diff options
Diffstat (limited to 'doc')
| -rw-r--r-- | doc/AUXILIARY_TEXTURE_INIT.md | 195 |
1 files changed, 79 insertions, 116 deletions
diff --git a/doc/AUXILIARY_TEXTURE_INIT.md b/doc/AUXILIARY_TEXTURE_INIT.md index d78c5ae..9cac70b 100644 --- a/doc/AUXILIARY_TEXTURE_INIT.md +++ b/doc/AUXILIARY_TEXTURE_INIT.md @@ -1,118 +1,116 @@ # Auxiliary Texture Initialization -**Technical decision document for auxiliary texture initialization order** +**Technical decision: Effect initialization order (resize → init)** + +--- ## Problem -Auxiliary textures (inter-effect shared textures like masks) were created with incorrect dimensions, causing "half resolution" rendering bugs. +Auxiliary textures created at wrong resolution (1280×720 defaults instead of actual window size). -### Root Cause +**Root Cause:** `init()` called before `resize()` in MainSequence. Effects registered textures using default dimensions. ```cpp -// Before fix (effect.cc:180-181) -entry.seq->init(this); // width_/height_ = 1280x720 (defaults) -entry.seq->resize(width, height); // THEN set actual dimensions (e.g., 2560x1440) +// BEFORE (effect.cc:180-181) +entry.seq->init(this); // Uses width_/height_ = 1280×720 +entry.seq->resize(width, height); // Too late - textures already created ``` -Effects called `register_auxiliary_texture()` during `init()` using default dimensions (1280×720) before `resize()` set actual window size. Compute shaders received uniforms with correct resolution but rendered to wrong-sized textures. - -### Affected Code - -- `CircleMaskEffect` - registers `"circle_mask"` texture -- `CNNEffect` - registers `"captured_frame"` texture -- `RotatingCubeEffect` - consumes `"circle_mask"` texture -- Any effect that calls `register_auxiliary_texture()` in `init()` - -## Solutions Considered - -### Option 1: Lazy Initialization ❌ - -**Approach:** Defer texture creation until first use (compute/render). - -**Pros:** -- Textures created with correct dimensions -- No reallocation on resize if dimensions unchanged -- Memory saved if effect never renders - -**Cons:** -- Requires `ensure_texture()` guards at every access point -- Cascade complexity: consumers (like RotatingCubeEffect) also need lazy bind group creation -- Multiple code paths, harder to debug -- Violates "initialize once" principle +**Affected:** +- CircleMaskEffect (circle_mask texture) +- CNNEffect (captured_frame texture) +- RotatingCubeEffect (consumer, hardcoded resolution in uniforms) -**Outcome:** Attempted but rejected due to complexity cascade. +--- -### Option 2: Reorder init/resize ✅ +## Solution: Reorder init/resize ✅ -**Approach:** Call `resize()` before `init()` to set dimensions first. +Call `resize()` before `init()` to set dimensions first. ```cpp -// After fix (effect.cc:179-180) +// AFTER (effect.cc:179-180) entry.seq->resize(width, height); // Set dimensions FIRST entry.seq->init(this); // Then init with correct dimensions ``` -**Pros:** -- **Simple:** 2-line change in MainSequence -- Dimensions available when needed during init() -- No lazy initialization complexity -- Clear, predictable initialization order - -**Cons:** -- Effects see resize() before init() (unusual but harmless) -- Base `Effect::resize()` just updates width_/height_ members +**Rejected Alternative:** Lazy initialization (ensure_texture() guards) - complexity cascade to consumers. -**Outcome:** Implemented. Clean, maintainable solution. +--- ## Implementation -### Changes Made +### 1. MainSequence Order Swap **File:** `src/gpu/effect.cc` -**Lines 179-180:** +Lines 179-180 and 189-190: Call `resize()` before `init()`. + +### 2. Auxiliary Texture Effects + +Register textures in `init()` using width_/height_ (now valid): + ```cpp -entry.seq->resize(width, height); // Set dimensions FIRST -entry.seq->init(this); // Then init with correct dimensions +void MyEffect::init(MainSequence* demo) { + demo_ = demo; + demo_->register_auxiliary_texture("my_texture", width_, height_); + // Create pipelines/bind groups... +} ``` -**Lines 189-190:** +Handle resize with early returns: + ```cpp -seq->resize(width_, height_); // Set dimensions FIRST -seq->init(this); // Then init with correct dimensions +void MyEffect::resize(int width, int height) { + if (width == width_ && height == height_) + return; + Effect::resize(width, height); + if (!demo_) + return; + demo_->resize_auxiliary_texture("my_texture", width, height); + // Recreate bind groups... +} ``` -### Resize Optimization +### 3. Effects with Renderer3D -Added early return in effects to avoid unnecessary GPU reallocation: +Add `initialized_` flag to guard resize(): ```cpp -void CircleMaskEffect::resize(int width, int height) { - if (width == width_ && height == height_) - return; // No-op if dimensions unchanged - - Effect::resize(width, height); +// Header +bool initialized_ = false; - if (!demo_ || !texture_initialized_) - return; +// init() +renderer_.init(ctx_.device, ctx_.queue, ctx_.format); +renderer_.resize(width_, height_); +initialized_ = true; - demo_->resize_auxiliary_texture("circle_mask", width, height); - // Recreate bind groups with new texture view... -} +// resize() +if (!initialized_) + return; // Don't resize uninitialized renderer +renderer_.resize(width_, height_); ``` -## Guidelines for New Effects +**Why flag?** `ctx_.device` exists before init() but Renderer3D not initialized yet. + +**Applied to:** FlashCubeEffect, Hybrid3DEffect + +### 4. Hardcoded Resolutions + +**RotatingCubeEffect:** Fixed hardcoded `vec2(1280.0f, 720.0f)` → `u.resolution` + +**Audit Results:** No other hardcoded resolutions in effects. Defaults in effect.cc:161-162 are test-only. + +--- + +## Guidelines ### Creating Auxiliary Textures ```cpp void MyEffect::init(MainSequence* demo) { demo_ = demo; - - // width_/height_ are VALID here (set by resize() before init()) demo_->register_auxiliary_texture("my_texture", width_, height_); - - // Create pipelines and bind groups referencing the texture + // width_/height_ valid (resize() called before init()) } ``` @@ -121,68 +119,33 @@ void MyEffect::init(MainSequence* demo) { ```cpp void MyEffect::init(MainSequence* demo) { demo_ = demo; - - // Safe to get_auxiliary_view() here - producer effects - // initialized before consumers (order in timeline) WGPUTextureView view = demo_->get_auxiliary_view("circle_mask"); + // Producer effects initialized before consumers (timeline order) } ``` -### Handling Resize - -```cpp -void MyEffect::resize(int width, int height) { - if (width == width_ && height == height_) - return; // Early exit if unchanged +### Using Resolutions - Effect::resize(width, height); +**Always use `u.resolution` from CommonPostProcessUniforms, never hardcode dimensions.** - // If you registered auxiliary texture, resize it: - if (demo_) { - demo_->resize_auxiliary_texture("my_texture", width, height); - // Recreate bind groups with new texture view - } -} -``` +--- ## Testing All 36 tests pass. Verified: -- CircleMaskEffect renders at correct resolution -- CNNEffect captured_frame has correct dimensions -- No artifacts or half-resolution bugs +- Auxiliary textures at correct resolution +- Renderer3D effects don't crash on resize-before-init +- No hardcoded resolution artifacts - Window resize works correctly -## Additional Fix Required - -After swapping init/resize order, effects with Renderer3D (FlashCubeEffect, Hybrid3DEffect) crashed when resize() tried to use uninitialized renderer. - -**Solution:** Add `initialized_` flag to track if init() was called. - -```cpp -// In header -bool initialized_ = false; - -// In init() -renderer_.init(ctx_.device, ctx_.queue, ctx_.format); -renderer_.resize(width_, height_); -initialized_ = true; // Mark as initialized - -// In resize() -if (!initialized_) - return; // Early exit if not initialized yet -``` - -**Why not check `ctx_.device`?** Device exists before init() (passed to constructor), but Renderer3D not initialized yet. +--- -## Related Documentation +## Related -- `doc/MASKING_SYSTEM.md` - Auxiliary texture usage patterns +- `doc/MASKING_SYSTEM.md` - Auxiliary texture patterns - `doc/EFFECT_WORKFLOW.md` - Effect creation workflow -- `src/gpu/effect.h` - MainSequence API reference +- `src/gpu/effect.h` - MainSequence API --- -**Decision made:** 2026-02-11 -**Implemented by:** Claude Sonnet 4.5 -**Status:** Active +**Decision:** 2026-02-11 | **Status:** Active | **By:** Claude Sonnet 4.5 |