From 19398e8eb5903a1c8fff533dbf08b2f8a2334b31 Mon Sep 17 00:00:00 2001 From: Ole Bueker Date: Tue, 7 Jul 2026 00:16:28 +0200 Subject: [PATCH 1/2] feat(scene): decouple render from fixed sim tick with interpolation (#502) Render an interpolated pose between the two most recent fixed-tick states so motion stays smooth when the display rate isn't a multiple of the sim rate (e.g. 60 Hz sim on a 144 Hz display). The fixed-tick accumulator already existed in Scene::OnUpdateRuntimeFixed (#484/#452); this adds interpolation on top. - OnUpdateRuntimeFixed snapshots each entity's local TransformComponent into m_InterpPrev before each step and m_InterpCurr after the last step, and publishes m_RenderInterpAlpha = accumulator / fixedDt. - RenderRuntime overwrites live locals with the blended pose (glm::mix for translation/scale, glm::slerp for rotation), re-propagates world matrices, renders, then restores the exact fixed-tick poses. Overwrite-then-restore makes every render read (GetWorldTransform world matrices AND direct TransformComponent::GetTransform() reads) see the blended pose without threading alpha through every draw call. Restore copies the whole component so the private RotationEuler can't drift; the primary fly-cam is excluded (a gameplay camera's view is blended into cameraTransform instead). - Purely presentational: sim state is restored before RenderRuntime returns, so determinism (#484) is preserved and post-render consumers read the exact tick pose. Editor toggle: EditorPreferences::RenderInterpolation (on by default), persisted + applied on Play and live via ApplyPreferences. OloRuntime uses the default. Test: RenderInterpolationSmoothsMotionTest drives a Lua constant-velocity mover through OnUpdateRuntimeFixed at 60/144 and proves the interpolated render pose advances in near-constant increments (judder-free, second difference ~0) while the raw sim pose stair-steps, and that interpolation never leaks a fractional pose into persisted sim state. Full suite: 4185 pass / 0 fail; existing fixed-timestep determinism tests unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- OloEditor/src/EditorLayer.cpp | 12 + .../src/Panels/EditorPreferencesPanel.cpp | 21 ++ OloEditor/src/Panels/EditorPreferencesPanel.h | 6 + OloEngine/src/OloEngine/Scene/Scene.cpp | 188 +++++++++++++++ OloEngine/src/OloEngine/Scene/Scene.h | 65 +++++ OloEngine/tests/CMakeLists.txt | 1 + .../RenderInterpolationSmoothsMotionTest.cpp | 227 ++++++++++++++++++ 7 files changed, 520 insertions(+) create mode 100644 OloEngine/tests/Functional/Scene/RenderInterpolationSmoothsMotionTest.cpp diff --git a/OloEditor/src/EditorLayer.cpp b/OloEditor/src/EditorLayer.cpp index f4a96572e..875d72635 100644 --- a/OloEditor/src/EditorLayer.cpp +++ b/OloEditor/src/EditorLayer.cpp @@ -2009,6 +2009,13 @@ namespace OloEngine Application::Get().SetFrameRateCap(m_Prefs.FrameRateCap); Application::Get().SetFrameTimeSmoothing(m_Prefs.FrameTimeSmoothing); + // Render interpolation (#502): apply live so toggling it in Preferences + // takes effect on the running Play session without a restart. + if (m_ActiveScene) + { + m_ActiveScene->SetRenderInterpolationEnabled(m_Prefs.RenderInterpolation); + } + auto& physicsSettings = Physics3DSystem::GetSettings(); physicsSettings.m_CaptureOnPlay = m_Prefs.CapturePhysicsOnPlay; @@ -3155,6 +3162,11 @@ namespace OloEngine } } + // Apply the render-interpolation preference to the fresh runtime scene + // (#502): Play renders interpolated poses between fixed ticks unless the + // user disabled it. + m_ActiveScene->SetRenderInterpolationEnabled(m_Prefs.RenderInterpolation); + m_ActiveScene->OnRuntimeStart(); // Stream quest/inventory gameplay events to the Console panel for this diff --git a/OloEditor/src/Panels/EditorPreferencesPanel.cpp b/OloEditor/src/Panels/EditorPreferencesPanel.cpp index 9e02e1226..7737427a1 100644 --- a/OloEditor/src/Panels/EditorPreferencesPanel.cpp +++ b/OloEditor/src/Panels/EditorPreferencesPanel.cpp @@ -280,6 +280,24 @@ namespace OloEngine ImGui::SetTooltip("EMA weight for the newest frame's delta.\nLower = smoother but laggier. 1.0 disables smoothing."); } } + + ImGui::Spacing(); + ImGui::Text("Render Interpolation"); + ImGui::Separator(); + ImGui::TextWrapped("Decouple rendering from the fixed simulation tick: the sim runs at a " + "stable fixed rate while each displayed frame renders a pose " + "interpolated between the last two ticks. Keeps motion smooth when the " + "refresh rate isn't a multiple of the sim rate (e.g. 60 Hz sim / 144 Hz " + "display). On by default."); + ImGui::Spacing(); + ImGui::Checkbox("Interpolate render pose between fixed ticks", &m_Draft.RenderInterpolation); + ImGui::SameLine(); + ImGui::TextDisabled("(?)"); + if (ImGui::IsItemHovered()) + { + ImGui::SetTooltip("Off = one render per sim tick (may judder at non-multiple refresh rates).\n" + "Purely a presentation blend — never affects simulation determinism."); + } } std::filesystem::path EditorPreferencesPanel::GetPrefsPath(const std::filesystem::path& projectDir) @@ -309,6 +327,7 @@ namespace OloEngine out << YAML::Key << "RenderBudgetMs" << YAML::Value << prefs.RenderBudgetMs; out << YAML::Key << "FrameRateCap" << YAML::Value << prefs.FrameRateCap; out << YAML::Key << "FrameTimeSmoothing" << YAML::Value << prefs.FrameTimeSmoothing; + out << YAML::Key << "RenderInterpolation" << YAML::Value << prefs.RenderInterpolation; out << YAML::Key << "EnableAutoSave" << YAML::Value << prefs.EnableAutoSave; out << YAML::Key << "AutoSaveIntervalSeconds" << YAML::Value << prefs.AutoSaveIntervalSeconds; out << YAML::Key << "McpAutoStart" << YAML::Value << prefs.McpAutoStart; @@ -419,6 +438,8 @@ namespace OloEngine if (node["FrameTimeSmoothing"]) if (f32 v = node["FrameTimeSmoothing"].as(); std::isfinite(v)) prefs.FrameTimeSmoothing = std::clamp(v, 0.01f, 1.0f); + if (node["RenderInterpolation"]) + prefs.RenderInterpolation = node["RenderInterpolation"].as(); if (node["EnableAutoSave"]) prefs.EnableAutoSave = node["EnableAutoSave"].as(); if (node["AutoSaveIntervalSeconds"]) diff --git a/OloEditor/src/Panels/EditorPreferencesPanel.h b/OloEditor/src/Panels/EditorPreferencesPanel.h index a6bac7d88..459008561 100644 --- a/OloEditor/src/Panels/EditorPreferencesPanel.h +++ b/OloEditor/src/Panels/EditorPreferencesPanel.h @@ -54,6 +54,12 @@ namespace OloEngine u32 FrameRateCap = 0; f32 FrameTimeSmoothing = 1.0f; + // Render interpolation (#502). When true, Play/runtime rendering blends + // between the two most recent fixed-tick poses (alpha = accumulator / + // fixedStep) so motion stays smooth at non-multiple refresh rates. Purely + // presentational — never affects simulation determinism. On by default. + bool RenderInterpolation = true; + // Physics debug bool CapturePhysicsOnPlay = false; diff --git a/OloEngine/src/OloEngine/Scene/Scene.cpp b/OloEngine/src/OloEngine/Scene/Scene.cpp index 67924ca8b..1b68a7316 100644 --- a/OloEngine/src/OloEngine/Scene/Scene.cpp +++ b/OloEngine/src/OloEngine/Scene/Scene.cpp @@ -102,6 +102,7 @@ #include "OloEngine/Project/Project.h" #include +#include #include // Box2D @@ -863,6 +864,12 @@ namespace OloEngine m_SimulationTick = 0; m_FixedTimeAccumulator = 0.0f; m_SimulationTime = 0.0f; + // Discard interpolation snapshots so the first rendered frame of a fresh + // run draws the exact tick-0 pose (no blend from a stale prior run). + m_InterpPrev.clear(); + m_InterpCurr.clear(); + m_HasInterpSnapshots = false; + m_RenderInterpAlpha = 0.0f; RandomUtils::SetGlobalSeed(Application::Get().GetRandomSeed()); // Unified diagnostics timeline (#306 item B): the single authoritative fire for @@ -1141,6 +1148,10 @@ namespace OloEngine m_SimulationTick = 0; m_FixedTimeAccumulator = 0.0f; m_SimulationTime = 0.0f; + m_InterpPrev.clear(); + m_InterpCurr.clear(); + m_HasInterpSnapshots = false; + m_RenderInterpAlpha = 0.0f; OnPhysics2DStart(); OnPhysics3DStart(); @@ -1567,16 +1578,110 @@ namespace OloEngine u32 steps = 0; while (m_FixedTimeAccumulator >= fixedDt && steps < kMaxFixedStepsPerFrame) { + // Render interpolation (issue #502): snapshot the live local + // transforms into m_InterpPrev BEFORE each step so, after the + // loop, m_InterpPrev holds the state one tick behind the final + // step and m_InterpCurr (captured below) holds the post-step + // state. RenderRuntime blends between them by alpha. Skipped + // entirely when interpolation is off. The overwrite each + // iteration is intentional — only the pre-last-step pose + // survives, which is exactly the "previous" state to blend from. + if (m_RenderInterpolationEnabled) + { + CaptureLocalTransforms(m_InterpPrev); + } SimulateRuntimeStep(fixedDt); m_FixedTimeAccumulator -= fixedDt; ++steps; } + + // Publish the fresh "current" pose and the blend factor for the + // render below. Only when a step actually ran this frame — an + // idle frame (accumulator < fixedDt) keeps the existing pair and + // just advances alpha, which is what fills the gap between ticks. + if (steps > 0 && m_RenderInterpolationEnabled) + { + CaptureLocalTransforms(m_InterpCurr); + m_HasInterpSnapshots = true; + } + + // Blend factor for this frame's render: how far the leftover + // accumulator has advanced toward the next fixed tick, in [0, 1]. + // Paused / fixedDt<=0 frames leave it untouched (last value), so a + // paused view shows the frozen pose it last rendered. + m_RenderInterpAlpha = std::clamp(m_FixedTimeAccumulator / fixedDt, 0.0f, 1.0f); } } RenderRuntime(safeFrameTs); } + void Scene::CaptureLocalTransforms(std::unordered_map& out) + { + out.clear(); + auto view = m_Registry.view(); + out.reserve(view.size()); + for (auto entity : view) + { + const auto& tc = view.get(entity); + out.emplace(static_cast(std::to_underlying(entity)), + InterpTransform{ tc.Translation, tc.GetRotation(), tc.Scale }); + } + } + + bool Scene::ShouldInterpolateThisFrame() const + { + // No point blending when the toggle is off, we have no prior state to + // blend from, or we're sitting exactly on a tick boundary (alpha == 0 + // renders the current pose verbatim, so skip the overwrite cost). + return m_RenderInterpolationEnabled && m_HasInterpSnapshots && m_RenderInterpAlpha > 0.0f; + } + + bool Scene::ComputeInterpolatedLocal(entt::entity entity, InterpTransform& out) const + { + if (!m_RenderInterpolationEnabled || !m_HasInterpSnapshots) + { + return false; + } + + const auto id = static_cast(std::to_underlying(entity)); + const auto prevIt = m_InterpPrev.find(id); + const auto currIt = m_InterpCurr.find(id); + // An entity present in only one snapshot (spawned or destroyed between + // the two ticks) has no meaningful blend — render it at its live pose. + if (prevIt == m_InterpPrev.end() || currIt == m_InterpCurr.end()) + { + return false; + } + + const InterpTransform& prev = prevIt->second; + const InterpTransform& curr = currIt->second; + const f32 a = m_RenderInterpAlpha; + + out.Translation = glm::mix(prev.Translation, curr.Translation, a); + out.Scale = glm::mix(prev.Scale, curr.Scale, a); + // Shortest-arc quaternion blend so a near-180° tick doesn't spin the + // long way. glm::slerp normalizes and picks the shorter hemisphere. + out.Rotation = glm::slerp(prev.Rotation, curr.Rotation, a); + return true; + } + + glm::mat4 Scene::GetInterpolatedLocalTransform(entt::entity entity) const + { + if (InterpTransform interp; ComputeInterpolatedLocal(entity, interp)) + { + return glm::translate(glm::mat4(1.0f), interp.Translation) * + glm::toMat4(interp.Rotation) * + glm::scale(glm::mat4(1.0f), interp.Scale); + } + // Fall back to the live local transform. + if (auto const* tc = m_Registry.try_get(entity)) + { + return tc->GetTransform(); + } + return glm::mat4(1.0f); + } + void Scene::UpdateStreaming() { // Refresh LocalizedTextComponent → TextComponent.TextString if the @@ -2455,6 +2560,11 @@ namespace OloEngine // Find the primary camera Camera const* mainCamera = nullptr; glm::mat4 cameraTransform; + // Remember the primary camera entity + whether it's the live fly-cam so + // the render-interpolation pass (below) can blend a gameplay camera's + // view but leave the display-rate fly-cam untouched (issue #502). + entt::entity primaryCameraEntity = entt::null; + bool primaryCameraIsFlyCam = false; { for (const auto view = m_Registry.view(); const auto entity : view) { @@ -2526,11 +2636,74 @@ namespace OloEngine mainCamera = &camera.Camera; cameraTransform = transform.GetTransform(); + primaryCameraEntity = entity; + primaryCameraIsFlyCam = camera.RuntimeControl; break; } } } + // ── Render interpolation (issue #502) ─────────────────────────────── + // Overwrite every entity's live local transform with the pose blended + // between the last two fixed-tick snapshots (alpha = accumulator / + // fixedStep), recompose world matrices, render, then restore the + // authoritative poses at the end of the frame. This makes EVERY render + // read — GetWorldTransform (world matrices) AND the direct + // TransformComponent::GetTransform() reads in RenderScene3D — see the + // interpolated pose without threading alpha through every draw call. The + // simulation state itself is untouched (restored before this function + // returns), so determinism (#484) is preserved and any post-render + // consumer this frame (SaveGameManager::Tick, editor gizmos) still reads + // the exact fixed-tick pose. + // + // The primary camera is excluded from the overwrite: its live local may + // have diverged from the snapshot this frame (the fly-cam mutates it at + // display rate above), so restoring from the snapshot would erase that + // movement. A gameplay (non-fly) camera is instead blended directly into + // cameraTransform so the VIEW interpolates without an ECS round-trip. + const bool interpolateThisFrame = ShouldInterpolateThisFrame(); + // Full-component copies (not just TRS): TransformComponent's Rotation / + // RotationEuler are private and SetRotation() re-derives the Euler + // representation, so restoring via the setter could drift RotationEuler + // even when the quat is bit-identical. Copying the whole component and + // assigning it back restores every field — incl. the private Euler and + // the matrix cache — exactly, guaranteeing zero simulation-state drift. + std::vector> restorePoses; + if (interpolateThisFrame) + { + if (mainCamera && !primaryCameraIsFlyCam) + { + cameraTransform = GetInterpolatedLocalTransform(primaryCameraEntity); + } + + for (const auto view = m_Registry.view(); const auto entity : view) + { + if (entity == primaryCameraEntity) + { + continue; + } + InterpTransform interp; + if (!ComputeInterpolatedLocal(entity, interp)) + { + continue; + } + auto& tc = view.get(entity); + // Save the authoritative pose before overwriting so it can be + // restored verbatim after the draw. + restorePoses.emplace_back(entity, tc); + tc.Translation = interp.Translation; + tc.Scale = interp.Scale; + tc.SetRotation(interp.Rotation); + } + + // Recompose parent-chain world matrices from the interpolated locals + // so GetWorldTransform() reads see the blended pose. + if (!restorePoses.empty()) + { + PropagateWorldTransforms(); + } + } + // Update camera VP matrix before resolving UI layout so world-anchor // projections use the current frame's camera, not the previous one. if (mainCamera) @@ -2682,6 +2855,21 @@ namespace OloEngine // instead of using whatever timestamp was last recorded. m_LastAnimationTime = -1.0f; } + + // Restore the authoritative fixed-tick poses overwritten for the + // interpolated draw above, then recompose world matrices so any + // post-render reader this frame sees the exact simulation state (#502). + if (!restorePoses.empty()) + { + for (const auto& [entity, pose] : restorePoses) + { + if (auto* tc = m_Registry.try_get(entity)) + { + *tc = pose; + } + } + PropagateWorldTransforms(); + } } void Scene::OnUpdateSimulation(const Timestep ts, EditorCamera const& camera) diff --git a/OloEngine/src/OloEngine/Scene/Scene.h b/OloEngine/src/OloEngine/Scene/Scene.h index bd838fdad..e861b9b4b 100644 --- a/OloEngine/src/OloEngine/Scene/Scene.h +++ b/OloEngine/src/OloEngine/Scene/Scene.h @@ -22,6 +22,7 @@ #include #include +#include #include #pragma warning(push) @@ -112,6 +113,37 @@ namespace OloEngine return m_SimulationTick; } + // ── Render interpolation (issue #502) ─────────────────────────────── + // Decouples the display rate from the fixed simulation tick. When + // enabled, OnUpdateRuntimeFixed keeps the two most recent fixed-tick + // states and RenderRuntime draws an interpolated pose using + // alpha = accumulator / fixedStep as the blend factor, so motion stays + // smooth even when the refresh rate isn't a multiple of the sim rate + // (e.g. 60 Hz sim on a 144 Hz display). Purely a presentation concern: + // it never mutates the persisted simulation state (poses are overwritten + // for the draw then restored), so it does NOT affect determinism (#484). + // On by default; the editor exposes a toggle. + void SetRenderInterpolationEnabled(bool enabled) + { + m_RenderInterpolationEnabled = enabled; + } + [[nodiscard("Store this!")]] bool IsRenderInterpolationEnabled() const + { + return m_RenderInterpolationEnabled; + } + // The blend factor used for the most recent render, in [0, 1]: + // accumulator / fixedStep after the last OnUpdateRuntimeFixed call. + [[nodiscard("Store this!")]] f32 GetRenderInterpolationAlpha() const + { + return m_RenderInterpAlpha; + } + // The interpolated LOCAL transform matrix rendering would use for + // `entity` this frame (lerp of the last two fixed-tick poses at the + // current alpha). Falls back to the entity's live transform when + // interpolation is disabled, no snapshot pair exists yet, or the entity + // isn't present in both snapshots. Exposed for tests / diagnostics. + [[nodiscard]] glm::mat4 GetInterpolatedLocalTransform(entt::entity entity) const; + [[nodiscard]] u32 GetViewportWidth() const { return m_ViewportWidth; @@ -630,6 +662,28 @@ namespace OloEngine void UpdateStreaming(); void SimulateRuntimeStep(Timestep ts); void RenderRuntime(Timestep ts); + + // ── Render interpolation snapshots (issue #502) ───────────────────── + // A per-entity local-transform pose captured at a fixed-tick boundary. + // Deliberately NOT named *Component so it never gets swept into the + // generated ECS/serializer tuples — it is transient render state. + struct InterpTransform + { + glm::vec3 Translation{ 0.0f }; + glm::quat Rotation{ 1.0f, 0.0f, 0.0f, 0.0f }; + glm::vec3 Scale{ 1.0f }; + }; + // Snapshot every entity's live local TransformComponent into `out` + // (clearing prior contents). Keyed by the raw entt entity id. + void CaptureLocalTransforms(std::unordered_map& out); + // Compose the interpolated pose for `entity` at the current alpha into + // `out`. Returns false (leaving `out` untouched) when interpolation is + // inactive or the entity lacks a snapshot pair — callers then use the + // live transform. `interpolate` short-circuits the enable/snapshot gate. + [[nodiscard]] bool ComputeInterpolatedLocal(entt::entity entity, InterpTransform& out) const; + // True when this frame should render interpolated poses: the toggle is + // on, a snapshot pair exists, and the blend factor is meaningfully < 1. + [[nodiscard]] bool ShouldInterpolateThisFrame() const; // Step 2D (Box2D) + 3D (Jolt, incl. m_SimulationTime-driven buoyancy) // physics one tick and sync the results back onto the ECS transforms // (Rigidbody2D / Rigidbody3D / CharacterController3D). This is the @@ -720,6 +774,17 @@ namespace OloEngine // once per SimulateRuntimeStep. f32 m_FixedTimeAccumulator = 0.0f; u64 m_SimulationTick = 0; + // Render interpolation (issue #502). The two most recent fixed-tick + // local-transform poses (m_InterpPrev = one tick behind m_InterpCurr), + // the blend factor for the last render (accumulator / fixedDt), whether + // a valid snapshot pair exists yet, and the enable toggle (on by + // default). Maps are keyed by the raw entt entity id; cleared/refilled + // each capture so destroyed entities drop out naturally. + std::unordered_map m_InterpPrev; + std::unordered_map m_InterpCurr; + f32 m_RenderInterpAlpha = 0.0f; + bool m_HasInterpSnapshots = false; + bool m_RenderInterpolationEnabled = true; // Deterministic simulation clock (seconds), advanced by exactly `ts` per // sim tick, used for time-driven physics (buoyancy wave phase) so it is // reproducible across frame pacings / rollback instead of wall-clock. diff --git a/OloEngine/tests/CMakeLists.txt b/OloEngine/tests/CMakeLists.txt index a8ec6c04d..54c150dcc 100644 --- a/OloEngine/tests/CMakeLists.txt +++ b/OloEngine/tests/CMakeLists.txt @@ -512,6 +512,7 @@ add_executable(OloEngine-Tests Functional/Scene/SceneStepAdvancesOneFrameTest.cpp Functional/Scene/DeterministicReplayProducesSameStateTest.cpp Functional/Scene/FixedTimestepDeterminismTest.cpp + Functional/Scene/RenderInterpolationSmoothsMotionTest.cpp Functional/Scene/PrimaryCameraResolutionTest.cpp Functional/Scene/EntityCreateWithUUIDTest.cpp Functional/AnimationPhysics/AnimationContinuesAfterPauseResumeTest.cpp diff --git a/OloEngine/tests/Functional/Scene/RenderInterpolationSmoothsMotionTest.cpp b/OloEngine/tests/Functional/Scene/RenderInterpolationSmoothsMotionTest.cpp new file mode 100644 index 000000000..bab088ebd --- /dev/null +++ b/OloEngine/tests/Functional/Scene/RenderInterpolationSmoothsMotionTest.cpp @@ -0,0 +1,227 @@ +#include "OloEnginePCH.h" + +// OLO_TEST_LAYER: Functional + +// ============================================================================= +// RenderInterpolationSmoothsMotionTest — Functional Test. +// +// Cross-subsystem seam under test: +// Render interpolation (issue #502) × the deterministic fixed-timestep loop +// (#484) × Lua-driven transform mutation. Scene::OnUpdateRuntimeFixed advances +// the gameplay simulation in fixed `fixedDt` steps from a variable frame delta, +// keeps the two most recent fixed-tick poses, and RenderRuntime draws a pose +// interpolated between them at alpha = accumulator / fixedDt. At a refresh rate +// that isn't a multiple of the sim rate (60 Hz sim / 144 Hz display) the +// *rendered* pose must advance in near-equal increments (judder-free) even +// though the *simulation* advances in discrete integer ticks. +// +// Scenario: a Lua script moves an entity at a constant one-unit-per-tick velocity +// (reading + incrementing its own translation.x, so any leak of an interpolated +// pose back into the persisted sim state would drift the trajectory). We drive +// the fixed-step entry with a constant non-multiple frame delta and, each frame, +// sample BOTH the interpolated render pose and the raw simulation pose. +// +// Because the mover is uniform, the interpolated render x is exactly linear in +// wall time (interp_x(n) = n * frameDelta / fixedDt - 1), so consecutive +// render-frame deltas are constant — the definition of judder-free. The raw sim +// x is a staircase (+0 on frames with no step, +1 on frames that step), which is +// the judder interpolation removes. +// +// Assertions: +// 1. The interpolated render pose is monotonic and its per-frame delta is +// near-constant (second difference ~0) — smooth motion at 144/60. +// 2. The raw sim pose visibly stair-steps (some 0-deltas, some 1-deltas) — the +// judder present without interpolation, proving the interpolation is what +// smooths #1 rather than the sim already being smooth. +// 3. Interpolation ON vs OFF produces a bit-identical raw sim trajectory — +// the render blend never mutates persisted simulation state, so determinism +// (#484) is not regressed. +// ============================================================================= + +#include "Functional/FunctionalTest.h" + +#include "OloEngine/Scene/Entity.h" +#include "OloEngine/Scene/Components.h" + +#include + +#include +#include +#include +#include + +using namespace OloEngine; +using namespace OloEngine::Functional; + +namespace +{ + std::filesystem::path WriteScript(const std::string& contents, const char* nameStem) + { + const auto* info = ::testing::UnitTest::GetInstance()->current_test_info(); + std::string fileName = std::string("olo_interp_") + nameStem + "_" + (info ? info->name() : "unknown") + ".lua"; + const auto path = std::filesystem::temp_directory_path() / fileName; + { + std::ofstream out(path, std::ios::binary | std::ios::trunc); + out << contents; + } + return path; + } + + // Constant one-unit-per-tick mover. Reads its own translation and increments + // it, so a corrupted (interpolated) pose leaking into the sim would drift the + // trajectory off the clean integer sequence — the determinism guard's teeth. + constexpr const char* kMoverScript = R"( +local script = {} +function script.OnUpdate(entityID, ts) + local id = math.tointeger(entityID) or 0 + local p = entity_utils.get_translation(id) + entity_utils.set_translation(id, vec3.new(p.x + 1, 0, 0)) +end +return script +)"; +} // namespace + +class RenderInterpolationSmoothsMotionTest : public FunctionalTest +{ + protected: + void BuildScene() override + { + EnableLua(); + // Small, round-trippable UUID: Lua's double number type can't represent + // a random > 2^53 UUID exactly (see LuaScriptMutatesTransformViaSceneTick). + m_Mover = GetScene().CreateEntityWithUUID(UUID{ 4242 }, "Mover"); + m_ScriptPath = WriteScript(kMoverScript, "mover"); + RegisterLuaScript(m_Mover, m_ScriptPath); + } + + void TearDown() override + { + FunctionalTest::TearDown(); + std::error_code ec; + std::filesystem::remove(m_ScriptPath, ec); + } + + Entity m_Mover; + std::filesystem::path m_ScriptPath; +}; + +TEST_F(RenderInterpolationSmoothsMotionTest, NonMultipleRefreshRateProducesJudderFreeInterpolatedMotion) +{ + // 60 Hz sim on a 144 Hz display — deliberately NOT a multiple, the case that + // judders when render is coupled to the fixed tick. + constexpr f32 kFixedDt = 1.0f / 60.0f; + constexpr f32 kFrameDt = 1.0f / 144.0f; + constexpr u32 kFrames = 72; // ~0.5 s → ~30 fixed steps, plenty of samples + // Expected constant per-frame advance of the interpolated pose, in units + // (one unit == one fixed tick of motion): frameDelta / fixedStep. + constexpr f32 kExpectedStride = kFrameDt / kFixedDt; // 60/144 = 0.41667 + + ASSERT_TRUE(GetScene().IsRenderInterpolationEnabled()) + << "interpolation is on by default; the smooth-motion assertions below assume it"; + + std::vector interpX; + std::vector rawX; + interpX.reserve(kFrames); + rawX.reserve(kFrames); + + for (u32 i = 0; i < kFrames; ++i) + { + GetScene().OnUpdateRuntimeFixed(kFrameDt, kFixedDt); + + const glm::mat4 rendered = GetScene().GetInterpolatedLocalTransform(m_Mover); + interpX.push_back(rendered[3].x); + rawX.push_back(m_Mover.GetComponent().Translation.x); + } + + // Analyze only the tail where a full prev/curr snapshot pair exists (both + // ticks represent real motion). The first ~3 frames precede the first fixed + // step (144/60 = 2.4 frames of wall time before one tick elapses). + constexpr u32 kWarmup = 6; + ASSERT_LT(kWarmup, kFrames); + + // (1) Monotonic + judder-free: consecutive interpolated deltas are all ~equal + // to the expected stride (second difference ~0). + f32 maxStrideError = 0.0f; + f32 maxInterpSecondDiff = 0.0f; + f32 prevDelta = kExpectedStride; + for (u32 i = kWarmup + 1; i < kFrames; ++i) + { + const f32 delta = interpX[i] - interpX[i - 1]; + EXPECT_GE(delta, -1e-4f) << "interpolated render pose went backwards at frame " << i + << " (delta " << delta << ")"; + maxStrideError = std::max(maxStrideError, std::fabs(delta - kExpectedStride)); + maxInterpSecondDiff = std::max(maxInterpSecondDiff, std::fabs(delta - prevDelta)); + prevDelta = delta; + } + EXPECT_LT(maxStrideError, 5e-3f) + << "interpolated render stride drifted from the expected constant " + << kExpectedStride << " — motion is not uniform at a non-multiple refresh rate"; + EXPECT_LT(maxInterpSecondDiff, 5e-3f) + << "interpolated render pose has a non-negligible second difference (judder)"; + + // (2) The raw sim pose stair-steps: it must show BOTH frames that don't + // advance (accumulator hasn't reached a tick) and frames that jump a full + // unit. This is the judder that (1) proves interpolation removes. + bool sawZeroStep = false; + bool sawUnitStep = false; + f32 maxRawSecondDiff = 0.0f; + f32 prevRawDelta = 0.0f; + for (u32 i = kWarmup + 1; i < kFrames; ++i) + { + const f32 rawDelta = rawX[i] - rawX[i - 1]; + if (std::fabs(rawDelta) < 1e-4f) + sawZeroStep = true; + if (std::fabs(rawDelta - 1.0f) < 1e-4f) + sawUnitStep = true; + maxRawSecondDiff = std::max(maxRawSecondDiff, std::fabs(rawDelta - prevRawDelta)); + prevRawDelta = rawDelta; + } + EXPECT_TRUE(sawZeroStep && sawUnitStep) + << "raw sim pose did not stair-step (zeroStep=" << sawZeroStep + << " unitStep=" << sawUnitStep << ") — the test's premise (coupled render judders) is void"; + // The interpolated series is dramatically smoother than the raw series. + EXPECT_GT(maxRawSecondDiff, 0.9f) << "raw sim judder should be ~1 unit at step boundaries"; + EXPECT_LT(maxInterpSecondDiff, maxRawSecondDiff * 0.25f) + << "interpolation did not measurably smooth the motion"; +} + +TEST_F(RenderInterpolationSmoothsMotionTest, InterpolationNeverLeaksIntoSimulationState) +{ + // The render blend overwrites poses transiently in RenderRuntime and restores + // them before the frame returns, so it must never corrupt the persisted sim + // state (#484 must not regress). The mover reads its own translation and adds + // one each tick: if an interpolated (fractional) pose leaked back into the + // persisted TransformComponent, the next tick's read would be fractional and + // the trajectory would drift off the exact integer tick sequence. So with + // interpolation ON, the raw sim pose must equal the fixed-tick count EXACTLY + // on every frame — even on frames where the rendered pose is mid-blend. + constexpr f32 kFixedDt = 1.0f / 60.0f; + constexpr f32 kFrameDt = 1.0f / 144.0f; // non-multiple: mid-blend most frames + constexpr u32 kFrames = 72; + + ASSERT_TRUE(GetScene().IsRenderInterpolationEnabled()); + + for (u32 i = 0; i < kFrames; ++i) + { + GetScene().OnUpdateRuntimeFixed(kFrameDt, kFixedDt); + + const f32 rawX = m_Mover.GetComponent().Translation.x; + const f32 expected = static_cast(GetScene().GetSimulationTick()); + // Exact integer equality: any leak of the fractional interpolated pose + // into the persisted transform would show here as a non-integer drift. + EXPECT_FLOAT_EQ(rawX, expected) + << "raw sim pose (" << rawX << ") diverged from the fixed-tick count (" + << expected << ") at frame " << i + << " — the render interpolation blend leaked into persisted sim state"; + + // And a mid-blend frame really is mid-blend: the rendered pose sits + // strictly between the two tick poses, so we're exercising the blend, not + // trivially sampling on tick boundaries. + if (const f32 alpha = GetScene().GetRenderInterpolationAlpha(); alpha > 0.01f && alpha < 0.99f && expected >= 1.0f) + { + const f32 renderedX = GetScene().GetInterpolatedLocalTransform(m_Mover)[3].x; + EXPECT_GT(renderedX, expected - 1.0f - 1e-4f); + EXPECT_LT(renderedX, expected + 1e-4f); + } + } +} From b85218c912d4b7fa3e964cabef2f5bba47382cc5 Mon Sep 17 00:00:00 2001 From: Ole Bueker Date: Tue, 7 Jul 2026 09:24:34 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(scene):=20address=20#502=20review=20?= =?UTF-8?q?=E2=80=94=20paused=20blend,=20buffer=20reuse,=20disable-clears-?= =?UTF-8?q?cache?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review follow-ups on the render-interpolation change (#579): - Paused frame-stepping: force m_RenderInterpAlpha = 0 while paused so RenderRuntime renders the live (frozen or just-stepped) pose exactly. Without this the stale pre-pause alpha/snapshot pair blended backward toward the previous tick and a single-step advance was invisible. - RenderRuntime restore buffer is now a function-local static thread_local (cleared, not reallocated, each frame) so the hot render path no longer heap-allocates a fresh vector per frame. Kept out of the Scene as a member because TransformComponent is incomplete in Scene.h (which deliberately omits the heavy Components.h); render is synchronous per game thread so a per-thread reused buffer is safe. - SetRenderInterpolationEnabled(false) now clears m_HasInterpSnapshots so re-enabling can't blend from a stale pose before the next fresh capture. - Use glm::mat4_cast (declared in the explicitly-included gtc/quaternion.hpp) instead of glm::toMat4 (gtx) — removes the transitive-include dependency. - Fix stale doc comments (ComputeInterpolatedLocal had a nonexistent parameter; ShouldInterpolateThisFrame now states the real alpha>0 gate) and the test header comment (it described an ON/OFF comparison the test doesn't perform). - Lua mover preserves y/z instead of zeroing them, so the fixture stays correct if reused with a non-zero start pose. Full suite: 4186 pass / 0 fail. Interpolation, determinism, and pause/step tests all green (the paused-alpha fix does not regress frame-by-frame stepping). Co-Authored-By: Claude Opus 4.8 (1M context) --- OloEngine/src/OloEngine/Scene/Scene.cpp | 31 ++++++++++++++++--- OloEngine/src/OloEngine/Scene/Scene.h | 19 +++++++++--- .../RenderInterpolationSmoothsMotionTest.cpp | 12 ++++--- 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/OloEngine/src/OloEngine/Scene/Scene.cpp b/OloEngine/src/OloEngine/Scene/Scene.cpp index 1b68a7316..79e9ef1cd 100644 --- a/OloEngine/src/OloEngine/Scene/Scene.cpp +++ b/OloEngine/src/OloEngine/Scene/Scene.cpp @@ -1555,6 +1555,13 @@ namespace OloEngine --m_StepFrames; SimulateRuntimeStep(fixedDt); } + + // Render the live (frozen or just-stepped) pose exactly while + // paused — force alpha to 0 so ShouldInterpolateThisFrame() is + // false. Without this the stale pre-pause alpha/snapshot pair + // would make RenderRuntime blend BACKWARD toward the previous + // tick, and a single-step advance would be invisible (issue #502). + m_RenderInterpAlpha = 0.0f; } else { @@ -1670,8 +1677,11 @@ namespace OloEngine { if (InterpTransform interp; ComputeInterpolatedLocal(entity, interp)) { + // glm::mat4_cast (not toMat4): mat4_cast is declared in + // , which this file includes explicitly, so + // the quaternion→matrix dependency isn't a transitive include. return glm::translate(glm::mat4(1.0f), interp.Translation) * - glm::toMat4(interp.Rotation) * + glm::mat4_cast(interp.Rotation) * glm::scale(glm::mat4(1.0f), interp.Scale); } // Fall back to the live local transform. @@ -2665,10 +2675,20 @@ namespace OloEngine // Full-component copies (not just TRS): TransformComponent's Rotation / // RotationEuler are private and SetRotation() re-derives the Euler // representation, so restoring via the setter could drift RotationEuler - // even when the quat is bit-identical. Copying the whole component and - // assigning it back restores every field — incl. the private Euler and - // the matrix cache — exactly, guaranteeing zero simulation-state drift. - std::vector> restorePoses; + // even when the quat is bit-identical (and the serializer persists the + // Euler — see SceneSerializer). Copying the whole component and assigning + // it back restores every field — incl. the private Euler and the matrix + // cache — exactly, guaranteeing zero simulation-state drift. + // + // A function-local `static thread_local` scratch buffer, cleared (not + // reallocated) each frame, so the hot render path doesn't heap-allocate a + // fresh vector per frame — the same "reuse capacity" intent as + // PropagateWorldTransforms' m_TransformOrder buffers. It can't be a Scene + // member because TransformComponent is incomplete in Scene.h (which + // deliberately doesn't include the heavy Components.h); render is + // synchronous per game thread, so a per-thread reused buffer is safe. + static thread_local std::vector> restorePoses; + restorePoses.clear(); if (interpolateThisFrame) { if (mainCamera && !primaryCameraIsFlyCam) @@ -2869,6 +2889,7 @@ namespace OloEngine } } PropagateWorldTransforms(); + restorePoses.clear(); } } diff --git a/OloEngine/src/OloEngine/Scene/Scene.h b/OloEngine/src/OloEngine/Scene/Scene.h index e861b9b4b..4701dd42d 100644 --- a/OloEngine/src/OloEngine/Scene/Scene.h +++ b/OloEngine/src/OloEngine/Scene/Scene.h @@ -126,6 +126,13 @@ namespace OloEngine void SetRenderInterpolationEnabled(bool enabled) { m_RenderInterpolationEnabled = enabled; + // Drop the cached snapshot pair when disabling so re-enabling doesn't + // blend from a stale pose before the next fresh capture — until then + // ShouldInterpolateThisFrame() falls back to the live pose. + if (!enabled) + { + m_HasInterpSnapshots = false; + } } [[nodiscard("Store this!")]] bool IsRenderInterpolationEnabled() const { @@ -676,13 +683,15 @@ namespace OloEngine // Snapshot every entity's live local TransformComponent into `out` // (clearing prior contents). Keyed by the raw entt entity id. void CaptureLocalTransforms(std::unordered_map& out); - // Compose the interpolated pose for `entity` at the current alpha into - // `out`. Returns false (leaving `out` untouched) when interpolation is - // inactive or the entity lacks a snapshot pair — callers then use the - // live transform. `interpolate` short-circuits the enable/snapshot gate. + // Compose the interpolated pose for `entity` at the current alpha + // (m_RenderInterpAlpha) into `out`. Returns false (leaving `out` + // untouched) when interpolation is disabled, no snapshot pair exists, or + // the entity is absent from one of the snapshots — callers then use the + // live transform. [[nodiscard]] bool ComputeInterpolatedLocal(entt::entity entity, InterpTransform& out) const; // True when this frame should render interpolated poses: the toggle is - // on, a snapshot pair exists, and the blend factor is meaningfully < 1. + // on, a snapshot pair exists, and m_RenderInterpAlpha > 0.0f (alpha == 0 + // renders the current pose verbatim, so the overwrite is skipped). [[nodiscard]] bool ShouldInterpolateThisFrame() const; // Step 2D (Box2D) + 3D (Jolt, incl. m_SimulationTime-driven buoyancy) // physics one tick and sync the results back onto the ECS transforms diff --git a/OloEngine/tests/Functional/Scene/RenderInterpolationSmoothsMotionTest.cpp b/OloEngine/tests/Functional/Scene/RenderInterpolationSmoothsMotionTest.cpp index bab088ebd..0cd72ee85 100644 --- a/OloEngine/tests/Functional/Scene/RenderInterpolationSmoothsMotionTest.cpp +++ b/OloEngine/tests/Functional/Scene/RenderInterpolationSmoothsMotionTest.cpp @@ -33,9 +33,11 @@ // 2. The raw sim pose visibly stair-steps (some 0-deltas, some 1-deltas) — the // judder present without interpolation, proving the interpolation is what // smooths #1 rather than the sim already being smooth. -// 3. Interpolation ON vs OFF produces a bit-identical raw sim trajectory — -// the render blend never mutates persisted simulation state, so determinism -// (#484) is not regressed. +// 3. With interpolation ON, the raw (persisted) sim pose equals the fixed-tick +// count EXACTLY on every frame — even mid-blend frames. The mover reads and +// increments its own translation, so any leak of a fractional interpolated +// pose into persisted state would drift it off the integer sequence. This is +// the determinism guard: the render blend never mutates sim state (#484). // ============================================================================= #include "Functional/FunctionalTest.h" @@ -75,7 +77,9 @@ local script = {} function script.OnUpdate(entityID, ts) local id = math.tointeger(entityID) or 0 local p = entity_utils.get_translation(id) - entity_utils.set_translation(id, vec3.new(p.x + 1, 0, 0)) + -- Preserve y/z (only x is the mover) so the fixture stays correct even if + -- reused with a non-zero starting pose. + entity_utils.set_translation(id, vec3.new(p.x + 1, p.y, p.z)) end return script )";