From 2ee457d5ba3d1ae1f426510657f7ec687d2fc91c Mon Sep 17 00:00:00 2001 From: ntomsic Date: Mon, 25 May 2026 18:49:30 -0500 Subject: [PATCH] pkg/glslang: typed Zig wrapper for the Vulkan compile shim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `pkg/glslang/vk.zig` exposing `compileToSpv(alloc, source, stage)` with a `Stage` enum, owning the malloc/free dance for the shim's out-pointers (separate free entry points for SPIR-V vs error string, both optional, both have to be dropped on the right path). Same shape step 1 used to promote the Vulkan binding: the renderer should consume `glslang.vk.*` typed APIs, not poke `glslang.c.ghastty_*` directly. Removed the two near-identical 25-line blocks of raw shim plumbing from `src/renderer/vulkan/shaders.zig` (production `Module.init` and the test-side `compileToSpv` helper). Net: +23 / -76 in shaders.zig. Local `Stage.glslangStage()` was dead and is dropped; new `vkBindingStage()` maps the renderer's `Stage` to `glslang.vk.Stage`. Verified via Docker (zig 0.15.2 linux-arm64): zig build -Drenderer=vulkan -Dapp-runtime=none → clean zig build -Drenderer=opengl -Dapp-runtime=none → clean Step 2 of 6 in the PR-17 review refactor. Co-Authored-By: claude-flow --- pkg/glslang/main.zig | 1 + pkg/glslang/vk.zig | 88 +++++++++++++++++++++++++++++ src/renderer/vulkan/shaders.zig | 98 ++++++++------------------------- 3 files changed, 111 insertions(+), 76 deletions(-) create mode 100644 pkg/glslang/vk.zig diff --git a/pkg/glslang/main.zig b/pkg/glslang/main.zig index 2743650c6..e9c835b10 100644 --- a/pkg/glslang/main.zig +++ b/pkg/glslang/main.zig @@ -4,6 +4,7 @@ const shader = @import("shader.zig"); pub const c = @import("c.zig").c; pub const testing = @import("test.zig"); +pub const vk = @import("vk.zig"); pub const init = initpkg.init; pub const finalize = initpkg.finalize; diff --git a/pkg/glslang/vk.zig b/pkg/glslang/vk.zig new file mode 100644 index 000000000..e418bcc5b --- /dev/null +++ b/pkg/glslang/vk.zig @@ -0,0 +1,88 @@ +//! Typed Zig wrapper around the Ghastty Vulkan-friendly glslang +//! compile shim (`pkg/glslang/override/ghastty_vk_shim.h`). The shim +//! itself is a small C entry point that wraps glslang's C++-only +//! `setAutoMapBindings` / `setAutoMapLocations` / `setEnvInput` knobs +//! the upstream C ABI doesn't expose. +//! +//! Callers use this instead of poking `glslang.c.ghastty_*` directly: +//! the malloc/free dance for the shim's out-pointers is finicky +//! (separate free entry points for SPIR-V and error strings, both +//! optional, both have to be dropped on the right path) and was +//! previously open-coded across two near-identical 25-line blocks +//! in `src/renderer/vulkan/shaders.zig`. This module is the binding +//! layer; the renderer just calls `compileToSpv` and gets a Zig +//! `[]const u32` slice. + +const std = @import("std"); +const Allocator = std.mem.Allocator; + +const c = @import("c.zig").c; + +const log = std.log.scoped(.glslang); + +pub const Stage = enum { + vertex, + fragment, + + fn cValue(self: Stage) c.ghastty_glslang_stage_t { + return switch (self) { + .vertex => c.GHASTTY_GLSLANG_STAGE_VERTEX, + .fragment => c.GHASTTY_GLSLANG_STAGE_FRAGMENT, + }; + } +}; + +pub const Error = error{ + /// `glslang_shader_preprocess` / `_parse` / `_program_link` / + /// `_program_SPIRV_generate` failed. The shim's error message + /// is logged via `std.log.err` before this error is returned — + /// no allocation is propagated to the caller. + GlslangFailed, +} || Allocator.Error; + +/// Compile a null-terminated GLSL source string to a Vulkan-flavored +/// SPIR-V binary. +/// +/// On success, returns a slice owned by `alloc`; the caller frees with +/// `alloc.free(spv)`. The shim hands back its own malloc'd buffer +/// which we copy into `alloc` so the caller's `defer alloc.free` works +/// without remembering a separate `ghastty_glslang_free_spirv` call. +/// +/// On failure, the shim's error string is logged with `std.log.err` +/// and `error.GlslangFailed` is returned — the C-side malloc'd error +/// buffer is freed before returning so callers don't have to. +pub fn compileToSpv( + alloc: Allocator, + source: [:0]const u8, + stage: Stage, +) Error![]const u32 { + var spv_ptr: [*c]u32 = undefined; + var spv_len: usize = 0; + var err_ptr: [*c]u8 = undefined; + + const rc = c.ghastty_glslang_compile_vulkan( + source.ptr, + stage.cValue(), + &spv_ptr, + &spv_len, + &err_ptr, + ); + if (rc != 0) { + if (err_ptr != null) { + log.err("ghastty_glslang_compile_vulkan: {s}", .{ + std.mem.span(@as([*:0]const u8, @ptrCast(err_ptr))), + }); + c.ghastty_glslang_free_error(err_ptr); + } else { + log.err("ghastty_glslang_compile_vulkan: unspecified failure", .{}); + } + return error.GlslangFailed; + } + defer c.ghastty_glslang_free_spirv(spv_ptr); + + // Copy out of the shim's malloc into `alloc` so the caller's + // free path is symmetric with every other allocator-owned slice. + const owned = try alloc.alloc(u32, spv_len); + @memcpy(owned, spv_ptr[0..spv_len]); + return owned; +} diff --git a/src/renderer/vulkan/shaders.zig b/src/renderer/vulkan/shaders.zig index ed2867b73..92b138d1e 100644 --- a/src/renderer/vulkan/shaders.zig +++ b/src/renderer/vulkan/shaders.zig @@ -103,10 +103,15 @@ pub const Stage = enum { vertex, fragment, - fn glslangStage(self: Stage) c_uint { + /// Map to the binding-layer enum that `glslang.vk.compileToSpv` + /// accepts. Same shape, different module — keeping the enum at + /// this level so the renderer's `.vertex` / `.fragment` literals + /// stay backend-flavored (the `vk_*` field on the struct also + /// reads off this enum). + fn vkBindingStage(self: Stage) glslang.vk.Stage { return switch (self) { - .vertex => glslang.c.GLSLANG_STAGE_VERTEX, - .fragment => glslang.c.GLSLANG_STAGE_FRAGMENT, + .vertex => .vertex, + .fragment => .fragment, }; } @@ -515,12 +520,12 @@ pub const Module = struct { /// The source is run through `vulkanizeGlsl` to swap OpenGL-only /// builtins for their Vulkan equivalents (`gl_VertexID` → /// `gl_VertexIndex`, `gl_InstanceID` → `gl_InstanceIndex`); then - /// the Ghastty Vulkan compile shim - /// (`pkg/glslang/override/ghastty_vk_shim.cpp`) finishes the job - /// with auto-map bindings / locations enabled. Same path covers - /// the renderer's built-in shaders AND user-supplied custom - /// shaders, so the OpenGL-flavored GLSL Ghostty already speaks - /// keeps working. + /// `glslang.vk.compileToSpv` (typed wrapper around the Vulkan + /// compile shim in `pkg/glslang/override/ghastty_vk_shim.cpp`) + /// finishes the job with auto-map bindings / locations enabled. + /// Same path covers the renderer's built-in shaders AND + /// user-supplied custom shaders, so the OpenGL-flavored GLSL + /// Ghostty already speaks keeps working. pub fn init( alloc: std.mem.Allocator, device: *const Device, @@ -540,36 +545,8 @@ pub const Module = struct { const translated = try vulkanizeGlsl(alloc, src); defer alloc.free(translated); - const c = glslang.c; - const c_stage: c.ghastty_glslang_stage_t = switch (stage) { - .vertex => c.GHASTTY_GLSLANG_STAGE_VERTEX, - .fragment => c.GHASTTY_GLSLANG_STAGE_FRAGMENT, - }; - - var spv_ptr: [*c]u32 = undefined; - var spv_len: usize = 0; - var err_ptr: [*c]u8 = undefined; - const rc = c.ghastty_glslang_compile_vulkan( - translated.ptr, - c_stage, - &spv_ptr, - &spv_len, - &err_ptr, - ); - if (rc != 0) { - if (err_ptr != null) { - log.err("ghastty_glslang_compile_vulkan: {s}", .{ - std.mem.span(@as([*:0]const u8, @ptrCast(err_ptr))), - }); - c.ghastty_glslang_free_error(err_ptr); - } else { - log.err("ghastty_glslang_compile_vulkan: unspecified failure", .{}); - } - return error.GlslangFailed; - } - defer c.ghastty_glslang_free_spirv(spv_ptr); - - const spv: []const u32 = spv_ptr[0..spv_len]; + const spv = try glslang.vk.compileToSpv(alloc, translated, stage.vkBindingStage()); + defer alloc.free(spv); return try initFromSpirv(device, spv, stage); } @@ -1587,11 +1564,11 @@ test "vulkanizeGlsl: layout with pre-existing set qualifier is unchanged" { // // `vulkanizeGlsl` unit tests above exercise the textual rewrite in // isolation. The integration tests below feed the rewriter's output -// through glslang via `ghastty_glslang_compile_vulkan` and assert -// the result is a valid SPIR-V binary. That covers the seam where -// a syntactically-fine rewrite still produces something glslang -// rejects (e.g. a `set = N` on a declaration glslang's -// `--auto-map-bindings` is also trying to assign). +// through `glslang.vk.compileToSpv` and assert the result is a valid +// SPIR-V binary. That covers the seam where a syntactically-fine +// rewrite still produces something glslang rejects (e.g. a `set = N` +// on a declaration glslang's `--auto-map-bindings` is also trying +// to assign). fn compileToSpv( alloc: std.mem.Allocator, @@ -1599,40 +1576,9 @@ fn compileToSpv( stage: Stage, ) ![]const u32 { glslang.testing.ensureInit() catch return error.GlslangFailed; - const translated = try vulkanizeGlsl(alloc, src); defer alloc.free(translated); - - var spv_ptr: [*c]u32 = undefined; - var spv_len: usize = 0; - var err_ptr: [*c]u8 = undefined; - const c_stage: glslang.c.ghastty_glslang_stage_t = switch (stage) { - .vertex => glslang.c.GHASTTY_GLSLANG_STAGE_VERTEX, - .fragment => glslang.c.GHASTTY_GLSLANG_STAGE_FRAGMENT, - }; - const rc = glslang.c.ghastty_glslang_compile_vulkan( - translated.ptr, - c_stage, - &spv_ptr, - &spv_len, - &err_ptr, - ); - if (rc != 0) { - if (err_ptr != null) { - std.log.err("compileToSpv: {s}", .{ - std.mem.span(@as([*:0]const u8, @ptrCast(err_ptr))), - }); - glslang.c.ghastty_glslang_free_error(err_ptr); - } - return error.GlslangFailed; - } - // Caller owns; copy out of glslang's malloc into the test allocator - // so cleanup is symmetric (the caller `defer alloc.free(out)`s). - const spv_words = spv_ptr[0..spv_len]; - const owned = try alloc.alloc(u32, spv_len); - @memcpy(owned, spv_words); - glslang.c.ghastty_glslang_free_spirv(spv_ptr); - return owned; + return try glslang.vk.compileToSpv(alloc, translated, stage.vkBindingStage()); } test "glslang integration: built-in bg_color fragment compiles" {