Improve ShaderCreateStep to work around cache issues

README.md

@@ -242,7 +242,7 @@ pub const xcb_connection_t = if (@hasDecl(root, "xcb_connection_t")) root.xcb_co
 For some times (such as those from Google Games Platform) no default is known. Usage of these without providing a concrete type in the project root generates a compile error.
 
 ### Shader compilation
-vulkan-zig provides functionality to help compiling shaders using glslc. It can be used from build.zig as follows:
+vulkan-zig provides functionality to help compiling shaders to spir-v using glslc. It can be used from build.zig as follows:
 
 ```zig
 const vkgen = @import("vulkan-zig/generator/index.zig");
@@ -254,15 +254,20 @@ pub fn build(b: *Builder) void {
     const gen = vkgen.VkGenerateStep(b, "path/to/vk.xml", "vk.zig");
     exe.addPackage(gen.package);
 
-    const shader_comp = vkgen.ShaderCompileStep.init(
+    const shader_comp = vkgen.ShaderCompileStep.create(
         builder,
         &[_][]const u8{"glslc", "--target-env=vulkan1.2"}, // Path to glslc and additional parameters
     );
-    exe.step.dependOn(&shader_comp.step);
+    exe.addPackage(shader_comp.getPackage("shaders"));
-    const spv_path = shader_comp.addShader("path/to/shader.frag");
+    shader_comp.add("shader", "path/to/shader.frag", .{});
 }
 ```
-Upon compilation, glslc is then invoked to compile each shader, and the result is placed within `zig-cache`. `addShader` returns the full path to the compiled shader code. This file can then be included in the project, as is done in [build.zig for the example](build.zig) by generating an additional file which uses `@embedFile`.
+Upon compilation, glslc is then invoked to compile each shader, and the result is placed within `zig-cache`. All shaders which are compiled using a particular `ShaderCompileStep` are imported in a single Zig file using `@embedFile`, and this file can be added to an executable as a package using `getPackage`. To slightly improve compile times, shader compilation is cached; as long as a shader's source and its compile commands stay the same, the shader is not recompiled. The spir-v code for any particular shader is aligned to that of a 32-bit integer as follows, as required by vkCreateShaderModule:
+```zig
+pub const ${name} align(@alignOf(u32)) = @embedFile("${path}").*;
+```
+
+See [build.zig](build.zig) for a working example.
 
 ## Limitations
 * Currently, the self-hosted version of Zig's cache-hash API is not yet ready for usage, which means that the bindings are regenerated every time an executable is built.

build.zig

@@ -2,117 +2,6 @@ const std = @import("std");
 const vkgen = @import("generator/index.zig");
 const Step = std.build.Step;
 const Builder = std.build.Builder;
