From a84104a40f0d8ab3abb15e3e235fded6ff8296ea Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sat, 3 Oct 2026 02:59:49 -0400 Subject: [PATCH] [Fix] (Pipe): the default framebuffer's record names no placeholder surface that only one share group holds. --- MobileGL/MG_Impl/Pipe/FramebufferEmit.h | 26 +++++++----- MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp | 42 ++++++++++++++++++- .../MG_Test/Wire/ConcurrentContextsTest.cpp | 6 +++ 3 files changed, 63 insertions(+), 11 deletions(-) diff --git a/MobileGL/MG_Impl/Pipe/FramebufferEmit.h b/MobileGL/MG_Impl/Pipe/FramebufferEmit.h index 887ff886..368cea05 100644 --- a/MobileGL/MG_Impl/Pipe/FramebufferEmit.h +++ b/MobileGL/MG_Impl/Pipe/FramebufferEmit.h @@ -524,22 +524,28 @@ namespace MobileGL::MG_Pipe { out.Target = static_cast(target); out.IsDefault = fbo.IsDefaultFramebuffer() ? 1 : 0; - // THE COLOUR POINTS. A default framebuffer keeps its one colour surface under - // BackLeft rather than under Color0, and the record has exactly one place to put - // it: Color[0], which is also the index MGPipeDrawBufferIndex maps that token to, - // so the array and the draw-buffer indices agree by construction. - if (out.IsDefault != 0) { - out.Color[0] = SurfaceOf(fbo, FramebufferAttachmentType::BackLeft); - } else { + // THE SURFACES - and a DEFAULT framebuffer's record names none. Its storage on the + // server is the surface the context is current on (the native window or pbuffer, + // Magma's swapchain); both servers answer every default-framebuffer question from + // IsDefault and never resolve a surface of this record. The frontend's attachments + // are placeholders: one process-wide set of textures every context's framebuffer 0 + // shares, created once, so their records live in the ONE share group that was + // bound when they were born. Naming them here sent every other group a handle it + // holds no record for ("texture {12, 0} has no applier resource record" on each of + // its draws) and marked bind bits on them under whichever context drew next. + // Publishing them into every group instead would build a driver texture per group + // for storage nothing reads. The extent still comes from them (FillGeometry), and + // the draw-buffer indices below still map BackLeft to slot 0. + if (out.IsDefault == 0) { for (SizeT i = 0; i < kMGPipeMaxColorAttachments; ++i) { out.Color[i] = SurfaceOf(fbo, static_cast( static_cast(FramebufferAttachmentType::Color0) + static_cast(i))); } + out.Depth = SurfaceOf(fbo, FramebufferAttachmentType::Depth); + out.Stencil = SurfaceOf(fbo, FramebufferAttachmentType::Stencil); + out.ReadSurface = SurfaceOf(fbo, fbo.GetReadBuffer()); } - out.Depth = SurfaceOf(fbo, FramebufferAttachmentType::Depth); - out.Stencil = SurfaceOf(fbo, FramebufferAttachmentType::Stencil); - out.ReadSurface = SurfaceOf(fbo, fbo.GetReadBuffer()); const auto& drawBuffers = fbo.GetDrawBuffers(); for (SizeT i = 0; i < kMGPipeMaxColorAttachments; ++i) { diff --git a/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp b/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp index 79b4e095..e56d93c0 100644 --- a/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp @@ -245,7 +245,7 @@ TEST(FramebufferEmit, TheEmitterIsOneNeverDestroyedProcessSingleton) { X(FramebufferEmit, EveryNonTexturePointCarriesTheUnknownSentinelsRatherThanZero) \ X(FramebufferEmit, ADeadFramebuffersNamedRecordLatchIsRetired) \ X(FramebufferEmit, TheRecordsElevenSurfacesAreTheWholePointSetTheServerReads) \ - X(FramebufferEmit, ADefaultFramebufferResizeReEmitsTheRecordWithItsNewExtent) + X(FramebufferEmit, ADefaultFramebufferResizeReEmitsTheRecordWithItsNewExtent) X(FramebufferEmit, TheDefaultFramebuffersRecordNamesNoSurfaceAShareGroupWouldHaveToHold) #define MGL_DECLARE_PULL_SKIP(Suite, Name) \ TEST(Suite, Name) { GTEST_SKIP() << "compiled only under MOBILEGL_PIPE_PUSH"; } @@ -706,6 +706,46 @@ TEST(FramebufferEmit, ADefaultFramebufferResizeReEmitsTheRecordWithItsNewExtent) EXPECT_EQ(applied->Height, 96u); } +// THE DEFAULT FRAMEBUFFER'S RECORD NAMES NO SURFACE. Its attachments are the frontend's +// placeholders - one process-wide set every context's framebuffer 0 shares, whose records live in +// the one share group that was bound when they were born - and the server's default framebuffer +// is the surface the context is current on. Red while the record named them: every other share +// group was sent texture handles it held no record for, and logged the miss on each draw. +TEST(FramebufferEmit, TheDefaultFramebuffersRecordNamesNoSurfaceAShareGroupWouldHaveToHold) { + FramebufferScope scope; + const auto color = MakeColorTexture(96, 64); + const auto depthStencil = MakeShared(97); + depthStencil->SetInternalFormat(TextureInternalFormat::Depth32FStencil8); + depthStencil->AllocateStorage(TextureUploadTarget::Texture2D, 0, + MipmapInput{IntVec3{64, 64, 1}, static_cast(64 * 64 * 8)}); + const auto fbo0 = MakeShared(0); // what MG_Impl::BuildDefaultFramebuffer builds + ASSERT_TRUE(fbo0->IsDefaultFramebuffer()); + for (const auto point : {FramebufferAttachmentType::Color0, FramebufferAttachmentType::FrontLeft, + FramebufferAttachmentType::FrontRight, FramebufferAttachmentType::BackLeft, + FramebufferAttachmentType::BackRight}) { + fbo0->AttachTexture(point, color, TextureUploadTarget::Texture2D); + } + fbo0->AttachTexture(FramebufferAttachmentType::Depth, depthStencil, TextureUploadTarget::Texture2D); + fbo0->AttachTexture(FramebufferAttachmentType::Stencil, depthStencil, TextureUploadTarget::Texture2D); + BindDrawAndRead(fbo0, fbo0); + Framebuffers().EmitFramebufferState(Ctx()); + + const MGPFramebufferState* const applied = MGPipeApplier().DrawFramebuffer(); + ASSERT_NE(applied, nullptr) << "the default framebuffer's record did not reach the applier"; + EXPECT_EQ(applied->IsDefault, 1) << "MGPFramebufferState::IsDefault"; + for (Uint i = 0; i < kMGPipeMaxColorAttachments; ++i) { + EXPECT_EQ(applied->Color[i].Kind, kMGPipeSurfaceKindNone) << "MGPSurface::Kind at colour point " << i; + EXPECT_TRUE(MGPipeHandleIsNull(applied->Color[i].Res)) << "MGPSurface::Res at colour point " << i; + } + EXPECT_EQ(applied->Depth.Kind, kMGPipeSurfaceKindNone) << "MGPSurface::Kind on the depth point"; + EXPECT_EQ(applied->Stencil.Kind, kMGPipeSurfaceKindNone) << "MGPSurface::Kind on the stencil point"; + EXPECT_EQ(applied->ReadSurface.Kind, kMGPipeSurfaceKindNone) << "MGPFramebufferState::ReadSurface"; + // What the record still carries: the extent, and slot 0 as the back buffer's draw buffer. + EXPECT_EQ(applied->Width, 64u) << "MGPFramebufferState::Width"; + EXPECT_EQ(applied->Height, 64u) << "MGPFramebufferState::Height"; + EXPECT_EQ(applied->DrawBuffers[0], 0) << "MGPFramebufferState::DrawBuffers[0]"; +} + // ============================ D-D2 ============================ // // RED BEFORE THIS PACKAGE LANDED. RenderbufferObject's three storage setters bump no version diff --git a/MobileGL/MG_Test/Wire/ConcurrentContextsTest.cpp b/MobileGL/MG_Test/Wire/ConcurrentContextsTest.cpp index df94b162..736ac933 100644 --- a/MobileGL/MG_Test/Wire/ConcurrentContextsTest.cpp +++ b/MobileGL/MG_Test/Wire/ConcurrentContextsTest.cpp @@ -454,6 +454,12 @@ namespace { ASSERT_EQ(r.initialized, 1) << "the client hung or did not come up; client log " << logBase << "\n" << server.Log(); ASSERT_EQ(r.finished, 1) << "the client did not finish"; + // Every record named something its own context's share group holds. The default + // framebuffer's placeholder textures were the standing exception: one process-wide set, + // recorded in the group bound when they were born and named by every context's draw. + const std::string serverLog = server.Log(); + EXPECT_EQ(serverLog.find("has no applier resource record"), std::string::npos) + << "a record named a texture its share group holds no record for"; const int threads = shape == Shape::ReadbackRace ? 1 + kRaceChurners : kThreads; for (int i = 0; i < threads; ++i) { const ThreadReport& t = r.threads[i];