diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index fa059d31..913bd1ba 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -2159,3 +2159,23 @@ if (MOBILEGL_BUILD_DISAGGREGATED) ) endforeach() endif() + +# P5b measured named-blit blockers, exercising both backend bound-form lowerings. +if (MOBILEGL_BUILD_DISAGGREGATED) + foreach(namedBlitBackend DirectGLES DirectVulkan) + mgl_itest_join_environment(MGL_ITEST_NAMED_BLIT_ENVIRONMENT + "MOBILEGL_BACKEND_TYPE=${namedBlitBackend}" "MOBILEGL_TRANSPORT=inproc" + "MOBILEGL_ITEST_REQUIRE_GPU=1" "MGITEST_SPLIT_LANE=1" + ${MGL_ITEST_CAPABILITY_ENV} ${MGL_ITEST_COMMON_ENV}) + foreach(namedBlitCase NamedBlitPreservesBindingsAndRestoresNextVerbsPixels NamedBlitDefaultEndpointPixels) + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "${namedBlitBackend}.Split.NamedBlit." + TEST_FILTER "F1WireScenario.${namedBlitCase}" + DISCOVERY_TIMEOUT 30 + PROPERTIES + LABELS "integration-split" + TIMEOUT ${MGL_ITEST_TIMEOUT} + ENVIRONMENT "${MGL_ITEST_NAMED_BLIT_ENVIRONMENT}\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/p5b-${namedBlitBackend}-${namedBlitCase}.log") + endforeach() + endforeach() +endif() diff --git a/MobileGL/MG_IntegrationTest/Scenarios/F1WireScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/F1WireScenario.cpp index 35e28c08..e681c1f2 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/F1WireScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/F1WireScenario.cpp @@ -219,4 +219,85 @@ TEST_F(F1WireScenario, GenerateMipmapPixels) { const int expected[4] = {64, 128, 191, 255}; for (int i = 0; i < 4; ++i) EXPECT_NEAR(pixel[i], expected[i], 1) << "F1.GenerateMipmap.pixels"; } +TEST_F(F1WireScenario, NamedBlitPreservesBindingsAndRestoresNextVerbsPixels) { + if (!Ready()) return; + Attach(GL_RGBA8); + glClearColor(1, 0, 0, 1); + glClear(GL_COLOR_BUFFER_BIT); + GLuint fbos[3]{}, textures[3]{}; + glGenFramebuffers(3, fbos); + glGenTextures(3, textures); + for (int i = 0; i < 3; ++i) { + glBindFramebuffer(GL_FRAMEBUFFER, fbos[i]); + glBindTexture(GL_TEXTURE_2D, textures[i]); + glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, 8, 8); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, textures[i], 0); + ASSERT_EQ(glCheckFramebufferStatus(GL_FRAMEBUFFER), GLenum(GL_FRAMEBUFFER_COMPLETE)); + glClearColor(0, i == 1 ? 1 : 0, 1, 1); + glClear(GL_COLOR_BUFFER_BIT); + } + glBindFramebuffer(GL_READ_FRAMEBUFFER, fbos[1]); + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, fbos[2]); + const auto before = PeekSplitRuntime().emitSeq; + glBlitNamedFramebuffer(fbo, fbos[0], 0, 0, 8, 8, 0, 0, 8, 8, GL_COLOR_BUFFER_BIT, GL_NEAREST); + EXPECT_GT(PeekSplitRuntime().emitSeq, before); + GLint read = 0, draw = 0; + glGetIntegerv(GL_READ_FRAMEBUFFER_BINDING, &read); + glGetIntegerv(GL_DRAW_FRAMEBUFFER_BINDING, &draw); + EXPECT_EQ(read, GLint(fbos[1])); + EXPECT_EQ(draw, GLint(fbos[2])); + + // No intervening bind: this must clear the restored draw FBO, not the DSA destination. + glClearColor(1, 0, 1, 1); + glClear(GL_COLOR_BUFFER_BIT); + glBindFramebuffer(GL_READ_FRAMEBUFFER, fbos[2]); + std::array pixel{}; + glReadPixels(2, 3, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel.data()); + EXPECT_EQ(pixel, (std::array{255, 0, 255, 255})) << "named blit restored draw before clear"; + glBindFramebuffer(GL_READ_FRAMEBUFFER, fbos[0]); + glReadPixels(2, 3, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel.data()); + EXPECT_EQ(pixel, (std::array{255, 0, 0, 255})) << "unbound named blit copied source"; + + glBindFramebuffer(GL_READ_FRAMEBUFFER, fbos[1]); + glBlitFramebuffer(0, 0, 8, 8, 0, 0, 8, 8, GL_COLOR_BUFFER_BIT, GL_NEAREST); + glBindFramebuffer(GL_READ_FRAMEBUFFER, fbos[2]); + glReadPixels(2, 3, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel.data()); + EXPECT_EQ(pixel, (std::array{0, 255, 255, 255})) << "ordinary blit follows restored bindings"; + EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); + glBindFramebuffer(GL_FRAMEBUFFER, fbo); + glDeleteFramebuffers(3, fbos); + glDeleteTextures(3, textures); +} + +TEST_F(F1WireScenario, NamedBlitDefaultEndpointPixels) { + if (!Ready()) return; + Attach(GL_RGBA8); + glClearColor(1, 0, 0, 1); + glClear(GL_COLOR_BUFFER_BIT); + glBindFramebuffer(GL_FRAMEBUFFER, 0); + glClearColor(0, 0, 1, 1); + glClear(GL_COLOR_BUFFER_BIT); + glBindFramebuffer(GL_FRAMEBUFFER, fbo); + glBlitNamedFramebuffer(fbo, 0, 0, 0, 8, 8, 0, 0, 8, 8, GL_COLOR_BUFFER_BIT, GL_NEAREST); + GLint read = 0, draw = 0; + glGetIntegerv(GL_READ_FRAMEBUFFER_BINDING, &read); + glGetIntegerv(GL_DRAW_FRAMEBUFFER_BINDING, &draw); + EXPECT_EQ(read, GLint(fbo)); + EXPECT_EQ(draw, GLint(fbo)); + glBindFramebuffer(GL_READ_FRAMEBUFFER, 0); + std::array pixel{}; + glReadPixels(2, 3, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel.data()); + EXPECT_EQ(pixel, (std::array{255, 0, 0, 255})) << "default draw endpoint"; + glBindFramebuffer(GL_FRAMEBUFFER, fbo); + glClearColor(0, 1, 0, 1); + glClear(GL_COLOR_BUFFER_BIT); + glBlitNamedFramebuffer(0, fbo, 0, 0, 8, 8, 0, 0, 8, 8, GL_COLOR_BUFFER_BIT, GL_NEAREST); + glGetIntegerv(GL_READ_FRAMEBUFFER_BINDING, &read); + glGetIntegerv(GL_DRAW_FRAMEBUFFER_BINDING, &draw); + EXPECT_EQ(read, GLint(fbo)); + EXPECT_EQ(draw, GLint(fbo)); + glReadPixels(2, 3, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel.data()); + EXPECT_EQ(pixel, (std::array{255, 0, 0, 255})) << "default read endpoint"; + EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)); +} } // namespace MGITest diff --git a/MobileGL/MG_Remote/CONTRACT-P5B.md b/MobileGL/MG_Remote/CONTRACT-P5B.md index 71cf7117..0b001117 100644 --- a/MobileGL/MG_Remote/CONTRACT-P5B.md +++ b/MobileGL/MG_Remote/CONTRACT-P5B.md @@ -404,6 +404,21 @@ the user-index span. --- +13. **Measured named blits use the existing `blit` row with a scoped client binding override.** + P5b's next trace census reaches `BlitNamedFramebuffer` in iris-BSL and + improved-transparency on both backends. The client saves its read/draw binding objects, + binds the named arguments in the client shadow only, validates `BlitNamedFramebuffer`, + and emits the original rectangles/mask/filter plus both original `MGPBlit` handles. + The barrier holds those bindings until the server's bound `BlitFramebuffer` finishes; + then RAII restores both client bindings. Their version changes make the next ordinary + verb republish the restored state. The existing classified BARRIER-PULLED accessors + remain the sole frontend read path; no pointer is added to the record or to a side channel, + and no driver call occurs on the client. Name 0 carries `kMGPipeDefaultFramebuffer`. + This adds no opcode and supersedes only named-blit's wave-3 deferral. *Overturned by:* + removing the lockstep barrier or adding process separation; then both framebuffer + handles need server-only resolution before dispatch. Pixel/binding gates cover unbound + endpoints, default endpoints and ordinary clear/blit immediately after restoration. + ## §7 The 71 slots after P5b's contract commit, and after the four packages On this head the partition is CONTRACT-P5 §7's, unchanged: **A = 2, B = 5, C = 64** diff --git a/MobileGL/MG_Remote/Client/EmitTables.cpp b/MobileGL/MG_Remote/Client/EmitTables.cpp index da77bb58..9b09a121 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.cpp +++ b/MobileGL/MG_Remote/Client/EmitTables.cpp @@ -10,7 +10,7 @@ // // THE PARTITION IS CONTRACT-P5.md §7's AND IS NOT RE-DERIVED HERE (R-15, ID-12): // class A 2 slots answered locally from the caps mirror, never emitted, never Fatal -// class B 48 slots emitted; class C 21 slots name their unmigrated verb. +// class B 49 slots emitted; class C 20 slots name their unmigrated verb. // The three counts are static_asserted to sum to kRemoteEmitSlotCount below, so a slot that // changes class without changing the arithmetic is a build break rather than a behaviour // change nobody reviewed. @@ -60,6 +60,7 @@ #include "WireTables.h" #include +#include #include namespace MobileGL::MG_Remote::Client { @@ -716,7 +717,7 @@ namespace MobileGL::MG_Remote::Client { MG_Pipe::MGPBlit record{}; // Same reasoning as Clear's Fbo: the read and draw bindings are gPipeInputs', set // by MGP_FILL(BlitFramebuffer) at GL_Framebuffer.cpp:660 and held still by the - // barrier. glBlitNamedFramebuffer, which DOES name two framebuffers, is class C. + // barrier. The named form below carries both handles after a scoped binding override. record.ReadFbo = MG_Pipe::kMGPipeNullHandle; record.DrawFbo = MG_Pipe::kMGPipeNullHandle; record.SrcX0 = srcX0; @@ -733,6 +734,57 @@ namespace MobileGL::MG_Remote::Client { nullptr, 0, nullptr); } + // DSA blits use the existing bound-form backend while the verb barrier holds. + // Only client shadow bindings change here; no driver call or frontend pointer crosses + // the wire. Bind() bumps their versions on entry and restore, so the next ordinary + // verb republishes the application's original bindings even if no GL bind intervenes. + class ScopedBlitBindings { + public: + using Fbo = MG_State::GLState::FramebufferObject; + ScopedBlitBindings(const SharedPtr& read, const SharedPtr& draw) + : m_read(MG_State::pGLContext->GetFramebufferBindingSlot(FramebufferTarget::Read)), + m_draw(MG_State::pGLContext->GetFramebufferBindingSlot(FramebufferTarget::Draw)), + m_savedRead(m_read.GetBoundObject()), m_savedDraw(m_draw.GetBoundObject()) { + m_read.Bind(read); + m_draw.Bind(draw); + } + ~ScopedBlitBindings() { + m_read.Bind(m_savedRead); + m_draw.Bind(m_savedDraw); + } + ScopedBlitBindings(const ScopedBlitBindings&) = delete; + ScopedBlitBindings& operator=(const ScopedBlitBindings&) = delete; + private: + BindingSlot& m_read; + BindingSlot& m_draw; + SharedPtr m_savedRead; + SharedPtr m_savedDraw; + }; + + void EmitBlitNamedFramebuffer( + const SharedPtr& read, + const SharedPtr& draw, + GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, GLint dstX0, + GLint dstY0, GLint dstX1, GLint dstY1, GLbitfield mask, GLenum filter) { + ClientSession& session = RequireSession("BlitNamedFramebuffer"); + const ScopedBlitBindings bindings(read, draw); + // The frontend's first validate preceded this temporary lowering. Refresh both + // emitted framebuffer state and the existing BARRIER-PULLED binding fields now. + MG_Pipe::MGPipeValidateForVerb(MG_Pipe::MGPipeVerb::BlitNamedFramebuffer); + BeforeReadOnlyVerb(); + MG_Pipe::MGPBlit record{}; + record.ReadFbo = MG_Pipe::MGPipeFramebufferEmitter::HandleFor(*read); + record.DrawFbo = MG_Pipe::MGPipeFramebufferEmitter::HandleFor(*draw); + record.SrcX0 = srcX0; record.SrcY0 = srcY0; + record.SrcX1 = srcX1; record.SrcY1 = srcY1; + record.DstX0 = dstX0; record.DstY0 = dstY0; + record.DstX1 = dstX1; record.DstY1 = dstY1; + record.Mask = static_cast(mask); + record.Filter = static_cast(filter); + session.EmitAndWait(MG_Pipe::MGPWireOp::Blit, &record, sizeof(record), nullptr, 0, + nullptr, 0, nullptr); + } + // How many bytes glReadPixels will pack for this rectangle, from the PACK half of the // pixel-store state. // @@ -1445,10 +1497,6 @@ namespace MobileGL::MG_Remote::Client { // The wave-3 tail. SetSwapInterval is hand-written below (it is not a GL.* slot). #define MGR_UNMIGRATED_TAIL_SLOTS(X) \ - X(BlitNamedFramebuffer, void, \ - (const SharedPtr&, \ - const SharedPtr&, GLint, GLint, GLint, GLint, GLint, \ - GLint, GLint, GLint, GLbitfield, GLenum)) \ X(GetTexImage, void, (GLenum, GLint, GLenum, GLenum, GLvoid*)) \ X(GetTextureImage, void, \ (const SharedPtr&, TextureUploadTarget, GLint, GLenum, \ @@ -1521,8 +1569,10 @@ namespace MobileGL::MG_Remote::Client { constexpr Uint32 kEmittedSlotsI1 = 7; constexpr Uint32 kEmittedSlotsT2 = 6; constexpr Uint32 kEmittedSlotsF1 = 11; + constexpr Uint32 kEmittedSlotsTail = 1; // BlitNamedFramebuffer constexpr Uint32 kEmittedSlots = - kEmittedSlotsP5 + kEmittedSlotsD1 + kEmittedSlotsI1 + kEmittedSlotsT2 + kEmittedSlotsF1; + kEmittedSlotsP5 + kEmittedSlotsD1 + kEmittedSlotsI1 + kEmittedSlotsT2 + kEmittedSlotsF1 + + kEmittedSlotsTail; constexpr Uint32 kLocallyAnsweredSlots = 2; // GetIntegeri_v, IsTimerQuerySupported // EACH PACKAGE'S OWNERSHIP, PINNED. A package that flips a slot removes one row and @@ -1533,7 +1583,7 @@ namespace MobileGL::MG_Remote::Client { static_assert(kUnmigratedI1 + kEmittedSlotsI1 == 7, "i1 owns the 7 image/compute/barrier/copy/SSBO slots"); static_assert(kUnmigratedT2 + kEmittedSlotsT2 == 7, "t2 owns the 7 XFB/tessellation slots"); static_assert(kUnmigratedF1 + kEmittedSlotsF1 == 11, "f1 owns the 11 clear/copy/mip slots"); - static_assert(kUnmigratedTail == 20, "the wave-3 tail is 20 slots and no P5b package owns one"); + static_assert(kUnmigratedTail + kEmittedSlotsTail == 20, "the original wave-3 tail owns 20 slots"); static_assert(kUnmigratedSlots + kEmittedSlots == 69, "class B and C own 69 slots"); static_assert(kLocallyAnsweredSlots + kEmittedSlots + kUnmigratedSlots == kRemoteEmitSlotCount, "the three classes no longer partition the 71 slots"); @@ -1589,6 +1639,7 @@ namespace MobileGL::MG_Remote::Client { table.GL.MultiDrawElementsIndirectCount = &EmitMultiDrawElementsIndirectCount; table.GL.ReadPixels = &EmitReadPixels; table.GL.BlitFramebuffer = &EmitBlitFramebuffer; + table.GL.BlitNamedFramebuffer = &EmitBlitNamedFramebuffer; table.Present = &EmitPresent; // ---- class B, P5b t2. Assigned AFTER the class-C block above, which is what makes // the flip a single-line change per slot: the Fatal thunk is overwritten, and a slot diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index 0c1e36f5..0c8cdcbb 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -138,8 +138,10 @@ namespace MobileGL::MG_Remote::Server { if (table->GL.BlitFramebuffer == nullptr) return false; // Same ruling as OnClear's: the read and draw framebuffers are already bound by the // set_framebuffer_state records that preceded this one, so the unnamed entry point is - // the one that matches what the server's state actually is. BlitNamedFramebuffer needs - // two frontend SharedPtrs, which table 2 lists as uncarried. + // the one that matches what the server's state actually is. For the named form the + // client temporarily binds the two named objects and validates before emitting; its + // existing BARRIER-PULLED fields stay fixed until this call returns. ReadFbo/DrawFbo + // retain the original identities for the later handle-only endpoint (CONTRACT-P5B §6). table->GL.BlitFramebuffer(blit.SrcX0, blit.SrcY0, blit.SrcX1, blit.SrcY1, blit.DstX0, blit.DstY0, blit.DstX1, blit.DstY1, static_cast(blit.Mask), diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index 4b35cdd0..93b7d745 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -177,8 +177,8 @@ namespace { TEST(RemoteEmitTable, TheThreeClassesPartitionAllSeventyOneSlots) { // P5 baseline five + f1 eleven + i1 seven + t2 six emitted slots. EXPECT_EQ(LocallyAnsweredSlotCount(), 2u); - EXPECT_EQ(ImplementedVerbCount(), 48u); - EXPECT_EQ(UnmigratedSlotCount(), 21u); + EXPECT_EQ(ImplementedVerbCount(), 49u); + EXPECT_EQ(UnmigratedSlotCount(), 20u); EXPECT_EQ(LocallyAnsweredSlotCount() + ImplementedVerbCount() + UnmigratedSlotCount(), kRemoteEmitSlotCount); }