Improve matc error reporting (#2741)

This commit is contained in:
Ben Doherty
2020-06-29 11:21:56 -07:00
committed by GitHub
parent aed8c7fcb4
commit 92f2004c4b
16 changed files with 460 additions and 88 deletions

View File

@@ -24,34 +24,46 @@
namespace filamat {
struct IncludeResult {
/**
* The full contents of the include file. This may contain additional, recursive include
* directives.
*/
utils::CString source;
// The include name of the root file, as if it were being included.
// I.e., 'foobar.h' in the case of #include "foobar.h"
const utils::CString includeName;
/**
* The name of the include file. This gets passed as "includerName" for any includes inside
* of source. This field isn't used by the include system; it's up to the callback to give
* meaning to this value and interpret it accordingly.
* In the case of DirIncluder, this is an empty string to represent the root include file,
* and a canonical path for subsequent included files.
*/
// The following fields should be filled out by the IncludeCallback when processing an include,
// or when calling resolveIncludes for the root file.
// The full contents of the include file. This may contain additional, recursive include
// directives.
utils::CString text;
// The line number for the first line of text (first line is 0).
size_t lineNumberOffset = 0;
// The name of the include file. This gets passed as "includerName" for any includes inside of
// source. This field isn't used by the include system; it's up to the callback to give meaning
// to this value and interpret it accordingly. In the case of DirIncluder, this is an empty
// string to represent the root include file, and a canonical path for subsequent included
// files.
utils::CString name;
};
/**
* A callback invoked by the include system when an #include "file.h" directive is found.
* @param headerName is the name referenced within the quotes.
* @param includerName is the value that was given to IncludeResult.name for this source file, or
*
* For example, if a file main.h includes file.h on line 10, then IncludeCallback would be called
* with the following:
* includeCallback("main.h", {.includeName = "file.h" })
* It's then up to the IncludeCallback to fill out the .text, .name, and (optionally)
* lineNumberOffset fields.
*
* @param includedBy is the value that was given to IncludeResult.name for this source file, or
* the empty string for the root source file.
* @param result is the IncludeResult that the callback should fill out.
* @return true, if the include was resolved successfully, false otherwise.
*
* For an example of implementing this callback, see tools/matc/src/matc/DirIncluder.h.
*/
using IncludeCallback = std::function<bool(
const utils::CString& headerName,
const utils::CString& includerName,
const utils::CString& includedBy,
IncludeResult& result)>;
} // namespace filamat

View File

@@ -195,6 +195,9 @@ public:
//! Set the name of this material.
MaterialBuilder& name(const char* name) noexcept;
//! Set the file name of this material file. Used in error reporting.
MaterialBuilder& fileName(const char* name) noexcept;
//! Set the shading model.
MaterialBuilder& shading(Shading shading) noexcept;
@@ -260,6 +263,10 @@ public:
* postProcess.color = float4(1.0);
* }
* ~~~~~
*
* @param code The source code of the material.
* @param line The line number offset of the material, where 0 is the first line. Used for error
* reporting
*/
MaterialBuilder& material(const char* code, size_t line = 0) noexcept;
@@ -291,6 +298,10 @@ public:
*
* }
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
* @param code The source code of the material.
* @param line The line number offset of the material, where 0 is the first line. Used for error
* reporting
*/
MaterialBuilder& materialVertex(const char* code, size_t line = 0) noexcept;
@@ -509,6 +520,7 @@ private:
bool isLit() const noexcept { return mShading != filament::Shading::UNLIT; }
utils::CString mMaterialName;
utils::CString mFileName;
class ShaderCode {
public:
@@ -519,7 +531,7 @@ private:
}
// Resolve all the #include directives, returns true if successful.
bool resolveIncludes(IncludeCallback callback) noexcept;
bool resolveIncludes(IncludeCallback callback, const utils::CString& fileName) noexcept;
const utils::CString& getResolved() const noexcept {
assert(mIncludesResolved);

View File

@@ -17,6 +17,8 @@
#include "Includes.h"
#include <utils/Log.h>
#include <utils/compiler.h>
#include <utils/sstream.h>
#include <string>
@@ -26,45 +28,151 @@ static bool isWhitespace(char c) {
return (c == ' ' || c == '\f' || c == '\n' || c == '\r' || c == '\t' || c == '\v');
}
bool resolveIncludes(const utils::CString& rootName, utils::CString& source,
IncludeCallback callback, size_t depth) {
bool resolveIncludes(IncludeResult& root, IncludeCallback callback,
const ResolveOptions& options, size_t depth) {
if (depth > 30) {
// This is probably an include cycle. Stop here and report an error so we don't overflow.
utils::slog.e << "Include depth > 30. Include cycle?" << utils::io::endl;
return false;
}
std::vector<FoundInclude> includes = parseForIncludes(source);
while (!includes.empty()) {
const size_t lineNumberOffset = root.lineNumberOffset;
utils::CString& text = root.text;
std::vector<FoundInclude> includes = parseForIncludes(text);
bool sourceDirty = false;
// If we weren't given an include name, use "0", which is default when no #line directives are
// used.
const char* rootIncludeName = root.includeName.empty() ? "0" : root.includeName.c_str();
auto insertLineDirective = [&options](utils::io::ostream& stream, size_t line, const char* filename) {
if (options.insertLineDirectiveCheck) {
stream << "#if defined(GL_GOOGLE_cpp_style_line_directive)\n";
// The #endif itself will count as a line, so subtact 1.
line--;
}
stream << "#line " << line << " \"" << filename << '\"';
if (options.insertLineDirectiveCheck) {
stream << "\n#endif";
}
};
// The #line directive must be on its own line and works like so:
// #line 10 "file.h"
// any code on this line is now considered line 10 of file.h
// Add #line directives before / after each #include. We work backwards, otherwise we'd
// invalidate the offsets in FoundInclude.
for (auto it = includes.rbegin(); it < includes.rend() && options.insertLineDirectives; ++it) {
const auto include = *it;
// Remember that text editors consider the first line of a file to be line 1.
// Consider the following file, called "root.h":
// 1
// 2 #include "foo.h"
// 3
// We want to insert an opening and closing #line directive:
// 1
// #line 1 "foo.h"
// 2 #include "foo.h"
// #line 3 "root.h"
// 3
// We want to insert a closing directive with a line number of 3.
// In this example, include.line is 2 and lineNumberOffset is 0.
// So, the math works out as such:
const size_t lineDirectiveLine = include.line + lineNumberOffset + 1;
utils::io::sstream closingDirective;
// This first newline is to ensure that the #line directive falls on a fresh line.
closingDirective << '\n';
// If there's a newline after the include, we'll use that to terminate the #line directive.
const size_t newlineCharacter = include.startPosition + include.length;
if (text.length() > newlineCharacter && text[newlineCharacter] == '\n') {
insertLineDirective(closingDirective, lineDirectiveLine, rootIncludeName);
} else {
// If there isn't one, be sure to add one. The included source might not have an
// newline at the end of the file.
// We subtract 1 to handle additional code after the include directive.
// E.g., this include statement:
// #include "foobar.h" more code on same line
//
// should get translated to:
// #line 1
// #include "foobar.h"
// #line 1
// more code on same line
insertLineDirective(closingDirective, lineDirectiveLine - 1, rootIncludeName);
closingDirective << '\n';
}
text.insert(include.startPosition + include.length, utils::CString(closingDirective.c_str()));
// The included source always starts on line 1.
utils::io::sstream openingDirective;
insertLineDirective(openingDirective, 1, include.name.c_str());
openingDirective << '\n';
text.insert(include.startPosition, utils::CString(openingDirective.c_str()));
sourceDirty = true;
}
// Add a line directive on the first line for the root include.
if (options.insertLineDirectives && depth == 0) {
utils::io::sstream lineDirective;
insertLineDirective(lineDirective, lineNumberOffset + 1, rootIncludeName);
lineDirective << '\n';
text.insert(0, utils::CString(lineDirective.c_str()));
sourceDirty = true;
}
// Re-parse for includes. If we've inserted any #line directives, then the line numbers have
// changed.
if (UTILS_LIKELY(sourceDirty)) {
includes = parseForIncludes(text);
}
while (!includes.empty() && options.resolveIncludes) {
const auto include = includes[0];
// Ask the includer to resolve this include.
if (!callback) {
return false;
}
IncludeResult result;
if (!callback(include.name, rootName, result)) {
IncludeResult resolved {
.includeName = include.name
};
if (!callback(root.name, resolved)) {
utils::slog.e << "The included file \"" << include.name.c_str()
<< "\" could not be found." << utils::io::endl;
return false;
}
// Recursively resolve all of its includes.
if (!resolveIncludes(result.name, result.source, callback, depth + 1)) {
if (!resolveIncludes(resolved, callback, options, depth + 1)) {
return false;
}
source.replace(include.startPosition, include.length, result.source);
text.replace(include.startPosition, include.length, resolved.text);
includes = parseForIncludes(source);
includes = parseForIncludes(text);
}
return true;
}
std::vector<FoundInclude> parseForIncludes(const utils::CString& source) {
std::string sourceString = source.c_str();
std::vector<FoundInclude> results;
if (source.empty()) {
return results;
}
std::string sourceString = source.c_str();
size_t result = sourceString.find("#include");
while(result != std::string::npos) {
@@ -102,7 +210,16 @@ std::vector<FoundInclude> parseForIncludes(const utils::CString& source) {
// Grab the include name.
const auto includeName = sourceString.substr(nameStart, nameEnd - nameStart + 1);
results.push_back({utils::CString(includeName.c_str()), includeStart, includeEnd - includeStart + 1});
// Calculate the line number of the include.
size_t lineNumber = 1;
for (size_t i = 0; i < includeStart; i++) {
if (source[i] == '\n') {
lineNumber++;
}
}
results.push_back({utils::CString(includeName.c_str()), includeStart,
includeEnd - includeStart + 1, lineNumber});
// Find next occurrence.
result = sourceString.find("#include", result);

View File

@@ -25,16 +25,30 @@
namespace filamat {
// Recursively handle all the includes inside of source.
struct ResolveOptions {
// If true, insert #line directives before / after each include.
bool insertLineDirectives = false;
// Surrounds line directives with #if defined(GL_GOOGLE_cpp_style_line_directive)
// Some drivers may complain about the use of cpp style #line directives if they don't support
// it.
bool insertLineDirectiveCheck = false;
// If true, process #include directives.
bool resolveIncludes = true;
};
// Recursively handle all the includes inside of root.
// Returns true if all includes were handled successfully, false otherwise.
// callback may be null, in which case any #include directives found will result in a failure.
bool resolveIncludes(const utils::CString& rootName, utils::CString& source,
IncludeCallback callback, size_t depth = 0);
bool resolveIncludes(IncludeResult& root, IncludeCallback callback,
const ResolveOptions& options, size_t depth = 0);
struct FoundInclude {
utils::CString name;
size_t startPosition;
size_t length;
size_t line; // the line number the include was found on (first line is 1)
};
std::vector<FoundInclude> parseForIncludes(const utils::CString& source);

View File

@@ -132,6 +132,11 @@ MaterialBuilder& MaterialBuilder::name(const char* name) noexcept {
return *this;
}
MaterialBuilder& MaterialBuilder::fileName(const char* fileName) noexcept {
mFileName = CString(fileName);
return *this;
}
MaterialBuilder& MaterialBuilder::material(const char* code, size_t line) noexcept {
mMaterialCode.setUnresolved(CString(code));
mMaterialCode.setLineOffset(line);
@@ -512,11 +517,23 @@ bool MaterialBuilder::checkLiteRequirements() noexcept {
return true;
}
bool MaterialBuilder::ShaderCode::resolveIncludes(IncludeCallback callback) noexcept {
bool MaterialBuilder::ShaderCode::resolveIncludes(IncludeCallback callback,
const utils::CString& fileName) noexcept {
if (!mCode.empty()) {
if (!::filamat::resolveIncludes(utils::CString(""), mCode, callback)) {
ResolveOptions options {
.insertLineDirectives = true,
.insertLineDirectiveCheck = true
};
IncludeResult source {
.includeName = fileName,
.text = mCode,
.lineNumberOffset = getLineOffset(),
.name = utils::CString("")
};
if (!::filamat::resolveIncludes(source, callback, options)) {
return false;
}
mCode = source.text;
}
mIncludesResolved = true;
@@ -708,8 +725,8 @@ Package MaterialBuilder::build() noexcept {
}
// Resolve all the #include directives within user code.
if (!mMaterialCode.resolveIncludes(mIncludeCallback) ||
!mMaterialVertexCode.resolveIncludes(mIncludeCallback)) {
if (!mMaterialCode.resolveIncludes(mIncludeCallback, mFileName) ||
!mMaterialVertexCode.resolveIncludes(mIncludeCallback, mFileName)) {
return Package::invalidPackage();
}

View File

@@ -64,6 +64,10 @@ io::sstream& CodeGenerator::generateProlog(io::sstream& out, ShaderType type,
break;
}
// This allows our includer system to use the #line directive to denote the source file for
// #included code. This way, glslang reports errors more accurately.
out << "#extension GL_GOOGLE_cpp_style_line_directive : enable\n\n";
if (mTargetApi == TargetApi::VULKAN) {
out << "#define TARGET_VULKAN_ENVIRONMENT\n";
}

View File

@@ -110,8 +110,7 @@ static void appendShader(utils::io::sstream& ss,
const utils::CString& shader, size_t lineOffset) noexcept {
if (!shader.empty()) {
size_t lines = countLines(ss.c_str());
ss << "#line " << lineOffset;
if (shader[0] != '\n') ss << "\n";
ss << "#line " << lineOffset + 1 << '\n';
ss << shader.c_str();
if (shader[shader.size() - 1] != '\n') {
ss << "\n";

View File

@@ -35,9 +35,8 @@ public:
return *this;
}
bool operator()(const utils::CString& headerName, const utils::CString& includerName,
filamat::IncludeResult& result) {
auto key = headerName.c_str();
bool operator()(const utils::CString& includedBy, filamat::IncludeResult& result) {
auto key = result.includeName.c_str();
auto found = mIncludeMap.find(key);
if (found == mIncludeMap.end()) {
@@ -47,12 +46,12 @@ public:
auto include = found->second;
if (!include.expectedIncluder.empty()) {
EXPECT_STREQ(includerName.c_str_safe(), include.expectedIncluder.c_str());
EXPECT_STREQ(includedBy.c_str_safe(), include.expectedIncluder.c_str());
}
if (!include.source.empty()) {
result.source = utils::CString(include.source.c_str());
result.name = headerName;
result.text = utils::CString(include.source.c_str());
result.name = result.includeName;
return true;
}

View File

@@ -35,10 +35,11 @@ TEST(IncludeParser, NoIncludes) {
TEST(IncludeParser, SingleInclude) {
utils::CString code(R"(#include "foobar.h")");
auto result = filamat::parseForIncludes(code);
EXPECT_EQ(result.size(), 1);
EXPECT_EQ(result[0].name, "foobar.h");
EXPECT_EQ(result[0].startPosition, 0);
EXPECT_EQ(result[0].length, 19);
EXPECT_EQ(1, result.size());
EXPECT_STREQ("foobar.h", result[0].name.c_str());
EXPECT_EQ(0, result[0].startPosition);
EXPECT_EQ(19, result[0].length);
EXPECT_EQ(1, result[0].line);
}
TEST(IncludeParser, MultipleIncludes) {
@@ -49,10 +50,12 @@ TEST(IncludeParser, MultipleIncludes) {
EXPECT_STREQ("foobar.h", result[0].name.c_str());
EXPECT_EQ(0, result[0].startPosition);
EXPECT_EQ(19, result[0].length);
EXPECT_EQ(1, result[0].line);
EXPECT_STREQ("bazbarfoo.h", result[1].name.c_str());
EXPECT_EQ(20, result[1].startPosition);
EXPECT_EQ(22, result[1].length);
EXPECT_EQ(2, result[1].line);
}
TEST(IncludeParser, EmptyInclude) {
@@ -63,6 +66,7 @@ TEST(IncludeParser, EmptyInclude) {
EXPECT_STREQ("", result[0].name.c_str_safe());
EXPECT_EQ(0, result[0].startPosition);
EXPECT_EQ(11, result[0].length);
EXPECT_EQ(1, result[0].line);
}
TEST(IncludeParser, Whitepsace) {
@@ -73,6 +77,7 @@ TEST(IncludeParser, Whitepsace) {
EXPECT_STREQ("foobarbaz.h", result[0].name.c_str());
EXPECT_EQ(2, result[0].startPosition);
EXPECT_EQ(27, result[0].length);
EXPECT_EQ(1, result[0].line);
}
TEST(IncludeParser, InvalidIncludes) {
@@ -89,6 +94,7 @@ TEST(IncludeParser, InvalidWithValidInclude) {
EXPECT_STREQ("foobar.h", result[0].name.c_str());
EXPECT_EQ(9, result[0].startPosition);
EXPECT_EQ(19, result[0].length);
EXPECT_EQ(1, result[0].line);
}
{
@@ -98,17 +104,30 @@ TEST(IncludeParser, InvalidWithValidInclude) {
EXPECT_STREQ("foo.h", result[0].name.c_str());
EXPECT_EQ(26, result[0].startPosition);
EXPECT_EQ(15, result[0].length);
EXPECT_EQ(1, result[0].line);
}
}
TEST(IncludeParser, LineNumbers) {
utils::CString code("#include \"one.h\"\n#include \"two.h\"\n\n#include \"four.h\"");
auto result = filamat::parseForIncludes(code);
EXPECT_EQ(3, result.size());
EXPECT_EQ(1, result[0].line);
EXPECT_EQ(2, result[1].line);
EXPECT_EQ(4, result[2].line);
}
// -------------------------------------------------------------------------------------------------
TEST(IncludeResolver, NoIncludes) {
utils::CString code("no includes");
MockIncluder includer;
bool result = filamat::resolveIncludes(utils::CString(""), code, includer);
filamat::IncludeResult source {
.text = code
};
bool result = filamat::resolveIncludes(source, includer, {});
EXPECT_TRUE(result);
EXPECT_STREQ("no includes", code.c_str());
EXPECT_STREQ("no includes", source.text.c_str());
}
TEST(IncludeResolver, SingleInclude) {
@@ -116,9 +135,12 @@ TEST(IncludeResolver, SingleInclude) {
MockIncluder includer;
includer
.sourceForInclude("test.h", "include");
bool result = filamat::resolveIncludes(utils::CString(""), code, includer);
filamat::IncludeResult source {
.text = code
};
bool result = filamat::resolveIncludes(source, includer, {});
EXPECT_TRUE(result);
EXPECT_STREQ("include", code.c_str());
EXPECT_STREQ("include", source.text.c_str());
}
TEST(IncludeResolver, MultipleIncludes) {
@@ -134,13 +156,16 @@ TEST(IncludeResolver, MultipleIncludes) {
.sourceForInclude("two.h", "2")
.sourceForInclude("three.h", "3");
bool result = filamat::resolveIncludes(utils::CString(""), code, includer);
filamat::IncludeResult source {
.text = code
};
bool result = filamat::resolveIncludes(source, includer, {});
EXPECT_TRUE(result);
EXPECT_STREQ(utils::CString(R"(
1
2
3
)").c_str(), code.c_str());
)").c_str(), source.text.c_str());
}
TEST(IncludeResolver, IncludeWithinInclude) {
@@ -156,13 +181,15 @@ TEST(IncludeResolver, IncludeWithinInclude) {
.sourceForInclude("three.h", "3")
.expectIncludeIncludedBy("three.h", "two.h");
bool result = filamat::resolveIncludes(utils::CString(""), code, includer);
filamat::IncludeResult source {
.text = code
};
bool result = filamat::resolveIncludes(source, includer, {});
EXPECT_TRUE(result);
EXPECT_STREQ(utils::CString(R"(
1
3
)").c_str(), code.c_str());
)").c_str(), source.text.c_str());
}
TEST(IncludeResolver, Includers) {
@@ -177,11 +204,14 @@ TEST(IncludeResolver, Includers) {
.sourceForInclude("three.h", "3")
.expectIncludeIncludedBy("two.h", "dir/one.h");
bool result = filamat::resolveIncludes(utils::CString(""), code, includer);
filamat::IncludeResult source {
.text = code
};
bool result = filamat::resolveIncludes(source, includer, {});
EXPECT_TRUE(result);
EXPECT_STREQ(utils::CString(R"(
3
)").c_str(), code.c_str());
)").c_str(), source.text.c_str());
}
TEST(IncludeResolver, IncludeFailure) {
@@ -194,7 +224,10 @@ TEST(IncludeResolver, IncludeFailure) {
includer
.sourceForInclude("one.h", "#include \"two.h\"");
bool result = filamat::resolveIncludes(utils::CString(""), code, includer);
filamat::IncludeResult source {
.text = code
};
bool result = filamat::resolveIncludes(source, includer, {});
EXPECT_FALSE(result);
}
{
@@ -204,7 +237,10 @@ TEST(IncludeResolver, IncludeFailure) {
MockIncluder includer;
bool result = filamat::resolveIncludes(utils::CString(""), code, includer);
filamat::IncludeResult source {
.text = code
};
bool result = filamat::resolveIncludes(source, includer, {});
EXPECT_FALSE(result);
}
}
@@ -219,11 +255,132 @@ TEST(IncludeResolver, Cycle) {
.sourceForInclude("foo.h", "#include \"bar.h\"")
.sourceForInclude("bar.h", "#include \"foo.h\"");
bool result = filamat::resolveIncludes(utils::CString(""), code, includer);
filamat::IncludeResult source {
.text = code
};
bool result = filamat::resolveIncludes(source, includer, {});
// Include cycles are disallowed. We should still terminate in finite time and report false.
EXPECT_FALSE(result);
}
// Helper function that lets us write comparison cases with line breaks.
void EXPECT_STREQ_TRIMMARGIN(const char* expected, const char* actual) {
size_t len = strlen(expected);
char* trimmed = (char*) malloc(len);
const char* end = expected + len;
const char* e = expected;
char* t = trimmed;
while (e < end) {
while (e < end) {
if (*e == '|') {
e += 2; // eat | and space
break;
}
e++;
}
while (e < end) {
if (*e == '\n') {
*t++ = *e++;
break;
}
*t++ = *e++;
}
}
*t++ = 0;
EXPECT_STREQ(trimmed, actual);
free(trimmed);
}
TEST(IncludeResolver, SingleIncludeLineDirective) {
utils::CString code("#include \"test.h\"");
MockIncluder includer;
includer
.sourceForInclude("test.h", "include");
filamat::ResolveOptions options = {
.insertLineDirectives = true,
.insertLineDirectiveCheck = false // makes it simplier to test
};
filamat::IncludeResult source {
.includeName = utils::CString("root.h"),
.text = code
};
bool result = filamat::resolveIncludes(source, includer, options);
EXPECT_TRUE(result);
EXPECT_STREQ_TRIMMARGIN(R"(
| #line 1 "root.h"
| #line 1 "test.h"
| include
| #line 1 "root.h"
| )", source.text.c_str());
}
TEST(IncludeResolver, MultipleIncludesLineDirective) {
utils::CString code("#include \"one.h\"\n#include \"two.h\"\n");
MockIncluder includer;
includer
.sourceForInclude("one.h", "1")
.sourceForInclude("two.h", "2");
filamat::ResolveOptions options = {
.insertLineDirectives = true,
.insertLineDirectiveCheck = false // makes it simplier to test
};
filamat::IncludeResult source {
.includeName = utils::CString("root.h"),
.text = code,
};
bool result = filamat::resolveIncludes(source, includer, options);
EXPECT_TRUE(result);
EXPECT_STREQ_TRIMMARGIN(R"(
| #line 1 "root.h"
| #line 1 "one.h"
| 1
| #line 2 "root.h"
| #line 1 "two.h"
| 2
| #line 3 "root.h"
| )", source.text.c_str());
}
TEST(IncludeResolver, MultipleIncludesSameLineLineDirective) {
// includes are on the same line
utils::CString code("#include \"one.h\"#include \"two.h\"\n");
MockIncluder includer;
includer
.sourceForInclude("one.h", "1")
.sourceForInclude("two.h", "2")
.expectIncludeIncludedBy("three.h", "two.h");
filamat::ResolveOptions options = {
.insertLineDirectives = true,
.insertLineDirectiveCheck = false // makes it simplier to test
};
filamat::IncludeResult source {
.includeName = utils::CString("root.h"),
.text = code
};
bool result = filamat::resolveIncludes(source, includer, options);
EXPECT_TRUE(result);
EXPECT_STREQ_TRIMMARGIN(R"(
| #line 1 "root.h"
| #line 1 "one.h"
| 1
| #line 1 "root.h"
| #line 1 "two.h"
| 2
| #line 2 "root.h"
| )", source.text.c_str());
}
// -------------------------------------------------------------------------------------------------
#include <utils/Log.h>

View File

@@ -260,6 +260,7 @@ public:
const_iterator cend() const noexcept { return end(); }
CString& replace(size_type pos, size_type len, const CString& str) noexcept;
CString& insert(size_type pos, const CString& str) noexcept { return replace(pos, 0, str); }
const_reference operator[](size_type pos) const noexcept {
assert(pos < size());

View File

@@ -68,11 +68,14 @@ foreach(SHADER_FILE ${SHADERS})
get_filename_component(SHADER_NAME ${SHADER_FILE} NAME)
set(SHADER_RAW ${CMAKE_CURRENT_SOURCE_DIR}/${SHADER_FILE})
set(SHADER_MIN ${MINIFIED_DIR}/${SHADER_NAME})
# For Debug builds, pass the "-Onone" flag to perform no minification. This is helpful for
# debugging shaders.
# For Debug builds, pass two additional flags to help with debugging:
# -Onone performs no minification.
# -lshader_name adds a #line directive at the beginning to help identify sources of errors.
set(OPT_FLAG "$<$<CONFIG:DEBUG>:-Onone>")
set(LINE_FLAG "$<$<CONFIG:DEBUG>:-l${SHADER_NAME}>")
add_custom_command(
OUTPUT ${SHADER_MIN}
COMMAND glslminifier "$<$<CONFIG:DEBUG>:-Onone>" -o ${SHADER_MIN} ${SHADER_RAW}
COMMAND glslminifier ${OPT_FLAG} ${LINE_FLAG} -o ${SHADER_MIN} ${SHADER_RAW}
DEPENDS glslminifier ${SHADER_RAW}
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
COMMENT "Minifying shader ${SHADER_NAME}"

View File

@@ -32,6 +32,8 @@ static bool g_writeToStdOut = true;
static const char* g_outputFile = "";
static const char* g_inputFile = "";
GlslMinifyOptions g_optimizationLevel = GlslMinifyOptions::ALL;
static bool g_outputLineDirectives = false;
static std::string g_lineDirectiveName;
static const char* USAGE = R"TXT(
GLSLMINIFIER minifies GLSL shader code by removing comments, blank lines and indentation.
@@ -48,6 +50,13 @@ Options:
Specify path to output file. If none provided, writes to stdout.
--optimization, -O [none]
Set the level of optimization. "none" performs a simple passthrough.
--line, -l [name]
Insert a #line directive on the first line of the shader with the given name.
For example, --line foobar.h will insert the following:
#if defined(GL_GOOGLE_cpp_style_line_directive)
#line 0 "foobar.h"
#endif
This option is meant to be used with -Onone optimization.
Example:
GLSLMINIFIER -o output.fs.min input.fs
@@ -76,12 +85,13 @@ static void license() {
}
static int handleArguments(int argc, char* argv[]) {
static constexpr const char* OPTSTR = "hLo:O:";
static constexpr const char* OPTSTR = "hLo:O:l:";
static const struct option OPTIONS[] = {
{ "help", no_argument, nullptr, 'h' },
{ "license", no_argument, nullptr, 'L' },
{ "output", required_argument, nullptr, 'o' },
{ "optimization", required_argument, nullptr, 'O' },
{ "line", required_argument, nullptr, 'l' },
{ nullptr, 0, nullptr, 0 } // termination of the option list
};
@@ -109,6 +119,10 @@ static int handleArguments(int argc, char* argv[]) {
std::cerr << "Warning: unknown optimization level." << std::endl;
}
break;
case 'l':
g_outputLineDirectives = true;
g_lineDirectiveName = arg;
break;
}
}
@@ -139,6 +153,17 @@ int main(int argc, char* argv[]) {
// Minify the GLSL.
string result = minifyGlsl(inputStr, g_optimizationLevel);
// Add a #line directive at the beginning of the file, if requested.
if (g_outputLineDirectives) {
// We must check for support for the line directive, otherwise drivers may complain if they
// don't support it.
std::string lineDirective =
std::string("#if defined(GL_GOOGLE_cpp_style_line_directive)\n") +
"#line 0 \"" + g_lineDirectiveName + "\"\n" +
"#endif\n";
result.insert(0, lineDirective);
}
if (g_writeToStdOut) {
cout << result;
return 0;

View File

@@ -22,20 +22,20 @@
namespace matc {
bool DirIncluder::operator()(const utils::CString& headerName, const utils::CString& includerName,
filamat::IncludeResult& result) {
auto getHeaderPath = [&includerName, &headerName, this]() {
// If includer name is empty, then search from the root include directory.
if (includerName.empty()) {
return mIncludeDirectory.concat(headerName.c_str());
bool DirIncluder::operator()(const utils::CString& includedBy, filamat::IncludeResult& result) {
auto getHeaderPath = [&result, &includedBy, this]() {
// includedBy is the path to the file that's including result.includeName.
// If it's empty, then search from the root include directory.
if (includedBy.empty()) {
return mIncludeDirectory.concat(result.includeName.c_str());
}
// Otherwise, search relative to the includer file.
utils::Path includer(includerName.c_str());
utils::Path includer(includedBy.c_str());
// TODO: this assert was firing only in CI during DirIncluder tests. Maybe because of
// inadequate file permissions.
// assert(includer.isFile());
return includer.getParent() + headerName.c_str();
return includer.getParent() + result.includeName.c_str();
};
const utils::Path headerPath = getHeaderPath();
@@ -60,7 +60,7 @@ bool DirIncluder::operator()(const utils::CString& headerName, const utils::CStr
stream.close();
result.source = utils::CString(contents.c_str());
result.text = utils::CString(contents.c_str());
result.name = utils::CString(headerPath.c_str());
return true;

View File

@@ -31,8 +31,7 @@ public:
mIncludeDirectory = dir;
}
bool operator()(const utils::CString& headerName, const utils::CString& includerName,
filamat::IncludeResult& result);
bool operator()(const utils::CString& includedBy, filamat::IncludeResult& result);
private:
utils::Path mIncludeDirectory;

View File

@@ -84,7 +84,9 @@ bool MaterialCompiler::processVertexShader(const MaterialLexeme& lexeme,
MaterialLexeme trimmedLexeme = lexeme.trimBlockMarkers();
std::string shaderStr = trimmedLexeme.getStringValue();
builder.materialVertex(shaderStr.c_str(), trimmedLexeme.getLine() + 1);
// getLine() returns a line number, with 1 being the first line, but .material wants a 0-based
// line number offset, where 0 is the first line.
builder.materialVertex(shaderStr.c_str(), trimmedLexeme.getLine() - 1);
return true;
}
@@ -94,7 +96,9 @@ bool MaterialCompiler::processFragmentShader(const MaterialLexeme& lexeme,
MaterialLexeme trimmedLexeme = lexeme.trimBlockMarkers();
std::string shaderStr = trimmedLexeme.getStringValue();
builder.material(shaderStr.c_str(), trimmedLexeme.getLine() + 1);
// getLine() returns a line number, with 1 being the first line, but .material wants a 0-based
// line number offset, where 0 is the first line.
builder.material(shaderStr.c_str(), trimmedLexeme.getLine() - 1);
return true;
}
@@ -276,6 +280,7 @@ bool MaterialCompiler::run(const Config& config) {
builder
.includeCallback(includer)
.fileName(materialFilePath.getName().c_str())
.platform(config.getPlatform())
.targetApi(config.getTargetApi())
.optimization(config.getOptimizationLevel())

View File

@@ -28,14 +28,18 @@ const utils::Path root = utils::Path(__FILE__).getParent();
TEST(DirIncluder, DISABLED_IncludeNonexistent) {
matc::DirIncluder includer;
{
IncludeResult _;
bool success = includer(CString("nonexistent.h"), CString(""), _);
IncludeResult i {
.includeName = CString("nonexistent.h")
};
bool success = includer(CString(""), i);
EXPECT_FALSE(success);
}
{
utils::CString includerFile((root + "Foo.h").getPath().c_str());
IncludeResult _;
bool success = includer(CString("nonexistent.h"), includerFile, _);
IncludeResult i {
.includeName = CString("nonexistent.h")
};
bool success = includer(includerFile, i);
EXPECT_FALSE(success);
}
}
@@ -44,13 +48,15 @@ TEST(DirIncluder, DISABLED_IncludeFile) {
matc::DirIncluder includer;
includer.setIncludeDirectory(root);
IncludeResult result;
bool success = includer(CString("Foo.h"), CString(""), result);
IncludeResult result {
.includeName = CString("Foo.h")
};
bool success = includer(CString(""), result);
EXPECT_TRUE(success);
// The result's source should be set to the contents of the includer file.
EXPECT_STREQ("// test include file", result.source.c_str());
EXPECT_STREQ("// test include file", result.text.c_str());
// The result's name should be set to the full path to the header file.
EXPECT_STREQ((root + "Foo.h").c_str(), result.name.c_str());
@@ -62,10 +68,12 @@ TEST(DirIncluder, DISABLED_IncludeFileFromIncluder) {
utils::CString includerFile((root + "Dir/Baz.h").c_str());
IncludeResult result;
bool success = includer(CString("Bar.h"), includerFile, result);
IncludeResult result {
.includeName = CString("Bar.h")
};
bool success = includer(includerFile, result);
EXPECT_STREQ("// Bar.h", result.source.c_str());
EXPECT_STREQ("// Bar.h", result.text.c_str());
EXPECT_STREQ((root + "Dir/Bar.h").c_str(), result.name.c_str());
}