Shader variants, sysroot libc++ headers, and a fix for the project.so stamp #37
No reviewers
Labels
No labels
bug
claude:blocked
claude:done
claude:failed
claude:in-progress
claude:ready
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
Catcrafts/Crafter.Build!37
Loading…
Reference in a new issue
No description provided.
Delete branch "claude/shader-variants-sysroot-libcxx"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Three changes, each as a commit followed by its test. The second and third are what Crafter.Graphics needs to run its UI path on drivers without
VK_EXT_descriptor_heap(branchclaude/descriptor-set-fallbackthere); the first is a bug the second one exposed.1.
fix(host-cache): the project.so stamp covered only the versionThe stamp was built with
std::format("{}\0{}\0{}\0{}", version, toolchainKey, sourcesStamp, compileCmd). A format string is read as a C string, so it ended at the first NUL and only the version was hashed. Aproject.socompiled against other module sources was therefore reused whenever the version string was unchanged: every working-tree build and every local Flatpak build.That matters here because the next change grows
Shaderfrom 56 to 104 bytes. A staleproject.sothat declares shaders aborts withfree(): invalid size.HostCacheSourceStampnow also asserts that changed declarations recompileproject.so. It fails without the fix.Effect on users: every existing
project.sois recompiled once.2.
feat(shader): per-entrydefinesand an outputvariantOne source can now be registered more than once:
Shader::OutputName()is the one place the file name is formed;Check,Compileand the forwarding of a dependency's shaders use it.a.vertnext toa.frag).Build()now fails and names both. An existing project with such a pair will stop building; it was already shipping one shader under the other's name.Tests:
ShaderCompile(API level, including per-variant staleness) and the newShaderVariantBuild(throughBuild(), including the collision error).3.
fix(sysroot): ordinary compiles use the sysroot's libc++ headersclang ranks the host toolchain's libc++ headers above
--sysroot.BuildStdPcmalready forced the sysroot's copy for the std module; ordinary compiles did not, so a unit with a textual standard include got the host's libc++. Against a musl sysroot that fails with "unknown rune table for this platform" (seen with Crafter.Asset's vendored gdeflate when cross-building for aarch64 musl).Non-wasm, non-mingw sysroot compiles now get
-nostdinc++ -isystem <sysroot>/usr/include/c++/v1, plus-Wno-unused-command-line-argumentbecause-nostdinc++leaves-stdlib=libc++unused on each compile. That silencer also hides the driver's unused-argument diagnostics for a user's own flags on sysroot builds.Test: the new
SysrootCompileFlagschecks the command text per target. README updated.Verification
crafter-build test: 26 passed, 2 skipped (WasmHelloRuns: no wasmtime here;ConcurrentCacheRace: no installed share dir).format --check: clean.lint: 47 findings, allnaming. 45 are onmasteralready (#36); the other two are theFailurescounter in the two new tests, the same pattern the 17 existing tests use.aarch64-alpine-linux-muslruns on a Fairphone 6.Not addressed
Shader::Checkstill compares only the source's mtime, so a change todefinesor to an included file does not recompile (#35).--targetnor--sysroot.🤖 Generated with Claude Code
The stamp that decides whether a compiled project file can be reused was meant to cover the crafter-build version, the toolchain key, the module-source stamp and the compile command. Only the version went in. The four parts were joined by a format string with "\0" between the fields, and a format string is read as a C string: it ended at the first NUL, so std::format saw "{}" and dropped the other arguments without a diagnostic. A project.so is loaded into the crafter-build process and fills in Configuration objects by their layout. With the stamp blind to the declarations, one built against other module sources was loaded as current whenever the version string was unchanged, which is every working-tree build (`git describe --dirty` does not follow the tree's contents) and every local Flatpak build. Growing a class that project files construct then corrupts the heap: a project.so built while Shader was 56 bytes, loaded after it had become 104, aborted with "free(): invalid size" as soon as it declared a shader. CI builds were spared only because each carries its own version. The parts are now appended to a std::string with NUL separators, the way the other stamps in this file are built. Every existing project.so carries a stamp of the version alone, so each is recompiled once. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>Crafter.Graphics has to ship each of its compute shaders in two builds of one source: one that indexes VK_EXT_descriptor_heap and one that reads the same slots from a descriptor set, for drivers without the extension. A Shader was a path, an entry point and a stage, and its output name was derived from the path alone, so one source could only be compiled once. Shader gains `defines`, spelled like the compiler's -D ("NAME" is defined to 1, "NAME=VALUE" to VALUE) and handed to glslang as a preamble, and `variant`, which names the output: "ui.comp.glsl" with variant "sets" becomes "ui.comp.sets.spv". OutputName() is the one place that name is formed; Check, Compile and the forwarding of a dependency's shaders into the consumer's bin dir all use it. Two fields make one mistake easy: defines set, variant forgotten. Both entries then compiled in parallel into the same file, the build reported success with whichever finished last, and Check() called both up to date from then on. The same has always been true of a.vert next to a.frag. Build() now refuses a configuration in which two entries share an output name and says which. A define without a name, or one spanning lines, and a variant with a path separator are rejected by name. Left to glslang the first two surface as "-1:<line>" with no file, and the last as a failed open after the whole compile. Not addressed: Check() still compares only the source's mtime, so a change to the defines, or to an included file, does not recompile (#35). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>Landed on
masterby fast-forward (c6b0bba..fe38cc1, the same seven commits; CI passed on this head). Closed by hand because the forge cannot mark a pushed fast-forward as merged in this repository.Pull request closed