diff --git a/implementations/Crafter.Build-Clang.cpp b/implementations/Crafter.Build-Clang.cpp index d2aee22..05fa4b8 100644 --- a/implementations/Crafter.Build-Clang.cpp +++ b/implementations/Crafter.Build-Clang.cpp @@ -502,10 +502,28 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map buildCancelled{false}; + // Two entries with the same output name would compile in parallel into + // one file: whichever finished last won, the build reported success, and + // Check() then called both up to date. The usual way in is one source + // registered twice with different defines and no variant; a.vert next to + // a.frag is another. + { + std::unordered_map outputs; + for (const Shader& shader : config.shaders) { + const auto [existing, inserted] = outputs.emplace(shader.OutputName().string(), &shader); + if (inserted) continue; + buildError = std::format("shaders {} and {} both compile to {}: give one of them a distinct Shader::variant", existing->second->path.string(), shader.path.string(), existing->first); + buildCancelled.store(true); + break; + } + } for (const Shader& shader : config.shaders) { + if (buildCancelled.load(std::memory_order_relaxed)) break; if (shader.Check(outputDir)) continue; threads.emplace_back([&shader, &outputDir, &shaderIncludeDirs, &buildError, &buildCancelled]() { - Progress::Task task(std::format("Compiling shader {}", shader.path.filename().string())); + // A source registered more than once shows up once per variant; + // name the variant so the lines can be told apart. + Progress::Task task(shader.variant.empty() ? std::format("Compiling shader {}", shader.path.filename().string()) : std::format("Compiling shader {} ({})", shader.path.filename().string(), shader.variant)); if (buildCancelled.load(std::memory_order_relaxed)) return; std::string result = shader.Compile(outputDir, shaderIncludeDirs); @@ -878,7 +896,7 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_mapBinDir(); for (const Shader& shader : dep->shaders) { - fs::path src = depBinDir / shader.path.filename().replace_extension("spv"); + fs::path src = depBinDir / shader.OutputName(); if (!fs::exists(src)) continue; copyTree(src, outputDir / src.filename()); } diff --git a/implementations/Crafter.Build-Shader.cpp b/implementations/Crafter.Build-Shader.cpp index e251c50..722e635 100644 --- a/implementations/Crafter.Build-Shader.cpp +++ b/implementations/Crafter.Build-Shader.cpp @@ -36,9 +36,17 @@ namespace { namespace Crafter { Shader::Shader(fs::path&& path, std::string&& entrypoint, ShaderType type) : path(std::move(path)), entrypoint(std::move(entrypoint)), type(type) { + } + fs::path Shader::OutputName() const { + // replace_extension swaps only the last extension, so a stage suffix + // survives: "ui.comp.glsl" becomes "ui.comp.spv", or "ui.comp.sets.spv" + // for the variant "sets". + fs::path name = path.filename(); + name.replace_extension(variant.empty() ? std::string("spv") : std::format("{}.spv", variant)); + return name; } bool Shader::Check(const fs::path& outputDir) const { - fs::path spv = outputDir / path.filename().replace_extension("spv"); + fs::path spv = outputDir / OutputName(); return fs::exists(spv) && fs::last_write_time(path) < fs::last_write_time(spv); } std::string Shader::Compile(const fs::path& outputDir, std::span includeDirs) const { @@ -52,6 +60,9 @@ namespace Crafter { auto fail = [&](std::string_view stage, std::string log) { glslang::FinalizeProcess(); std::string out = path.string(); + // Several variants of one source compile in parallel; without the + // variant the message would not say which of them failed. + if (!variant.empty()) out += std::format(" ({})", variant); out += ": "; out += stage; if (!log.empty()) { @@ -75,8 +86,34 @@ namespace Crafter { const char* shaderSource = src.data(); const std::int32_t shaderSourceLen = static_cast(src.size()); + // A variant is one component of a file name. Caught here, because a + // separator would otherwise surface as "failed to open SPIR-V output" + // after the whole compile. + if (variant.find_first_of("/\\") != std::string::npos) { + return fail(std::format("shader variant '{}' must not contain a path separator", variant), {}); + } + + // One #define line per entry, split like the compiler's -D: "NAME" + // defines it to 1, "NAME=VALUE" to VALUE. glslang places the preamble + // ahead of the whole source (it finds #version by scanning first), so + // line numbers in the user's file are unaffected. It keeps the + // pointer it is handed instead of copying, so the string is declared + // here to outlive parse() below. + std::string preamble; + for (const std::string& define : defines) { + const std::size_t eq = define.find('='); + const std::string name = define.substr(0, eq); + // An error inside the preamble is reported as "-1:" with no + // file and no macro name, so reject the malformed ones by name. + if (name.empty() || define.find_first_of("\r\n") != std::string::npos) { + return fail(std::format("invalid shader define '{}': expected NAME or NAME=VALUE on one line", define), {}); + } + preamble += std::format("#define {} {}\n", name, eq == std::string::npos ? std::string("1") : define.substr(eq + 1)); + } + glslang::TShader shader(glslangType); shader.setStringsWithLengthsAndNames(&shaderSource, &shaderSourceLen, fileNameList, 1); + if (!preamble.empty()) shader.setPreamble(preamble.c_str()); shader.setEntryPoint(entrypoint.c_str()); shader.setSourceEntryPoint(entrypoint.c_str()); shader.setEnvTarget(glslang::EShTargetSpv, glslang::EShTargetSpv_1_4); @@ -110,7 +147,7 @@ namespace Crafter { glslang::GlslangToSpv(*intermediate, spirv, &logger); std::string spvLog = logger.getAllMessages(); - fs::path filename = path.filename().replace_extension("spv"); + fs::path filename = OutputName(); std::ofstream file(outputDir/filename, std::ios::binary); if (!file) { return fail("failed to open SPIR-V output", (outputDir/filename).string()); diff --git a/interfaces/Crafter.Build-Shader.cppm b/interfaces/Crafter.Build-Shader.cppm index 4b4d841..b059cfb 100644 --- a/interfaces/Crafter.Build-Shader.cppm +++ b/interfaces/Crafter.Build-Shader.cppm @@ -28,7 +28,25 @@ namespace Crafter { fs::path path; std::string entrypoint; ShaderType type; + // Preprocessor macros for this compile, spelled like the compiler's -D: + // "NAME" (defined to 1) or "NAME=VALUE". They are placed ahead of the + // source, so one file can be registered more than once and take a + // different #ifdef branch each time. Note the spelling differs from + // Configuration::defines, which is a list of {name, value} pairs. + std::vector defines; + // Names the output when one source is compiled more than once: empty + // gives ".spv", "sets" gives ".sets.spv". One file-name + // component: no path separators. Every entry of a configuration must + // end up with its own OutputName(); Build() fails if two collide, so + // each extra compile of a source needs its own variant. Check() + // compares only the source's mtime: changing defines without touching + // the source leaves the old output in place. + std::string variant; Shader(fs::path&& path, std::string&& entrypoint, ShaderType type); + // File name of the compiled SPIR-V, relative to the output directory. + // The single source of truth for it: Check, Compile and the artifact + // forwarding in Build() all go through here. + fs::path OutputName() const; bool Check(const fs::path& outputDir) const; std::string Compile(const fs::path& outputDir, std::span includeDirs) const; };