From 3e85f1083e71fd10d4208fd4c33b9fabb9a3954b Mon Sep 17 00:00:00 2001 From: gamer147 Date: Mon, 27 Jul 2026 12:01:35 -0400 Subject: [PATCH] Prevent FIELD redraw flicker --- docs/engine-re.md | 11 + .../ScriptPresentationBarrierTests.cs | 74 +++++ .../Hosting/ScriptPresentationBarrier.cs | 47 ++++ godot/GodotAdvHost.cs | 257 ++++++++++++------ godot/Main.cs | 41 ++- 5 files changed, 344 insertions(+), 86 deletions(-) create mode 100644 engine/Age.Engine.Tests/ScriptPresentationBarrierTests.cs create mode 100644 engine/Age.Engine/Hosting/ScriptPresentationBarrier.cs diff --git a/docs/engine-re.md b/docs/engine-re.md index 3e7c5f8..2e117e0 100644 --- a/docs/engine-re.md +++ b/docs/engine-re.md @@ -780,6 +780,17 @@ the old idle sprite composited beneath the moving clone until each segment ends. post-bind static-alpha consumption for looping objects as a transient opacity latch, cleared by the next `draw-texture` bind. This preserves ADV's distinct pre-animation/static alpha-zero CG initialization. +The same route exposed an independent publication-atomicity requirement. Each tile step reaches +`FIELD@0x4eb2 -> 0x9225`, which calls `DRAWMAP`, `DRAWOBJ`, and `DRAWMINIMAP`. `DRAWMAP` first erases its +old terrain handle ranges and then rebuilds them; `DRAWMINIMAP` recreates mutable surfaces `0x42..0x44` +from transparent pixels before filling the terrain and copying object/unit markers. Those are ordinary +VM-side construction bursts, not a series of front-buffer presents. Letting Godot's render thread sample +between their individual erase/fill/copy instructions displayed a partially blank minimap at every tile +and could display the erased terrain state for one frame when a complete route returned. The interactive +host now holds one writer-side presentation barrier for the whole script burst. Explicit presentation, +transition, sleep, text, movie, and input services temporarily expose the completed state; callbacks +serviced inside a parked input wait reacquire the writer side for their own atomic burst. + **Port implication:** replace the current “any spritesheet is active” redraw predicate with native-style shared current/previous sampling and request a composition only when at least one visible sampled cell changes. The FIELD `sleep(1)` input-poll loop must also stop requesting a redraw when no retained mutation diff --git a/engine/Age.Engine.Tests/ScriptPresentationBarrierTests.cs b/engine/Age.Engine.Tests/ScriptPresentationBarrierTests.cs new file mode 100644 index 0000000..de79a6b --- /dev/null +++ b/engine/Age.Engine.Tests/ScriptPresentationBarrierTests.cs @@ -0,0 +1,74 @@ +using System.Threading; +using System.Threading.Tasks; +using Age.Engine.Hosting; +using Xunit; + +public class ScriptPresentationBarrierTests +{ + [Fact] + public void ScriptBurstWithholdsPresentationUntilExit() + { + var barrier = new ScriptPresentationBarrier(); + + barrier.EnterScript(); + Assert.False(barrier.TryEnterPresentation()); + + barrier.ExitScript(); + Assert.True(barrier.TryEnterPresentation()); + barrier.ExitPresentation(); + } + + [Fact] + public void NestedScriptsRemainOneAtomicBurst() + { + var barrier = new ScriptPresentationBarrier(); + + barrier.EnterScript(); + barrier.EnterScript(); + barrier.ExitScript(); + Assert.False(barrier.TryEnterPresentation()); + + barrier.ExitScript(); + Assert.True(barrier.TryEnterPresentation()); + barrier.ExitPresentation(); + } + + [Fact] + public void ServiceBoundaryTemporarilyAllowsPresentation() + { + var barrier = new ScriptPresentationBarrier(); + + barrier.EnterScript(); + bool suspended = barrier.SuspendScript(); + Assert.True(suspended); + Assert.True(barrier.TryEnterPresentation()); + barrier.ExitPresentation(); + + barrier.ResumeScript(suspended); + Assert.False(barrier.TryEnterPresentation()); + barrier.ExitScript(); + } + + [Fact] + public async Task RenderThreadCannotEnterWhileVmThreadIsRebuilding() + { + var barrier = new ScriptPresentationBarrier(); + using var entered = new ManualResetEventSlim(); + using var release = new ManualResetEventSlim(); + Task vmBurst = Task.Run(() => + { + barrier.EnterScript(); + entered.Set(); + release.Wait(); + barrier.ExitScript(); + }); + + await Task.Run(entered.Wait); + Assert.False(barrier.TryEnterPresentation()); + + release.Set(); + await vmBurst; + Assert.True(barrier.TryEnterPresentation()); + barrier.ExitPresentation(); + } +} diff --git a/engine/Age.Engine/Hosting/ScriptPresentationBarrier.cs b/engine/Age.Engine/Hosting/ScriptPresentationBarrier.cs new file mode 100644 index 0000000..2971fb7 --- /dev/null +++ b/engine/Age.Engine/Hosting/ScriptPresentationBarrier.cs @@ -0,0 +1,47 @@ +using System; +using System.Threading; + +namespace Age.Engine.Hosting; + +/// +/// Keeps one VM opcode burst atomic with respect to the interactive compositor. The VM owns the +/// write side while script code is running; the render thread samples through the read side only +/// after the VM reaches a presentation, sleep, or input service boundary. +/// +public sealed class ScriptPresentationBarrier +{ + private readonly ReaderWriterLockSlim _lock = new(LockRecursionPolicy.NoRecursion); + private int _scriptDepth; + + public void EnterScript() + { + if (_scriptDepth++ == 0) + _lock.EnterWriteLock(); + } + + public void ExitScript() + { + if (_scriptDepth <= 0) + throw new InvalidOperationException("No script burst is active."); + if (--_scriptDepth == 0) + _lock.ExitWriteLock(); + } + + /// Temporarily expose the completed burst state while a host service is parked. + public bool SuspendScript() + { + if (_scriptDepth <= 0 || !_lock.IsWriteLockHeld) return false; + _lock.ExitWriteLock(); + return true; + } + + public void ResumeScript(bool suspended) + { + if (suspended) _lock.EnterWriteLock(); + } + + public bool TryEnterPresentation() + => !_lock.IsWriteLockHeld && _lock.TryEnterReadLock(0); + + public void ExitPresentation() => _lock.ExitReadLock(); +} diff --git a/godot/GodotAdvHost.cs b/godot/GodotAdvHost.cs index 67de8ac..3eeaccd 100644 --- a/godot/GodotAdvHost.cs +++ b/godot/GodotAdvHost.cs @@ -24,6 +24,11 @@ public sealed class GodotAdvHost : IHost private readonly string _rootScene; private readonly object _scriptContextLock = new(); private readonly Stack _scriptContexts = new(); + private readonly ScriptPresentationBarrier _presentationBarrier = new(); + private readonly AutoResetEvent _presentationRequestConsumed = new(false); + private readonly bool _synchronizeExplicitPresentation; + private long _explicitPresentationRequestGeneration; + private long _consumedPresentationRequestGeneration; private readonly object _imageLock = new(); private readonly Dictionary _images = new(); // packed catalog id -> decoded pixels // Mutable AGE surfaces are published by replacing immutable RgbaImage snapshots, so the compositor @@ -83,10 +88,12 @@ public sealed class GodotAdvHost : IHost public readonly List<(int Offset, string Text)> Captured = new(); public GodotAdvHost(Main main, ResourceMap res, string scene, Age.Engine.Hosting.FrameClock clock, - PageLocatorState locator, GodotTimelineLog? timeline = null) + PageLocatorState locator, GodotTimelineLog? timeline = null, + bool synchronizeExplicitPresentation = true) { _main = main; _res = res; _rootScene = scene; _clock = clock; _locator = locator; _timeline = timeline; + _synchronizeExplicitPresentation = synchronizeExplicitPresentation; } public void ReportWarning(string message) => System.Console.Error.WriteLine(message); @@ -98,6 +105,7 @@ public sealed class GodotAdvHost : IHost public void EnterScriptContext(string scriptName) { + _presentationBarrier.EnterScript(); string scene = System.IO.Path.GetFileNameWithoutExtension(scriptName).ToUpperInvariant(); lock (_scriptContextLock) _scriptContexts.Push(scene); _timeline?.Event("script-context-enter", new() { ["scene"] = scene }); @@ -109,6 +117,25 @@ public sealed class GodotAdvHost : IHost lock (_scriptContextLock) if (_scriptContexts.TryPop(out var popped)) scene = popped; if (scene != null) _timeline?.Event("script-context-exit", new() { ["scene"] = scene }); + _presentationBarrier.ExitScript(); + } + + public bool TryEnterPresentation() => _presentationBarrier.TryEnterPresentation(); + + public void ExitPresentation() => _presentationBarrier.ExitPresentation(); + + private bool SuspendScriptForPresentation() => _presentationBarrier.SuspendScript(); + + private void ResumeScriptAfterPresentation(bool suspended) + => _presentationBarrier.ResumeScript(suspended); + + private void RequestSynchronizedPresentation() + { + long requested = Interlocked.Increment(ref _explicitPresentationRequestGeneration); + Interlocked.Exchange(ref _presentRequested, 1); + if (!_synchronizeExplicitPresentation) return; + while (Interlocked.Read(ref _consumedPresentationRequestGeneration) < requested && !_stopping) + _presentationRequestConsumed.WaitOne(50); } public void ShowText(int offset, string text) @@ -127,14 +154,23 @@ public sealed class GodotAdvHost : IHost ["offset"] = $"0x{offset:x}", ["x"] = _advTextX, ["y"] = _advTextY, ["glyphs"] = text.Length, ["delay_ms"] = 50, }); - while (IsTextRevealing && !_stopping) + bool scriptSuspended = SuspendScriptForPresentation(); + try { - lock (_textLock) + RequestSynchronizedPresentation(); + while (IsTextRevealing && !_stopping) { - if (_advTextForceComplete || _clock.NowMs - _advTextStartedMs >= text.Length * 50L) - IsTextRevealing = false; + lock (_textLock) + { + if (_advTextForceComplete || _clock.NowMs - _advTextStartedMs >= text.Length * 50L) + IsTextRevealing = false; + } + if (IsTextRevealing) _frameSignal.WaitOne(50); } - if (IsTextRevealing) _frameSignal.WaitOne(50); + } + finally + { + ResumeScriptAfterPresentation(scriptSuspended); } _timeline?.State("running", new() { ["text_reveal_complete"] = true }); } @@ -363,9 +399,6 @@ public sealed class GodotAdvHost : IHost Pages++; _locator.Wait(Pages); _main.CallDeferred("PageBreak"); - // Publish retained mutations accumulated before the wait once. A static input wait is not itself a - // reason to rebuild the 800x600 background every frame; ambient channels are queried separately. - System.Threading.Interlocked.Exchange(ref _presentRequested, 1); lock (_textLock) { _activeWaitLayout = layoutSlot == 0 ? _currentAdvLayout : layoutSlot; @@ -377,39 +410,60 @@ public sealed class GodotAdvHost : IHost var autoTimer = new AdvAutoAdvanceTimer(); bool autoAdvanced = false; bool messageSkipped = false; - while (!_stopping) + bool scriptSuspended = SuspendScriptForPresentation(); + try { - while (true) + // Publish the completed pre-wait burst once. Callback scripts temporarily reacquire the + // write side below, so each hover/click update is likewise exposed only after it returns. + RequestSynchronizedPresentation(); + while (!_stopping) { - bool markerWasEnabled; - lock (_textLock) markerWasEnabled = _waitIndicatorEnabled; - bool keepServicing = serviceInputCallback(); - // The native callback returns through the shared ADV redraw/wait path, whose op 0x72 - // re-arms a marker stopped by nested HISTORY. Our blocking host keeps the parent wait - // parked, so restore that parent-owned state at the equivalent callback boundary. - lock (_textLock) + while (true) { - if (markerWasEnabled && !_waitIndicatorEnabled) + bool markerWasEnabled; + lock (_textLock) markerWasEnabled = _waitIndicatorEnabled; + if (scriptSuspended) ResumeScriptAfterPresentation(true); + bool keepServicing; + try { - _waitIndicatorEnabled = true; - _waitIndicatorStartedMs = _clock.NowMs; + keepServicing = serviceInputCallback(); } + finally + { + if (scriptSuspended && !SuspendScriptForPresentation()) + throw new InvalidOperationException("Input callback did not retain the script burst."); + } + // The native callback returns through the shared ADV redraw/wait path, whose op 0x72 + // re-arms a marker stopped by nested HISTORY. Our blocking host keeps the parent wait + // parked, so restore that parent-owned state at the equivalent callback boundary. + lock (_textLock) + { + if (markerWasEnabled && !_waitIndicatorEnabled) + { + _waitIndicatorEnabled = true; + _waitIndicatorStartedMs = _clock.NowMs; + } + } + if (!keepServicing) break; } - if (!keepServicing) break; + if (_messageSkipActive) + { + messageSkipped = true; + break; + } + if (autoTimer.Poll(autoWaitState(), _main.IsVoicePlaybackActive, _clock.NowMs)) + { + autoAdvanced = true; + break; + } + if (_gate.Wait(0)) break; + WaitHandle.WaitAny(new[] { _gate.AvailableWaitHandle, _inputCallbackSignal, _frameSignal }); + if (_gate.Wait(0)) break; } - if (_messageSkipActive) - { - messageSkipped = true; - break; - } - if (autoTimer.Poll(autoWaitState(), _main.IsVoicePlaybackActive, _clock.NowMs)) - { - autoAdvanced = true; - break; - } - if (_gate.Wait(0)) break; - WaitHandle.WaitAny(new[] { _gate.AvailableWaitHandle, _inputCallbackSignal, _frameSignal }); - if (_gate.Wait(0)) break; + } + finally + { + ResumeScriptAfterPresentation(scriptSuspended); } IsWaiting = false; lock (_textLock) _waitIndicatorEnabled = false; @@ -558,30 +612,44 @@ public sealed class GodotAdvHost : IHost public void WaitForForegroundTransition(GfxState gfx) { int started = gfx.StartForegroundTransitions(_clock.NowMs); - if (started == 0 && !gfx.HasActiveTimedPresentation(_clock.NowMs) && !HasActiveMoviePresentation()) return; - _foregroundGfx = gfx; - System.Threading.Interlocked.Exchange(ref _transitionStartedAtMs, _clock.NowMs); - IsTransitionWaiting = true; - _timeline?.State("transition-start", new() { ["count"] = started }); - int lastBucket = -1; - while ((gfx.HasActiveTimedPresentation(_clock.NowMs) || HasActiveMoviePresentation()) && !_stopping) + bool hasActivePresentation = + gfx.HasActiveTimedPresentation(_clock.NowMs) || HasActiveMoviePresentation(); + if (hasActivePresentation) { - var active = gfx.SnapshotForegroundTransitions(_clock.NowMs); - int bucket = active.Count == 0 ? 100 : (int)System.Math.Floor(active[0].Progress * 10); - if (bucket != lastBucket) - { - lastBucket = bucket; - _timeline?.State("transition-progress", new() - { - ["progress"] = active.Count == 0 ? 1.0 : active[0].Progress, - ["forced"] = active.Count != 0 && active[0].Forced, - }); - } - _frameSignal.WaitOne(50); + _foregroundGfx = gfx; + System.Threading.Interlocked.Exchange(ref _transitionStartedAtMs, _clock.NowMs); + IsTransitionWaiting = true; + _timeline?.State("transition-start", new() { ["count"] = started }); + } + bool scriptSuspended = SuspendScriptForPresentation(); + try + { + RequestSynchronizedPresentation(); + if (!hasActivePresentation) return; + int lastBucket = -1; + while ((gfx.HasActiveTimedPresentation(_clock.NowMs) || HasActiveMoviePresentation()) && !_stopping) + { + var active = gfx.SnapshotForegroundTransitions(_clock.NowMs); + int bucket = active.Count == 0 ? 100 : (int)System.Math.Floor(active[0].Progress * 10); + if (bucket != lastBucket) + { + lastBucket = bucket; + _timeline?.State("transition-progress", new() + { + ["progress"] = active.Count == 0 ? 1.0 : active[0].Progress, + ["forced"] = active.Count != 0 && active[0].Forced, + }); + } + _frameSignal.WaitOne(50); + } + // The active query becomes false at the exact transition/movie endpoint. Publish that terminal sample + // once so the last visible frame cannot remain fractionally incomplete. + RequestSynchronizedPresentation(); + } + finally + { + ResumeScriptAfterPresentation(scriptSuspended); } - // The active query becomes false at the exact transition/movie endpoint. Publish that terminal sample - // once so the last visible frame cannot remain fractionally incomplete. - System.Threading.Interlocked.Exchange(ref _presentRequested, 1); IsTransitionWaiting = false; System.Threading.Interlocked.Exchange(ref _transitionStartedAtMs, -1); _foregroundGfx = null; @@ -610,7 +678,15 @@ public sealed class GodotAdvHost : IHost { ["started"] = started, ["completed"] = completed, }); - System.Threading.Interlocked.Exchange(ref _presentRequested, 1); + bool scriptSuspended = SuspendScriptForPresentation(); + try + { + RequestSynchronizedPresentation(); + } + finally + { + ResumeScriptAfterPresentation(scriptSuspended); + } } // Op 0x25's native mode-4 path advances an 8-bit alpha accumulator. Values <=64 use the @@ -640,26 +716,35 @@ public sealed class GodotAdvHost : IHost ["interval_argument"] = intervalArgument, ["duration_ms"] = duration, ["source_objects"] = source.Count, ["target_objects"] = target.Count, }); - while (!_stopping) + bool scriptSuspended = SuspendScriptForPresentation(); + try { - bool complete; + RequestSynchronizedPresentation(); + while (!_stopping) + { + bool complete; + lock (_screenTransitionLock) + complete = _screenTransition == null || _screenTransition.Forced + || _clock.NowMs - start >= duration; + if (complete) break; + _frameSignal.WaitOne(50); + } + // Publish the exact target endpoint before the following surface releases/root reload. lock (_screenTransitionLock) - complete = _screenTransition == null || _screenTransition.Forced - || _clock.NowMs - start >= duration; - if (complete) break; - _frameSignal.WaitOne(50); + if (_screenTransition != null) _screenTransition.Forced = true; + RequestSynchronizedPresentation(); + // Do not let the VM release both source surfaces (or immediately reload SYSTEM4) until the + // main thread has actually published the terminal target frame. + while (!_stopping) + { + lock (_screenTransitionLock) + if (_screenTransition == null) break; + _frameSignal.WaitOne(50); + } } - // Publish the exact target endpoint before the following surface releases/root reload. - lock (_screenTransitionLock) - if (_screenTransition != null) _screenTransition.Forced = true; - System.Threading.Interlocked.Exchange(ref _presentRequested, 1); - // Do not let the VM release both source surfaces (or immediately reload SYSTEM4) until the - // main thread has actually published the terminal target frame. - while (!_stopping) + finally { - lock (_screenTransitionLock) - if (_screenTransition == null) break; - _frameSignal.WaitOne(50); + ResumeScriptAfterPresentation(scriptSuspended); } IsTransitionWaiting = false; System.Threading.Interlocked.Exchange(ref _transitionStartedAtMs, -1); @@ -694,7 +779,12 @@ public sealed class GodotAdvHost : IHost lock (_screenTransitionLock) screenTransitionActive = _screenTransition != null; var reasons = HostPresentationReason.None; if (System.Threading.Interlocked.Exchange(ref _presentRequested, 0) != 0) + { reasons |= HostPresentationReason.HostRequest; + long requested = Interlocked.Read(ref _explicitPresentationRequestGeneration); + Interlocked.Exchange(ref _consumedPresentationRequestGeneration, requested); + _presentationRequestConsumed.Set(); + } if (screenTransitionActive) reasons |= HostPresentationReason.ScreenTransition; GfxPresentationReason gfxReasons = gfx.ConsumePresentationReasons(_clock.NowMs); if ((gfxReasons & GfxPresentationReason.RetainedMutation) != 0) @@ -821,10 +911,19 @@ public sealed class GodotAdvHost : IHost long deadline = _clock.NowMs + ms; _timeline?.State("sleep", new() { ["duration_ms"] = ms, ["deadline_ms"] = deadline }); IsSleeping = true; - while (_clock.NowMs < deadline) + bool scriptSuspended = SuspendScriptForPresentation(); + try { - if (_stopping) break; - _frameSignal.WaitOne(50); + RequestSynchronizedPresentation(); + while (_clock.NowMs < deadline) + { + if (_stopping) break; + _frameSignal.WaitOne(50); + } + } + finally + { + ResumeScriptAfterPresentation(scriptSuspended); } IsSleeping = false; _timeline?.State("running", new() { ["sleep_complete"] = true }); @@ -1023,6 +1122,7 @@ public sealed class GodotAdvHost : IHost _modalMovieCancelled = false; _modalMovieWaiting = true; + bool scriptSuspended = false; try { if (!StartMovie(asset, resourceId, surfaceSlot, movieFlags, 0, modal: true, @@ -1032,6 +1132,8 @@ public sealed class GodotAdvHost : IHost ["resource"] = resourceId, ["playback"] = playbackId, ["surface"] = surfaceSlot, ["file"] = asset.Name, }); + scriptSuspended = SuspendScriptForPresentation(); + RequestSynchronizedPresentation(); while (!_stopping && !_modalMovieCancelled) { if (!_movieSurfaces.IsActive(surfaceSlot)) break; @@ -1050,6 +1152,7 @@ public sealed class GodotAdvHost : IHost } finally { + ResumeScriptAfterPresentation(scriptSuspended); _modalMovieWaiting = false; _modalMovieCancelled = false; } diff --git a/godot/Main.cs b/godot/Main.cs index e2db12f..93899af 100644 --- a/godot/Main.cs +++ b/godot/Main.cs @@ -245,7 +245,13 @@ public partial class Main : Godot.Control _locator = new PageLocatorState(scene, _selftest ? null : pageMapPath); _locatorHud.Visible = _locatorHudVisible; var resources = scripts != null ? new ResourceMap(scripts.Catalog) : ResourceMap.Load(); - _host = new GodotAdvHost(this, resources, scene, _clock, _locator, _timeline) { SleepScale = sleepScale, TraceOps = _gfxLogPath != null }; + _host = new GodotAdvHost( + this, resources, scene, _clock, _locator, _timeline, + synchronizeExplicitPresentation: !_selftest) + { + SleepScale = sleepScale, + TraceOps = _gfxLogPath != null, + }; _trace = new GodotTraceSink(_locator, _timeline); if (_perfLogPath != null) _perf = new PerformanceFrameLog(_perfLogPath); // --trace-histogram: aggregate op/call-site execution counts of the REAL Godot run (headless flow @@ -382,15 +388,32 @@ public partial class Main : Godot.Control perf?.RecordMovies(PerformanceFrameLog.Timestamp() - phase); phase = perf != null ? PerformanceFrameLog.Timestamp() : 0; - HostPresentationReason presentationReasons = !_selftest && _vm != null && _host != null - ? _host.ConsumePresentationReasons(_vm.Gfx) - : HostPresentationReason.None; - bool shouldRecomposite = presentationReasons != HostPresentationReason.None; - perf?.RecordPresentationReasons((int)presentationReasons); - perf?.RecordShouldRecomposite(PerformanceFrameLog.Timestamp() - phase); + HostPresentationReason presentationReasons = HostPresentationReason.None; + bool presentationEntered = !_selftest && _vm != null && _host != null + && _host.TryEnterPresentation(); long allocationPhase = perf != null ? PerformanceFrameLog.AllocatedBytes() : 0; - if (shouldRecomposite) - Recomposite(); // native publishes retained mutations only at present/service boundaries + bool shouldRecomposite = false; + try + { + if (presentationEntered) + { + presentationReasons = _host!.ConsumePresentationReasons(_vm!.Gfx); + shouldRecomposite = presentationReasons != HostPresentationReason.None; + perf?.RecordPresentationReasons((int)presentationReasons); + perf?.RecordShouldRecomposite(PerformanceFrameLog.Timestamp() - phase); + if (shouldRecomposite) + Recomposite(); // native publishes retained mutations only at present/service boundaries + } + else + { + perf?.RecordPresentationReasons(0); + perf?.RecordShouldRecomposite(PerformanceFrameLog.Timestamp() - phase); + } + } + finally + { + if (presentationEntered) _host!.ExitPresentation(); + } perf?.RecordRecomposeAllocation(PerformanceFrameLog.AllocatedBytes() - allocationPhase); phase = perf != null ? PerformanceFrameLog.Timestamp() : 0;