From 2e2e01048da5c466102bb57d64148aff72f4a558 Mon Sep 17 00:00:00 2001 From: skal Date: Tue, 17 Feb 2026 12:51:29 +0100 Subject: refactor(gpu): Add RAII wrapper for WGPU resources to eliminate manual cleanup Introduces WGPUResource template with automatic release on destruction. Reduces boilerplate in effect destructors and prevents resource leaks. - set() for one-time initialization - replace() for per-frame recreation - Field ordering documented for dependency management Converted 3 effects (Heptagon, Flash, Passthrough) and Effect base class. All tests pass (34/34). Co-Authored-By: Claude Sonnet 4.5 --- src/effects/flash_effect.cc | 28 ++++++----------------- src/effects/flash_effect.h | 6 ++--- src/effects/gaussian_blur_effect.cc | 2 +- src/effects/heptagon_effect.cc | 27 ++++++---------------- src/effects/heptagon_effect.h | 6 ++--- src/effects/passthrough_effect.cc | 20 +++++++---------- src/effects/passthrough_effect.h | 5 +++-- src/gpu/effect.cc | 8 +++---- src/gpu/effect.h | 7 +++--- src/gpu/wgpu_resource.h | 45 +++++++++++++++++++++++++++++++++++++ 10 files changed, 85 insertions(+), 69 deletions(-) create mode 100644 src/gpu/wgpu_resource.h diff --git a/src/effects/flash_effect.cc b/src/effects/flash_effect.cc index 2eba2ff..93ff4dd 100644 --- a/src/effects/flash_effect.cc +++ b/src/effects/flash_effect.cc @@ -9,29 +9,15 @@ Flash::Flash(const GpuContext& ctx, const std::vector& inputs, const std::vector& outputs, float start_time, float end_time) - : Effect(ctx, inputs, outputs, start_time, end_time), pipeline_(nullptr), - bind_group_(nullptr) { + : Effect(ctx, inputs, outputs, start_time, end_time) { HEADLESS_RETURN_IF_NULL(ctx_.device); init_uniforms_buffer(); create_nearest_sampler(); create_dummy_scene_texture(); - pipeline_ = create_post_process_pipeline( - ctx_.device, WGPUTextureFormat_RGBA8Unorm, flash_shader_wgsl); -} - -Flash::~Flash() { - if (bind_group_) - wgpuBindGroupRelease(bind_group_); - if (pipeline_) - wgpuRenderPipelineRelease(pipeline_); - if (sampler_) - wgpuSamplerRelease(sampler_); - if (dummy_texture_view_) - wgpuTextureViewRelease(dummy_texture_view_); - if (dummy_texture_) - wgpuTextureRelease(dummy_texture_); + pipeline_.set(create_post_process_pipeline( + ctx_.device, WGPUTextureFormat_RGBA8Unorm, flash_shader_wgsl)); } void Flash::render(WGPUCommandEncoder encoder, @@ -43,8 +29,8 @@ void Flash::render(WGPUCommandEncoder encoder, uniforms_buffer_.update(ctx_.queue, params); // Update bind group (use dummy texture for scene effect) - pp_update_bind_group(ctx_.device, pipeline_, &bind_group_, - dummy_texture_view_, uniforms_buffer_.get(), + pp_update_bind_group(ctx_.device, pipeline_.get(), bind_group_.get_address(), + dummy_texture_view_.get(), uniforms_buffer_.get(), {nullptr, 0}); // Render pass @@ -57,8 +43,8 @@ void Flash::render(WGPUCommandEncoder encoder, WGPURenderPassEncoder pass = wgpuCommandEncoderBeginRenderPass(encoder, &pass_desc); - wgpuRenderPassEncoderSetPipeline(pass, pipeline_); - wgpuRenderPassEncoderSetBindGroup(pass, 0, bind_group_, 0, nullptr); + wgpuRenderPassEncoderSetPipeline(pass, pipeline_.get()); + wgpuRenderPassEncoderSetBindGroup(pass, 0, bind_group_.get(), 0, nullptr); wgpuRenderPassEncoderDraw(pass, 3, 1, 0, 0); wgpuRenderPassEncoderEnd(pass); wgpuRenderPassEncoderRelease(pass); diff --git a/src/effects/flash_effect.h b/src/effects/flash_effect.h index d46cd56..052957d 100644 --- a/src/effects/flash_effect.h +++ b/src/effects/flash_effect.h @@ -4,18 +4,18 @@ #pragma once #include "gpu/effect.h" #include "gpu/uniform_helper.h" +#include "gpu/wgpu_resource.h" class Flash : public Effect { public: Flash(const GpuContext& ctx, const std::vector& inputs, const std::vector& outputs, float start_time, float end_time); - ~Flash() override; void render(WGPUCommandEncoder encoder, const UniformsSequenceParams& params, NodeRegistry& nodes) override; private: - WGPURenderPipeline pipeline_; - WGPUBindGroup bind_group_; + RenderPipeline pipeline_; + BindGroup bind_group_; }; diff --git a/src/effects/gaussian_blur_effect.cc b/src/effects/gaussian_blur_effect.cc index 1d39256..0548b4a 100644 --- a/src/effects/gaussian_blur_effect.cc +++ b/src/effects/gaussian_blur_effect.cc @@ -34,7 +34,7 @@ void GaussianBlur::render(WGPUCommandEncoder encoder, // Update bind group WGPUBindGroupEntry entries[4] = {}; entries[0].binding = PP_BINDING_SAMPLER; - entries[0].sampler = sampler_; + entries[0].sampler = sampler_.get(); entries[1].binding = PP_BINDING_TEXTURE; entries[1].textureView = input_view; entries[2].binding = PP_BINDING_UNIFORMS; diff --git a/src/effects/heptagon_effect.cc b/src/effects/heptagon_effect.cc index 46d6f7f..c9ec17c 100644 --- a/src/effects/heptagon_effect.cc +++ b/src/effects/heptagon_effect.cc @@ -10,28 +10,15 @@ Heptagon::Heptagon(const GpuContext& ctx, const std::vector& inputs, const std::vector& outputs, float start_time, float end_time) - : Effect(ctx, inputs, outputs, start_time, end_time), pipeline_(nullptr), bind_group_(nullptr) { + : Effect(ctx, inputs, outputs, start_time, end_time) { HEADLESS_RETURN_IF_NULL(ctx_.device); init_uniforms_buffer(); create_nearest_sampler(); create_dummy_scene_texture(); - pipeline_ = create_post_process_pipeline( - ctx_.device, WGPUTextureFormat_RGBA8Unorm, heptagon_shader_wgsl); -} - -Heptagon::~Heptagon() { - if (bind_group_) - wgpuBindGroupRelease(bind_group_); - if (pipeline_) - wgpuRenderPipelineRelease(pipeline_); - if (sampler_) - wgpuSamplerRelease(sampler_); - if (dummy_texture_view_) - wgpuTextureViewRelease(dummy_texture_view_); - if (dummy_texture_) - wgpuTextureRelease(dummy_texture_); + pipeline_.set(create_post_process_pipeline( + ctx_.device, WGPUTextureFormat_RGBA8Unorm, heptagon_shader_wgsl)); } void Heptagon::render(WGPUCommandEncoder encoder, @@ -44,8 +31,8 @@ void Heptagon::render(WGPUCommandEncoder encoder, uniforms_buffer_.update(ctx_.queue, params); // Create bind group (use dummy texture for scene effect) - pp_update_bind_group(ctx_.device, pipeline_, &bind_group_, - dummy_texture_view_, uniforms_buffer_.get(), + pp_update_bind_group(ctx_.device, pipeline_.get(), bind_group_.get_address(), + dummy_texture_view_.get(), uniforms_buffer_.get(), {nullptr, 0}); // Render pass @@ -58,8 +45,8 @@ void Heptagon::render(WGPUCommandEncoder encoder, WGPURenderPassEncoder pass = wgpuCommandEncoderBeginRenderPass(encoder, &pass_desc); - wgpuRenderPassEncoderSetPipeline(pass, pipeline_); - wgpuRenderPassEncoderSetBindGroup(pass, 0, bind_group_, 0, nullptr); + wgpuRenderPassEncoderSetPipeline(pass, pipeline_.get()); + wgpuRenderPassEncoderSetBindGroup(pass, 0, bind_group_.get(), 0, nullptr); wgpuRenderPassEncoderDraw(pass, 3, 1, 0, 0); // Fullscreen triangle wgpuRenderPassEncoderEnd(pass); wgpuRenderPassEncoderRelease(pass); diff --git a/src/effects/heptagon_effect.h b/src/effects/heptagon_effect.h index 4b7b887..4563e47 100644 --- a/src/effects/heptagon_effect.h +++ b/src/effects/heptagon_effect.h @@ -4,18 +4,18 @@ #include "gpu/effect.h" #include "gpu/uniform_helper.h" +#include "gpu/wgpu_resource.h" class Heptagon : public Effect { public: Heptagon(const GpuContext& ctx, const std::vector& inputs, const std::vector& outputs, float start_time, float end_time); - ~Heptagon(); void render(WGPUCommandEncoder encoder, const UniformsSequenceParams& params, NodeRegistry& nodes) override; private: - WGPURenderPipeline pipeline_; - WGPUBindGroup bind_group_; + RenderPipeline pipeline_; + BindGroup bind_group_; }; diff --git a/src/effects/passthrough_effect.cc b/src/effects/passthrough_effect.cc index 02f14da..24eefca 100644 --- a/src/effects/passthrough_effect.cc +++ b/src/effects/passthrough_effect.cc @@ -9,15 +9,14 @@ Passthrough::Passthrough(const GpuContext& ctx, const std::vector& inputs, const std::vector& outputs, float start_time, float end_time) - : Effect(ctx, inputs, outputs, start_time, end_time), pipeline_(nullptr), - bind_group_(nullptr) { + : Effect(ctx, inputs, outputs, start_time, end_time) { HEADLESS_RETURN_IF_NULL(ctx_.device); init_uniforms_buffer(); create_linear_sampler(); - pipeline_ = create_post_process_pipeline_simple( - ctx_.device, WGPUTextureFormat_RGBA8Unorm, passthrough_shader_wgsl); + pipeline_.set(create_post_process_pipeline_simple( + ctx_.device, WGPUTextureFormat_RGBA8Unorm, passthrough_shader_wgsl)); } void Passthrough::render(WGPUCommandEncoder encoder, @@ -33,7 +32,7 @@ void Passthrough::render(WGPUCommandEncoder encoder, // Manually create bind group with only 3 entries (no effect params needed) WGPUBindGroupEntry entries[3] = {}; entries[0].binding = PP_BINDING_SAMPLER; - entries[0].sampler = sampler_; + entries[0].sampler = sampler_.get(); entries[1].binding = PP_BINDING_TEXTURE; entries[1].textureView = input_view; entries[2].binding = PP_BINDING_UNIFORMS; @@ -41,14 +40,11 @@ void Passthrough::render(WGPUCommandEncoder encoder, entries[2].size = sizeof(UniformsSequenceParams); WGPUBindGroupDescriptor bg_desc = {}; - bg_desc.layout = wgpuRenderPipelineGetBindGroupLayout(pipeline_, 0); + bg_desc.layout = wgpuRenderPipelineGetBindGroupLayout(pipeline_.get(), 0); bg_desc.entryCount = 3; bg_desc.entries = entries; - if (bind_group_) { - wgpuBindGroupRelease(bind_group_); - } - bind_group_ = wgpuDeviceCreateBindGroup(ctx_.device, &bg_desc); + bind_group_.replace(wgpuDeviceCreateBindGroup(ctx_.device, &bg_desc)); // Render pass WGPURenderPassColorAttachment color_attachment = {}; @@ -60,8 +56,8 @@ void Passthrough::render(WGPUCommandEncoder encoder, WGPURenderPassEncoder pass = wgpuCommandEncoderBeginRenderPass(encoder, &pass_desc); - wgpuRenderPassEncoderSetPipeline(pass, pipeline_); - wgpuRenderPassEncoderSetBindGroup(pass, 0, bind_group_, 0, nullptr); + wgpuRenderPassEncoderSetPipeline(pass, pipeline_.get()); + wgpuRenderPassEncoderSetBindGroup(pass, 0, bind_group_.get(), 0, nullptr); wgpuRenderPassEncoderDraw(pass, 3, 1, 0, 0); // Fullscreen triangle wgpuRenderPassEncoderEnd(pass); wgpuRenderPassEncoderRelease(pass); diff --git a/src/effects/passthrough_effect.h b/src/effects/passthrough_effect.h index 3813fa8..1c60e02 100644 --- a/src/effects/passthrough_effect.h +++ b/src/effects/passthrough_effect.h @@ -4,6 +4,7 @@ #include "gpu/effect.h" #include "gpu/uniform_helper.h" +#include "gpu/wgpu_resource.h" class Passthrough : public Effect { public: @@ -15,6 +16,6 @@ class Passthrough : public Effect { NodeRegistry& nodes) override; private: - WGPURenderPipeline pipeline_; - WGPUBindGroup bind_group_; + RenderPipeline pipeline_; + BindGroup bind_group_; }; diff --git a/src/gpu/effect.cc b/src/gpu/effect.cc index cd4bc16..d0693ec 100644 --- a/src/gpu/effect.cc +++ b/src/gpu/effect.cc @@ -62,15 +62,15 @@ void Effect::init_uniforms_buffer() { } void Effect::create_linear_sampler() { - sampler_ = gpu_create_linear_sampler(ctx_.device); + sampler_.set(gpu_create_linear_sampler(ctx_.device)); } void Effect::create_nearest_sampler() { - sampler_ = gpu_create_nearest_sampler(ctx_.device); + sampler_.set(gpu_create_nearest_sampler(ctx_.device)); } void Effect::create_dummy_scene_texture() { TextureWithView dummy = gpu_create_dummy_scene_texture(ctx_.device); - dummy_texture_ = dummy.texture; - dummy_texture_view_ = dummy.view; + dummy_texture_.set(dummy.texture); + dummy_texture_view_.set(dummy.view); } diff --git a/src/gpu/effect.h b/src/gpu/effect.h index 82aa6e1..d47a8b7 100644 --- a/src/gpu/effect.h +++ b/src/gpu/effect.h @@ -7,6 +7,7 @@ #include "gpu/gpu.h" #include "gpu/sequence.h" #include "gpu/uniform_helper.h" +#include "gpu/wgpu_resource.h" #include #include @@ -54,9 +55,9 @@ class Effect { // Common resources for most effects UniformBuffer uniforms_buffer_; - WGPUSampler sampler_ = nullptr; - WGPUTexture dummy_texture_ = nullptr; - WGPUTextureView dummy_texture_view_ = nullptr; + Sampler sampler_; + Texture dummy_texture_; + TextureView dummy_texture_view_; // Helper: Initialize uniforms buffer (call in subclass constructor) void init_uniforms_buffer(); diff --git a/src/gpu/wgpu_resource.h b/src/gpu/wgpu_resource.h new file mode 100644 index 0000000..e448b18 --- /dev/null +++ b/src/gpu/wgpu_resource.h @@ -0,0 +1,45 @@ +// WGPU Resource RAII Wrapper +// Automatic release on destruction +// +// IMPORTANT: Field ordering matters for destruction order. +// Members are destroyed in reverse declaration order. +// Declare dependencies before dependents: +// RenderPipeline pipeline_; // Destroyed last +// BindGroup bind_group_; // Destroyed first (may reference pipeline) + +#pragma once +#include "platform/platform.h" +#include "util/fatal_error.h" + +template +class WGPUResource { + public: + WGPUResource() : ptr_(nullptr) {} + ~WGPUResource() { if (ptr_) Release(ptr_); } + + void set(T ptr) { + FATAL_ASSERT(ptr_ == nullptr); + ptr_ = ptr; + } + + void replace(T ptr) { + if (ptr_) Release(ptr_); + ptr_ = ptr; + } + + T get() const { return ptr_; } + T* get_address() { return &ptr_; } + + private: + T ptr_; + WGPUResource(const WGPUResource&) = delete; + WGPUResource& operator=(const WGPUResource&) = delete; +}; + +using BindGroup = WGPUResource; +using RenderPipeline = WGPUResource; +using ComputePipeline = WGPUResource; +using Sampler = WGPUResource; +using Texture = WGPUResource; +using TextureView = WGPUResource; +using Buffer = WGPUResource; -- cgit v1.2.3