diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 06974e5e..2671b81c 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -77,6 +77,7 @@ add_executable(MobileGLIntegrationTest Scenarios/PixelStoreSweepScenario.cpp Scenarios/PrimitiveRestartScenario.cpp Scenarios/FragCoordOriginScenario.cpp + Scenarios/ClientVertexArrayScenario.cpp Scenarios/ClearThenReadPixelsScenario.cpp Scenarios/SampleVariablesScenario.cpp Scenarios/DepthStencilReadbackScenario.cpp @@ -2069,6 +2070,26 @@ if (MOBILEGL_BUILD_DISAGGREGATED) ENVIRONMENT "${MGL_ITEST_GLES_SPLIT_ENVIRONMENT}" ) + # ---- P5e package vi (MG_Remote/CONTRACT-P5E.md §5.1, ID-82 / ruling 2) -------------------- + # + # THE LANE ENTRY THAT DID NOT EXIST, and its absence was the brief's open item (a). Nothing + # else registered here draws from a vertex array in the APPLICATION's own memory: every + # other scenario binds GL_ARRAY_BUFFER first, so its glVertexAttribPointer argument is a + # byte offset. This is the one draw shape whose bytes have no wire form - the server uploads + # them by dereferencing a client pointer per draw - and it is therefore both the positive + # control for vi's decision to KEEP that arm alive under lockstep and the only entry that + # can reach the run-ahead refusal when ra publishes the caps bit. + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectGLES.Split." + TEST_LIST MGL_SPLIT_VI_CLIENT_ARRAY_TESTS + TEST_FILTER "ClientVertexArrayScenario.*" + DISCOVERY_TIMEOUT 30 + PROPERTIES + LABELS "integration-gpu\;integration-split" + TIMEOUT ${MGL_ITEST_TIMEOUT} + ENVIRONMENT "${MGL_ITEST_GLES_SPLIT_ENVIRONMENT}" + ) + gtest_discover_tests(MobileGLIntegrationTest TEST_PREFIX "DirectGLES.Split." TEST_LIST MGL_SPLIT_SYNC_TESTS diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in b/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in index ca76b5c5..3bc1c1f6 100644 --- a/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in +++ b/MobileGL/MG_IntegrationTest/Harness/SplitLogPaths.cmake.in @@ -3,8 +3,12 @@ file(MAKE_DIRECTORY "@CMAKE_CURRENT_BINARY_DIR@/split-logs") # P5b t2's two lanes ride the same rule: one private log path per entry, or # SplitLogPaths.PrivateAndDistinct is red for them (ID-53). +# P5e vi's client-array lane joins the same list for the same reason: its two entries are the +# only ones that can reach Fatal{UnmigratedVerb, "DrawArrays+CLIENT_ARRAYS"} once ra publishes +# the caps bit, and a shared log file is a marker nobody can attribute. foreach(entry IN LISTS MGL_SPLIT_CLEAR_TESTS MGL_SPLIT_TRIANGLE_TESTS MGL_SPLIT_PMAP_TESTS - MGL_SPLIT_T2_TESS_TESTS MGL_SPLIT_T2_XFB_TESTS MGL_SPLIT_SYNC_TESTS) + MGL_SPLIT_T2_TESS_TESTS MGL_SPLIT_T2_XFB_TESTS MGL_SPLIT_SYNC_TESTS + MGL_SPLIT_VI_CLIENT_ARRAY_TESTS) set_tests_properties("${entry}" PROPERTIES ENVIRONMENT "MOBILEGL_LOG_FILE_PATH=@CMAKE_CURRENT_BINARY_DIR@/split-logs/${entry}.log") endforeach() diff --git a/MobileGL/MG_IntegrationTest/Scenarios/ClientVertexArrayScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/ClientVertexArrayScenario.cpp new file mode 100644 index 00000000..ac615a74 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/ClientVertexArrayScenario.cpp @@ -0,0 +1,189 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/ClientVertexArrayScenario.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// Scenario - A VERTEX ARRAY THAT LIVES IN THE APPLICATION'S OWN MEMORY, DRAWN OVER THE WIRE. +// +// WHY THIS FILE EXISTS AT ALL (P5e vi, ID-82 / BRIEF-P5E §6 open item (a)). The brief asked +// whether any existing scenario exercises a client-memory vertex array under split, and named +// DoublePrecisionScenario as the candidate. IT DOES NOT, and nothing else does either: every +// glVertexAttribPointer in this directory is issued with a buffer bound to GL_ARRAY_BUFFER, so +// its last argument is a byte OFFSET and not a host pointer (DoublePrecisionScenario uses the +// binding-model calls against real buffer objects, DoublePrecisionScenario.cpp:944-959). So the +// one draw shape whose bytes have no wire form was, until this file, never drawn over the wire +// in any lane. +// +// WHAT IT PINS, in the two arms it can be in: +// +// TODAY (lockstep, and the monolith control) the draw is NOT refused - ruling 4: the client +// is parked in WaitForApplied for exactly this record, so the server's per-draw upload of the +// application's bytes (Managers.cpp's SyncClientSideAttributesForDrawArrays, the kimi audit's +// row 14) reads memory that is not moving. The assertion is therefore about PIXELS: the quad +// has to arrive. That is what makes vi's "monolith-only would have been wrong here" concrete +// rather than an argument in a report - delete the split arm of the upload and this goes red. +// +// UNDER RUN-AHEAD it is REFUSED BY NAME on the GL thread, Fatal{UnmigratedVerb, +// "DrawArrays+CLIENT_ARRAYS"} (CONTRACT-P5E §5.1), because the client is a frame ahead and +// those bytes are moving under the reader. This file is the red-once for that refusal: it is +// the only lane entry that can reach it. Staging the bytes as a per-attribute {BindingIndex, +// MGHostSpan} tail - the shape kDrawHasUserIndices already has for client INDICES - is P8's, +// and retires the refusal along with this scenario's second half. +// +// Both draw entry points that carry the upload are driven, because they are two arms and not +// one: DrawArrays uploads the single range, MultiDrawArrays uploads one range per sub-draw. + +#include +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + constexpr const char* kVS = R"(#version 330 core +in vec2 aPos; +void main() { gl_Position = vec4(aPos, 0.0, 1.0); } +)"; + + constexpr const char* kFS = R"(#version 330 core +out vec4 o_color; +void main() { o_color = vec4(0.0, 1.0, 0.0, 1.0); } +)"; + + class ClientVertexArrayScenario : public ScenarioTest {}; + + // THE WHOLE POINT IS THE ABSENCE OF A BUFFER. glBindBuffer(GL_ARRAY_BUFFER, 0) before + // glVertexAttribPointer makes the last argument a HOST POINTER rather than an offset, + // which is the one vertex source EmitVertexBuffers publishes as Res == + // kMGPipeNullHandle - "not a hole: it is exactly how the server learns this attribute + // is client-sourced, upload it yourself" (VertexInputEmit.h). + // + // A VAO is still bound, because ES core requires one and because the server's upload + // hangs off the VAO's twin. + struct ClientArrayQuad { + GLuint vao = 0; + const float* vertices = nullptr; + + void Bind(const float* quad) { + vertices = quad; + glGenVertexArrays(1, &vao); + glBindVertexArray(vao); + glBindBuffer(GL_ARRAY_BUFFER, 0); + glEnableVertexAttribArray(0); + glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 2 * sizeof(float), vertices); + } + void Release() { + glBindVertexArray(0); + if (vao != 0) glDeleteVertexArrays(1, &vao); + vao = 0; + } + }; + + // Full-viewport strip, so "did the draw arrive" is one pixel read rather than a shape + // comparison - the claim is about the vertex SOURCE, not about rasterisation. + const float kQuad[] = {-1.0f, -1.0f, 1.0f, -1.0f, -1.0f, 1.0f, 1.0f, 1.0f}; + + Rgba8 CentreAfter(int width, int height) { + const Image image = ReadPixelsRect(0, 0, width, height); + return image.At(width / 2, height / 2); + } + + } // namespace + + TEST_F(ClientVertexArrayScenario, AClientMemoryVertexArrayReachesTheDrawItFeeds) { + if (!Ready()) return; + HeadlessGL& gl = Gl(); + const int width = gl.Width(); + const int height = gl.Height(); + + std::string error; + const unsigned int program = CompileProgram(kVS, kFS, &error); + ASSERT_NE(program, 0u) << error; + + BindDefaultFramebuffer(); + glViewport(0, 0, width, height); + glDisable(GL_SCISSOR_TEST); + glDisable(GL_DEPTH_TEST); + ClearTo(0.0f, 0.0f, 1.0f, 1.0f); + + ClientArrayQuad quad; + quad.Bind(kQuad); + glUseProgram(program); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + + const Rgba8 centre = CentreAfter(width, height); + EXPECT_GT(static_cast(centre.g), 200) + << "the client-memory vertex array never reached the draw: the centre is " << centre + << ", i.e. still the clear colour. Under split this means the per-draw upload of " + "the application's own bytes did not run - the arm CONTRACT-P5E §5.1 keeps alive " + "for lockstep, and refuses only under run-ahead"; + EXPECT_LT(static_cast(centre.b), 60) + << "the quad drew, but the clear colour is still showing through: " << centre; + + quad.Release(); + glUseProgram(0); + glDeleteProgram(program); + EXPECT_EQ(FirstGLError(), 0u); + } + + TEST_F(ClientVertexArrayScenario, EverySubDrawOfAMultiDrawArraysGetsItsOwnClientRange) { + if (!Ready()) return; + HeadlessGL& gl = Gl(); + const int width = gl.Width(); + const int height = gl.Height(); + + std::string error; + const unsigned int program = CompileProgram(kVS, kFS, &error); + ASSERT_NE(program, 0u) << error; + + BindDefaultFramebuffer(); + glViewport(0, 0, width, height); + glDisable(GL_SCISSOR_TEST); + glDisable(GL_DEPTH_TEST); + ClearTo(0.0f, 0.0f, 1.0f, 1.0f); + + // Two strips out of one client array: the left half and the right half, so a + // MultiDrawArrays that uploaded only the FIRST sub-draw's range leaves one side blue. + static const float kTwoHalves[] = { + -1.0f, -1.0f, 0.0f, -1.0f, -1.0f, 1.0f, 0.0f, 1.0f, + 0.0f, -1.0f, 1.0f, -1.0f, 0.0f, 1.0f, 1.0f, 1.0f, + }; + ClientArrayQuad quad; + quad.Bind(kTwoHalves); + glUseProgram(program); + + const GLint firsts[2] = {0, 4}; + const GLsizei counts[2] = {4, 4}; + glMultiDrawArrays(GL_TRIANGLE_STRIP, firsts, counts, 2); + + const Image image = ReadPixelsRect(0, 0, width, height); + const Rgba8 left = image.At(width / 4, height / 2); + const Rgba8 right = image.At((3 * width) / 4, height / 2); + EXPECT_GT(static_cast(left.g), 200) + << "the FIRST sub-draw's client range did not reach the draw: " << left; + EXPECT_GT(static_cast(right.g), 200) + << "the SECOND sub-draw's client range did not reach the draw (" << right + << "): an upload that ran once for the whole call instead of once per sub-draw " + "leaves exactly this half unpainted"; + + quad.Release(); + glUseProgram(0); + glDeleteProgram(program); + EXPECT_EQ(FirstGLError(), 0u); + } + +} // namespace MGITest diff --git a/MobileGL/MG_Test/Pipe/VertexInputEmitTest.cpp b/MobileGL/MG_Test/Pipe/VertexInputEmitTest.cpp index b826465a..e1ac461f 100644 --- a/MobileGL/MG_Test/Pipe/VertexInputEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/VertexInputEmitTest.cpp @@ -129,6 +129,7 @@ namespace { X(VertexInputEmit, IsLongAndFloat64TravelSeparately) \ X(VertexInputEmit, ABaseInstanceChangeAloneStillEmitsTheVertexBufferSet) \ X(VertexInputEmit, AnUnchangedSetWithAnUnchangedBaseInstanceEmitsNothing) \ + X(VertexInputEmit, TheVertexBufferWindowCoversEveryAttributeTheCsoDeclaresEnabled) \ X(VertexInputEmit, RebindingTheSameVaoEmitsABindAndNoCreate) \ X(VertexInputEmit, PingPongingBetweenTwoVaosNeverRecreatesEither) \ X(VertexInputEmit, DestroyedVertexArraysReturnTheirCsoSlotsAndRecords) \ @@ -433,6 +434,78 @@ namespace { EXPECT_EQ(Emitter().VertexBufferSetCount(), 2u); } + // ==================== P5e (vi), ID-95 / ruling 19 / CONTRACT-P5E §5.3 ==================== + // + // THE WINDOW RULE, PINNED RATHER THAN ARGUED: set_vertex_buffers' window must COVER every + // attribute the vertex-elements CSO declares ENABLED. + // + // Why it needs a case at all (scout S1's caveat R4). The two views are emitted from the + // same validate step and both walk the same `Enabled` bits, so today they cannot disagree - + // which is exactly the shape of unpinned invariant that a later narrowing turns into a + // silent wrong picture. An attribute the RECORD says is enabled but that falls outside the + // window resolves VertexBufferForAttributeIndex == nullptr on the server and is SKIPPED at + // the attribute walk (Managers.cpp): the array stays enabled in the driver VAO with no + // buffer bound under it, which is not a refusal and not a black triangle, it is whatever + // that attribute last pointed at. + // + // The oracle is the RECORD, not the emitter's arithmetic: whatever create_vertex_elements + // declared enabled must be inside [Start, Start+Count). Sparse on purpose - a hole at 1..6 + // is what distinguishes "the window covers the enabled set" from "the window happens to be + // as long as the attribute array". + // + // The sink half is CheckUnitWindows (tx2's, called at every draw/dispatch), whose verdict + // for a narrow window is Fatal{ProtocolCorruption, "SetVertexBuffers.Count"}. This case is + // the PRODUCER half and is the one that goes red if the emitter narrows. + TEST(VertexInputEmit, TheVertexBufferWindowCoversEveryAttributeTheCsoDeclaresEnabled) { + EmitterScope scope; + const SharedPtr vao = MakeVao(1); + const SharedPtr buffer = Ctx().CreateBufferObject(1); + buffer->Respecify(4096, nullptr); + + // Enabled: 0 and 7. Disabled: everything between, and everything above. + const Uint kEnabled[] = {0u, 7u}; + for (Uint index : kEnabled) { + vao->SetAttributeFormat(index, 4, DataType::Float32, false, 16, 0, false); + vao->BindAttributeBuffer(index, buffer); + vao->EnableAttribute(index); + } + + ASSERT_GT(Emitter().EmitVertexElements(Ctx()), 0u); + ASSERT_GT(Emitter().EmitVertexBuffers(Ctx(), 0), 0u); + + const MGPVertexBuffers& window = Emitter().LastVertexBuffers(); + const auto& declared = Emitter().LastAttributes(); + const Uint32 declaredCount = Emitter().LastElements().AttributeCount; + + Uint32 covered = 0; + for (Uint32 i = 0; i < declaredCount; ++i) { + if (!declared[i].Enabled) continue; + ++covered; + SCOPED_TRACE(::testing::Message() << "attribute " << i); + EXPECT_GE(i, window.Start) + << "create_vertex_elements declares attribute " << i + << " ENABLED but set_vertex_buffers' window starts at " << window.Start + << ". The backend resolves no binding for it and SKIPS it, leaving the driver " + "array enabled over whatever it last pointed at (ID-95 / ruling 19)"; + EXPECT_LT(i, window.Start + window.Count) + << "create_vertex_elements declares attribute " << i + << " ENABLED but set_vertex_buffers' window ends at " << (window.Start + window.Count) + << " - the same skip, at the other end. A narrower window is the sink's " + "Fatal{ProtocolCorruption, \"SetVertexBuffers.Count\"}"; + } + ASSERT_EQ(covered, 2u) + << "the record did not declare the two attributes this case enabled, so the window " + "check above had nothing to be about"; + + // And the window's own entries agree with the record about WHICH attribute each is, so + // "covered" cannot be satisfied by a window of the right LENGTH over the wrong indices. + for (Uint32 i = 0; i < window.Count; ++i) { + EXPECT_EQ(Emitter().LastEntries()[i].BindingIndex, window.Start + i) + << "entry " << i << " of the window does not name attribute " << (window.Start + i) + << "; the server indexes this window BY ATTRIBUTE"; + } + } + // D-G3's per-handle latch. create_vertex_elements is re-issued on the SAME handle when a // configuration moves, and the latch is stored per handle rather than globally so that // rebinding cannot look like a configuration change. diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index 80beaa1a..9c25a928 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -5004,6 +5004,168 @@ TEST(DirectGLESSlotTable, ACompositeHandleDoesNotGrowTheOrdinaryTable) { EXPECT_EQ(table.CompositeCapacityForTest(), 4u) << "the recycle re-grew the band"; } +// ========================================================================================== +// P5e (vi), CONTRACT-P5E §5.1: THE DRAW'S BUFFERS COME FROM THE RECORD, NOT FROM THE VAO. +// ========================================================================================== +// +// The two cases below are BRIEF-P5E's red-onces (2) and (3) for this package, and they are +// built the way ID-102 says a red-once has to be: the thing under test is THE PRODUCTION +// DECISION ITSELF (BufferImpl::ResolveDrawVertexBuffersFromRecord and +// ResolveDrawIndexBufferFromRecord, which are the only enumeration +// SyncNeccessaryBuffers' record arm has), not a copy of it written here. Both take the applier +// state and NOTHING ELSE, so the only way to revert §5.1's substitution is to put a frontend +// read back inside one of them - and then these go red naming the field. +// +// THE SETUP IS THE DIVERGENCE ITSELF: the frontend VAO is pointed at a buffer the RECORD does +// not name. That state is unreachable in one process today (the client re-emits at every +// validate point, so the two always agree); under run-ahead it is the ordinary state of the +// world, because the client has moved on by the time the record is applied. Producing it here +// by hand is the only way to ask "which one did you read" at all. +// +// WHAT IS NOT HERE, and deliberately: the ensure, the memo and the clean probe. They need an ES +// context and a driver, and DirectGLES.Split.* is where they are exercised +// (ClientVertexArrayScenario, TriangleScenario, IndexedDrawFamilyScenario and the rest of the +// lane draw through exactly this arm). +TEST(DirectGLESVertexInputDraw, TheAttributeWalkTakesItsBuffersFromTheRecordNotTheFrontendVao) { + using namespace MobileGL; + using namespace MobileGL::MG_Backend::DirectGLES; + using namespace MobileGL::MG_State::GLState; +#if !MOBILEGL_BUILD_DISAGGREGATED + GTEST_SKIP() << "the record arm of the attribute walk is compiled only under " + "MOBILEGL_BUILD_DISAGGREGATED"; +#else + // A, B and C, with handles minted the way the client mints them. + auto bufferA = MakeShared(0u); + auto bufferB = MakeShared(0u); + auto bufferC = MakeShared(0u); + const MG_Pipe::MGPipeHandle a = + MG_Pipe::MGPipeSlots().Acquire(MG_Pipe::MGPipeKind::Buffer, bufferA->GetLifetimeId()); + const MG_Pipe::MGPipeHandle b = + MG_Pipe::MGPipeSlots().Acquire(MG_Pipe::MGPipeKind::Buffer, bufferB->GetLifetimeId()); + const MG_Pipe::MGPipeHandle c = + MG_Pipe::MGPipeSlots().Acquire(MG_Pipe::MGPipeKind::Buffer, bufferC->GetLifetimeId()); + ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(a)); + ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(b)); + ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(c)); + + // THE PUBLISHED STATE: attribute 0 fetches from A, attribute 1 from B, attribute 2 from A + // again (the dedupe), attribute 3 is a client-memory array (null Res, nothing to ensure). + MG_Pipe::MGPipeApplierState& st = MG_Pipe::MGPipeApplier(); + const MG_Pipe::MGPipeApplierState saved = st; + st.VertexBufferStart = 0; + st.VertexBufferCount = 4; + for (Uint32 i = 0; i < 4; ++i) { + st.VertexBuffers[i] = MG_Pipe::MGPVertexBuffer{}; + st.VertexBuffers[i].BindingIndex = i; + } + st.VertexBuffers[0].Res = a; + st.VertexBuffers[1].Res = b; + st.VertexBuffers[2].Res = a; + st.VertexBuffers[3].Res = MG_Pipe::kMGPipeNullHandle; + + // THE FRONTEND, MOVED ON: every enabled attribute now points at C, and nothing was + // re-emitted. A walk that read GetAllAttributes() answers {C}. The VAO is BOUND IN A REAL + // CONTEXT rather than free-standing, because that is the only shape in which the revert + // this case exists to catch - MGB_CTX->GetBoundVertexArray()->GetAllAttributes() - has + // anything to read at all; a free-standing object would make the revert crash instead of + // disagree, which is a red for the wrong reason. + UniquePtr previousContext = Move(MG_State::pGLContext); + MG_State::pGLContext = MakeUnique(); + MG_State::pGLContext->CreateVertexArrayObject(1); + MG_State::pGLContext->BindVertexArray(1); + const SharedPtr vao = MG_State::pGLContext->GetBoundVertexArray(); + ASSERT_NE(vao, nullptr); + for (Uint i = 0; i < 4; ++i) { + vao->SetAttributeFormat(i, 2, DataType::Float32, false, 8, 0, false, false, 8); + vao->BindAttributeBuffer(i, bufferC); + vao->EnableAttribute(i); + } + + BufferImpl::DrawVertexBufferRequest requests[VertexArrayObject::MAX_VERTEX_ATTRIBS]; + const Uint count = BufferImpl::ResolveDrawVertexBuffersFromRecord( + st, requests, VertexArrayObject::MAX_VERTEX_ATTRIBS); + + ASSERT_EQ(count, 2u) + << "the walk did not resolve st.VertexBuffers[Start..+Count): two DISTINCT handles are " + "named there (A twice and B), plus one client-memory array with no store to ensure"; + EXPECT_EQ(requests[0].Res, a) + << "the first buffer is not st.VertexBuffers[0].Res. If it is the frontend VAO's C, the " + "walk went back to GetAllAttributes() and §5.1's substitution is reverted"; + EXPECT_EQ(requests[1].Res, b) << "the second buffer is not st.VertexBuffers[1].Res"; + EXPECT_EQ(requests[0].BindingIndex, 0u); + EXPECT_EQ(requests[1].BindingIndex, 1u); + for (Uint i = 0; i < count; ++i) { + EXPECT_NE(requests[i].Res, c) + << "the walk ensured the buffer the FRONTEND VAO points at, which is the object a " + "run-ahead client has already moved on from"; + } + + st = saved; + MG_State::pGLContext.reset(); + MG_State::pGLContext = Move(previousContext); + MG_Pipe::MGPipeSlots().Free(MG_Pipe::MGPipeKind::Buffer, a); + MG_Pipe::MGPipeSlots().Free(MG_Pipe::MGPipeKind::Buffer, b); + MG_Pipe::MGPipeSlots().Free(MG_Pipe::MGPipeKind::Buffer, c); +#endif +} + +// Red-once (3), the index half. The claim has TWO parts and the second is what this package +// added: the handle comes from st.IndexBuffer.Res, and IndexBufferSerial comes with it - the +// legacy and push-monolith arms re-read the VAO's element slot on every indexed draw and so see +// a rebind for free, while this arm reads nothing and needs the serial in the key +// (ResolvedDrawBuffers::iboSerial). A rebind of the SAME buffer still moves the serial, which is +// exactly the case an identity-only compare would call a hit. +TEST(DirectGLESVertexInputDraw, TheIndexArmTakesItsBufferAndItsSerialFromTheRecord) { + using namespace MobileGL; + using namespace MobileGL::MG_Backend::DirectGLES; + using namespace MobileGL::MG_State::GLState; +#if !MOBILEGL_BUILD_DISAGGREGATED + GTEST_SKIP() << "the record arm of the index-buffer sync is compiled only under " + "MOBILEGL_BUILD_DISAGGREGATED"; +#else + auto published = MakeShared(0u); + auto rebound = MakeShared(0u); + const MG_Pipe::MGPipeHandle publishedHandle = + MG_Pipe::MGPipeSlots().Acquire(MG_Pipe::MGPipeKind::Buffer, published->GetLifetimeId()); + ASSERT_FALSE(MG_Pipe::MGPipeHandleIsNull(publishedHandle)); + + MG_Pipe::MGPipeApplierState& st = MG_Pipe::MGPipeApplier(); + const MG_Pipe::MGPipeApplierState saved = st; + st.IndexBuffer = MG_Pipe::MGPIndexBuffer{}; + st.IndexBuffer.Res = publishedHandle; + st.IndexBufferSerial = 11; + + // The frontend's element slot has moved to another buffer with nothing emitted, in a real + // context for the reason the case above gives. + UniquePtr previousContext = Move(MG_State::pGLContext); + MG_State::pGLContext = MakeUnique(); + MG_State::pGLContext->CreateVertexArrayObject(1); + MG_State::pGLContext->BindVertexArray(1); + const SharedPtr vao = MG_State::pGLContext->GetBoundVertexArray(); + ASSERT_NE(vao, nullptr); + vao->GetIndexBufferBindingSlot().Bind(rebound); + + BufferImpl::DrawIndexBufferRequest request = BufferImpl::ResolveDrawIndexBufferFromRecord(st); + EXPECT_EQ(request.Res, publishedHandle) + << "the index arm did not read st.IndexBuffer.Res; if it answered the VAO's element " + "slot it is reading a binding the client has already changed"; + EXPECT_EQ(request.Serial, 11u) << "IndexBufferSerial did not travel with the handle"; + + // The same buffer re-bound: the handle is unchanged and ONLY the serial says so. + st.IndexBufferSerial = 12; + request = BufferImpl::ResolveDrawIndexBufferFromRecord(st); + EXPECT_EQ(request.Res, publishedHandle); + EXPECT_EQ(request.Serial, 12u) + << "a re-emitted set_index_buffer on the SAME handle is invisible to this arm without " + "the serial, and the memo would read clean over it"; + + st = saved; + MG_State::pGLContext.reset(); + MG_State::pGLContext = Move(previousContext); + MG_Pipe::MGPipeSlots().Free(MG_Pipe::MGPipeKind::Buffer, publishedHandle); +#endif +} + #else // G2 wants the pull and the push build to list the SAME ctest entries. The twin table only // exists under MOBILEGL_PIPE_PUSH, so in the pull build each case above keeps its name and @@ -5103,4 +5265,11 @@ TEST(DirectGLESBufferDrawProbe, UnderSplitTheRecordAloneAnswersTheLiveHostMapQue TEST(DirectGLESBufferDrawProbe, ACapsMaskWithoutTheResourceFamilyEmitsNothingAndCountsTheRefusal) { GTEST_SKIP() << "the handle-keyed resource table is compiled only under MOBILEGL_PIPE_PUSH"; } +// P5e (vi): G2/G14's skip twins for the two record-arm cases above. +TEST(DirectGLESVertexInputDraw, TheAttributeWalkTakesItsBuffersFromTheRecordNotTheFrontendVao) { + GTEST_SKIP() << "the record arm of the attribute walk is compiled only under MOBILEGL_PIPE_PUSH"; +} +TEST(DirectGLESVertexInputDraw, TheIndexArmTakesItsBufferAndItsSerialFromTheRecord) { + GTEST_SKIP() << "the record arm of the index-buffer sync is compiled only under MOBILEGL_PIPE_PUSH"; +} #endif // MOBILEGL_PIPE_PUSH