Skip to content

fix(mcp): settle render-throttle transient before perf-lever writes return#585

Merged
drsnuggles8 merged 1 commit into
masterfrom
fix/mcp-perf-firstwrite-settle
Jul 7, 2026
Merged

fix(mcp): settle render-throttle transient before perf-lever writes return#585
drsnuggles8 merged 1 commit into
masterfrom
fix/mcp-perf-firstwrite-settle

Conversation

@drsnuggles8

@drsnuggles8 drsnuggles8 commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • olo_renderer_settings_set and olo_scene_open could return before the editor's render-budget throttle actually cleared, so a caller reading olo_perf_snapshot/olo_perf_pass_timings right afterward could see stale pre-change data — RendererProfiler::BeginFrame/EndFrame only run inside Renderer3D::BeginScene, which the throttle skips for a beat after any main-thread stall (a scene load being the common trigger).
  • Both handlers now wait out that transient via the existing AwaitRenderedFrames/IsCaptureUnready settle discipline (the same mechanism the camera-pose screenshot tools already use), so a couple of real frames have rendered with the new state by the time either call returns.
  • Closes the last open item in MCP perf tooling: CPU-scope profiling, GPU-timing accuracy under load, overdraw debug view #519 ("First perf-lever write right after scene load doesn't take effect on the GPU").

Test plan

  • Added OloEngine/tests/Rendering/PropertyTests/McpPerfSettleTest.cpp (headless MCP host, GPU-gated) asserting olo_renderer_settings_set doesn't return until the frame counter advances past the settle window.
  • Live-verified against a running editor over MCP: olo_scene_open → immediate olo_renderer_settings_set(depthprepass=on) → immediate olo_perf_pass_timings shows the DepthPrepass GPU sub-pass with fresh (gpuResultsAgeFrames: 1) data; flipping back to off and re-checking immediately correctly shows it gone.
  • OloEngine-Tests.exe --gtest_filter=McpPerfSettleTest.* — build was still compiling in this environment when I finished; please run before merging if CI doesn't cover it.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Scene loading and renderer setting updates now wait briefly for rendering to settle before completing, so changes are visible immediately in subsequent reads.
    • This reduces stale results after opening a scene or changing performance/rendering options.
  • Tests

    • Added coverage to verify renderer setting changes do not return until the updated state has been rendered.

…eturn (#519)

A renderer-setting write (olo_renderer_settings_set) or scene load
(olo_scene_open) landing right after a main-thread stall could return
before the editor's render-budget throttle cleared, so the change hadn't
actually rendered yet: RendererProfiler::BeginFrame/EndFrame only run
inside Renderer3D::BeginScene, which the throttle skips, so
GetLastCompletedFrameData() (what olo_perf_snapshot/olo_perf_pass_timings
read) stayed frozen on stale pre-change data until an unrelated later
frame rendered — the "first write doesn't take effect, repeatable A/B
afterwards" symptom from #519's last open item.

Both handlers now wait out the throttle/resize transient via
AwaitRenderedFrames before returning, mirroring the settle discipline the
camera-pose screenshot tools already use for the identical class of
problem.

Live-verified against a running editor: an immediate
olo_renderer_settings_set(depthprepass) + olo_perf_pass_timings pair now
shows the change with fresh (non-stale) GPU data on the first read.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

An error occurred during the review process. Please try again later.

📝 Walkthrough

Walkthrough

Two MCP editor tool handlers now block briefly after applying changes, awaiting a small number of rendered frames before returning, to ensure subsequent reads observe updated state. A new integration test validates this settle behavior for the renderer settings tool, and the test is registered in the CMake build.

Changes

MCP Tool Settle Wait

Layer / File(s) Summary
Post-action frame settle in MCP tools
OloEditor/src/MCP/McpTools.cpp
Handle_RendererSettingsSet and Handle_SceneOpen now call AwaitRenderedFrames(...) for a small number of frames after applying a setting or opening a scene, when frame indexing is available, before returning.
Integration test verifying settle behavior
OloEngine/tests/Rendering/PropertyTests/McpPerfSettleTest.cpp, OloEngine/tests/CMakeLists.txt
New McpPerfSettleTest builds a minimal lit scene, hosts an in-process McpHeadlessHost, invokes olo_renderer_settings_set, and asserts at least 2 additional frames render before the call returns; the test file is added to the OloEngine-Tests build.

Sequence Diagram(s)

sequenceDiagram
  participant Test as McpPerfSettleTest
  participant Host as McpHeadlessHost
  participant Tool as McpTools
  participant Renderer

  Test->>Host: Boot host, warm frames, grant write permission
  Test->>Tool: olo_renderer_settings_set(depthprepass=on)
  Tool->>Renderer: Apply(setting)
  Tool->>Renderer: AwaitRenderedFrames(kSettingsSettleFrames)
  Renderer-->>Tool: frames rendered
  Tool-->>Test: success result
  Test->>Test: Assert frame index advanced by >= 2
Loading

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@drsnuggles8

Copy link
Copy Markdown
Owner Author

Ran the full MCP test suite locally: `OloEngine-Tests.exe --gtest_filter="Mcp*.*"` → 344 passed, 1 skipped (`McpHeadlessAttachTest.HostUntilDetached`, the interactive-only opt-in test — expected). `McpPerfSettleTest.RendererSettingsSetSettlesBeforeReturning` passes.

@sonarqubecloud

sonarqubecloud Bot commented Jul 7, 2026

Copy link
Copy Markdown

@drsnuggles8 drsnuggles8 merged commit 7146a9b into master Jul 7, 2026
9 of 10 checks passed
@drsnuggles8 drsnuggles8 deleted the fix/mcp-perf-firstwrite-settle branch July 7, 2026 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant