From a91f89c8ea15665853176c05597760d0fcf6e0df Mon Sep 17 00:00:00 2001 From: skal Date: Wed, 20 May 2026 22:44:44 +0200 Subject: fix: code review cleanup — bugs, dead code, factorization, simplification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bugs: - B1: fix dead tempo debug (prev_tempo captured after assignment) - B2: fix ReloadAssetsFromFile leak for disk-loaded assets; simplify DropAsset - B3: fix get_free_pool_slot leak (unregister synth + free data on reuse) - B4: volatile -> std::atomic with acquire/release in miniaudio_backend, synth - B5: fix unaligned reads in scene_loader (memcpy-based read_f32/read_u32) - B6: fix shader module + BGL + pipeline layout leaks in gpu.cc, pipeline_builder Dead code: - D1: remove unused particle_defs.h - D3: remove create_post_process_pipeline_simple (zero callers) - D4: remove empty gpu_draw() - D5: remove write-only Hybrid3D::initialized_ - D6: remove legacy pending buffer path in audio.cc Factorization: - F1: Effect::run_fullscreen_pass() replaces boilerplate in 5 effects - F2: particle_common.wgsl snippet, #include in 3 WGSL shaders - F3: gpu_create_shader_module() helper, used in 3 call sites - F5: get_world_aabb() shared between bvh.cc and physics.cc - F6: samples_to_seconds() replaces 6 inline expressions - F7: gpu_create_linear/nearest_sampler use SamplerCache; add nearest() preset Simplification: - S9+S1: WgslSamplerType param; Scene2Effect collapsed to thin wrapper - S4: FFT heap allocs -> stack arrays (zero allocs on hot path) - S5: ObjectType::CUBE documented as legacy alias for BOX; default changed - S6: bind group dirty-flag in Renderer3D; remove duplicate pipeline set - S7: create_gpu_procedural() helper in texture_manager (~80 lines removed) 37/37 tests passing. handoff(Claude): code review batch — all items verified, no regressions. --- src/gpu/texture_manager.cc | 154 +++++++++++++-------------------------------- src/gpu/texture_manager.h | 7 +++ src/gpu/wgsl_effect.cc | 8 ++- src/gpu/wgsl_effect.h | 5 +- 4 files changed, 61 insertions(+), 113 deletions(-) (limited to 'src/gpu') diff --git a/src/gpu/texture_manager.cc b/src/gpu/texture_manager.cc index bdeb508..20e215d 100644 --- a/src/gpu/texture_manager.cc +++ b/src/gpu/texture_manager.cc @@ -285,142 +285,76 @@ void TextureManager::dispatch_compute(const std::string& func_name, wgpuTextureViewRelease(target_view); } -void TextureManager::create_gpu_noise_texture( - const std::string& name, const GpuProceduralParams& params) { - extern const char* gen_noise_compute_wgsl; - get_or_create_compute_pipeline("gen_noise", gen_noise_compute_wgsl, 16); - - WGPUTextureDescriptor tex_desc = {}; - tex_desc.usage = - WGPUTextureUsage_StorageBinding | WGPUTextureUsage_TextureBinding; - tex_desc.dimension = WGPUTextureDimension_2D; - tex_desc.size = {(uint32_t)params.width, (uint32_t)params.height, 1}; - tex_desc.format = WGPUTextureFormat_RGBA8Unorm; - tex_desc.mipLevelCount = 1; - tex_desc.sampleCount = 1; - WGPUTexture texture = wgpuDeviceCreateTexture(device_, &tex_desc); +void TextureManager::create_gpu_procedural( + const std::string& name, const std::string& func_name, + const char* shader_code, const GpuProceduralParams& params, + const void* uniform_data, size_t uniform_size) { + get_or_create_compute_pipeline(func_name, shader_code, uniform_size); - struct NoiseParams { - uint32_t width; - uint32_t height; - float seed; - float frequency; - }; - NoiseParams uniforms = {(uint32_t)params.width, (uint32_t)params.height, - params.params[0], params.params[1]}; - dispatch_compute("gen_noise", texture, params, &uniforms, - sizeof(NoiseParams)); + TextureWithView tv = gpu_create_storage_texture_2d( + device_, (uint32_t)params.width, (uint32_t)params.height, + WGPUTextureFormat_RGBA8Unorm); - WGPUTextureView view = - gpu_create_texture_view_2d(texture, WGPUTextureFormat_RGBA8Unorm); + dispatch_compute(func_name, tv.texture, params, uniform_data, uniform_size); GpuTexture gpu_tex; - gpu_tex.texture = texture; - gpu_tex.view = view; + gpu_tex.texture = tv.texture; + gpu_tex.view = tv.view; gpu_tex.width = params.width; gpu_tex.height = params.height; textures_[name] = gpu_tex; #if !defined(STRIP_ALL) - printf("Generated GPU noise texture: %s (%dx%d)\n", name.c_str(), - params.width, params.height); + printf("Generated GPU %s texture: %s (%dx%d)\n", func_name.c_str(), + name.c_str(), params.width, params.height); #endif } +void TextureManager::create_gpu_noise_texture( + const std::string& name, const GpuProceduralParams& params) { + extern const char* gen_noise_compute_wgsl; + struct NoiseParams { + uint32_t width, height; + float seed, frequency; + }; + NoiseParams u = {(uint32_t)params.width, (uint32_t)params.height, + params.params[0], params.params[1]}; + create_gpu_procedural(name, "gen_noise", gen_noise_compute_wgsl, params, &u, + sizeof(u)); +} + void TextureManager::create_gpu_perlin_texture( const std::string& name, const GpuProceduralParams& params) { extern const char* gen_perlin_compute_wgsl; - get_or_create_compute_pipeline("gen_perlin", gen_perlin_compute_wgsl, 32); - - WGPUTextureDescriptor tex_desc = {}; - tex_desc.usage = - WGPUTextureUsage_StorageBinding | WGPUTextureUsage_TextureBinding; - tex_desc.dimension = WGPUTextureDimension_2D; - tex_desc.size = {(uint32_t)params.width, (uint32_t)params.height, 1}; - tex_desc.format = WGPUTextureFormat_RGBA8Unorm; - tex_desc.mipLevelCount = 1; - tex_desc.sampleCount = 1; - WGPUTexture texture = wgpuDeviceCreateTexture(device_, &tex_desc); - struct PerlinParams { - uint32_t width; - uint32_t height; - float seed; - float frequency; - float amplitude; - float amplitude_decay; + uint32_t width, height; + float seed, frequency, amplitude, amplitude_decay; uint32_t octaves; float _pad0; }; - PerlinParams uniforms = {(uint32_t)params.width, - (uint32_t)params.height, - params.params[0], - params.params[1], - params.num_params > 2 ? params.params[2] : 1.0f, - params.num_params > 3 ? params.params[3] : 0.5f, - params.num_params > 4 ? (uint32_t)params.params[4] - : 4u, - 0.0f}; - dispatch_compute("gen_perlin", texture, params, &uniforms, - sizeof(PerlinParams)); - - WGPUTextureView view = - gpu_create_texture_view_2d(texture, WGPUTextureFormat_RGBA8Unorm); - - GpuTexture gpu_tex; - gpu_tex.texture = texture; - gpu_tex.view = view; - gpu_tex.width = params.width; - gpu_tex.height = params.height; - textures_[name] = gpu_tex; - -#if !defined(STRIP_ALL) - printf("Generated GPU perlin texture: %s (%dx%d)\n", name.c_str(), - params.width, params.height); -#endif + PerlinParams u = {(uint32_t)params.width, + (uint32_t)params.height, + params.params[0], + params.params[1], + params.num_params > 2 ? params.params[2] : 1.0f, + params.num_params > 3 ? params.params[3] : 0.5f, + params.num_params > 4 ? (uint32_t)params.params[4] : 4u, + 0.0f}; + create_gpu_procedural(name, "gen_perlin", gen_perlin_compute_wgsl, params, &u, + sizeof(u)); } void TextureManager::create_gpu_grid_texture( const std::string& name, const GpuProceduralParams& params) { extern const char* gen_grid_compute_wgsl; - get_or_create_compute_pipeline("gen_grid", gen_grid_compute_wgsl, 16); - - WGPUTextureDescriptor tex_desc = {}; - tex_desc.usage = - WGPUTextureUsage_StorageBinding | WGPUTextureUsage_TextureBinding; - tex_desc.dimension = WGPUTextureDimension_2D; - tex_desc.size = {(uint32_t)params.width, (uint32_t)params.height, 1}; - tex_desc.format = WGPUTextureFormat_RGBA8Unorm; - tex_desc.mipLevelCount = 1; - tex_desc.sampleCount = 1; - WGPUTexture texture = wgpuDeviceCreateTexture(device_, &tex_desc); - struct GridParams { - uint32_t width; - uint32_t height; - uint32_t grid_size; - uint32_t thickness; + uint32_t width, height, grid_size, thickness; }; - GridParams uniforms = { - (uint32_t)params.width, (uint32_t)params.height, - params.num_params > 0 ? (uint32_t)params.params[0] : 32u, - params.num_params > 1 ? (uint32_t)params.params[1] : 2u}; - dispatch_compute("gen_grid", texture, params, &uniforms, sizeof(GridParams)); - - WGPUTextureView view = - gpu_create_texture_view_2d(texture, WGPUTextureFormat_RGBA8Unorm); - - GpuTexture gpu_tex; - gpu_tex.texture = texture; - gpu_tex.view = view; - gpu_tex.width = params.width; - gpu_tex.height = params.height; - textures_[name] = gpu_tex; - -#if !defined(STRIP_ALL) - printf("Generated GPU grid texture: %s (%dx%d)\n", name.c_str(), params.width, - params.height); -#endif + GridParams u = {(uint32_t)params.width, (uint32_t)params.height, + params.num_params > 0 ? (uint32_t)params.params[0] : 32u, + params.num_params > 1 ? (uint32_t)params.params[1] : 2u}; + create_gpu_procedural(name, "gen_grid", gen_grid_compute_wgsl, params, &u, + sizeof(u)); } #if !defined(STRIP_GPU_COMPOSITE) diff --git a/src/gpu/texture_manager.h b/src/gpu/texture_manager.h index ec30c7b..a5462ae 100644 --- a/src/gpu/texture_manager.h +++ b/src/gpu/texture_manager.h @@ -93,6 +93,13 @@ class TextureManager { const GpuProceduralParams& params, const void* uniform_data, size_t uniform_size); + // Shared helper: create pipeline + storage texture + dispatch + store result + void create_gpu_procedural(const std::string& name, + const std::string& func_name, + const char* shader_code, + const GpuProceduralParams& params, + const void* uniform_data, size_t uniform_size); + #if !defined(STRIP_GPU_COMPOSITE) void dispatch_composite(const std::string& func_name, WGPUTexture target, const GpuProceduralParams& params, diff --git a/src/gpu/wgsl_effect.cc b/src/gpu/wgsl_effect.cc index 1cb0ecb..4f658a5 100644 --- a/src/gpu/wgsl_effect.cc +++ b/src/gpu/wgsl_effect.cc @@ -10,13 +10,17 @@ WgslEffect::WgslEffect(const GpuContext& ctx, const std::vector& outputs, float start_time, float end_time, const char* shader_code, WGPULoadOp load_op, - WgslEffectParams initial_params) + WgslEffectParams initial_params, + WgslSamplerType sampler_type) : Effect(ctx, inputs, outputs, start_time, end_time), effect_params(initial_params), load_op_(load_op) { HEADLESS_RETURN_IF_NULL(ctx_.device); - create_linear_sampler(); + if (sampler_type == WgslSamplerType::Nearest) + create_nearest_sampler(); + else + create_linear_sampler(); params_buffer_.init(ctx_.device); pipeline_.set(create_post_process_pipeline(ctx_.device, diff --git a/src/gpu/wgsl_effect.h b/src/gpu/wgsl_effect.h index 062f885..f487ef7 100644 --- a/src/gpu/wgsl_effect.h +++ b/src/gpu/wgsl_effect.h @@ -16,6 +16,8 @@ struct WgslEffectParams { }; static_assert(sizeof(WgslEffectParams) == 32, "WgslEffectParams must be 32 bytes"); +enum class WgslSamplerType { Linear, Nearest }; + class WgslEffect : public Effect { public: // Mutate per-frame for dynamic parameter modulation. @@ -25,7 +27,8 @@ class WgslEffect : public Effect { const std::vector& outputs, float start_time, float end_time, const char* shader_code, WGPULoadOp load_op = WGPULoadOp_Clear, - WgslEffectParams initial_params = {}); + WgslEffectParams initial_params = {}, + WgslSamplerType sampler_type = WgslSamplerType::Linear); void render(WGPUCommandEncoder encoder, const UniformsSequenceParams& params, NodeRegistry& nodes) override; -- cgit v1.2.3