-const Encoder = std.base64.standard_no_pad.Encoder;
-
-pub const ResourceGenStep = struct {
-    step: Step,
-    shader_step: *vkgen.ShaderCompileStep,
-    builder: *Builder,
-    package: std.build.Pkg,
-    output_file: std.build.GeneratedFile,
-    resources: std.ArrayList(u8),
-    base64_sources: std.ArrayList(Base64Source),
-    const Base64Source = struct {
-        file_name: []u8,
-        str_name: []const u8,
-    };
-
-    pub fn init(builder: *Builder, out: []const u8) *ResourceGenStep {
-        const self = builder.allocator.create(ResourceGenStep) catch unreachable;
-        const full_out_path = std.fs.path.join(builder.allocator, &[_][]const u8{
-            builder.build_root,
-            builder.cache_root,
-            out,
-        }) catch unreachable;
-
-        self.* = .{
-            .step = Step.init(.custom, "resources", builder.allocator, make),
-            .shader_step = vkgen.ShaderCompileStep.init(builder, &[_][]const u8{ "glslc", "--target-env=vulkan1.2" }, "shaders"),
-            .builder = builder,
-            .package = .{
-                .name = "resources",
-                .source = .{ .generated = &self.output_file },
-                .dependencies = null,
-            },
-            .output_file = .{
-                .step = &self.step,
-                .path = full_out_path,
-            },
-            .resources = std.ArrayList(u8).init(builder.allocator),
-            .base64_sources = std.ArrayList(Base64Source).init(builder.allocator),
-        };
-
-        self.step.dependOn(&self.shader_step.step);
-        return self;
-    }
-
-    fn renderPath(path: []const u8, writer: anytype) void {
-        const separators = &[_]u8{ std.fs.path.sep_windows, std.fs.path.sep_posix };
-        var i: usize = 0;
-        while (std.mem.indexOfAnyPos(u8, path, i, separators)) |j| {
-            writer.writeAll(path[i..j]) catch unreachable;
-            switch (std.fs.path.sep) {
-                std.fs.path.sep_windows => writer.writeAll("\\\\") catch unreachable,
-                std.fs.path.sep_posix => writer.writeByte(std.fs.path.sep_posix) catch unreachable,
-                else => unreachable,
-            }
-
-            i = j + 1;
-        }
-        writer.writeAll(path[i..]) catch unreachable;
-    }
-
-    pub fn addShader(self: *ResourceGenStep, name: []const u8, source: []const u8) void {
-        const shader_out_path = self.shader_step.add(source, .{});
-        var writer = self.resources.writer();
-
-        writer.print("pub const {s} align(@alignOf(u32)) = @embedFile(\"", .{name}) catch unreachable;
-        renderPath(shader_out_path, writer);
-        writer.writeAll("\").*;\n") catch unreachable;
-    }
-
-    /// Instead of using @embedFile as addShader does, write SPIR-V binaries to resources as base64-encoded strings
-    pub fn addShaderBase64(self: *ResourceGenStep, name: []const u8, source: []const u8) void {
-        const shader_out_path = self.shader_step.add(source, .{});
-        var fixed_shader_out_path = std.ArrayList(u8).init(self.builder.allocator);
-        defer fixed_shader_out_path.deinit();
-        var path_writer = fixed_shader_out_path.writer();
-        renderPath(shader_out_path, path_writer);
-
-        var base64_source = self.base64_sources.addOne() catch unreachable;
-        base64_source.str_name = name;
-        base64_source.file_name = fixed_shader_out_path.toOwnedSlice() catch unreachable;
-    }
-
-    fn make(step: *Step) !void {
-        const self = @fieldParentPtr(ResourceGenStep, "step", step);
-        const cwd = std.fs.cwd();
-
-        // Read SPIR-V binaries, encode as base64 string, and write the base64 string to resources
-        for (self.base64_sources.items) |base64_source| {
-            const spv_file = std.fs.cwd().readFileAllocOptions(
-                self.builder.allocator,
-                base64_source.file_name,
-                std.math.pow(u32, 2, 21), // max spv file size 2^21 bytes ~= 2 MB
-                std.math.pow(u32, 2, 13), // size hint 2^13 ~= 8 KB
-                @alignOf(u32),
-                null,
-            ) catch unreachable;
-            defer self.builder.allocator.free(spv_file);
-
-            const base64_len = Encoder.calcSize(spv_file.len);
-            var base64_str_mem = self.builder.allocator.alloc(u8, base64_len) catch unreachable;
-            const base64_str = Encoder.encode(base64_str_mem, spv_file);
-
-            var writer = self.resources.writer();
-            writer.print("pub const {s} = \"{s}\";\n", .{base64_source.str_name, base64_str}) catch unreachable;
-        }
-
-        const dir = std.fs.path.dirname(self.output_file.path.?).?;
-        try cwd.makePath(dir);
-        try cwd.writeFile(self.output_file.path.?, self.resources.items);
-    }
-};
 
 pub fn build(b: *Builder) void {
     const target = b.standardTargetOptions(.{});
@@ -135,10 +24,13 @@ pub fn build(b: *Builder) void {
     const gen = vkgen.VkGenerateStep.init(b, vk_xml_path, "vk.zig");
     triangle_exe.addPackage(gen.package);
 
-    const res = ResourceGenStep.init(b, "resources.zig");
+    const shaders = vkgen.ShaderCompileStep.create(
-    res.addShaderBase64("triangle_vert", "examples/shaders/triangle.vert");
+        b,
-    res.addShaderBase64("triangle_frag", "examples/shaders/triangle.frag");
+        &[_][]const u8{ "glslc", "--target-env=vulkan1.2" },
-    triangle_exe.addPackage(res.package);
+    );
+    shaders.add("triangle_vert", "examples/shaders/triangle.vert", .{});
+    shaders.add("triangle_frag", "examples/shaders/triangle.frag", .{});
+    triangle_exe.addPackage(shaders.getPackage("shaders"));
 
     const triangle_run_cmd = triangle_exe.run();
     triangle_run_cmd.step.dependOn(b.getInstallStep());

examples/triangle.zig

@@ -1,7 +1,7 @@
 const std = @import("std");
 const vk = @import("vulkan");
 const c = @import("c.zig");
-const resources = @import("resources");
+const shaders = @import("shaders");
 const GraphicsContext = @import("graphics_context.zig").GraphicsContext;
 const Swapchain = @import("swapchain.zig").Swapchain;
 const Allocator = std.mem.Allocator;
@@ -44,6 +44,11 @@ pub fn main() !void {
     if (c.glfwInit() != c.GLFW_TRUE) return error.GlfwInitFailed;
     defer c.glfwTerminate();
 
+    if (c.glfwVulkanSupported() != c.GLFW_TRUE) {
+        std.log.err("GLFW could not find libvulkan", .{});
+        return error.NoVulkan;
+    }
+
     var extent = vk.Extent2D{ .width = 800, .height = 600 };
 
     c.glfwWindowHint(c.GLFW_CLIENT_API, c.GLFW_NO_API);
@@ -338,7 +343,7 @@ fn createRenderPass(gc: *const GraphicsContext, swapchain: Swapchain) !vk.Render
         .store_op = .store,
         .stencil_load_op = .dont_care,
         .stencil_store_op = .dont_care,
-        .initial_layout = .@"undefined",
+        .initial_layout = .undefined,
         .final_layout = .present_src_khr,
     };
 
@@ -378,15 +383,15 @@ fn createPipeline(
 ) !vk.Pipeline {
     const vert = try gc.vkd.createShaderModule(gc.dev, &.{
         .flags = .{},
-        .code_size = resources.triangle_vert.len,
+        .code_size = shaders.triangle_vert.len,
-        .p_code = @ptrCast([*]const u32, &resources.triangle_vert),
+        .p_code = @ptrCast([*]const u32, &shaders.triangle_vert),
     }, null);
     defer gc.vkd.destroyShaderModule(gc.dev, vert, null);
 
     const frag = try gc.vkd.createShaderModule(gc.dev, &.{
         .flags = .{},
-        .code_size = resources.triangle_frag.len,
+        .code_size = shaders.triangle_frag.len,
-        .p_code = @ptrCast([*]const u32, &resources.triangle_frag),
+        .p_code = @ptrCast([*]const u32, &shaders.triangle_frag),
     }, null);
     defer gc.vkd.destroyShaderModule(gc.dev, frag, null);
 

generator/build_integration.zig

@@ -2,6 +2,7 @@ const std = @import("std");
 const path = std.fs.path;
 const Builder = std.build.Builder;
 const Step = std.build.Step;
+const GeneratedFile = std.build.GeneratedFile;
 
 /// Stage the shader should be built for. This is passed to the -fshader-stage
 /// argument when invoking glslc.
@@ -18,70 +19,137 @@ pub const ShaderStage = enum {
 /// Invokes glslc (or another shader compiler passed to `init`) for each shader
 /// added via `addShader`.
 pub const ShaderCompileStep = struct {
-    const AddFileParams = struct {
+    /// The directory within the zig-cache directory that is used to store
+    /// shader artifacts.
+    pub const cache_dir = "shaders";
+
+    /// This structure contains additional options that can be passed to glslc when shaders are compiled.
+    pub const ShaderOptions = struct {
+        /// The entry point to use when compiling the shader.
         entry_point: ?[]const u8 = null,
+
+        /// The stage to use when building. If not null, this is passed to
+        /// the -fshader-stage argument.
         stage: ?ShaderStage = null,
-        output_filename: ?[]const u8 = null,
+
+        /// To ensure that if compilation options change, the shader is recompiled
+        /// properly.
+        fn hash(self: ShaderOptions, hasher: anytype) void {
+            if (self.entry_point) |entry_point| {
+                hasher.update(entry_point);
+            }
+            if (self.stage) |stage| {
+                hasher.update(std.mem.asBytes(&@enumToInt(stage)));
+            }
+        }
     };
 
     /// Structure representing a shader to be compiled.
     const Shader = struct {
+        /// The name of the shader in the generated file.
+        /// Must be unique for all shaders added to this ShaderCompileStep.
+        name: []const u8,
+
         /// The path to the shader, relative to the current build root.
         source_path: []const u8,
 
-        /// The full output path where the compiled shader binary is placed.
+        /// Miscellaneous options to pass when compiling the shader.
-        full_out_path: []const u8,
+        options: ShaderOptions,
-
-        /// The entry point to use when compiling the shader.
-        entry_point: ?[]const u8,
-
-        /// The stage to use when building. If not null, this is passed to
-        /// the -fshader-stage argument.
-        stage: ?ShaderStage,
     };
 
     step: Step,
-    builder: *Builder,
+    b: *Builder,
 
     /// The command and optional arguments used to invoke the shader compiler.
     glslc_cmd: []const []const u8,
 
-    /// The directory within `zig-cache/` that the compiled shaders are placed in.
-    output_dir: []const u8,
-
     /// List of shaders that are to be compiled.
     shaders: std.ArrayList(Shader),
 
-    /// Create a ShaderCompilerStep for `builder`. When this step is invoked by the build
+    /// The main Zig file that contains all the shaders. Each shader is included as
-    /// system, `<glcl_cmd...> <shader_source> -o <dst_addr>` is invoked for each shader.
+    /// `pub const ${name} align(@alignOf(u32))= @embedFile("${path").*;`
-    pub fn init(builder: *Builder, glslc_cmd: []const []const u8, output_dir: []const u8) *ShaderCompileStep {
+    generated_file: GeneratedFile,
+
+    /// Create a ShaderCompileStep for `builder`. When this step is invoked by the build
+    /// system, `<glcl_cmd...> <shader_source> -o <path>` is invoked for each shader.
+    pub fn create(builder: *Builder, glslc_cmd: []const []const u8) *ShaderCompileStep {
         const self = builder.allocator.create(ShaderCompileStep) catch unreachable;
         self.* = .{
-            .step = Step.init(.custom, "shader-compile", builder.allocator, make),
+            .step = Step.init(.custom, "shaders", builder.allocator, make),
-            .builder = builder,
+            .b = builder,
-            .output_dir = output_dir,
             .glslc_cmd = builder.dupeStrings(glslc_cmd),
             .shaders = std.ArrayList(Shader).init(builder.allocator),
+            .generated_file = undefined,
         };
+        self.generated_file = .{ .step = &self.step };
         return self;
     }
 
+    /// Returns the shaders package with name `package_name`.
+    pub fn getPackage(self: *ShaderCompileStep, package_name: []const u8) std.build.Pkg {
+        return .{ .name = package_name, .source = self.getSource() };
+    }
+
+    /// Returns the file source for the generated shader resource code.
+    pub fn getSource(self: *ShaderCompileStep) std.build.FileSource {
+        return .{ .generated = &self.generated_file };
+    }
+
     /// Add a shader to be compiled. `src` is shader source path, relative to the project root.
     /// Returns the full path where the compiled binary will be stored upon successful compilation.
     /// This path can then be used to include the binary into an executable, for example by passing it
     /// to @embedFile via an additional generated file. `entry_point` is the entry point to pass to the compiler.
     /// `stage` is an optional shader stage to pass to the compiler with the flag `-fshader-stage` when building the shader.
-    pub fn add(self: *ShaderCompileStep, src: []const u8, params: AddFileParams) []const u8 {
+    pub fn add(self: *ShaderCompileStep, name: []const u8, src: []const u8, options: ShaderOptions) void {
-        const full_out_path = path.join(self.builder.allocator, &[_][]const u8{
+        const full_source_path = std.fs.path.join(self.b.allocator, &.{
-            self.builder.build_root,
+            self.b.build_root,
-            self.builder.cache_root,
+            src,
-            if (params.output_filename) |out| out else std.fmt.allocPrint(self.builder.allocator, "{s}.spv", .{src}) catch unreachable,
         }) catch unreachable;
-        var src_cpy = self.builder.allocator.alloc(u8, src.len) catch unreachable;
+        self.shaders.append(.{
-        _ = std.mem.copy(u8, src_cpy, src);
+            .name = name,
+            .source_path = full_source_path,
+            .options = options,
+        }) catch unreachable;
+    }
 
-        self.shaders.append(.{ .source_path = src_cpy, .full_out_path = full_out_path, .entry_point = params.entry_point, .stage = params.stage }) catch unreachable;
+    /// Create a hash of a shader's source contents.
-        return full_out_path;
+    fn hashShaderToFileName(self: *ShaderCompileStep, shader: Shader) ![64]u8 {
+        const source = std.fs.cwd().readFileAlloc(
+            self.b.allocator,
+            shader.source_path,
+            std.math.maxInt(usize),
+        ) catch |err| switch (err) {
+            error.FileNotFound => {
+                std.log.err("could not open shader '{s}'", .{shader.source_path});
+                return error.FileNotFound;
+            },
+            else => |e| return e,
+        };
+
+        var hasher = std.crypto.hash.blake2.Blake2b384.init(.{});
+        // Random bytes to make ShaderCompileStep unique. Refresh with new random
+        // bytes when the implementation is changed in a non-backwards-compatible way.
+        hasher.update("Pw7Z*9Q8r!fLY8&!");
+        // Make sure that there is no cache hit if the shader's source has changed.
+        hasher.update(source);
+        // Not only the shader source must be the same to ensure uniqueness -
+        // the compilation options must be the same as well!
+        shader.options.hash(&hasher);
+        // And the compile command, too.
+        for (self.glslc_cmd) |cmd| {
+            hasher.update(cmd);
+        }
+
+        return digest(&hasher);
+    }
+
+    /// Create a base-64 hash digest from a hasher, which we can use as file name.
+    fn digest(hasher: anytype) [64]u8 {
+        var hash_digest: [48]u8 = undefined;
+        hasher.final(&hash_digest);
+        var hash: [64]u8 = undefined;
+        _ = std.fs.base64_encoder.encode(&hash, &hash_digest);
+        return hash;
     }
 
     /// Internal build function.
@@ -89,26 +157,69 @@ pub const ShaderCompileStep = struct {
         const self = @fieldParentPtr(ShaderCompileStep, "step", step);
         const cwd = std.fs.cwd();
 
-        var cmd = std.ArrayList([]const u8).init(self.builder.allocator);
+        var cmd = std.ArrayList([]const u8).init(self.b.allocator);
         try cmd.appendSlice(self.glslc_cmd);
         const base_cmd_len = cmd.items.len;
 
+        var shaders_file_contents = std.ArrayList(u8).init(self.b.allocator);
+        const shaders_out = shaders_file_contents.writer();
+
+        const shaders_dir = try std.fs.path.join(
+            self.b.allocator,
+            &.{ self.b.build_root, self.b.cache_root, cache_dir },
+        );
+        try cwd.makePath(shaders_dir);
+
         for (self.shaders.items) |shader| {
+            const shader_basename = try self.hashShaderToFileName(shader);
+            const shader_out_path = try std.fs.path.join(self.b.allocator, &.{
+                shaders_dir,
+                &shader_basename,
+            });
+
+            // This path must be relative to the shaders zig file - which is in the same directory
+            try shaders_out.print("pub const {s} align(@alignOf(u32)) = @embedFile(\"{s}\").*;\n", .{
+                shader.name,
+                &shader_basename,
+            });
+
+            // If we have a cache hit, we can save some compile time by not invoking glslc.
+            compile_shader: {
+                std.fs.accessAbsolute(shader_out_path, .{}) catch |err| switch (err) {
+                    error.FileNotFound => break :compile_shader,
+                    else => |e| return e,
+                };
+
+                continue;
+            }
+
             cmd.items.len = base_cmd_len;
 
-            if (shader.entry_point) |entry_point| {
+            if (shader.options.entry_point) |entry_point| {
-                try cmd.append(try std.fmt.allocPrint(self.builder.allocator, "-fentry-point={s}", .{entry_point}));
+                try cmd.append(try std.fmt.allocPrint(self.b.allocator, "-fentry-point={s}", .{entry_point}));
             }
 
-            if (shader.stage) |stage| {
+            if (shader.options.stage) |stage| {
-                try cmd.append(try std.fmt.allocPrint(self.builder.allocator, "-fshader-stage={s}", .{@tagName(stage)}));
+                try cmd.append(try std.fmt.allocPrint(self.b.allocator, "-fshader-stage={s}", .{@tagName(stage)}));
             }
 
-            const dir = path.dirname(shader.full_out_path).?;
+            try cmd.appendSlice(&.{ shader.source_path, "-o", shader_out_path });
-            try cwd.makePath(dir);
+            try self.b.spawnChild(cmd.items);
-
-            try cmd.appendSlice(&.{ shader.source_path, "-o", shader.full_out_path });
-            try self.builder.spawnChild(cmd.items);
         }
+
+        // Generate a file name for the shaders zig source based on the contents of shaders_file_contents.
+        // In this case we don't need to omit writing the file - Zig does this check already for us.
+        var hasher = std.crypto.hash.blake2.Blake2b384.init(.{});
+        // Note: don't need to seed the hasher - it transitively contains the seed from
+        // hashShaderToFileName. Change that if the implementation changes.
+        hasher.update(shaders_file_contents.items);
+
+        const shaders_path = try std.fs.path.join(
+            self.b.allocator,
+            &.{ shaders_dir, &digest(&hasher) },
+        );
+
+        try cwd.writeFile(shaders_path, shaders_file_contents.items);
+        self.generated_file.path = shaders_path;
     }
 };