From bdb1a4e95a545f3f4d88630b8aec6ab771776d99 Mon Sep 17 00:00:00 2001 From: skal Date: Tue, 17 Feb 2026 13:05:11 +0100 Subject: refactor(effects): Streamline uniforms initialization Centralized uniforms_buffer_ initialization and updates to Effect base class: - init_uniforms_buffer() now automatic in Effect::Effect() - uniforms_buffer_.update() now automatic in dispatch_render() - Removed redundant calls from all effect subclasses - Updated effect.h comments to reflect automatic behavior - Updated EFFECT_WORKFLOW.md templates Benefits: - 16 lines removed from effect implementations - Consistent pattern enforced at compile time - Reduced boilerplate for new effects Tests: 34/34 passing handoff(Claude): Effect base class now handles uniforms automatically --- doc/EFFECT_WORKFLOW.md | 39 +++++++++++++++++++++---------------- src/effects/flash_effect.cc | 4 ---- src/effects/gaussian_blur_effect.cc | 4 +--- src/effects/heptagon_effect.cc | 4 ---- src/effects/particles_effect.cc | 5 ----- src/effects/passthrough_effect.cc | 4 ---- src/effects/peak_meter_effect.cc | 3 --- src/effects/placeholder_effect.cc | 3 --- src/gpu/effect.cc | 6 ++---- src/gpu/effect.h | 13 +++++-------- 10 files changed, 30 insertions(+), 55 deletions(-) diff --git a/doc/EFFECT_WORKFLOW.md b/doc/EFFECT_WORKFLOW.md index c4010db..46aebd2 100644 --- a/doc/EFFECT_WORKFLOW.md +++ b/doc/EFFECT_WORKFLOW.md @@ -44,14 +44,18 @@ void render(WGPUCommandEncoder encoder, void declare_nodes(NodeRegistry& registry) override; ``` -**Uniforms**: +**Uniforms** (auto-updated by base class): ```cpp +// Available in render() via params: params.time; // Physical seconds params.beat_time; // Musical beats params.beat_phase; // Fractional beat 0.0-1.0 params.audio_intensity; // Audio peak params.resolution; // vec2(width, height) params.aspect_ratio; // width/height + +// uniforms_buffer_ automatically initialized and updated +// No need to call init_uniforms_buffer() or uniforms_buffer_.update() ``` ### 2. Add Shader to Assets @@ -143,17 +147,15 @@ class MyEffect : public Effect { MyEffect::MyEffect(const GpuContext& ctx, const std::vector& inputs, - const std::vector& outputs) - : Effect(ctx, inputs, outputs), pipeline_(nullptr), bind_group_(nullptr) { - uniforms_buffer_.init(ctx_.device); - pipeline_ = create_post_process_pipeline(ctx_.device, - WGPUTextureFormat_RGBA8Unorm, - my_shader_wgsl); -} + const std::vector& outputs, + float start_time, float end_time) + : Effect(ctx, inputs, outputs, start_time, end_time) { + HEADLESS_RETURN_IF_NULL(ctx_.device); -MyEffect::~MyEffect() { - if (bind_group_) wgpuBindGroupRelease(bind_group_); - if (pipeline_) wgpuRenderPipelineRelease(pipeline_); + // uniforms_buffer_ already initialized by base class + pipeline_.set(create_post_process_pipeline(ctx_.device, + WGPUTextureFormat_RGBA8Unorm, + my_shader_wgsl)); } void MyEffect::render(WGPUCommandEncoder encoder, @@ -162,9 +164,9 @@ void MyEffect::render(WGPUCommandEncoder encoder, WGPUTextureView input_view = nodes.get_view(input_nodes_[0]); WGPUTextureView output_view = nodes.get_view(output_nodes_[0]); - uniforms_buffer_.update(ctx_.queue, params); - pp_update_bind_group(ctx_.device, pipeline_, &bind_group_, input_view, - uniforms_buffer_.get(), {nullptr, 0}); + // uniforms_buffer_ already updated by base class dispatch_render() + pp_update_bind_group(ctx_.device, pipeline_.get(), bind_group_.get_address(), + input_view, uniforms_buffer_.get(), {nullptr, 0}); WGPURenderPassColorAttachment color_attachment = { .view = output_view, @@ -196,9 +198,12 @@ void MyEffect::render(WGPUCommandEncoder encoder, class My3DEffect : public Effect { std::string depth_node_; - My3DEffect(const GpuContext& ctx, ...) - : Effect(ctx, inputs, outputs), - depth_node_(outputs[0] + "_depth") {} + My3DEffect(const GpuContext& ctx, ..., float start_time, float end_time) + : Effect(ctx, inputs, outputs, start_time, end_time), + depth_node_(outputs[0] + "_depth") { + HEADLESS_RETURN_IF_NULL(ctx_.device); + // Custom uniforms if needed (don't use base uniforms_buffer_) + } void declare_nodes(NodeRegistry& registry) override { registry.declare_node(depth_node_, NodeType::DEPTH24, -1, -1); diff --git a/src/effects/flash_effect.cc b/src/effects/flash_effect.cc index 93ff4dd..7064e9c 100644 --- a/src/effects/flash_effect.cc +++ b/src/effects/flash_effect.cc @@ -12,7 +12,6 @@ Flash::Flash(const GpuContext& ctx, const std::vector& inputs, : Effect(ctx, inputs, outputs, start_time, end_time) { HEADLESS_RETURN_IF_NULL(ctx_.device); - init_uniforms_buffer(); create_nearest_sampler(); create_dummy_scene_texture(); @@ -25,9 +24,6 @@ void Flash::render(WGPUCommandEncoder encoder, // Get output view (scene effects typically write to output, ignore input) WGPUTextureView output_view = nodes.get_view(output_nodes_[0]); - // Update uniforms - uniforms_buffer_.update(ctx_.queue, params); - // Update bind group (use dummy texture for scene effect) pp_update_bind_group(ctx_.device, pipeline_.get(), bind_group_.get_address(), dummy_texture_view_.get(), uniforms_buffer_.get(), diff --git a/src/effects/gaussian_blur_effect.cc b/src/effects/gaussian_blur_effect.cc index 0548b4a..a925dee 100644 --- a/src/effects/gaussian_blur_effect.cc +++ b/src/effects/gaussian_blur_effect.cc @@ -12,7 +12,6 @@ GaussianBlur::GaussianBlur(const GpuContext& ctx, : Effect(ctx, inputs, outputs, start_time, end_time), pipeline_(nullptr), bind_group_(nullptr) { HEADLESS_RETURN_IF_NULL(ctx_.device); - init_uniforms_buffer(); create_linear_sampler(); params_buffer_.init(ctx_.device); @@ -27,8 +26,7 @@ void GaussianBlur::render(WGPUCommandEncoder encoder, WGPUTextureView input_view = nodes.get_view(input_nodes_[0]); WGPUTextureView output_view = nodes.get_view(output_nodes_[0]); - // Update uniforms - uniforms_buffer_.update(ctx_.queue, params); + // Update effect-specific params params_buffer_.update(ctx_.queue, blur_params_); // Update bind group diff --git a/src/effects/heptagon_effect.cc b/src/effects/heptagon_effect.cc index c9ec17c..c02efb3 100644 --- a/src/effects/heptagon_effect.cc +++ b/src/effects/heptagon_effect.cc @@ -13,7 +13,6 @@ Heptagon::Heptagon(const GpuContext& ctx, : Effect(ctx, inputs, outputs, start_time, end_time) { HEADLESS_RETURN_IF_NULL(ctx_.device); - init_uniforms_buffer(); create_nearest_sampler(); create_dummy_scene_texture(); @@ -27,9 +26,6 @@ void Heptagon::render(WGPUCommandEncoder encoder, // Get output view (scene effects typically write to output, ignore input) WGPUTextureView output_view = nodes.get_view(output_nodes_[0]); - // Update uniforms - uniforms_buffer_.update(ctx_.queue, params); - // Create bind group (use dummy texture for scene effect) pp_update_bind_group(ctx_.device, pipeline_.get(), bind_group_.get_address(), dummy_texture_view_.get(), uniforms_buffer_.get(), diff --git a/src/effects/particles_effect.cc b/src/effects/particles_effect.cc index 3c9feb7..d0336f6 100644 --- a/src/effects/particles_effect.cc +++ b/src/effects/particles_effect.cc @@ -14,8 +14,6 @@ Particles::Particles(const GpuContext& ctx, : Effect(ctx, inputs, outputs, start_time, end_time) { HEADLESS_RETURN_IF_NULL(ctx_.device); - init_uniforms_buffer(); - // Initialize particles buffer std::vector init_p(NUM_PARTICLES); for (int i = 0; i < NUM_PARTICLES; ++i) { @@ -66,9 +64,6 @@ Particles::Particles(const GpuContext& ctx, void Particles::render(WGPUCommandEncoder encoder, const UniformsSequenceParams& params, NodeRegistry& nodes) { - // Update uniforms - uniforms_buffer_.update(ctx_.queue, params); - // Run compute pass (particle simulation) WGPUComputePassEncoder compute = wgpuCommandEncoderBeginComputePass(encoder, nullptr); diff --git a/src/effects/passthrough_effect.cc b/src/effects/passthrough_effect.cc index 24eefca..217b5d2 100644 --- a/src/effects/passthrough_effect.cc +++ b/src/effects/passthrough_effect.cc @@ -12,7 +12,6 @@ Passthrough::Passthrough(const GpuContext& ctx, : Effect(ctx, inputs, outputs, start_time, end_time) { HEADLESS_RETURN_IF_NULL(ctx_.device); - init_uniforms_buffer(); create_linear_sampler(); pipeline_.set(create_post_process_pipeline_simple( @@ -26,9 +25,6 @@ void Passthrough::render(WGPUCommandEncoder encoder, WGPUTextureView input_view = nodes.get_view(input_nodes_[0]); WGPUTextureView output_view = nodes.get_view(output_nodes_[0]); - // Update uniforms - uniforms_buffer_.update(ctx_.queue, params); - // Manually create bind group with only 3 entries (no effect params needed) WGPUBindGroupEntry entries[3] = {}; entries[0].binding = PP_BINDING_SAMPLER; diff --git a/src/effects/peak_meter_effect.cc b/src/effects/peak_meter_effect.cc index d077302..27adba4 100644 --- a/src/effects/peak_meter_effect.cc +++ b/src/effects/peak_meter_effect.cc @@ -12,8 +12,6 @@ PeakMeter::PeakMeter(const GpuContext& ctx, : Effect(ctx, inputs, outputs, start_time, end_time), pipeline_(nullptr), bind_group_(nullptr) { HEADLESS_RETURN_IF_NULL(ctx_.device); - init_uniforms_buffer(); - const char* shader_main = R"( struct VertexOutput { @builtin(position) position: vec4, @@ -76,7 +74,6 @@ void PeakMeter::render(WGPUCommandEncoder encoder, WGPUTextureView input_view = nodes.get_view(input_nodes_[0]); WGPUTextureView output_view = nodes.get_view(output_nodes_[0]); - uniforms_buffer_.update(ctx_.queue, params); pp_update_bind_group(ctx_.device, pipeline_, &bind_group_, input_view, uniforms_buffer_.get(), {nullptr, 0}); diff --git a/src/effects/placeholder_effect.cc b/src/effects/placeholder_effect.cc index e024a6b..beb5f33 100644 --- a/src/effects/placeholder_effect.cc +++ b/src/effects/placeholder_effect.cc @@ -17,7 +17,6 @@ Placeholder::Placeholder(const GpuContext& ctx, HEADLESS_RETURN_IF_NULL(ctx_.device); - init_uniforms_buffer(); create_linear_sampler(); pipeline_ = create_post_process_pipeline( @@ -30,8 +29,6 @@ void Placeholder::render(WGPUCommandEncoder encoder, WGPUTextureView input_view = nodes.get_view(input_nodes_[0]); WGPUTextureView output_view = nodes.get_view(output_nodes_[0]); - uniforms_buffer_.update(ctx_.queue, params); - pp_update_bind_group(ctx_.device, pipeline_, &bind_group_, input_view, uniforms_buffer_.get(), {nullptr, 0}); diff --git a/src/gpu/effect.cc b/src/gpu/effect.cc index d0693ec..a94b4a0 100644 --- a/src/gpu/effect.cc +++ b/src/gpu/effect.cc @@ -15,6 +15,7 @@ Effect::Effect(const GpuContext& ctx, const std::vector& inputs, FATAL_CHECK(start_time <= end_time, "Invalid time range: %f > %f\n", start_time, end_time); HEADLESS_RETURN_IF_NULL(ctx_.device); + uniforms_buffer_.init(ctx_.device); } void Effect::dispatch_render(WGPUCommandEncoder encoder, @@ -28,6 +29,7 @@ void Effect::dispatch_render(WGPUCommandEncoder encoder, if (!active && input_nodes_.size() == 1 && output_nodes_.size() == 1) { blit_input_to_output(encoder, nodes); } else if (active) { + uniforms_buffer_.update(ctx_.queue, params); render(encoder, params, nodes); } // Multi-output effects: output undefined when inactive (validated at compile time) @@ -57,10 +59,6 @@ void Effect::blit_input_to_output(WGPUCommandEncoder encoder, &extent); } -void Effect::init_uniforms_buffer() { - uniforms_buffer_.init(ctx_.device); -} - void Effect::create_linear_sampler() { sampler_.set(gpu_create_linear_sampler(ctx_.device)); } diff --git a/src/gpu/effect.h b/src/gpu/effect.h index d47a8b7..bfd5743 100644 --- a/src/gpu/effect.h +++ b/src/gpu/effect.h @@ -53,22 +53,19 @@ class Effect { int width_ = 1280; int height_ = 720; - // Common resources for most effects - UniformBuffer uniforms_buffer_; + // Common resources (initialized automatically in base class) + UniformBuffer uniforms_buffer_; // Auto-updated in dispatch_render() Sampler sampler_; Texture dummy_texture_; TextureView dummy_texture_view_; - // Helper: Initialize uniforms buffer (call in subclass constructor) - void init_uniforms_buffer(); - - // Helper: Create linear sampler (call in subclass constructor) + // Helper: Create linear sampler (call in subclass constructor if needed) void create_linear_sampler(); - // Helper: Create nearest sampler (call in subclass constructor) + // Helper: Create nearest sampler (call in subclass constructor if needed) void create_nearest_sampler(); - // Helper: Create dummy texture for scene effects (call in subclass constructor) + // Helper: Create dummy texture for scene effects (call in subclass constructor if needed) void create_dummy_scene_texture(); private: -- cgit v1.2.3