erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

fix(diag): resolve the multisampled backbuffer before reading it

Browse source 
Every automated pixel gate and every blank-world verdict in Campaign V is
produced by FrameScreenshotController reading the default framebuffer with
glReadPixels. The window is created with the quality preset's MSAA sample
count, so that framebuffer is normally 4x multisampled -- and glReadPixels
against a multisampled read framebuffer is undefined per the GL spec. The
instrument the campaign has been using to decide "did the world render?"
rested on an operation with no specified result.

That is not a theoretical complaint. The blank-world investigation spent
several rounds unable to tell "the renderer drew nothing" apart from "the
readback did not return what the renderer drew", and it took an out-of-process
desktop grab to separate them. A gate cannot arbitrate a rendering defect
while its own read is unspecified.

So the capture resolves first: when the default framebuffer is multisampled
it blits the whole colour buffer into a single-sampled RGBA8 framebuffer with
identical rectangles and GL_NEAREST -- the defined resolve -- and reads that.
A single-sampled default framebuffer keeps the original direct read, so
non-MSAA captures stay byte-for-byte what they were. The blit disables and
restores the scissor test, because a blit is subject to it and a frame that
left a rectangle armed would otherwise resolve only part of the image; that
is the same self-contained-GL-state rule the render passes follow. The
resolve target is created and destroyed per capture -- captures are rare, and
a cache would have to track resize and context teardown for no gain.

GlGpuDevice.CaptureBackbuffer had the identical undefined read. It now routes
through the same path rather than being a second instrument to keep sound.

The IDefaultFramebufferSurface seam grows the draw binding, the sample count,
and the resolve operations, so the bind/query/blit/read/restore order stays
assertable without a GL context; two new tests pin the resolve order and the
resolve target's release on a failing read.

Gates: Release build green. App tests 3,866 passed / 3 skipped. Offline pixel
gate against fed636b9 passes at a differing fraction of 4.08e-05 against the
0.001 threshold -- which is exactly the same-commit control pair measured at
this commit, i.e. indistinguishable from ambient noise. Same-commit controls
re-measured at 17 px (fed636b9) and 23 px (here) out of 563,200; the recorded
band in plan section 5.1 widens to 15-23 px, fraction <= 4.1e-05.

Plan section 5.5.1 records what the connected investigation established: the
interleaved A/B attribution (4/5 vs 0/5, p ~ 0.024), the desktop witness
showing every depth-tested draw missing while the atmosphere clear and the
complete retained UI present, the probe evidence that the CPU dispatched
3,331 statics with no GL error, and the falsification list -- including the
ring glBufferSubData hazard, which condition 1 shipped against and did not
fix.

Section 5.5.2 records this session's second investigation, run against a
staged (never committed) V4c with log-only glGet* probes, and it closes the
shared-3-D-state hypothesis. The depth plane is bit-identical on blank and
rendered frames -- test on, write mask on, GL_LESS, clear value 1.0, range
[0,1], full viewport, full colour mask, no clip distances. The camera
constants are sane and advancing. Forcing gl_ClipDistance off left the blank
rate unchanged at 3/5. glGetGraphicsResetStatus returned NO_ERROR in all
1,814 samples across four blank runs, which also retires the "GPU-side fault"
reading in its context-reset form.

Two sharper facts replace it. Replacing only the frame clear colour with
magenta makes a blank frame come back uniformly magenta under the complete
retained UI, so no 3-D fragment is rasterized at all -- the world is not
drawn-then-hidden, fogged, or overdrawn. And on a blank run the client's own
capture of framebuffer 0 is RGBA(0,0,0,0) in every pixel, including pixels
where the UI is visibly on screen at that moment. That survives this commit's
resolve fix, so it is a second, independent instrument fault: the screenshot-
byte verdict used by the repeat and A/B gates measures the readback, not the
renderer, and those gates need to assert on the desktop witness instead.

V4c is NOT re-landed. No fix was attempted, because the mechanism is not
renderer state and does not sit in V4c's surface as this hypothesis predicted.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-07-28 00:19:03 +02:00
parent fed636b9c0
commit e6362da5c2
4 changed files with 456 additions and 26 deletions
Download patch file Download diff file Expand all files Collapse all files

195
src/AcDream.App/Diagnostics/FrameScreenshotController.cs
View file

