[Fix] (P13 W4a): monolith indirect draws stop reading the command buffer back when no attribute is client memory

MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp

@@ -2483,6 +2483,20 @@ namespace MobileGL::MG_Backend::DirectGLES {
         // there is one. FALSE means the caller must SKIP the draw: the elements it would fetch are
         // not in the store the driver is about to read, and issuing it anyway is a wrong picture
         // rather than an error.
+        // Does the VAO feed any enabled attribute from client memory? `vertexRate`, when given,
+        // is set when one of those attributes advances per vertex rather than per instance.
+        static Bool VaoHasEnabledClientArray(const MG_State::GLState::VertexArrayObject& vao,
+                                             Bool* vertexRate = nullptr) {
+            Bool any = false;
+            const auto& attributes = vao.GetAllAttributes();
+            for (SizeT i = 0; i < attributes.size(); ++i) {
+                if (!attributes[i].Enabled || attributes[i].Buffer) continue;
+                any = true;
+                if (vertexRate != nullptr) *vertexRate |= attributes[i].Divisor == 0;
+            }
+            return any;
+        }
+
         static Bool SyncClientSideVertexArraysForFetch(Uint8 indexSize, Uint32 elementStart, GLsizei count,
                                                        GLsizei instanceCount, GLint baseVertex, Uint32 baseInstance,
                                                        const void* clientIndices, Uint64 clientIndexBytes) {
@@ -2498,15 +2512,8 @@ namespace MobileGL::MG_Backend::DirectGLES {
 
             // A draw whose attributes all come from buffers has nothing to snapshot, and asking
             // the plan would read the index stream to answer a question nobody asked.
-            Bool clientArrays = false;
             Bool clientVertexRate = false;
-            const auto& attributes = currentVAO->GetAllAttributes();
-            for (SizeT i = 0; i < attributes.size(); ++i) {
-                if (!attributes[i].Enabled || attributes[i].Buffer) continue;
-                clientArrays = true;
-                clientVertexRate |= attributes[i].Divisor == 0;
-            }
-            if (!clientArrays) return true;
+            if (!VaoHasEnabledClientArray(*currentVAO, &clientVertexRate)) return true;
 
             auto* twin = ResolveVaoTwin(currentVAO);
             if (twin == nullptr) return false;
@@ -2554,6 +2561,15 @@ namespace MobileGL::MG_Backend::DirectGLES {
             const Uint8* commandBytes, SizeT commandOffset, GLsizei stride, GLsizei index, Uint8 indexSize,
             const SharedPtr<MG_State::GLState::BufferObject>& commandBuffer) {
             if (MG_Config::Transport != MG_Config::TransportMode::Monolith) return true;
+            // P13 W4a (ID-P8-14a): ASK FIRST WHETHER THERE IS ANYTHING TO SNAPSHOT. The command
+            // words below are read only to size a client-memory fetch; a VAO whose attributes all
+            // come from buffers has none, and reading them anyway cost a whole-buffer readback of a
+            // GPU-written command buffer per command, per viewport pass (the hang 62bfe461 left
+            // behind on a phone).
+            {
+                const auto& currentVAO = MG_Pipe::gPipeInputs.GetBoundVertexArray();
+                if (!currentVAO || !VaoHasEnabledClientArray(*currentVAO)) return true;
+            }
             const Uint8* source = commandBytes;
             if (commandBuffer) {
                 commandBuffer->SyncGpuWrites();

MobileGL/MG_Impl/Pipe/PipeFill.cpp

@@ -1205,6 +1205,9 @@ namespace MobileGL::MG_Pipe {
         record.Res = handle;
         record.Offset = offset;
         record.Size = size;
+        if (MG_Util::PipeStats::Enabled()) {
+            MG_Util::PipeStats::AddCalls(MG_Util::PipeStats::CallClass::ResourceReadbacks, 1);
+        }
         // The answer travels back through MGPipeClientOnBufferWriteback, and the server's
         // epoch bump happens AFTER that writeback, never before.
         MGPipeRouteResourceReadback(record);

MobileGL/MG_IntegrationTest/Harness/WireIndirectPeek.cpp

@@ -27,6 +27,7 @@ namespace MGITest {
         out->hostWaits = Stats::TotalCalls(Stats::CallClass::WireHostWaits);
         out->indirectWaits = Stats::TotalCalls(Stats::CallClass::WireHostWaitsIndirect);
         out->nativeDispatches = Stats::TotalCalls(Stats::CallClass::WireIndirectNativeDispatches);
+        out->resourceReadbacks = Stats::TotalCalls(Stats::CallClass::ResourceReadbacks);
         return true;
     }
 #else

MobileGL/MG_IntegrationTest/Harness/WireIndirectPeek.h

@@ -22,6 +22,7 @@ namespace MGITest {
         unsigned long long hostWaits = 0;      // wire-host-waits (`whw`)
         unsigned long long indirectWaits = 0;  // wire-host-waits-indirect (`whwi`)
         unsigned long long nativeDispatches = 0;  // P8-SV: wire-indirect-native-dispatches (`wdsp`)
+        unsigned long long resourceReadbacks = 0; // P13 W4a: resource-readbacks (client emissions)
     };
 
     bool PeekWireIndirectCounters(WireIndirectCounters* out);

MobileGL/MG_IntegrationTest/Scenarios/GpuWrittenDrawInputScenario.cpp

@@ -42,6 +42,7 @@
 #include "../Harness/HeadlessGL.h"
 #include "../Harness/ScenarioFixture.h"
 #include "../Harness/SplitRuntimePeek.h"
+#include "../Harness/WireIndirectPeek.h"
 
 #ifdef GLAPI
 #undef GLAPI
@@ -797,4 +798,70 @@ void main() {
                "second GPU write";
     }
 
+
+    // P13 W4a (ID-P8-14a): AN INDIRECT DRAW WITH NO CLIENT-MEMORY ARRAY READS NOTHING BACK. The
+    // monolith indirect executors used to pull a GPU-written command buffer back into its shadow
+    // once PER COMMAND, PER VIEWPORT PASS - before asking whether any attribute came from client
+    // memory, which is the only reason the words are read on the CPU at all. On a phone that was a
+    // multi-minute hang (62bfe461). The positive control is the same draw with a client-memory
+    // attribute: there the words ARE needed, so the readback must still happen and still draw.
+    TEST_F(GpuWrittenDrawInputScenario, AnIndirectDrawWithNoClientArrayReadsNothingBack) {
+        if (!Ready()) return;
+        WireIndirectCounters before{};
+        if (!PeekWireIndirectCounters(&before)) GTEST_SKIP() << "no in-process counter peek on this platform";
+        const GLuint program = Program(kFlatVertex, kFlatFragment);
+        ASSERT_NE(program, 0u);
+        const std::vector<GLfloat> vertices = HalvesVertices();
+        BindVertices(vertices.data(), static_cast<GLsizeiptr>(vertices.size() * sizeof(GLfloat)));
+        constexpr int kCommands = 4;
+        Store(m_commands, GL_DRAW_INDIRECT_BUFFER, kCommands * sizeof(ArraysCommand), nullptr);
+        // Four copies of {6, 1, 3, 0}: the left half, four times.
+        WriteWithCompute(R"(#version 430 core
+layout(local_size_x = 1) in;
+layout(std430, binding = 0) buffer Commands { uint command[]; };
+void main() {
+    for (int i = 0; i < 4; ++i) {
+        command[4 * i + 0] = 6u; command[4 * i + 1] = 1u; command[4 * i + 2] = 3u; command[4 * i + 3] = 0u;
+    }
+}
+)",
+                         m_commands, 0, GL_COMMAND_BARRIER_BIT);
+        glBindVertexArray(m_vao);
+        glBindBuffer(GL_DRAW_INDIRECT_BUFFER, m_commands);
+        PeekWireIndirectCounters(&before);
+        Image image = Render(program, [] { glMultiDrawArraysIndirect(GL_TRIANGLES, nullptr, kCommands, 0); });
+        WireIndirectCounters after{};
+        ASSERT_TRUE(PeekWireIndirectCounters(&after));
+        EXPECT_EQ(FirstGLError(), 0u);
+        EXPECT_TRUE(IsGreen(Half(image, 0))) << "the compute-written commands did not draw (" << Half(image, 0) << ")";
+        EXPECT_FALSE(IsGreen(Half(image, 1))) << "the right half drew: the commands were not the ones written";
+        EXPECT_EQ(after.resourceReadbacks - before.resourceReadbacks, 0u)
+            << "an indirect draw whose attributes all come from buffers read a buffer back into its "
+               "shadow; nothing on the CPU needs the command words for it";
+
+        // The positive control: attribute 0 from client memory, so the words size a snapshot.
+        WriteWithCompute(R"(#version 430 core
+layout(local_size_x = 1) in;
+layout(std430, binding = 0) buffer Commands { uint command[]; };
+void main() { command[0] = 6u; command[1] = 1u; command[2] = 3u; command[3] = 0u; }
+)",
+                         m_commands, 0, GL_COMMAND_BARRIER_BIT);
+        glBindVertexArray(m_vao);
+        glBindBuffer(GL_ARRAY_BUFFER, 0);
+        glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 2 * sizeof(GLfloat), vertices.data());
+        glBindBuffer(GL_DRAW_INDIRECT_BUFFER, m_commands);
+        if (FirstGLError() != 0u) {
+            // A context that refuses client-memory arrays has no snapshot for the control to need.
+            GTEST_SKIP() << "client-memory vertex arrays are refused here; the positive control has no subject";
+        }
+        PeekWireIndirectCounters(&before);
+        image = Render(program, [] { glMultiDrawArraysIndirect(GL_TRIANGLES, nullptr, 1, 0); });
+        ASSERT_TRUE(PeekWireIndirectCounters(&after));
+        EXPECT_EQ(FirstGLError(), 0u);
+        EXPECT_TRUE(IsGreen(Half(image, 0))) << "the client-array control did not draw (" << Half(image, 0) << ")";
+        EXPECT_GE(after.resourceReadbacks - before.resourceReadbacks, 1u)
+            << "the positive control read nothing back: either the counter is dead or the command words "
+               "a client-array snapshot needs were taken from a stale shadow";
+    }
+
 } // namespace MGITest

MobileGL/MG_Util/Metrics/PipeStats.cpp

@@ -233,7 +233,7 @@ namespace MobileGL::MG_Util::PipeStats {
             "residual-pulls", "server-verb-boundaries", "wire-records",
             "wire-indirect-native-draws", "wire-indirect-cpu-expansions", "wire-indirect-barriers",
             "wire-host-waits", "wire-host-waits-indirect", "wire-host-wait-us",
-            "wire-indirect-native-dispatches",
+            "wire-indirect-native-dispatches", "resource-readbacks",
         };
         const char* const kGateNames[kGateCount] = {
             "espryt-render-state", "espryt-texture-sync-list", "espryt-unit-bindings-epoch",

MobileGL/MG_Util/Metrics/PipeStats.h

@@ -284,6 +284,14 @@ namespace MobileGL::MG_Util::PipeStats {
         // wire store (VulkanRenderer::DispatchWireComputeIndirect). Their barriers count in `wibar`
         // with the draws': one barrier per shader write, whichever indirect read consumes it.
         WireIndirectNativeDispatches,
+        // P13 W4a (ID-P8-14a): one per resource_readback the CLIENT emitted - a whole buffer, or
+        // one slice of it, pulled back into the frontend shadow because a CPU reader needed bytes
+        // a GPU had written. Counted at the emitter (MGPipeEmitResourceReadbackRange), in every
+        // mode. It exists for the regression it was added with: monolith's indirect executors
+        // read the command buffer back once PER COMMAND, PER VIEWPORT PASS, whenever it carried a
+        // GPU write - before even asking whether the draw had a client-memory array to snapshot.
+        // Not on the summary line; read through TotalCalls by the scenario that pins it.
+        ResourceReadbacks,
         Count
     };