diff --git a/src/AcDream.App/Diagnostics/FrameScreenshotController.cs b/src/AcDream.App/Diagnostics/FrameScreenshotController.cs index 0170a5e3..a11380be 100644 --- a/src/AcDream.App/Diagnostics/FrameScreenshotController.cs +++ b/src/AcDream.App/Diagnostics/FrameScreenshotController.cs @@ -31,9 +31,18 @@ internal sealed class FrameScreenshotController GL gl, string directory, Action? log = null) - : this((width, height) => ReadDefaultFramebuffer(gl, width, height), directory, log) + : this( + CreateReader(gl), + directory, + log) + { + } + + private static Func CreateReader(GL gl) { ArgumentNullException.ThrowIfNull(gl); + var surface = new GlDefaultFramebufferSurface(gl); + return (width, height) => ReadDefaultFramebuffer(surface, width, height); } internal FrameScreenshotController( @@ -126,20 +135,93 @@ internal sealed class FrameScreenshotController return flipped; } - private static unsafe byte[] ReadDefaultFramebuffer(GL gl, int width, int height) + /// + /// The GL surface state a default-framebuffer capture touches, isolated so + /// the bind/read/restore order is assertable without a GL context. + /// + internal interface IDefaultFramebufferSurface { + /// The name currently bound to GL_READ_FRAMEBUFFER. + uint ReadFramebufferBinding { get; } + + void BindReadFramebuffer(uint framebuffer); + + void ReadRgba(int width, int height, byte[] destination); + } + + /// + /// Reads the default framebuffer — framebuffer name 0, the backbuffer — + /// and nothing else. + /// + /// + /// + /// glReadPixels reads whatever is bound to GL_READ_FRAMEBUFFER, + /// so a capture that does not name its source silently captures whichever + /// offscreen target the previous renderer left bound. The frame this runs in + /// draws several: PrivateEntityViewportRenderer clears its paperdoll + /// and appraisal FBOs to exactly RGBA(0,0,0,0), which is what a leaked + /// binding writes to disk — a fully transparent PNG that reads as a + /// blank-world failure while the backbuffer on screen was correct. + /// + /// + /// This was latent for as long as something else rebound framebuffer 0 often + /// enough to mask it (before Campaign V slice V4c, GL BeginPass did so + /// on every pass — see plan §5.4). The capture states its own source instead + /// of inheriting one, and restores the caller's binding so a diagnostic + /// capture cannot perturb the frame it observes. + /// + /// + internal static byte[] ReadDefaultFramebuffer( + IDefaultFramebufferSurface surface, + int width, + int height) + { + ArgumentNullException.ThrowIfNull(surface); byte[] pixels = new byte[checked(width * height * 4)]; - fixed (byte* pointer = pixels) + uint previous = surface.ReadFramebufferBinding; + surface.BindReadFramebuffer(0u); + try { - gl.ReadPixels( - 0, - 0, - (uint)width, - (uint)height, - PixelFormat.Rgba, - PixelType.UnsignedByte, - pointer); + surface.ReadRgba(width, height, pixels); + } + finally + { + surface.BindReadFramebuffer(previous); } return pixels; } + + private sealed class GlDefaultFramebufferSurface : IDefaultFramebufferSurface + { + private readonly GL _gl; + + public GlDefaultFramebufferSurface(GL gl) => _gl = gl; + + public uint ReadFramebufferBinding + { + get + { + _gl.GetInteger(GetPName.ReadFramebufferBinding, out int binding); + return (uint)binding; + } + } + + public void BindReadFramebuffer(uint framebuffer) => + _gl.BindFramebuffer(FramebufferTarget.ReadFramebuffer, framebuffer); + + public unsafe void ReadRgba(int width, int height, byte[] destination) + { + fixed (byte* pointer = destination) + { + _gl.ReadPixels( + 0, + 0, + (uint)width, + (uint)height, + PixelFormat.Rgba, + PixelType.UnsignedByte, + pointer); + } + } + } } diff --git a/tests/AcDream.App.Tests/Diagnostics/WorldLifecycleAutomationControllerTests.cs b/tests/AcDream.App.Tests/Diagnostics/WorldLifecycleAutomationControllerTests.cs index 43be61a7..d5bd1509 100644 --- a/tests/AcDream.App.Tests/Diagnostics/WorldLifecycleAutomationControllerTests.cs +++ b/tests/AcDream.App.Tests/Diagnostics/WorldLifecycleAutomationControllerTests.cs @@ -93,6 +93,73 @@ public sealed class WorldLifecycleAutomationControllerTests Assert.NotEmpty(error); } + /// + /// glReadPixels reads GL_READ_FRAMEBUFFER. A capture that does not name + /// framebuffer 0 captures whichever offscreen target the previous renderer + /// left bound - the paperdoll/appraisal FBOs clear to RGBA(0,0,0,0), so the + /// artifact is a fully transparent PNG that reads as a blank-world failure + /// while the backbuffer on screen was correct. + /// + [Fact] + public void DefaultFramebufferRead_BindsFramebufferZeroAndRestoresTheCallersBinding() + { + var surface = new RecordingFramebufferSurface(boundOnEntry: 7u, fill: 0x42); + + byte[] pixels = FrameScreenshotController.ReadDefaultFramebuffer(surface, 2, 3); + + Assert.Equal( + ["bind 0", "read 2x3", "bind 7"], + surface.Calls); + Assert.Equal(0u, surface.ReadFramebufferBindingDuringRead); + Assert.Equal(2 * 3 * 4, pixels.Length); + Assert.All(pixels, value => Assert.Equal(0x42, value)); + Assert.Equal(7u, surface.ReadFramebufferBinding); + } + + [Fact] + public void DefaultFramebufferRead_RestoresTheCallersBindingWhenTheReadThrows() + { + var surface = new RecordingFramebufferSurface(boundOnEntry: 9u, fill: 0) + { + ReadFailure = new InvalidOperationException("GL_OUT_OF_MEMORY"), + }; + + Assert.Throws( + () => FrameScreenshotController.ReadDefaultFramebuffer(surface, 1, 1)); + + Assert.Equal(["bind 0", "read 1x1", "bind 9"], surface.Calls); + Assert.Equal(9u, surface.ReadFramebufferBinding); + } + + private sealed class RecordingFramebufferSurface(uint boundOnEntry, byte fill) + : FrameScreenshotController.IDefaultFramebufferSurface + { + private uint _binding = boundOnEntry; + + public List Calls { get; } = []; + + public Exception? ReadFailure { get; init; } + + public uint? ReadFramebufferBindingDuringRead { get; private set; } + + public uint ReadFramebufferBinding => _binding; + + public void BindReadFramebuffer(uint framebuffer) + { + _binding = framebuffer; + Calls.Add($"bind {framebuffer}"); + } + + public void ReadRgba(int width, int height, byte[] destination) + { + Calls.Add($"read {width}x{height}"); + ReadFramebufferBindingDuringRead = _binding; + if (ReadFailure is not null) + throw ReadFailure; + Array.Fill(destination, fill); + } + } + [Fact] public void Checkpoint_WritesCanonicalRevealAndResourceSnapshot() {