@ -1,3 +1,4 @@
using AcDream.App.Rendering.Wb;
using Silk.NET.OpenGL;
using SixLabors.ImageSharp;
using SixLabors.ImageSharp.PixelFormats;
@ -45,6 +46,21 @@ internal sealed class FrameScreenshotController
return (width, height) => ReadDefaultFramebuffer(surface, width, height);
}
/// <summary>
/// Reads the default framebuffer through the same resolve-aware path the
/// screenshot gates use. Shared so there is exactly one implementation of
/// "read the backbuffer" in the process — see
/// <see cref="ReadDefaultFramebuffer(IDefaultFramebufferSurface, int, int)"/>.
/// </summary>
internal static byte[] ReadDefaultFramebuffer(GL gl, int width, int height)
{
ArgumentNullException.ThrowIfNull(gl);
return ReadDefaultFramebuffer(
new GlDefaultFramebufferSurface(gl),
width,
height);
}
internal FrameScreenshotController(
Func<int, int, byte[]> readRgba,
string directory,
@ -144,8 +160,36 @@ internal sealed class FrameScreenshotController
/// <summary>The name currently bound to <c>GL_READ_FRAMEBUFFER</c>.</summary>
uint ReadFramebufferBinding { get; }
/// <summary>The name currently bound to <c>GL_DRAW_FRAMEBUFFER</c>.</summary>
uint DrawFramebufferBinding { get; }
/// <summary>
/// <c>GL_SAMPLES</c> for the default framebuffer. Queried with
/// framebuffer 0 bound to both targets, because the value is
/// framebuffer-dependent state and would otherwise report whichever
/// offscreen target the frame left bound.
/// </summary>
int DefaultFramebufferSamples { get; }
void BindReadFramebuffer(uint framebuffer);
void BindDrawFramebuffer(uint framebuffer);
/// <summary>
/// Creates a single-sampled RGBA8 colour framebuffer of the given size
/// and returns its name.
/// </summary>
uint CreateResolveTarget(int width, int height);
void DeleteResolveTarget(uint framebuffer);
/// <summary>
/// Blits the whole colour buffer from the bound read framebuffer to the
/// bound draw framebuffer with <c>GL_NEAREST</c> and identical rectangles
/// — the multisample resolve.
/// </summary>
void BlitColorNearest(int width, int height);
void ReadRgba(int width, int height, byte[] destination);
}
@ -170,6 +214,27 @@ internal sealed class FrameScreenshotController
/// of inheriting one, and restores the caller's binding so a diagnostic
/// capture cannot perturb the frame it observes.
/// </para>
/// <para>
/// <b>Multisampling.</b> The window is created with the quality preset's
/// MSAA sample count, so the default framebuffer is normally 4x multisampled,
/// and <c>glReadPixels</c> against a multisampled read framebuffer is
/// <i>undefined</i> per the GL spec (GL 4.6 §18.2: an INVALID_OPERATION is
/// generated only for framebuffer objects; for the default framebuffer the
/// result is simply unspecified, and AMD returns real pixels most of the time
/// and something else the rest). Every automated pixel gate and every blank-
/// world verdict in Campaign V reads through here, so an unspecified read is
/// an unsound instrument, not a cosmetic issue. When the default framebuffer
/// is multisampled the capture resolves it first — blit the whole colour
/// buffer into a single-sampled RGBA8 framebuffer with identical rectangles
/// and <c>GL_NEAREST</c>, which is the defined resolve — and reads that.
/// A single-sampled default framebuffer keeps the original direct read, so
/// non-MSAA captures stay byte-for-byte what they were.
/// </para>
/// <para>
/// The resolve target is created and destroyed per capture rather than
/// cached: captures are rare (a handful per gate run), and a cache would have
/// to track window resizes and context teardown for no measurable gain.
/// </para>
/// </remarks>
internal static byte[] ReadDefaultFramebuffer(
IDefaultFramebufferSurface surface,
@ -178,22 +243,50 @@ internal sealed class FrameScreenshotController
{
ArgumentNullException.ThrowIfNull(surface);
byte[] pixels = new byte[checked(width * height * 4)];
uint previous = surface.ReadFramebufferBinding;
uint previousRead = surface.ReadFramebufferBinding;
uint previousDraw = surface.DrawFramebufferBinding;
surface.BindReadFramebuffer(0u);
surface.BindDrawFramebuffer(0u);
try
{
if (surface.DefaultFramebufferSamples > 1)
ResolveThenRead(surface, width, height, pixels);
else
surface.ReadRgba(width, height, pixels);
}
finally
{
surface.BindReadFramebuffer(previousRead);
surface.BindDrawFramebuffer(previousDraw);
}
return pixels;
}
private static void ResolveThenRead(
IDefaultFramebufferSurface surface,
int width,
int height,
byte[] pixels)
{
uint resolve = surface.CreateResolveTarget(width, height);
try
{
// Read is still framebuffer 0 — the multisampled source.
surface.BindDrawFramebuffer(resolve);
surface.BlitColorNearest(width, height);
surface.BindReadFramebuffer(resolve);
surface.ReadRgba(width, height, pixels);
}
finally
{
surface.BindReadFramebuffer(previous);
surface.DeleteResolveTarget(resolve);
}
return pixels;
}
private sealed class GlDefaultFramebufferSurface : IDefaultFramebufferSurface
{
private readonly GL _gl;
private uint _resolveRenderbuffer;
public GlDefaultFramebufferSurface(GL gl) => _gl = gl;
@ -206,9 +299,105 @@ internal sealed class FrameScreenshotController
}
}
public uint DrawFramebufferBinding
{
get
{
_gl.GetInteger(GetPName.DrawFramebufferBinding, out int binding);
return (uint)binding;
}
}
public int DefaultFramebufferSamples
{
get
{
_gl.GetInteger(GetPName.Samples, out int samples);
return samples;
}
}
public void BindReadFramebuffer(uint framebuffer) =>
_gl.BindFramebuffer(FramebufferTarget.ReadFramebuffer, framebuffer);
public void BindDrawFramebuffer(uint framebuffer) =>
_gl.BindFramebuffer(FramebufferTarget.DrawFramebuffer, framebuffer);
public uint CreateResolveTarget(int width, int height)
{
// RGBA8 matches the default framebuffer's colour encoding — the app
// never enables GL_FRAMEBUFFER_SRGB — so the blit is a pure resolve
// with no encoding conversion.
uint renderbuffer = _gl.GenRenderbuffer();
_gl.BindRenderbuffer(RenderbufferTarget.Renderbuffer, renderbuffer);
_gl.RenderbufferStorage(
RenderbufferTarget.Renderbuffer,
InternalFormat.Rgba8,
(uint)width,
(uint)height);
_gl.BindRenderbuffer(RenderbufferTarget.Renderbuffer, 0u);
uint framebuffer = _gl.GenFramebuffer();
_gl.BindFramebuffer(FramebufferTarget.DrawFramebuffer, framebuffer);
_gl.FramebufferRenderbuffer(
FramebufferTarget.DrawFramebuffer,
FramebufferAttachment.ColorAttachment0,
RenderbufferTarget.Renderbuffer,
renderbuffer);
GLEnum status =
_gl.CheckFramebufferStatus(FramebufferTarget.DrawFramebuffer);
if (status != GLEnum.FramebufferComplete)
{
_gl.BindFramebuffer(FramebufferTarget.DrawFramebuffer, 0u);
_gl.DeleteFramebuffer(framebuffer);
_gl.DeleteRenderbuffer(renderbuffer);
throw new InvalidOperationException(
$"multisample resolve framebuffer {width}x{height} is "
+ $"incomplete: {status}");
}
GLHelpers.ThrowOnResourceError(
_gl,
$"create screenshot resolve target {width}x{height}");
_resolveRenderbuffer = renderbuffer;
return framebuffer;
}
public void DeleteResolveTarget(uint framebuffer)
{
_gl.DeleteFramebuffer(framebuffer);
if (_resolveRenderbuffer != 0u)
_gl.DeleteRenderbuffer(_resolveRenderbuffer);
_resolveRenderbuffer = 0u;
}
public void BlitColorNearest(int width, int height)
{
// A blit is subject to the scissor test, so a frame that left a
// scissor rectangle armed would resolve only part of the image.
// This operation states the state it needs and restores it — the
// same self-contained-GL-state rule the render passes follow.
bool scissor = _gl.IsEnabled(EnableCap.ScissorTest);
if (scissor)
_gl.Disable(EnableCap.ScissorTest);
_gl.BlitFramebuffer(
0,
0,
width,
height,
0,
0,
width,
height,
ClearBufferMask.ColorBufferBit,
BlitFramebufferFilter.Nearest);
if (scissor)
_gl.Enable(EnableCap.ScissorTest);
GLHelpers.ThrowOnResourceError(
_gl,
$"resolve default framebuffer {width}x{height}");
}
public unsafe void ReadRgba(int width, int height, byte[] destination)
{
fixed (byte* pointer = destination)

21
src/AcDream.App/Rendering/Gpu/Gl/GlGpuDevice.cs
View file

@ -500,20 +500,15 @@ internal sealed class GlGpuDevice : IGpuDevice
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(width);
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(height);
byte[] pixels = new byte[checked(width * height * 4)];
unsafe
{
fixed (byte* pointer = pixels)
{
_gl.ReadPixels(0, 0, (uint)width, (uint)height, PixelFormat.Rgba, PixelType.UnsignedByte, pointer);
}
}
// FrameScreenshotController owns the one backbuffer read in the process:
// it names framebuffer 0 rather than inheriting a binding, and it
// resolves the multisampled default framebuffer before reading, because
// glReadPixels against a multisampled read framebuffer is undefined.
// A second implementation here would be a second instrument to keep
// sound. FlipRows is the same top-left-origin flip the screenshot gates
// rely on, so this seam stays byte-for-byte compatible with them.
byte[] pixels = FrameScreenshotController.ReadDefaultFramebuffer(_gl, width, height);
GLHelpers.ThrowOnResourceError(_gl, $"capture backbuffer {width}x{height}");
// FrameScreenshotController.FlipRows is the same top-left-origin flip
// the existing screenshot gates already rely on — reusing it (rather
// than reimplementing the loop) is what keeps this seam byte-for-byte
// compatible with those gates.
return FrameScreenshotController.FlipRows(pixels, width, height);
}