Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions OloEditor/src/MCP/McpTools.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3689,6 +3689,32 @@

if (result.is_object() && result.contains("__error"))
return ToolResult::Error(result["__error"].get<std::string>());

// Settle before returning (#519 "first perf-lever write right after
// scene load doesn't take effect on the GPU"). A lever flip right
// after a heavy scene load lands while the editor's render-budget
// throttle is still skipping frames — the just-blocked main thread
// reports a huge timestep for the next OnUpdate, which trips
// skipRender for a beat. While throttled, Renderer3D::BeginScene
// never runs, so RendererProfiler::BeginFrame/EndFrame don't either:
// GetLastCompletedFrameData() (what olo_perf_snapshot reads) stays
// frozen on whatever rendered before the write, making the change
// invisible until an unrelated later frame finally renders. Waiting
// out the same throttle/resize transient the screenshot tools
// already respect (AwaitRenderedFrames/IsCaptureUnready) guarantees
// at least a couple of real frames have executed with the new
// setting by the time this call returns, so an immediately
// following olo_perf_snapshot reads live data instead of a stale
// pre-change frame.
constexpr int kSettingsSettleFrames = 2;
if (server.Context().GetFrameIndex)
{
const u64 baseFrame = server.MarshalRead([&server]() -> Json

Check warning on line 3712 in OloEditor/src/MCP/McpTools.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove the redundant return type of this lambda.

See more on https://sonarcloud.io/project/issues?id=drsnuggles8_OloEngineBase&issues=AZ8-C5LoVRbCU165GYxN&open=AZ8-C5LoVRbCU165GYxN&pullRequest=585
{ return Json{ { "frame", server.Context().GetFrameIndex() } }; })
.value("frame", static_cast<u64>(0));
AwaitRenderedFrames(server, baseFrame, kSettingsSettleFrames);
}

return ToolResult::Text(result.dump(2));
}

Expand Down Expand Up @@ -3751,6 +3777,29 @@

if (result.is_object() && result.contains("__error"))
return ToolResult::Error(result["__error"].get<std::string>());

// Settle before returning (#519 "first perf-lever write right after
// scene load doesn't take effect on the GPU"). The load above ran
// synchronously inside the MarshalRead job, blocking the main-thread
// frame pump for however long it took; the very next OnUpdate sees
// that stall as an inflated timestep and trips the render-budget
// throttle for a beat, during which Renderer3D::BeginScene (and so
// RendererProfiler::BeginFrame/EndFrame) never runs. A caller that
// immediately writes a renderer setting and reads back
// olo_perf_snapshot in that window sees stale pre-load data, not the
// new state — Handle_RendererSettingsSet settles on its own end too,
// but waiting here as well means a plain scene-load caller (no
// follow-up settings write) also gets a scene that has actually
// rendered before the tool returns.
constexpr int kPostLoadSettleFrames = 2;
if (server.Context().GetFrameIndex)
{
const u64 baseFrame = server.MarshalRead([&server]() -> Json

Check warning on line 3797 in OloEditor/src/MCP/McpTools.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove the redundant return type of this lambda.

See more on https://sonarcloud.io/project/issues?id=drsnuggles8_OloEngineBase&issues=AZ8-C5LoVRbCU165GYxO&open=AZ8-C5LoVRbCU165GYxO&pullRequest=585
{ return Json{ { "frame", server.Context().GetFrameIndex() } }; })
.value("frame", static_cast<u64>(0));
AwaitRenderedFrames(server, baseFrame, kPostLoadSettleFrames);
}

return ToolResult::Text(result.dump(2));
}

