Files

scawful 5c4cd57ff8 backend-infra-engineer: Post v0.3.9-hotfix7 snapshot (build cleanup)

2025-12-22 00:20:49 +00:00

15 KiB

Raw Permalink Blame History

Complete Duplicate Rendering Investigation

Date: 2025-11-25 Status: Investigation Complete - Root Cause Analysis Issue: Elements inside editor cards appear twice (visually stacked)

Executive Summary

Traced the complete call chain from main loop to editor content rendering. No duplicate Update() or Draw() calls found. The issue is NOT caused by multiple rendering paths in the editor system.

Key Finding: The diagnostic code added to EditorCard::Begin() will definitively identify if cards are being rendered twice. If no duplicates are detected by the diagnostic, the issue lies outside the EditorCard system (likely ImGui draw list submission or Z-ordering).

Complete Call Chain (Main Loop → Editor Content)

1. Main Loop (`controller.cc`)

Controller::OnLoad() [Line 56]
  ├─ ImGui::NewFrame() [Line 63-65] ← SINGLE CALL
  ├─ DockSpace Setup [Lines 67-116]
  │  ├─ Calculate sidebar offsets [Lines 70-78]
  │  ├─ Create main dockspace window [Lines 103-116]
  │  └─ EditorManager::DrawMenuBar() [Line 112]
  │
  └─ EditorManager::Update() [Line 124] ← SINGLE CALL
     └─ DoRender() [Line 134]
        └─ ImGui::Render() ← SINGLE CALL

Verdict: ✅ Clean single-path rendering - no duplicates at main loop level

2. EditorManager Update Flow (`editor_manager.cc:616-843`)

EditorManager::Update()
  ├─ [Lines 617-626] Process deferred actions
  ├─ [Lines 632-662] Draw UI systems (popups, toasts, dialogs)
  ├─ [Lines 664-693] Draw UICoordinator (welcome screen, command palette)
  │
  ├─ [Lines 698-772] Draw Sidebar (BEFORE ROM check)
  │  ├─ Check: IsCardSidebarVisible() && !IsSidebarCollapsed()
  │  ├─ Mutual exclusion: IsTreeViewMode() ?
  │  │  ├─ TRUE  → DrawTreeSidebar() [Line 758]
  │  │  └─ FALSE → DrawSidebar() [Line 761]
  │  └─ Note: Different window names prevent overlap
  │     - DrawSidebar() → "##EditorCardSidebar"
  │     - DrawTreeSidebar() → "##TreeSidebar"
  │
  ├─ [Lines 774-778] Draw RightPanelManager (BEFORE ROM check)
  │  └─ RightPanelManager::Draw() → "##RightPanel"
  │
  ├─ [Lines 802-812] Early return if no ROM loaded
  │
  └─ [Lines 1043-1056] Update active editors (ONLY PATH TO EDITOR UPDATE)
     └─ for (editor : active_editors_)
        └─ if (*editor->active())
           └─ editor->Update() ← SINGLE CALL PER EDITOR PER FRAME

Verdict: ✅ Only one editor->Update() call per active editor per frame

3. Editor Update Implementation (e.g., OverworldEditor)

File: src/app/editor/overworld/overworld_editor.cc:228

OverworldEditor::Update()
  ├─ [Lines 240-258] Create local EditorCard instances
  │  └─ EditorCard overworld_canvas_card(...)
  │     EditorCard tile16_card(...)
  │     ... (8 cards total)
  │
  ├─ [Lines 294-300] Overworld Canvas Card
  │  └─ if (show_overworld_canvas_)
  │     if (overworld_canvas_card.Begin(&show_overworld_canvas_))
  │        DrawToolset()
  │        DrawOverworldCanvas()
  │     overworld_canvas_card.End() ← ALWAYS CALLED
  │
  ├─ [Lines 303-308] Tile16 Selector Card
  │  └─ if (show_tile16_selector_)
  │     if (tile16_card.Begin(&show_tile16_selector_))
  │        DrawTile16Selector()
  │     tile16_card.End()
  │
  └─ ... (6 more cards, same pattern)

Pattern: Each card follows strict Begin/End pairing:

if (visibility_flag) {
  if (card.Begin(&visibility_flag)) {
    // Draw content ONCE
  }
  card.End(); // ALWAYS called after Begin()
}

Verdict: ✅ No duplicate Begin() calls - each card rendered exactly once per Update()

4. EditorCard Rendering (`editor_layout.cc`)

