From de590c2a25ad545b4489fe44e24960c8b519ebdb Mon Sep 17 00:00:00 2001 From: Nathan Date: Tue, 26 May 2026 15:02:23 -0500 Subject: [PATCH] renderer/vulkan: revert built-in SPV precompile until cross-shader bug found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even with image_vert + image_frag runtime-compiled while the other 7 used precompiled SPV, kitty images still rendered at full-window size. That rules out the SPV bytes for those two shaders specifically — the bug is some cross-shader interaction the precompile path triggers that runtime glslang doesn't. Hypotheses worth chasing later: - Auto-map binding numbers depend on glslang TLS state. Build- time spawns a fresh process per shader → fresh state each time. Runtime compiles 9 sequentially → shared TLS pool accumulates assignments. Final bindings/locations could diverge in ways that affect a specific pipeline's draw. - ProcessDeferred's SetupBuiltinSymbolTable might cache cross- compile and produce different inlined builtins between "fresh process" and "Nth call in same process". - The std140 layout we verified matches IS correct — host Globals struct ↔ SPV decorate offsets line up. So it's not a UBO layout mismatch. Going back to all-runtime Module.init via glslang. ~5 MB heaptrack-only leak comes back; user-visible image rendering correctness restored. VulkanSpv.zig + src/vulkan_spvgen.zig + the build step stay in the tree (small build overhead, no runtime cost since the generated module isn't imported). A future investigation can turn it back on by adding back the @import("vulkan_spv") + 9 Module.initFromSpirv calls once we understand the cross-shader issue. Co-Authored-By: claude-flow --- src/renderer/vulkan/shaders.zig | 53 +++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/src/renderer/vulkan/shaders.zig b/src/renderer/vulkan/shaders.zig index 8382f5ee9..3f83e8a08 100644 --- a/src/renderer/vulkan/shaders.zig +++ b/src/renderer/vulkan/shaders.zig @@ -30,19 +30,11 @@ const DescriptorPool = vulkan.DescriptorPool; const Pipeline = @import("Pipeline.zig"); const math = @import("../../math.zig"); -/// Build-time-precompiled SPIR-V blobs for the 9 built-in -/// shaders. Generated by `src/build/VulkanSpv.zig`. Each decl -/// is `[]const u32` backed by a comptime-aligned u8 array -/// (align(@alignOf(u32))) so the underlying storage is on a -/// 4-byte boundary — Module.initFromSpirv can consume the -/// slices directly without alignment surgery. -/// -/// Skipping the runtime glslang call path for built-ins -/// eliminates the per-process TPoolAllocator high-water-mark -/// leak that the first-surface compile otherwise leaves -/// behind (~5 MB heaptrack delta). Custom (user) shaders still -/// go through glslang at runtime via shadertoy.spirvFromGlsl. -const vulkan_spv = @import("vulkan_spv"); +// Build-time-precompiled SPIR-V blobs are generated by +// `src/build/VulkanSpv.zig` and exposed as the `vulkan_spv` +// module — currently UNUSED at runtime; see the long +// explanation in `Shaders.init` for why we went back to +// runtime glslang compilation for built-ins. const log = std.log.scoped(.vulkan); @@ -920,16 +912,33 @@ pub const Shaders = struct { // runtime via shadertoy.spirvFromGlsl, and the per-frame // shadertoy post-pipeline below still allocates via // `alloc`, so the parameter is still load-bearing. + // All built-ins runtime-compiled via glslang. The build- + // time SPV precompile path (commits c921c4b2e / 99d26181e) + // saves ~5 MB of TPoolAllocator leak per heaptrack but + // somehow corrupts kitty image rendering (images fill the + // whole window). The diagnostic in 7c045d694... showed + // even runtime-compiling image_vert + image_frag while + // leaving the other 7 precompiled DIDN'T fix it — so the + // bug isn't local to those shaders' SPV, it's some cross- + // shader interaction (descriptor binding cross-talk? + // ProcessDeferred state divergence between fresh-process + // build-time vs sequential-compile runtime?) we don't + // understand yet. + // + // VulkanSpv.zig + src/vulkan_spvgen.zig + the generated + // vulkan_spv import stay in tree so a future investigation + // can flip the swap back on. ~5 MB cosmetic leak is the + // wrong trade-off vs visibly broken images. var modules: Modules = .{ - .bg_color_frag = try Module.initFromSpirv(device, vulkan_spv.bg_color_frag, .fragment), - .bg_image_frag = try Module.initFromSpirv(device, vulkan_spv.bg_image_frag, .fragment), - .bg_image_vert = try Module.initFromSpirv(device, vulkan_spv.bg_image_vert, .vertex), - .cell_bg_frag = try Module.initFromSpirv(device, vulkan_spv.cell_bg_frag, .fragment), - .cell_text_frag = try Module.initFromSpirv(device, vulkan_spv.cell_text_frag, .fragment), - .cell_text_vert = try Module.initFromSpirv(device, vulkan_spv.cell_text_vert, .vertex), - .full_screen_vert = try Module.initFromSpirv(device, vulkan_spv.full_screen_vert, .vertex), - .image_frag = try Module.initFromSpirv(device, vulkan_spv.image_frag, .fragment), - .image_vert = try Module.initFromSpirv(device, vulkan_spv.image_vert, .vertex), + .bg_color_frag = try Module.init(alloc, device, source.bg_color_frag, .fragment), + .bg_image_frag = try Module.init(alloc, device, source.bg_image_frag, .fragment), + .bg_image_vert = try Module.init(alloc, device, source.bg_image_vert, .vertex), + .cell_bg_frag = try Module.init(alloc, device, source.cell_bg_frag, .fragment), + .cell_text_frag = try Module.init(alloc, device, source.cell_text_frag, .fragment), + .cell_text_vert = try Module.init(alloc, device, source.cell_text_vert, .vertex), + .full_screen_vert = try Module.init(alloc, device, source.full_screen_vert, .vertex), + .image_frag = try Module.init(alloc, device, source.image_frag, .fragment), + .image_vert = try Module.init(alloc, device, source.image_vert, .vertex), }; errdefer { inline for (.{