Expand Down
5 changes: 5 additions & 0 deletions OloEngine/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,11 @@ add_executable(OloEngine-Tests
# loop. Header-only host helper (McpHeadlessHost.h); pulls in the real MCP
# tools (McpTools.cpp + McpScriptApi.cpp) added to the source list below.
Rendering/PropertyTests/McpHeadlessAttachTest.cpp
# Pins the #519 "first perf-lever write after scene load doesn't take
# effect" settle-wait fix: olo_renderer_settings_set now waits out the
# render-budget throttle transient before returning. Same McpHeadlessHost
# infra as McpHeadlessAttachTest.cpp above.
Rendering/PropertyTests/McpPerfSettleTest.cpp
Rendering/PropertyTests/VideoOverlayVisualEvidenceTest.cpp
Rendering/PropertyTests/AutoExposureEvidenceTest.cpp
Rendering/PropertyTests/TestFailureCapture.cpp
Expand Down
138 changes: 138 additions & 0 deletions OloEngine/tests/Rendering/PropertyTests/McpPerfSettleTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
// =============================================================================
// McpPerfSettleTest.cpp
//
// Pins the #519 "Smaller findings" fix: the first perf-lever write right after
// a heavy scene load (or any render-throttled beat) didn't take effect on the
// GPU — a caller that flipped a renderer setting then immediately read
// olo_perf_snapshot saw stale pre-change data, because RendererProfiler's
// BeginFrame/EndFrame (and so GetLastCompletedFrameData(), what the snapshot
// reads) only run inside Renderer3D::BeginScene, which the editor's render-
// budget throttle skips entirely for a beat after a stall. Handle_
// RendererSettingsSet (McpTools.cpp) now waits out that transient — the same
// AwaitRenderedFrames/IsCaptureUnready settle discipline the camera-pose
// screenshot tools already use — before returning, so a caller never
// round-trips on the "changed but not yet rendered" window.
//
// This test proves the settle-wait actually executes (not just that the tool
// still returns successfully): olo_renderer_settings_set must pump at least
// `kSettingsSettleFrames` (2) more rendered frames before its response comes
// back, which is only true if AwaitRenderedFrames blocks the response as
// intended. Without the fix the call returns near-instantly (0-1 pumps).
//
// Classification: integration (real Renderer3D bring-up + in-process MCP
// dispatch via McpHeadlessHost). Sibling of McpHeadlessAttachTest.
// =============================================================================

// OLO_TEST_LAYER: integration

#include "OloEnginePCH.h"

#include "McpHeadlessHost.h"
#include "RenderPropertyTest.h"
#include "RendererAttachedTest.h"

#include "MCP/McpServer.h"

#include "OloEngine/Renderer/Camera/EditorCamera.h"
#include "OloEngine/Renderer/Framebuffer.h"
#include "OloEngine/Renderer/Mesh.h"
#include "OloEngine/Renderer/MeshPrimitives.h"
#include "OloEngine/Renderer/Renderer3D.h"
#include "OloEngine/Renderer/ResourceHandle.h"
#include "OloEngine/Scene/Components.h"
#include "OloEngine/Scene/Entity.h"

#include <gtest/gtest.h>

namespace OloEngine::Tests
{
namespace
{
using Json = OloEngine::MCP::Json;

constexpr u32 kWidth = 320;
constexpr u32 kHeight = 180;
} // namespace

// A minimal lit scene, same shape as McpHeadlessAttachTest's fixture — only
// needs SOMETHING rendering so Renderer3D is fully initialized and
// olo_renderer_settings_set has a live graph to act on.
class McpPerfSettleTest : public RendererAttachedTest
{
protected:
void BuildScene() override
{
Scene& scene = GetScene();
EnableRendering(kWidth, kHeight);

Entity light = scene.CreateEntity("Sun");
auto& dl = light.AddComponent<DirectionalLightComponent>();
dl.m_Direction = glm::normalize(glm::vec3(-0.4f, -0.8f, -0.45f));
dl.m_Intensity = 3.0f;

Entity cube = scene.CreateEntity("Cube");
auto& mc = cube.AddComponent<MeshComponent>();
mc.m_Primitive = MeshPrimitive::Cube;
if (Ref<Mesh> mesh = MeshPrimitives::CreateCube())
mc.m_MeshSource = mesh->GetMeshSource();
cube.AddComponent<MaterialComponent>();
}

[[nodiscard]] EditorCamera MakeCamera() const
{
EditorCamera camera(45.0f, static_cast<f32>(kWidth) / static_cast<f32>(kHeight), 0.1f, 1000.0f);
camera.SetViewportSize(static_cast<f32>(kWidth), static_cast<f32>(kHeight));
camera.SetPose(glm::vec3(0.0f, 1.2f, 4.5f), /*yaw=*/0.0f, /*pitch=*/0.18f);
return camera;
}
};

TEST_F(McpPerfSettleTest, RendererSettingsSetSettlesBeforeReturning)
{
OLO_ENSURE_GPU_OR_SKIP();

EditorCamera camera = MakeCamera();
RunEditorFrames(camera, 2); // warm the graph up before hosting

McpHeadlessHost host(McpHeadlessHost::Hooks{
.GetScene = [this]() -> Ref<Scene>
{ return GetSceneRef(); },
.GetCompositeFramebuffer = []() -> Ref<Framebuffer>
{ return Renderer3D::ResolveFrameGraphFramebuffer(ResourceNames::UIComposite); },
.Camera = &camera,
.RenderWidth = kWidth,
.RenderHeight = kHeight,
.RenderOneFrame = [this, &camera]()
{ RunEditorFrames(camera, 1); },
});

const u16 port = host.Start();
ASSERT_NE(port, 0) << "McpHeadlessHost failed to bind a port for the MCP server";

// olo_renderer_settings_set is a consented WRITE tool.
host.Server().SetAllowWrites(true);

// Get past the host's own frame==0 "unready" special-case before timing
// the settle window, so the measurement below isolates the NEW settle
// logic in Handle_RendererSettingsSet rather than that startup case.
host.PumpOnce();
host.PumpOnce();

const u64 before = host.FrameIndex();
const Json resp = host.CallTool("olo_renderer_settings_set", Json{ { "setting", "depthprepass" }, { "value", "on" } });
const u64 after = host.FrameIndex();

ASSERT_TRUE(resp.contains("result")) << "olo_renderer_settings_set returned an error: " << resp.dump(2);
EXPECT_FALSE(resp["result"].value("isError", false)) << resp.dump(2);

// The core regression guard: the call must not return until at least a
// couple more frames have actually rendered with the new setting
// applied — proving AwaitRenderedFrames genuinely blocked the response
// rather than the tool returning as soon as the write landed.
EXPECT_GE(after - before, 2u)
<< "olo_renderer_settings_set returned without settling rendered frames (#519 regression: "
"a caller reading olo_perf_snapshot immediately afterward could see stale pre-change data)";

host.Stop();
}
} // namespace OloEngine::Tests
Loading