EditorCard::Begin(bool* p_open) [Lines 256-366]
  ├─ [Lines 257-261] Check visibility flag
  │  └─ if (p_open && !*p_open) return false
  │
  ├─ [Lines 263-285] 🔍 DUPLICATE DETECTION (NEW)
  │  └─ Track which cards have called Begin() this frame
  │     if (duplicate detected)
  │        fprintf(stderr, "DUPLICATE DETECTED: '%s' frame %d")
  │        duplicate_detected_ = true
  │
  ├─ [Lines 288-292] Handle collapsed state
  ├─ [Lines 294-336] Setup ImGui window
  └─ [Lines 352-356] Call ImGui::Begin()
     └─ imgui_begun_ = true ← Tracks that End() must be called

EditorCard::End() [Lines 369-380]
  └─ if (imgui_begun_)
     ImGui::End()
     imgui_begun_ = false

Diagnostic Behavior:

Frame tracking resets on ImGui::GetFrameCount() change
Each Begin() call checks if card name already in cards_begun_this_frame_
Duplicate detected → logs to stderr and sets flag
This will definitively identify double Begin() calls

Verdict: ✅ Diagnostic will catch any duplicate Begin() calls

5. RightPanelManager (ProposalDrawer, AgentChat, Settings)

File: src/app/editor/ui/right_panel_manager.cc

RightPanelManager::Draw() [Lines 117-181]
  └─ if (active_panel_ != PanelType::kNone)
     ImGui::Begin("##RightPanel", ...)
     DrawPanelHeader(...)
     switch (active_panel_)
        case kProposals: DrawProposalsPanel() [Line 162]
           └─ proposal_drawer_->DrawContent() [Line 238]
              NOT Draw()! Only DrawContent()!
        case kAgentChat: DrawAgentChatPanel()
        case kSettings: DrawSettingsPanel()
     ImGui::End()

Key Discovery: ProposalDrawer has TWO methods:

Draw() - Creates own window (lines 75-107 in proposal_drawer.cc) ← NEVER CALLED
DrawContent() - Renders inside existing window (line 238) ← ONLY THIS IS USED

Verification in EditorManager:

// Line 827 in editor_manager.cc
// Proposal drawer is now drawn through RightPanelManager
// Removed duplicate direct call - DrawProposalsPanel() in RightPanelManager handles it

Verdict: ✅ ProposalDrawer::Draw() is dead code - only DrawContent() used

What Was Ruled Out

❌ Multiple Update() Calls

EditorManager::Update() calls editor->Update() exactly once per active editor (line 1047)
Controller::OnLoad() calls EditorManager::Update() exactly once per frame (line 124)
No loops, no recursion, no duplicate paths

❌ ImGui Begin/End Mismatches

Every EditorCard::Begin() has matching End() call
imgui_begun_ flag prevents double End() calls
Verified in OverworldEditor: 8 cards × 1 Begin + 1 End each = balanced

❌ Sidebar Double Rendering

DrawSidebar() and DrawTreeSidebar() are mutually exclusive
Different window names: ##EditorCardSidebar vs ##TreeSidebar
Only one is called based on IsTreeViewMode() check (lines 757-763)

❌ RightPanel vs Direct Drawer Calls

ProposalDrawer::Draw() is never called (confirmed with grep)
Only DrawContent() used via RightPanelManager::DrawProposalsPanel()
Comment at line 827 confirms duplicate call was removed

❌ EditorCard Registry Drawing Cards

card_registry_.ShowCard() only sets visibility flags
Cards are not drawn by registry - only drawn in editor Update() methods
Registry only manages: visibility state, sidebar UI, card browser

❌ Multi-Viewport Issues

ImGuiConfigFlags_ViewportsEnable is NOT enabled
Only ImGuiConfigFlags_DockingEnable is active
Single viewport architecture - no platform windows

Possible Root Causes (Outside Editor System)

If the diagnostic does NOT detect duplicate Begin() calls, the issue must be:

1. ImGui Draw List Submission

Hypothesis: Draw data is being submitted to GPU twice

// In Controller::DoRender()
ImGui::Render();                     // Generate draw lists
renderer_->Clear();                  // Clear framebuffer
ImGui_ImplSDLRenderer2_RenderDrawData(...); // Submit to GPU
renderer_->Present();                // Swap buffers

Check:

Are draw lists being submitted twice?
Is ImGui_ImplSDLRenderer2_RenderDrawData() called more than once?
Add: printf("RenderDrawData called: frame %d\n", ImGui::GetFrameCount());

2. Z-Ordering / Layering Bug

Hypothesis: Two overlapping windows with same content at same position

