ExampleShaders compiles outside crafter-build, so it can pass while an example fails to build #174

Open
opened 2026-10-31 14:57:57 +00:00 by jorijnvdgraaf · 0 comments

Summary

tests/ExampleShaders (#164) is meant to fail when an example's shader stops compiling the way crafter-build compiles it. It runs whatever glslang executable is on PATH with its own copy of crafter-build's flags. It also finds the shaders by scanning files rather than from the examples' registrations. So it can pass while an example fails to build, and fail where the example builds. Its declaration has two smaller problems on top.

Line numbers refer to 6e94cc6.

1. Another compiler, other defines

  • Inside the Flatpak the test runs /usr/bin/glslang 16.4.0 (the host has 16.6.0), while crafter-build compiles with the glslang it links, built from glslang main, unpinned. main already rejects something neither executable does: layout(constant_id = 1) const uint b = a * 2u; compiles with 16.4.0 and 16.6.0 but fails in crafter-build ("cannot be applied to a constant computed from other specialization constants"). An example shader with that line passes ExampleShaders while its example fails to build.
  • tests/ExampleShaders/main.cpp:115 passes -DCRAFTER_GRAPHICS_DESCRIPTOR_SETS, which glslang defines as empty, while crafter-build emits #define CRAFTER_GRAPHICS_DESCRIPTOR_SETS 1 for a define given without a value (Crafter.Build-Shader.cpp:159 in Crafter.Build). A shader that tests #if CRAFTER_GRAPHICS_DESCRIPTOR_SETS builds in its example and fails here with "preprocessor evaluation: bad expression".

2. A file scan instead of the registrations

:89-92 compile only examples/<Name>/*.glsl, one level deep, and :107 adds the descriptor-set build only when the file's own text mentions ui-shared.glsl. That has three consequences:

  • A shader registered from a subdirectory, or with another extension, is never compiled.
  • A shader that reaches ui-shared.glsl through its own header gets no descriptor-set build.
  • An include-only .glsl fails with "no stage known" (:102).

The comment's "a new shader is never silently left out" (:14) does not hold. Today the 16 shaders the examples register are exactly the 16 it finds.

3. The declaration

  • project.cpp:557 declares it for every native target, x86_64-w64-mingw32 included. There the tool:glslang probe succeeds on the Linux side, but under Wine std::system("glslang ...") goes through cmd.exe, which cannot run it. All 17 checks fail and the test returns EXIT_FAILURE instead of 77. UIFusedShader and PushConstantRewrite share the pattern.
  • LibraryTest builds a private copy of the whole library for a test that only imports std and runs glslang. That is 173 MB under build/ExampleShaders-*/, rebuilt after every library edit, and the binary links Vulkan, Wayland, xkbcommon and udev. crafter-build's own cfg.AddTest("ExampleShaders").Requires("tool:glslang") builds main.cpp alone; AGENTS.md would need a note for the exception to LibraryTest.

Suggested fix

Have crafter-build compile the shaders. Declare each example's shaders in the test's config.shaders as the example registers them, with one variant per example so outputs like raygen.spv do not collide. That fixes 1 and 2, and leaves the test binary at most checking that the outputs exist. Failing that, move the declaration out of the Windows build (or probe for glslang inside the test and return 77), and declare it with AddTest.

Found in a code review of the #163 change.

## Summary `tests/ExampleShaders` (#164) is meant to fail when an example's shader stops compiling the way crafter-build compiles it. It runs whatever `glslang` executable is on `PATH` with its own copy of crafter-build's flags. It also finds the shaders by scanning files rather than from the examples' registrations. So it can pass while an example fails to build, and fail where the example builds. Its declaration has two smaller problems on top. Line numbers refer to `6e94cc6`. ## 1. Another compiler, other defines - Inside the Flatpak the test runs `/usr/bin/glslang` 16.4.0 (the host has 16.6.0), while crafter-build compiles with the glslang it links, built from glslang `main`, unpinned. `main` already rejects something neither executable does: `layout(constant_id = 1) const uint b = a * 2u;` compiles with 16.4.0 and 16.6.0 but fails in crafter-build ("cannot be applied to a constant computed from other specialization constants"). An example shader with that line passes `ExampleShaders` while its example fails to build. - `tests/ExampleShaders/main.cpp:115` passes `-DCRAFTER_GRAPHICS_DESCRIPTOR_SETS`, which glslang defines as empty, while crafter-build emits `#define CRAFTER_GRAPHICS_DESCRIPTOR_SETS 1` for a define given without a value (`Crafter.Build-Shader.cpp:159` in Crafter.Build). A shader that tests `#if CRAFTER_GRAPHICS_DESCRIPTOR_SETS` builds in its example and fails here with "preprocessor evaluation: bad expression". ## 2. A file scan instead of the registrations `:89-92` compile only `examples/<Name>/*.glsl`, one level deep, and `:107` adds the descriptor-set build only when the file's own text mentions `ui-shared.glsl`. That has three consequences: - A shader registered from a subdirectory, or with another extension, is never compiled. - A shader that reaches `ui-shared.glsl` through its own header gets no descriptor-set build. - An include-only `.glsl` fails with "no stage known" (`:102`). The comment's "a new shader is never silently left out" (`:14`) does not hold. Today the 16 shaders the examples register are exactly the 16 it finds. ## 3. The declaration - `project.cpp:557` declares it for every native target, `x86_64-w64-mingw32` included. There the `tool:glslang` probe succeeds on the Linux side, but under Wine `std::system("glslang ...")` goes through cmd.exe, which cannot run it. All 17 checks fail and the test returns `EXIT_FAILURE` instead of 77. `UIFusedShader` and `PushConstantRewrite` share the pattern. - `LibraryTest` builds a private copy of the whole library for a test that only imports `std` and runs glslang. That is 173 MB under `build/ExampleShaders-*/`, rebuilt after every library edit, and the binary links Vulkan, Wayland, xkbcommon and udev. crafter-build's own `cfg.AddTest("ExampleShaders").Requires("tool:glslang")` builds `main.cpp` alone; AGENTS.md would need a note for the exception to `LibraryTest`. ## Suggested fix Have crafter-build compile the shaders. Declare each example's shaders in the test's `config.shaders` as the example registers them, with one variant per example so outputs like `raygen.spv` do not collide. That fixes 1 and 2, and leaves the test binary at most checking that the outputs exist. Failing that, move the declaration out of the Windows build (or probe for glslang inside the test and return 77), and declare it with `AddTest`. Found in a code review of the #163 change.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
Catcrafts/Crafter.Graphics#174
No description provided.