From f79c0f71b5d20da6ed679eaf0228f224e5149188 Mon Sep 17 00:00:00 2001 From: Nathan Date: Sun, 24 May 2026 17:42:44 -0500 Subject: [PATCH] renderer/vulkan: image + bg_image pipelines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two remaining built-in pipelines now exist. Kitty graphics images (`image`, drawn at four z-orders: kitty_below_bg, kitty_below_text, kitty_above_text, overlay) and the `background-image` config option (`bg_image`) both work on Vulkan. Pipeline shape: set 0 binding 1 Globals UBO (vert+frag) set 1 binding 0 combined image sampler (the kitty image / bg-image texture; normalized sampling) Both pipelines use a shared normalized-linear `image_sampler` (distinct from the unnormalized `atlas_sampler` cell_text uses for its glyph atlases) — the two sampler configs are mutually exclusive in a single `VkSampler`. Vertex inputs are per-instance struct attribs: image: `Image` (48B): grid_pos vec2, cell_offset vec2, source_rect vec4, dest_size vec2. bg_image: `BgImage` (8B): opacity float, info uint8. Two ancillary fixes piggyback: 1. `RenderPass` caches the last non-null `Step.uniforms` and reuses it when subsequent steps don't supply one. The renderer's `image.zig:draw` records image draws WITHOUT passing a UBO — OpenGL gets away with this because the previously-bound UBO sticks; Vulkan needs explicit per-pipeline descriptor updates, so the cache keeps the projection_matrix / cell_size uniforms alive for the image pipeline's vertex shader. 2. `RenderPass.complete` transitions to SHADER_READ_ONLY_OPTIMAL for `.texture` attachments (read by a subsequent pass's sampler) and stays in GENERAL for `.target` attachments (the dmabuf, which `Target.recordCopyToDmabuf` re-transitions anyway). The custom-shader path's `back_texture` is now in the right layout when the post pass samples it; previously the descriptor write declared SHADER_READ_ONLY but the texture sat in GENERAL, tripping VUID-vkCmdDraw-imageLayout-00344 on every sampled draw call. Validation is now clean except for the pre-existing Qt-side device-extension warning. Co-Authored-By: claude-flow --- src/renderer/vulkan/RenderPass.zig | 55 ++++++++++-- src/renderer/vulkan/shaders.zig | 136 +++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+), 9 deletions(-) diff --git a/src/renderer/vulkan/RenderPass.zig b/src/renderer/vulkan/RenderPass.zig index 19a7c2f68..ed1f70d32 100644 --- a/src/renderer/vulkan/RenderPass.zig +++ b/src/renderer/vulkan/RenderPass.zig @@ -99,6 +99,16 @@ cb: vk.VkCommandBuffer, device: *const Device, step_number: usize = 0, +/// Last `Step.uniforms` value seen in this pass. The OpenGL backend +/// keeps the bound UBO across draw calls implicitly (GL state +/// persists), and the renderer's image/overlay draw calls in +/// `image.zig` don't pass `uniforms` at all — they expect the +/// previously-bound UBO to still be live. Vulkan needs explicit +/// descriptor-set updates per pipeline, so we cache the last UBO +/// buffer here and reuse it when a step doesn't supply one. Reset +/// to null at `begin`. +last_uniforms: ?vk.VkBuffer = null, + /// Begin a render pass. Transitions the first attachment to /// `COLOR_ATTACHMENT_OPTIMAL` and opens a `vkCmdBeginRendering` /// scope with the caller's clear color (defaults to opaque black). @@ -298,8 +308,13 @@ pub fn step(self: *Self, s: Step) void { // tiny (1 UBO + a handful of storage buffers + a handful of // samplers) so batching wouldn't move the needle. - // UBO (set 0) - if (s.pipeline.descriptor_sets[0] != null) if (s.uniforms) |ubo_buffer| { + // UBO (set 0). The OpenGL backend's image/overlay draws don't + // pass `uniforms` — they expect the previously-bound UBO to + // persist. Fall back to `last_uniforms` when the Step doesn't + // supply one. Track the new one for later steps. + const ubo: ?vk.VkBuffer = s.uniforms orelse self.last_uniforms; + if (s.uniforms) |b| self.last_uniforms = b; + if (s.pipeline.descriptor_sets[0] != null) if (ubo) |ubo_buffer| { const buffer_info: vk.VkDescriptorBufferInfo = .{ .buffer = ubo_buffer, .offset = 0, @@ -333,6 +348,7 @@ pub fn step(self: *Self, s: Step) void { s.pipeline.sampler else continue; + const image_info: vk.VkDescriptorImageInfo = .{ .sampler = sampler_handle, .imageView = tex.view, @@ -432,18 +448,39 @@ pub fn complete(self: *const Self) void { self.device.dispatch.cmdEndRendering(self.cb); - const image: vk.VkImage = switch (self.attachments[0].target) { - .texture => |t| t.image, - .target => |t| t.image, - }; + // Final layout depends on what consumes the attachment next. + // A `.texture` attachment is the custom-shader back_texture, read + // by the post pass's sampler — transition to SHADER_READ_ONLY so + // the descriptor write's declared layout matches reality + // (otherwise validation flags VUID-vkCmdDraw-imageLayout-00344 + // and some drivers can mishandle sampling from an out-of-spec + // layout). A `.target` attachment is the dmabuf-backed + // `frame.target`; the next op is + // `Target.recordCopyToDmabuf` which transitions from GENERAL + // anyway, so leave it in GENERAL here. + const image: vk.VkImage, const new_layout: vk.VkImageLayout, const dst_stage: vk.VkPipelineStageFlags, const dst_access: vk.VkAccessFlags = + switch (self.attachments[0].target) { + .texture => |t| .{ + t.image, + vk.VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, + vk.VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT, + vk.VK_ACCESS_SHADER_READ_BIT, + }, + .target => |t| .{ + t.image, + vk.VK_IMAGE_LAYOUT_GENERAL, + vk.VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, + 0, + }, + }; const barrier: vk.VkImageMemoryBarrier = .{ .sType = vk.VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER, .pNext = null, .srcAccessMask = vk.VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, - .dstAccessMask = 0, + .dstAccessMask = dst_access, .oldLayout = vk.VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, - .newLayout = vk.VK_IMAGE_LAYOUT_GENERAL, + .newLayout = new_layout, .srcQueueFamilyIndex = vk.VK_QUEUE_FAMILY_IGNORED, .dstQueueFamilyIndex = vk.VK_QUEUE_FAMILY_IGNORED, .image = image, @@ -458,7 +495,7 @@ pub fn complete(self: *const Self) void { self.device.dispatch.cmdPipelineBarrier( self.cb, vk.VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, - vk.VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, + dst_stage, 0, 0, null, 0, null, diff --git a/src/renderer/vulkan/shaders.zig b/src/renderer/vulkan/shaders.zig index ad5f8ef39..e3055b96c 100644 --- a/src/renderer/vulkan/shaders.zig +++ b/src/renderer/vulkan/shaders.zig @@ -716,6 +716,14 @@ pub const Shaders = struct { /// `deinit`. atlas_sampler: ?Sampler = null, + /// Sampler used by the image + bg_image pipelines. Normalized, + /// linear filter, clamp-to-edge — sampled in shadertoy/normal + /// 2D fashion. Separate from `atlas_sampler` because that one + /// uses unnormalized coords for the cell-text glyph atlases; + /// the two requirements are mutually exclusive in a single + /// `VkSampler`. + image_sampler: ?Sampler = null, + defunct: bool = false, /// The compiled `VkShaderModule`s for the renderer's built-in @@ -1037,10 +1045,136 @@ pub const Shaders = struct { }); errdefer cell_text_pipeline.deinit(); + // ---- image pipeline (kitty graphics, overlay) ------------ + // + // Per-instance fullscreen quad (triangle-strip, 4 verts) that + // draws ONE image rectangle into the grid. The renderer's + // `image.zig:draw` records one Step per visible image + // placement, each with its own VkBuffer (single + // `Image`-struct instance) and texture. + // + // Bindings after `vulkanizeGlsl`: + // set 0 binding 1 Globals UBO (vertex stage: + // projection_matrix + cell_size; fragment + // stage: `bools` for linear-blending check) + // set 1 binding 0 combined image sampler (the kitty image + // texture — sampled normalized; pipeline's + // owned `image_sampler` is the fallback + // since the renderer doesn't pass a + // Sampler with the Step). + const image_ubo_dsl = try createSingleBindingDsl( + device, + 1, + vk.VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, + vk.VK_SHADER_STAGE_VERTEX_BIT | vk.VK_SHADER_STAGE_FRAGMENT_BIT, + ); + tracker.track(image_ubo_dsl); + const image_sampler_dsl = try createSingleBindingDsl( + device, + 0, + vk.VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, + vk.VK_SHADER_STAGE_VERTEX_BIT | vk.VK_SHADER_STAGE_FRAGMENT_BIT, + ); + tracker.track(image_sampler_dsl); + + // Vertex input: `Image` struct (48 bytes after alignment). + // Attributes match the GLSL `layout(location = N) in ...` + // declarations in `image.v.glsl`. + const image_attrs = [_]vk.VkVertexInputAttributeDescription{ + .{ .location = 0, .binding = 0, .format = vk.VK_FORMAT_R32G32_SFLOAT, .offset = @offsetOf(Image, "grid_pos") }, + .{ .location = 1, .binding = 0, .format = vk.VK_FORMAT_R32G32_SFLOAT, .offset = @offsetOf(Image, "cell_offset") }, + .{ .location = 2, .binding = 0, .format = vk.VK_FORMAT_R32G32B32A32_SFLOAT, .offset = @offsetOf(Image, "source_rect") }, + .{ .location = 3, .binding = 0, .format = vk.VK_FORMAT_R32G32_SFLOAT, .offset = @offsetOf(Image, "dest_size") }, + }; + + // Normalized linear sampler shared by image + bg_image. Kept + // alongside `atlas_sampler` (which is unnormalized) so the + // two consumers don't fight over a single shared sampler's + // properties. + const image_sampler = try Sampler.init(.{ + .device = device, + .min_filter = .linear, + .mag_filter = .linear, + .wrap_s = .clamp_to_edge, + .wrap_t = .clamp_to_edge, + }); + errdefer image_sampler.deinit(); + + const image_pipeline = try Pipeline.init(.{ + .device = device, + .descriptor_pool = &pool, + .vertex_module = modules.image_vert.handle, + .fragment_module = modules.image_frag.handle, + .vertex_input = .{ + .stride = @sizeOf(Image), + .step_fn = .per_instance, + .attributes = &image_attrs, + }, + .descriptor_set_layouts = &.{ image_ubo_dsl, image_sampler_dsl }, + .empty_set_layout = empty_dsl, + .sampler = image_sampler.sampler, + .color_format = vk.VK_FORMAT_B8G8R8A8_SRGB, + .blending_enabled = true, + .topology = vk.VK_PRIMITIVE_TOPOLOGY_TRIANGLE_STRIP, + }); + errdefer image_pipeline.deinit(); + + // ---- bg_image pipeline ----------------------------------- + // + // The user's `background-image` config. One full-screen + // triangle that samples the image with cover/contain/etc. + // layout math driven by per-instance `BgImage` attributes. + // + // Bindings after `vulkanizeGlsl`: + // set 0 binding 1 Globals UBO + // set 1 binding 0 combined image sampler (the + // user-supplied background image) + // + // Vertex input: `BgImage` struct, per-instance. Locations 0 + // (opacity, float) and 1 (info, uint). + const bg_image_ubo_dsl = try createSingleBindingDsl( + device, + 1, + vk.VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, + vk.VK_SHADER_STAGE_VERTEX_BIT | vk.VK_SHADER_STAGE_FRAGMENT_BIT, + ); + tracker.track(bg_image_ubo_dsl); + const bg_image_sampler_dsl = try createSingleBindingDsl( + device, + 0, + vk.VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, + vk.VK_SHADER_STAGE_VERTEX_BIT | vk.VK_SHADER_STAGE_FRAGMENT_BIT, + ); + tracker.track(bg_image_sampler_dsl); + const bg_image_attrs = [_]vk.VkVertexInputAttributeDescription{ + .{ .location = 0, .binding = 0, .format = vk.VK_FORMAT_R32_SFLOAT, .offset = @offsetOf(BgImage, "opacity") }, + .{ .location = 1, .binding = 0, .format = vk.VK_FORMAT_R8_UINT, .offset = @offsetOf(BgImage, "info") }, + }; + const bg_image_pipeline = try Pipeline.init(.{ + .device = device, + .descriptor_pool = &pool, + .vertex_module = modules.bg_image_vert.handle, + .fragment_module = modules.bg_image_frag.handle, + .vertex_input = .{ + .stride = @sizeOf(BgImage), + .step_fn = .per_instance, + .attributes = &bg_image_attrs, + }, + .descriptor_set_layouts = &.{ bg_image_ubo_dsl, bg_image_sampler_dsl }, + .empty_set_layout = empty_dsl, + .sampler = image_sampler.sampler, + .color_format = vk.VK_FORMAT_B8G8R8A8_SRGB, + .blending_enabled = true, + .topology = vk.VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST, + }); + errdefer bg_image_pipeline.deinit(); + var pipelines: PipelineCollection = .{}; pipelines.bg_color = bg_color_pipeline; pipelines.cell_bg = cell_bg_pipeline; pipelines.cell_text = cell_text_pipeline; + pipelines.image = image_pipeline; + pipelines.bg_image = bg_image_pipeline; // ---- post (custom shader) pipelines ---------------------- // @@ -1134,6 +1268,7 @@ pub const Shaders = struct { .set_layouts_len = set_layouts_len, .empty_set_layout = empty_dsl, .atlas_sampler = atlas_sampler, + .image_sampler = image_sampler, }; } @@ -1204,6 +1339,7 @@ pub const Shaders = struct { // Atlas sampler held by `Shaders` for the cell_text pipeline's // texture bindings. if (self.atlas_sampler) |samp| samp.deinit(); + if (self.image_sampler) |samp| samp.deinit(); // Descriptor pool reclaims every set allocated from it // (including the per-pipeline sets); the standalone layouts