// ImGui windows at same coordinates with same content
ImGui::SetNextWindowPos(ImVec2(100, 100));
ImGui::Begin("Window1");
DrawContent(); // Content rendered
ImGui::End();

// Another window at SAME position
ImGui::SetNextWindowPos(ImVec2(100, 100));
ImGui::Begin("Window2");
DrawContent(); // SAME content rendered again
ImGui::End();

Check:

ImGui Metrics window → Show "Windows" section
Look for duplicate windows with same position
Check window Z-order and docking state

3. Texture Double-Binding

Hypothesis: Textures are bound/drawn twice in rendering backend

// In SDL2 renderer backend
SDL_RenderCopy(renderer, texture, ...); // First draw
// ... some code ...
SDL_RenderCopy(renderer, texture, ...); // Accidental second draw

Check:

SDL2 render target state
Multiple texture binding in same frame
Backend drawing primitives twice

4. Stale ImGui State

Hypothesis: Old draw commands not cleared between frames

// Missing clear in backend
void NewFrame() {
  // Should clear old draw data here!
  ImGui_ImplSDLRenderer2_NewFrame();
}

Check:

Is ImGui::NewFrame() clearing old state?
Backend implementation of NewFrame() correct?
Add: ImGui::GetDrawData()->CmdListsCount logging

Recommended Next Steps

Step 1: Run with Diagnostic

cmake --build build --target yaze -j4
./build/bin/yaze --rom_file=zelda3.sfc --editor=Overworld 2>&1 | grep "DUPLICATE"

Expected Output:

If duplicates exist: [EditorCard] DUPLICATE DETECTED: 'Overworld Canvas' Begin() called twice in frame 1234
If no duplicates: (no output)

Step 2: Check Programmatically

// In EditorManager::Update() after line 1056, add:
if (gui::EditorCard::HasDuplicateRendering()) {
  LOG_ERROR("Duplicate card rendering detected: %s",
            gui::EditorCard::GetDuplicateCardName().c_str());
  // Breakpoint here to inspect call stack
}

Step 3A: If Duplicates Detected

Trace the duplicate Begin() call:

Set breakpoint in EditorCard::Begin() at line 279 (duplicate detection)
Condition: duplicate_detected_ == true
Inspect call stack to find second caller
Fix the duplicate code path

Step 3B: If No Duplicates Detected

Issue is outside EditorCard system:

Enable ImGui Metrics: ImGui::ShowMetricsWindow()
Check "Windows" section for duplicate windows

Add logging to Controller::DoRender():

static int render_count = 0;
printf("DoRender #%d: DrawData CmdLists=%d\n",
       ++render_count, ImGui::GetDrawData()->CmdListsCount);

Inspect SDL2 backend for double submission
Check for stale GPU state between frames

Step 4: Alternative Debugging

If issue persists, try:

// In OverworldEditor::Update(), add frame tracking
static int last_frame = -1;
int current_frame = ImGui::GetFrameCount();
if (current_frame == last_frame) {
  LOG_ERROR("OverworldEditor::Update() called TWICE in frame %d!", current_frame);
}
last_frame = current_frame;

Architecture Insights

Editor Rendering Pattern

Decentralized Card Creation:

Each editor creates EditorCard instances locally in its Update() method
Cards are not global - they're stack-allocated temporaries
Visibility is managed by pointers to bool flags that persist across frames

Example:

// In OverworldEditor::Update() - called ONCE per frame
gui::EditorCard tile16_card("Tile16 Selector", ICON_MD_GRID_3X3);
if (show_tile16_selector_) {  // Persistent flag
  if (tile16_card.Begin(&show_tile16_selector_)) {
    DrawTile16Selector(); // Content rendered ONCE
  }
  tile16_card.End();
}
// Card destroyed at end of Update() - stack unwinding

Registry vs Direct Rendering

EditorCardRegistry:

Purpose: Manage visibility flags, sidebar UI, card browser
Does NOT render cards - only manages state
Does render: Sidebar buttons, card browser UI, tree view

Direct Rendering (in editors):

Each editor creates and renders its own cards
Registry provides visibility flag pointers
Editor checks flag, renders if true

Separation of Concerns

Clear boundaries:

Controller - Main loop, window management, single Update() call
EditorManager - Editor lifecycle, session management, single editor->Update() per editor
Editor (e.g., OverworldEditor) - Card creation, content rendering, one Begin/End pair per card
EditorCard - ImGui window wrapper, duplicate detection, Begin/End state tracking
EditorCardRegistry - Visibility management, sidebar UI, no direct card rendering

