[Feat] (MG_Remote): lower named framebuffer blits through scoped wire bindings

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()

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<GLubyte, 4> pixel{};
+    glReadPixels(2, 3, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel.data());
+    EXPECT_EQ(pixel, (std::array<GLubyte, 4>{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<GLubyte, 4>{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<GLubyte, 4>{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<GLubyte, 4> pixel{};
+    glReadPixels(2, 3, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel.data());
+    EXPECT_EQ(pixel, (std::array<GLubyte, 4>{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<GLubyte, 4>{255, 0, 0, 255})) << "default read endpoint";
+    EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR));
+}
 } // namespace MGITest

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**

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 <MG_Impl/Pipe/FramebufferEmit.h>
+#include <MG_Impl/Pipe/PipeFill.h>
 #include <MG_Impl/Pipe/TextureEmit.h>
 
 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<Fbo>& read, const SharedPtr<Fbo>& 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<Fbo>& m_read;
+            BindingSlot<Fbo>& m_draw;
+            SharedPtr<Fbo> m_savedRead;
+            SharedPtr<Fbo> m_savedDraw;
+        };
+
+        void EmitBlitNamedFramebuffer(
+            const SharedPtr<MG_State::GLState::FramebufferObject>& read,
+            const SharedPtr<MG_State::GLState::FramebufferObject>& 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<Uint32>(mask);
+            record.Filter = static_cast<Uint32>(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<MG_State::GLState::FramebufferObject>&,                                     \
-       const SharedPtr<MG_State::GLState::FramebufferObject>&, GLint, GLint, GLint, GLint, GLint,  \
-       GLint, GLint, GLint, GLbitfield, GLenum))                                                   \
     X(GetTexImage, void, (GLenum, GLint, GLenum, GLenum, GLvoid*))                                 \
     X(GetTextureImage, void,                                                                       \
       (const SharedPtr<MG_State::GLState::ITextureObject>&, 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

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
+        // the one that matches what the server's state actually is. For the named form the
-        // two frontend SharedPtrs, which table 2 lists as uncarried.
+        // 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<GLbitfield>(blit.Mask),

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(ImplementedVerbCount(), 49u);
-    EXPECT_EQ(UnmigratedSlotCount(), 21u);
+    EXPECT_EQ(UnmigratedSlotCount(), 20u);
     EXPECT_EQ(LocallyAnsweredSlotCount() + ImplementedVerbCount() + UnmigratedSlotCount(),
               kRemoteEmitSlotCount);
 }