diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index dead37f8..3b55e563 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/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& 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(); diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index bb45694f..71b8446e 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/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); diff --git a/MobileGL/MG_IntegrationTest/Harness/WireIndirectPeek.cpp b/MobileGL/MG_IntegrationTest/Harness/WireIndirectPeek.cpp index e334e725..55b3b15c 100644 --- a/MobileGL/MG_IntegrationTest/Harness/WireIndirectPeek.cpp +++ b/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 diff --git a/MobileGL/MG_IntegrationTest/Harness/WireIndirectPeek.h b/MobileGL/MG_IntegrationTest/Harness/WireIndirectPeek.h index 010ce2ab..38f7191c 100644 --- a/MobileGL/MG_IntegrationTest/Harness/WireIndirectPeek.h +++ b/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); diff --git a/MobileGL/MG_IntegrationTest/Scenarios/GpuWrittenDrawInputScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/GpuWrittenDrawInputScenario.cpp index cf818f85..50a9e464 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/GpuWrittenDrawInputScenario.cpp +++ b/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 vertices = HalvesVertices(); + BindVertices(vertices.data(), static_cast(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 diff --git a/MobileGL/MG_Util/Metrics/PipeStats.cpp b/MobileGL/MG_Util/Metrics/PipeStats.cpp index ecd561bc..4d8ade84 100644 --- a/MobileGL/MG_Util/Metrics/PipeStats.cpp +++ b/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", diff --git a/MobileGL/MG_Util/Metrics/PipeStats.h b/MobileGL/MG_Util/Metrics/PipeStats.h index 93d34af0..219c2bfd 100644 --- a/MobileGL/MG_Util/Metrics/PipeStats.h +++ b/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 };