This architecture prevents duplicate rendering by design - there is only ONE path from main loop to card content.

Diagnostic Code Summary

Location: src/app/gui/app/editor_layout.h (lines 121-135) and editor_layout.cc (lines 17-285)

Static Tracking Variables:

static int last_frame_count_ = 0;
static std::vector<std::string> cards_begun_this_frame_;
static bool duplicate_detected_ = false;
static std::string duplicate_card_name_;

Detection Logic:

// In EditorCard::Begin()
int current_frame = ImGui::GetFrameCount();
if (current_frame != last_frame_count_) {
  // New frame - reset tracking
  cards_begun_this_frame_.clear();
  duplicate_detected_ = false;
}

// Check for duplicate
for (const auto& card_name : cards_begun_this_frame_) {
  if (card_name == window_name_) {
    duplicate_detected_ = true;
    fprintf(stderr, "[EditorCard] DUPLICATE: '%s' frame %d\n",
            window_name_.c_str(), current_frame);
  }
}
cards_begun_this_frame_.push_back(window_name_);

Public API:

static void ResetFrameTracking();           // Manual reset (optional)
static bool HasDuplicateRendering();        // Check if duplicate detected
static const std::string& GetDuplicateCardName(); // Get duplicate card name

Conclusion

The editor system has a clean, single-path rendering architecture. No code paths exist that could cause duplicate card rendering through the normal Update() flow.

If duplicate rendering occurs:

The diagnostic WILL detect it if it's in EditorCard::Begin()
If diagnostic doesn't fire, issue is outside EditorCard (ImGui backend, GPU state, Z-order)

Next Agent Action:

Build and run with diagnostic
Report findings based on stderr output
Follow appropriate Step 3A or 3B from "Recommended Next Steps"

Files Referenced

Core Investigation Files:

/Users/scawful/Code/yaze/src/app/controller.cc - Main loop (lines 56-165)
/Users/scawful/Code/yaze/src/app/editor/editor_manager.cc - Update flow (lines 616-1079)
/Users/scawful/Code/yaze/src/app/editor/overworld/overworld_editor.cc - Editor Update (lines 228-377)
/Users/scawful/Code/yaze/src/app/gui/app/editor_layout.cc - EditorCard implementation (lines 256-380)
/Users/scawful/Code/yaze/src/app/editor/ui/right_panel_manager.cc - Panel system (lines 117-242)
/Users/scawful/Code/yaze/src/app/editor/system/editor_card_registry.cc - Card registry (lines 456-787)
/Users/scawful/Code/yaze/src/app/editor/system/proposal_drawer.h - Draw vs DrawContent (lines 39-43)

Diagnostic Code:

/Users/scawful/Code/yaze/src/app/gui/app/editor_layout.h (lines 121-135)
/Users/scawful/Code/yaze/src/app/gui/app/editor_layout.cc (lines 17-285)

Previous Investigation:

/Users/scawful/Code/yaze/docs/internal/handoff-duplicate-rendering-investigation.md

15 KiB Raw Permalink Blame History Unescape Escape

Complete Duplicate Rendering Investigation

Executive Summary

Complete Call Chain (Main Loop → Editor Content)

1. Main Loop (controller.cc)

2. EditorManager Update Flow (editor_manager.cc:616-843)

3. Editor Update Implementation (e.g., OverworldEditor)

4. EditorCard Rendering (editor_layout.cc)

5. RightPanelManager (ProposalDrawer, AgentChat, Settings)

What Was Ruled Out

❌ Multiple Update() Calls

❌ ImGui Begin/End Mismatches

❌ Sidebar Double Rendering

❌ RightPanel vs Direct Drawer Calls

❌ EditorCard Registry Drawing Cards

❌ Multi-Viewport Issues

Possible Root Causes (Outside Editor System)

1. ImGui Draw List Submission

2. Z-Ordering / Layering Bug

3. Texture Double-Binding

4. Stale ImGui State

Recommended Next Steps

Step 1: Run with Diagnostic

Step 2: Check Programmatically

Step 3A: If Duplicates Detected

Step 3B: If No Duplicates Detected

Step 4: Alternative Debugging

Architecture Insights

Editor Rendering Pattern

Registry vs Direct Rendering

Separation of Concerns

Diagnostic Code Summary

Conclusion

Files Referenced

15 KiB

Raw Permalink Blame History

1. Main Loop (`controller.cc`)

2. EditorManager Update Flow (`editor_manager.cc:616-843`)

4. EditorCard Rendering (`editor_layout.cc`)