From beba025618b2b8eae0a554d36b1f57d1bf97e3fd Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 20 Sep 2026 11:26:28 -0400 Subject: [PATCH] [MG_Impl, MG_Remote] (Disaggregated): own client vertex and index data before draw emission --- MobileGL/MG_Impl/Pipe/OwnedDrawInputs.h | 243 ++++++++++++++++++ MobileGL/MG_Impl/Pipe/VertexInputEmit.h | 25 +- .../Scenarios/ClientVertexArrayScenario.cpp | 213 +++++++++++++-- MobileGL/MG_Remote/Client/EmitTables.cpp | 143 +++-------- 4 files changed, 499 insertions(+), 125 deletions(-) create mode 100644 MobileGL/MG_Impl/Pipe/OwnedDrawInputs.h diff --git a/MobileGL/MG_Impl/Pipe/OwnedDrawInputs.h b/MobileGL/MG_Impl/Pipe/OwnedDrawInputs.h new file mode 100644 index 00000000..bffa9080 --- /dev/null +++ b/MobileGL/MG_Impl/Pipe/OwnedDrawInputs.h @@ -0,0 +1,243 @@ +// MobileGL - MobileGL/MG_Impl/Pipe/OwnedDrawInputs.h +// 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 +// +// Client memory becomes ordinary owned buffer resources before a draw. +#pragma once + +#if MOBILEGL_BUILD_DISAGGREGATED && MOBILEGL_PIPE_PUSH +#include +#include +#include +#include +#include + +namespace MobileGL::MG_Pipe { + + // These private buffers have no GL name or frontend binding. Their normal + // resource records own the bytes on the server, and normal resource retirement + // keeps an already submitted draw alive after this client-side scope ends. + // Restore the application's wire bindings before destroying the private handles. + class MGPipeOwnedDrawInputs { + public: + using Buffer = MG_State::GLState::BufferObject; + using Context = MG_State::GLState::GLContext; + + explicit MGPipeOwnedDrawInputs(Context& context) : m_context(context) {} + ~MGPipeOwnedDrawInputs() { + if (m_vertexBindingsChanged) + MGPipeVertexInputEmitterInstance().EmitVertexBuffers(m_context, m_baseInstance); + if (m_indexBindingChanged) + MGPipeVertexInputEmitterInstance().EmitIndexBuffer(m_context); + } + + Bool Prepare(MGPDrawInfo& info, const MGPDrawRange* ranges, Uint32 rangeCount, + const void* clientIndices, Uint64 clientIndexBytes, const MGPDrawIndirect* indirect) { + m_baseInstance = info.StartInstance; + const auto& vao = m_context.GetBoundVertexArray(); + if (!vao) return (info.Flags & kDrawClientArrays) == 0 && clientIndexBytes == 0; + + // A single owned EBO also represents flattened MultiDraw client indices. + // Start remains an element offset, so base vertex and draw IDs are unchanged. + if (clientIndexBytes != 0) { + if (!clientIndices || clientIndexBytes > std::numeric_limits::max()) return false; + m_indexBuffer = MakeOwnedBuffer(clientIndices, static_cast(clientIndexBytes), BufferTarget::Index); + info.IndexResource = MGPipeResourceTrackerInstance().Find(*m_indexBuffer); + info.Flags &= ~static_cast(kDrawHasUserIndices); + MGPIndexBuffer binding{}; + binding.Res = info.IndexResource; + binding.IndexSize = info.IndexSize; + MGPipeRouteSetIndexBuffer(binding); + m_indexBindingChanged = true; + } + + if ((info.Flags & kDrawClientArrays) == 0) return true; + const auto& attributes = vao->GetAllAttributes(); + Bool needsVertexIndices = false; + for (const auto& attribute : attributes) + needsVertexIndices |= attribute.Enabled && !attribute.Buffer && attribute.Divisor == 0; + + Vector fetches; + if (indirect != nullptr) { + if (!ReadIndirectFetches(info.IndexSize, *indirect, fetches)) return false; + } else { + fetches.reserve(rangeCount); + for (Uint32 i = 0; i < rangeCount; ++i) + fetches.push_back({ranges[i], info.InstanceCount, info.StartInstance}); + } + + Vector vertices; + if (needsVertexIndices && !ReadVertexIndices(info, fetches, clientIndices, clientIndexBytes, vertices)) + return false; + Array handles{}; + for (SizeT location = 0; location < attributes.size(); ++location) { + const auto& attribute = attributes[location]; + if (!attribute.Enabled || attribute.Buffer) continue; + const SizeT elementBytes = AttributeBytes(attribute); + if (!elementBytes || attribute.Stride < 0) return false; + Vector instances; + const Vector* referenced = &vertices; + if (attribute.Divisor != 0) { + for (const auto& fetch : fetches) { + if (!fetch.Range.Count || !fetch.Instances) continue; + const Uint64 first = fetch.BaseInstance; + const Uint64 last = first + (fetch.Instances - 1u) / attribute.Divisor; + for (Uint64 at = first; at <= last; ++at) instances.push_back(at); + } + SortUnique(instances); + referenced = &instances; + } + const Uint64 stride = static_cast(attribute.Stride); + const Uint64 last = referenced->empty() ? 0 : referenced->back(); + if (stride != 0 && last > (std::numeric_limits::max() - elementBytes) / stride) + return false; + const SizeT byteCount = static_cast(last * stride + elementBytes); + Vector bytes(byteCount, 0); + if (!referenced->empty()) { + if (attribute.Offset == 0 || byteCount > std::numeric_limits::max() - attribute.Offset) + return false; + const auto* source = reinterpret_cast(attribute.Offset); + if (stride == 0) { + std::memcpy(bytes.data(), source, elementBytes); + } else { + // Only dereference fetched elements: sparse indices and first>0 + // do not grant permission to read intervening application memory. + for (const Uint64 vertex : *referenced) { + const SizeT offset = static_cast(vertex * stride); + std::memcpy(bytes.data() + offset, source + offset, elementBytes); + } + } + } + m_vertexBuffers[location] = MakeOwnedBuffer(bytes.data(), bytes.size(), BufferTarget::Vertex); + handles[location] = MGPipeResourceTrackerInstance().Find(*m_vertexBuffers[location]); + } + MGPipeVertexInputEmitterInstance().EmitVertexBuffers(m_context, info.StartInstance, &handles); + m_vertexBindingsChanged = true; + info.Flags &= ~static_cast(kDrawClientArrays); + return true; + } + + private: + struct Fetch { + MGPDrawRange Range; + Uint32 Instances; + Uint32 BaseInstance; + }; + + static SizeT AttributeBytes(const MG_State::GLState::VertexAttribute& attribute) { + if (attribute.IsBgra || attribute.Type == DataType::Int2101010Rev || + attribute.Type == DataType::Uint2101010Rev) return 4; + SizeT component = 0; + switch (attribute.Type) { + case DataType::Int8: case DataType::Uint8: component = 1; break; + case DataType::Int16: case DataType::Uint16: case DataType::Float16: component = 2; break; + case DataType::Int32: case DataType::Uint32: case DataType::Float32: case DataType::Fixed32: + component = 4; break; + case DataType::Float64: component = 8; break; + default: return 0; + } + return attribute.Size > 0 && attribute.Size <= 4 ? component * attribute.Size : 0; + } + + static void SortUnique(Vector& values) { + std::sort(values.begin(), values.end()); + values.erase(std::unique(values.begin(), values.end()), values.end()); + } + + static UniquePtr MakeOwnedBuffer(const void* bytes, SizeT size, BufferTarget target) { + auto buffer = MakeUnique(0); + const auto handle = MGPipeResourceTrackerInstance().Find(*buffer); + MGPipeResourceTrackerInstance().NoteBoundAs(handle, target); + buffer->SetUsage(BufferUsage::StreamDraw); + buffer->Respecify(size, bytes); + return buffer; + } + + static Bool ReadBuffer(const SharedPtr& buffer, Uint64 offset, SizeT size, void* destination) { + if (!buffer || offset > buffer->GetSize() || size > buffer->GetSize() - offset) return false; + // This waits for the actual server writeback when an earlier GPU draw, + // dispatch, or capture produced the index/command bytes. + buffer->SyncGpuWrites(); + buffer->DownloadSubData(destination, static_cast(offset), size); + return true; + } + + Bool ReadIndirectFetches(Uint8 indexSize, const MGPDrawIndirect& indirect, Vector& fetches) { + const auto& commands = m_context.GetBufferBindingSlot(BufferTarget::DrawIndirect).GetBoundObject(); + Uint32 count = indirect.DrawCount; + if (!MGPipeHandleIsNull(indirect.ParameterBuffer)) { + const auto& parameter = m_context.GetBufferBindingSlot(BufferTarget::Parameter).GetBoundObject(); + Uint32 actual = 0; + if (!ReadBuffer(parameter, indirect.ParameterOffset, sizeof(actual), &actual)) return false; + count = std::min(count, actual); + } + const Uint64 commandBytes = indexSize ? 5 * sizeof(Uint32) : 4 * sizeof(Uint32); + const Uint64 stride = indirect.Stride ? indirect.Stride : commandBytes; + fetches.reserve(count); + for (Uint32 i = 0; i < count; ++i) { + const Uint64 displacement = static_cast(i) * stride; + if (displacement > std::numeric_limits::max() - indirect.Offset) return false; + Uint32 words[5]{}; + if (!ReadBuffer(commands, indirect.Offset + displacement, commandBytes, words)) return false; + Int32 bias = 0; + if (indexSize) std::memcpy(&bias, &words[3], sizeof(bias)); + fetches.push_back({{words[2], words[0], bias}, words[1], words[indexSize ? 4 : 3]}); + } + return true; + } + + Bool ReadVertexIndices(const MGPDrawInfo& info, const Vector& fetches, + const void* clientIndices, Uint64 clientIndexBytes, Vector& vertices) { + const auto& vao = m_context.GetBoundVertexArray(); + const auto& elementBuffer = vao->GetIndexBufferBindingSlot().GetBoundObject(); + const Bool restart = (info.Flags & kDrawPrimitiveRestart) != 0; + const Bool fixedRestart = m_context.IsCapabilityEnabled(CapabilityInput::PrimitiveRestartFixedIndex); + const Uint32 restartIndex = fixedRestart + ? (info.IndexSize == 1 ? 0xffu : info.IndexSize == 2 ? 0xffffu : 0xffffffffu) + : info.RestartIndex; + for (const auto& fetch : fetches) { + if (!fetch.Range.Count || !fetch.Instances) continue; + if (!info.IndexSize) { + for (Uint64 n = 0; n < fetch.Range.Count; ++n) + vertices.push_back(static_cast(fetch.Range.Start) + n); + continue; + } + const Uint64 offset = static_cast(fetch.Range.Start) * info.IndexSize; + const Uint64 size = static_cast(fetch.Range.Count) * info.IndexSize; + if (size > std::numeric_limits::max()) return false; + Vector indexBytes; + const Uint8* data = nullptr; + if (clientIndices != nullptr) { + if (offset > clientIndexBytes || size > clientIndexBytes - offset) return false; + data = static_cast(clientIndices) + static_cast(offset); + } else { + indexBytes.resize(static_cast(size)); + if (!ReadBuffer(elementBuffer, offset, indexBytes.size(), indexBytes.data())) return false; + data = indexBytes.data(); + } + for (Uint32 n = 0; n < fetch.Range.Count; ++n) { + Uint32 index = 0; + std::memcpy(&index, data + static_cast(n) * info.IndexSize, info.IndexSize); + if (restart && index == restartIndex) continue; + const Int64 vertex = static_cast(index) + fetch.Range.IndexBias; + if (vertex < 0) return false; + vertices.push_back(static_cast(vertex)); + } + } + SortUnique(vertices); + return true; + } + + Context& m_context; + Array, kMGPipeMaxVertexAttribs> m_vertexBuffers{}; + UniquePtr m_indexBuffer; + Uint32 m_baseInstance = 0; + Bool m_vertexBindingsChanged = false; + Bool m_indexBindingChanged = false; + }; +} // namespace MobileGL::MG_Pipe +#endif diff --git a/MobileGL/MG_Impl/Pipe/VertexInputEmit.h b/MobileGL/MG_Impl/Pipe/VertexInputEmit.h index 716fb81e..f7af6b75 100644 --- a/MobileGL/MG_Impl/Pipe/VertexInputEmit.h +++ b/MobileGL/MG_Impl/Pipe/VertexInputEmit.h @@ -193,12 +193,12 @@ namespace MobileGL::MG_Pipe { // is the highest ENABLED attribute plus one, which is the 32-slot prefix walk the // dirty bit is specified over. // - // A client-memory array is Res == kMGPipeNullHandle, and that is not a hole: it is - // exactly how the server learns "this attribute is client-sourced, upload it - // yourself". Its store genuinely does not exist at this moment - the client-array - // uploader runs after PrepareForDraw, at the draw entry point - and moving that - // resolution to the client is P8's. - Uint64 EmitVertexBuffers(GLContext& ctx, Uint32 baseInstance) { + // Validate has no draw range yet, so a client-memory array initially names + // the null handle. Under a transport, OwnedDrawInputs snapshots the actual + // fetches at draw emission and calls this again with ownedClientBuffers. + // Monolith keeps its existing backend-side client-array upload. + Uint64 EmitVertexBuffers(GLContext& ctx, Uint32 baseInstance, + const Array* ownedClientBuffers = nullptr) { const auto& vao = ctx.GetBoundVertexArray(); Uint32 count = 0; if (vao) { @@ -212,6 +212,9 @@ namespace MobileGL::MG_Pipe { entry.Res = attrib.Buffer ? MGPipeSlots().Acquire(MGPipeKind::Buffer, attrib.Buffer->GetLifetimeId()) : kMGPipeNullHandle; + if (ownedClientBuffers != nullptr && attrib.Enabled && !attrib.Buffer) { + entry.Res = (*ownedClientBuffers)[i]; + } // D-A3's sticky mask, ORed HERE rather than only sampled at a storage op. // This is the bit that survives the DSA idiom: a buffer defined through // glNamedBuffer* may never be bound at any resource emission, but a draw @@ -380,9 +383,19 @@ namespace MobileGL::MG_Pipe { for (SizeT i = 0; i < kAttribs; ++i) { m_attributes[i] = MGPipeBuildVertexAttribWire(vao.GetAttribute(static_cast(i)), vao.GetAttributeBindingIndex(static_cast(i))); +#if MOBILEGL_BUILD_DISAGGREGATED + // Client addresses never cross the transport. Draw emission snapshots + // their referenced elements into owned buffers with a zero byte origin. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith && + !vao.GetAttribute(static_cast(i)).Buffer) m_attributes[i].Offset = 0; +#endif } for (SizeT i = 0; i < kBindings; ++i) { m_bindingPoints[i] = MGPipeBuildVertexBindingPointWire(vao.GetBindingPoint(static_cast(i))); +#if MOBILEGL_BUILD_DISAGGREGATED + if (MG_Config::Transport != MG_Config::TransportMode::Monolith && + !vao.GetBindingPoint(static_cast(i)).Buffer) m_bindingPoints[i].Offset = 0; +#endif } // Attributes first, then binding points, both ascending and contiguous. constexpr SizeT kAttribBytes = kAttribs * sizeof(MGPVertexAttribWire); diff --git a/MobileGL/MG_IntegrationTest/Scenarios/ClientVertexArrayScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/ClientVertexArrayScenario.cpp index ac615a74..f32d90db 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/ClientVertexArrayScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/ClientVertexArrayScenario.cpp @@ -17,26 +17,17 @@ // 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. +// Under a transport the client snapshots referenced elements into owned buffer +// resources before returning. Both lockstep and run-ahead must render the pixels; +// neither may borrow the application's addresses on the apply thread. The added +// cases also cover host overwrites, instancing and GPU-produced fetch ranges. // // 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 #include @@ -102,6 +93,35 @@ void main() { o_color = vec4(0.0, 1.0, 0.0, 1.0); } return image.At(width / 2, height / 2); } + GLuint ClientInputComputeProgram(const char* source) { + const GLuint shader = glCreateShader(GL_COMPUTE_SHADER); + glShaderSource(shader, 1, &source, nullptr); + glCompileShader(shader); + GLint compiled = GL_FALSE; + glGetShaderiv(shader, GL_COMPILE_STATUS, &compiled); + if (!compiled) { + char log[4096]{}; + glGetShaderInfoLog(shader, sizeof(log), nullptr, log); + ADD_FAILURE() << log; + glDeleteShader(shader); + return 0; + } + const GLuint program = glCreateProgram(); + glAttachShader(program, shader); + glLinkProgram(program); + glDeleteShader(shader); + GLint linked = GL_FALSE; + glGetProgramiv(program, GL_LINK_STATUS, &linked); + if (!linked) { + char log[4096]{}; + glGetProgramInfoLog(program, sizeof(log), nullptr, log); + ADD_FAILURE() << log; + glDeleteProgram(program); + return 0; + } + return program; + } + } // namespace TEST_F(ClientVertexArrayScenario, AClientMemoryVertexArrayReachesTheDrawItFeeds) { @@ -128,9 +148,7 @@ void main() { o_color = vec4(0.0, 1.0, 0.0, 1.0); } 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"; + << ", i.e. still the clear colour. The draw did not consume its owned client-vertex snapshot"; EXPECT_LT(static_cast(centre.b), 60) << "the quad drew, but the clear colour is still showing through: " << centre; @@ -186,4 +204,163 @@ void main() { o_color = vec4(0.0, 1.0, 0.0, 1.0); } EXPECT_EQ(FirstGLError(), 0u); } + TEST_F(ClientVertexArrayScenario, ClientVerticesAndIndicesKeepTheirBytesAfterTheDrawReturns) { + if (!Ready()) return; + std::string error; + const GLuint program = CompileProgram(kVS, kFS, &error); + ASSERT_NE(program, 0u) << error; + BindDefaultFramebuffer(); + glViewport(0, 0, Gl().Width(), Gl().Height()); + glDisable(GL_DEPTH_TEST); + glDisable(GL_SCISSOR_TEST); + ClearTo(0, 0, 1, 1); + std::array positions{-1, -1, 0, -1, -1, 1, 0, 1}; + std::array indices{0, 1, 2, 2, 1, 3}; + ClientArrayQuad quad; + quad.Bind(positions.data()); + glUseProgram(program); + glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_SHORT, indices.data()); + for (size_t i = 0; i < positions.size(); i += 2) positions[i] += 1; + glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_SHORT, indices.data()); + // No readback or Finish between the draws and the host writes. + std::fill(positions.begin(), positions.end(), -100.0f); + std::fill(indices.begin(), indices.end(), 0); + const Image image = ReadPixelsRect(0, 0, Gl().Width(), Gl().Height()); + EXPECT_GT(image.At(Gl().Width() / 4, Gl().Height() / 2).g, 200); + EXPECT_GT(image.At(3 * Gl().Width() / 4, Gl().Height() / 2).g, 200); + quad.Release(); + glUseProgram(0); + glDeleteProgram(program); + EXPECT_EQ(FirstGLError(), 0u); + } + + TEST_F(ClientVertexArrayScenario, ClientAttributesKeepBaseVertexAndDividedBaseInstanceFetches) { + if (!Ready()) return; + const char* vertex = R"(#version 430 core +layout(location=0) in vec2 aPos; +layout(location=1) in vec2 aOffset; +void main() { gl_Position = vec4(aPos + aOffset, 0.0, 1.0); } +)"; + std::string error; + const GLuint program = CompileProgram(vertex, kFS, &error); + ASSERT_NE(program, 0u) << error; + BindDefaultFramebuffer(); + glViewport(0, 0, Gl().Width(), Gl().Height()); + glDisable(GL_DEPTH_TEST); + glDisable(GL_SCISSOR_TEST); + ClearTo(0, 0, 1, 1); + const float positions[]{-0.4f, -0.8f, 0.4f, -0.8f, -0.4f, 0.8f, 0.4f, 0.8f}; + const float offsets[]{100, 100, -0.5f, 0, 0.5f, 0}; + const GLushort indices[]{1, 2, 3, 3, 2, 4}; + ClientArrayQuad quad; + quad.Bind(positions); + glEnableVertexAttribArray(1); + glVertexAttribPointer(1, 2, GL_FLOAT, GL_FALSE, 2 * sizeof(float), offsets); + glVertexAttribDivisor(1, 2); + glUseProgram(program); + glDrawElementsInstancedBaseVertexBaseInstance(GL_TRIANGLES, 6, GL_UNSIGNED_SHORT, indices, 3, -1, 1); + const Image image = ReadPixelsRect(0, 0, Gl().Width(), Gl().Height()); + EXPECT_GT(image.At(Gl().Width() / 4, Gl().Height() / 2).g, 200); + EXPECT_GT(image.At(3 * Gl().Width() / 4, Gl().Height() / 2).g, 200); + quad.Release(); + glUseProgram(0); + glDeleteProgram(program); + EXPECT_EQ(FirstGLError(), 0u); + } + + TEST_F(ClientVertexArrayScenario, GpuWrittenIndicesSelectTheClientVerticesBeforeTheyAreCopied) { + if (!Ready()) return; + const GLuint compute = ClientInputComputeProgram(R"(#version 430 core +layout(local_size_x=1) in; +layout(std430, binding=0) buffer Indices { uint value[]; }; +void main() { + value[0]=4u; value[1]=5u; value[2]=6u; + value[3]=6u; value[4]=5u; value[5]=7u; +} +)"); + ASSERT_NE(compute, 0u); + std::string error; + const GLuint program = CompileProgram(kVS, kFS, &error); + ASSERT_NE(program, 0u) << error; + BindDefaultFramebuffer(); + glViewport(0, 0, Gl().Width(), Gl().Height()); + glDisable(GL_DEPTH_TEST); + glDisable(GL_SCISSOR_TEST); + ClearTo(0, 0, 1, 1); + const float positions[]{100, 100, 100, 100, 100, 100, 100, 100, + -1, -1, 1, -1, -1, 1, 1, 1}; + ClientArrayQuad quad; + quad.Bind(positions); + GLuint ebo = 0; + glGenBuffers(1, &ebo); + glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, ebo); + const GLuint poison[6]{}; + glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(poison), poison, GL_DYNAMIC_COPY); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, ebo); + glUseProgram(compute); + glDispatchCompute(1, 1, 1); + glMemoryBarrier(GL_ELEMENT_ARRAY_BARRIER_BIT | GL_SHADER_STORAGE_BARRIER_BIT); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, 0); + glUseProgram(program); + glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_INT, nullptr); + EXPECT_GT(CentreAfter(Gl().Width(), Gl().Height()).g, 200); + quad.Release(); + glUseProgram(0); + glDeleteBuffers(1, &ebo); + glDeleteProgram(program); + glDeleteProgram(compute); + EXPECT_EQ(FirstGLError(), 0u); + } + + TEST_F(ClientVertexArrayScenario, GpuWrittenIndirectCommandsDefineTheClientAttributeSnapshot) { + if (!Ready()) return; + const GLuint compute = ClientInputComputeProgram(R"(#version 430 core +layout(local_size_x=1) in; +layout(std430, binding=0) buffer Command { uint value[]; }; +void main() { value[0]=4u; value[1]=1u; value[2]=4u; value[3]=1u; } +)"); + ASSERT_NE(compute, 0u); + const char* vertex = R"(#version 430 core +layout(location=0) in vec2 aPos; +layout(location=1) in vec2 aOffset; +void main() { gl_Position = vec4(aPos + aOffset, 0.0, 1.0); } +)"; + std::string error; + const GLuint program = CompileProgram(vertex, kFS, &error); + ASSERT_NE(program, 0u) << error; + BindDefaultFramebuffer(); + glViewport(0, 0, Gl().Width(), Gl().Height()); + glDisable(GL_DEPTH_TEST); + glDisable(GL_SCISSOR_TEST); + ClearTo(0, 0, 1, 1); + const float positions[]{100, 100, 100, 100, 100, 100, 100, 100, + -1, -1, 1, -1, -1, 1, 1, 1}; + const float offsets[]{100, 100, 0, 0}; + ClientArrayQuad quad; + quad.Bind(positions); + glEnableVertexAttribArray(1); + glVertexAttribPointer(1, 2, GL_FLOAT, GL_FALSE, 2 * sizeof(float), offsets); + glVertexAttribDivisor(1, 1); + GLuint commands = 0; + glGenBuffers(1, &commands); + glBindBuffer(GL_DRAW_INDIRECT_BUFFER, commands); + const GLuint poison[4]{}; + glBufferData(GL_DRAW_INDIRECT_BUFFER, sizeof(poison), poison, GL_DYNAMIC_COPY); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, commands); + glUseProgram(compute); + glDispatchCompute(1, 1, 1); + glMemoryBarrier(GL_COMMAND_BARRIER_BIT | GL_SHADER_STORAGE_BARRIER_BIT); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, 0); + glUseProgram(program); + glDrawArraysIndirect(GL_TRIANGLE_STRIP, nullptr); + EXPECT_GT(CentreAfter(Gl().Width(), Gl().Height()).g, 200); + quad.Release(); + glUseProgram(0); + glBindBuffer(GL_DRAW_INDIRECT_BUFFER, 0); + glDeleteBuffers(1, &commands); + glDeleteProgram(program); + glDeleteProgram(compute); + EXPECT_EQ(FirstGLError(), 0u); + } + } // namespace MGITest diff --git a/MobileGL/MG_Remote/Client/EmitTables.cpp b/MobileGL/MG_Remote/Client/EmitTables.cpp index 23334f80..a1a443ca 100644 --- a/MobileGL/MG_Remote/Client/EmitTables.cpp +++ b/MobileGL/MG_Remote/Client/EmitTables.cpp @@ -53,10 +53,12 @@ // set_index_buffer / the buffer's own constructor did that), for MGPDrawInfo::IndexResource and // the two indirect-buffer handles. #include +#include #include #include #include +#include #include "WireTables.h" #include @@ -214,18 +216,6 @@ namespace MobileGL::MG_Remote::Client { return *session; } - // The two hooks b1 wrote and deliberately left with no caller, because the call site is - // this file's. ORDER: the push first (it produces resource_subdata records that must - // precede the verb on SEG_CMD), then the mark walk, then the verb record. The deferred - // destroy drain rides the same boundary: it replays, on this GL thread, the death - // announcements whose last SharedPtr dropped on the apply thread (PipeMutation.h's - // deferred destroy queue), and its records must also precede this verb. - void BeforeDrawVerb() { - PersistentMapTracker::Instance().PushDrawConsumers(); - MG_Pipe::MGPipeDrainDeferredDestroys(); - MarkGpuWritesForDraw(); - } - // A verb that reads buffers but starts no shader: clear, blit, readback, present. The // push still has to run - a coherent map is read by the GPU on any of them - but there // is no shader that could write one, so no mark walk. @@ -383,11 +373,12 @@ namespace MobileGL::MG_Remote::Client { nullptr, 0, nullptr, 0, nullptr); } - // P5e (vi), ID-82: declared here because the REDUCED DrawArrays path below predates the - // nineteen-slot block that defines them, and it needs the same two answers. One - // definition each, further down, beside the rest of the draw plan. + // All draw entry points share the owned-input preparation and emission below. [[noreturn]] void RefuseDrawByName(const char* slot, const char* qualifier); - Bool RunAheadWouldSkipTheWait(ClientSession& session); + void EmitDrawRecord(const char* slot, MG_Pipe::MGPDrawInfo& info, + const MG_Pipe::MGPDrawRange* ranges, Uint32 numDraws, + const void* clientIndices, Uint64 clientIndexBytes, + const MG_Pipe::MGPDrawIndirect* indirect); // Does the bound VAO fetch any ENABLED attribute out of the application's own memory. // The test is EmitVertexBuffers' own (`attrib.Enabled && !attrib.Buffer` is exactly what @@ -405,28 +396,7 @@ namespace MobileGL::MG_Remote::Client { } void EmitDrawArrays(GLenum mode, GLint first, GLsizei count) { - ClientSession& session = RequireSession("DrawArrays"); - - // P5e (vi): the same refusal the nineteen-slot block raises in EmitDrawRecord, and - // this is the entry point the contract names by name - "DrawArrays+CLIENT_ARRAYS". - // The probe runs only on this path's own draw, before the hooks, and it is inert - // under lockstep (see RunAheadWouldSkipTheWait). const Bool clientArrays = BoundVaoHasClientVertexArrays(MG_State::pGLContext.get()); - if (clientArrays && RunAheadWouldSkipTheWait(session)) { - RefuseDrawByName("DrawArrays", "CLIENT_ARRAYS"); - } - - BeforeDrawVerb(); - - if (g_dropDrawEmission) { - // E2's LOAD-BEARING negative control. Same shape as the clear drop and the same - // rule: everything above still ran - the persistent-map push and the GPU-write - // mark walk both happened - so the ONLY difference from the live arm is that - // this frame's geometry never crossed the ring. A lane that still matches its - // golden with this armed did not get its picture from the wire. - ++g_droppedDrawEmissions; - return; - } MG_Pipe::MGPDrawInfo info{}; info.Mode = static_cast(mode); @@ -450,8 +420,7 @@ namespace MobileGL::MG_Remote::Client { // payload and Fatals on a disagreement, on THIS side - so a NumDraws that drifted // from the tail is a producer-side abort rather than a corrupt stream a peer has to // diagnose. - session.EmitAndWait(MG_Pipe::MGPWireOp::DrawVbo, &info, sizeof(info), &range, - sizeof(range), nullptr, 0, nullptr); + EmitDrawRecord("DrawArrays", info, &range, 1, nullptr, 0, nullptr); } // ============================================================================= @@ -467,11 +436,8 @@ namespace MobileGL::MG_Remote::Client { // ID-18), honours the E2 draw-drop control for every draw record and not only the P5 // one, stages a client index array into SEG_STAGE when there is one, and emits. // - // WHAT IS REFUSED BY NAME, ID-57's shape (Fatal{UnmigratedVerb, "+"}), - // never rendered wrong and never fallen through to the driver (R-4): - // +CLIENT_INDICES a glMultiDrawElements* with no element buffer bound: `indices[i]` - // are drawcount separate client pointers and one span names one run; - // P8's HostResolve.cpp flattens it. No measured workload has one. + // Client arrays and client indices become owned ordinary buffer resources. + // Remaining invalid/unrepresentable draw shapes are named before encoding: // +CLIENT_COMMANDS an indirect draw with no GL_DRAW_INDIRECT_BUFFER bound: `indirect` // would be a host pointer, which rule B forbids on the wire. // +UNBOUND_PARAMETER an *IndirectCount with no GL_PARAMETER_BUFFER (the frontend has @@ -481,27 +447,6 @@ namespace MobileGL::MG_Remote::Client { // indices (or past 2^32 of them): the record spells Start in // indices, and rounding would draw from the wrong element. - // P5e (vi), ID-82 / CONTRACT-P5E §5.1: "would this session publish a draw and move on". - // - // IT IS THE CONJUNCTION MGPipeTypes.h STATES FOR RunAheadArmed(), spelled here rather - // than called, and that is a SEAM, not a duplication: ClientSession::RunAheadArmed() is - // package ra's to land (it latches the same three at the first caps adoption after Start - // and may afterwards only turn OFF). Until it exists this is the same answer computed - // from the same three inputs, and it is inert for the whole phase because the third one - // - the server's kCapRunAheadApply - is not published until the integration commit - // (kMGPipeP5eRunAheadReady). WHEN ra LANDS, THIS BODY BECOMES `session.RunAheadArmed()`: - // one latch, one answer, and no way for the refusal to disagree with the wait rule about - // which arm the session is on. - Bool RunAheadWouldSkipTheWait(ClientSession& session) { -#if MOBILEGL_BUILD_DISAGGREGATED - return session.BarrierArmed() && MG_Config::Ipc.RunAhead != 0 && - session.Caps().HasCap(MG_Pipe::kCapRunAheadApply); -#else - (void)session; - return false; -#endif - } - [[noreturn]] void RefuseDrawByName(const char* slot, const char* qualifier) { char name[96]; std::snprintf(name, sizeof(name), "%s+%s", slot, qualifier); @@ -555,25 +500,22 @@ namespace MobileGL::MG_Remote::Client { const void* clientIndices, Uint64 clientIndexBytes, const MG_Pipe::MGPDrawIndirect* indirect) { ClientSession& session = RequireSession(slot); - - // P5e (vi), ID-82 / ruling 2, BEFORE the pre-verb hooks and before the record: a - // draw that fetches vertices out of the application's own memory has no wire form, - // and under run-ahead the server would dereference a client pointer into memory - // this thread is already past. It is refused BY NAME on the GL thread, in the - // shape ID-57 gives every unmigrated draw shape, rather than rendered wrong (R-4). - // - // Under LOCKSTEP it is not refused and nothing here changes: the client is parked - // in WaitForApplied for exactly this record, so the server's upload reads memory - // that is not moving - which is ruling 4's "barriered records keep P5C's - // semantics", and is why this is a run-ahead gate and not a transport gate. - // Staging the bytes as a per-attribute {BindingIndex, MGHostSpan} tail, shaped like - // kDrawHasUserIndices, is P8's and retires the refusal with it. - if ((info.Flags & static_cast(MG_Pipe::kDrawClientArrays)) != 0 && - RunAheadWouldSkipTheWait(session)) { - RefuseDrawByName(slot, "CLIENT_ARRAYS"); + PersistentMapTracker::Instance().PushDrawConsumers(); + MG_Pipe::MGPipeDrainDeferredDestroys(); + // Snapshot before marking THIS draw's potential GPU writes: resolving + // a GPU-produced EBO here must not clear its pending mark for this draw. + UniquePtr ownedInputs; + if ((info.Flags & MG_Pipe::kDrawClientArrays) != 0 || clientIndexBytes != 0) { + if (MG_State::pGLContext == nullptr) RefuseDrawByName(slot, "NO_CONTEXT"); + ownedInputs = MakeUnique(*MG_State::pGLContext); + if (!ownedInputs->Prepare(info, ranges, numDraws, clientIndices, clientIndexBytes, indirect)) { + MG_State::pGLContext->RecordError(ErrorCode::InvalidOperation, + MakeUnique("MG_Remote/Client", slot, + "the client vertex/index fetch range cannot be represented by its storage")); + return; + } } - - BeforeDrawVerb(); + MarkGpuWritesForDraw(); if (g_dropDrawEmission) { // E2's negative control covers EVERY draw record, not only DrawArrays: the @@ -587,22 +529,7 @@ namespace MobileGL::MG_Remote::Client { Wire::WireTail tails[2] = {{ranges, static_cast(numDraws) * sizeof(MG_Pipe::MGPDrawRange)}, {nullptr, 0}}; Uint32 tailCount = 1; - MG_Pipe::MGHostSpan span{}; - if (clientIndices != nullptr && clientIndexBytes != 0) { - // The P8 resolve-on-client rule, applied: the bytes exist on the client only, - // so the client stages them whole and the record names the run - Ptr = nullptr, - // Seg = SEG_STAGE (rule B). The encoder runs all four honesty arms on the span - // before it is published, and the sink resolves it through MGPipeHostBytes. - const MG_Pipe::MGPBlobRef staged = - session.Encoder().StageBytes(clientIndices, clientIndexBytes); - span.Ptr = nullptr; - span.Seg = staged.Seg; - span.Offset = staged.Offset; - span.Size = staged.Size; - info.Flags |= MG_Pipe::kDrawHasUserIndices; - tails[1] = {&span, sizeof(span)}; - tailCount = 2; - } else if (indirect != nullptr) { + if (indirect != nullptr) { info.Flags |= MG_Pipe::kDrawIsIndirect; tails[1] = {indirect, sizeof(*indirect)}; tailCount = 2; @@ -735,17 +662,31 @@ namespace MobileGL::MG_Remote::Client { const RemoteDrawBindings bindings = ReadDrawBindings(); const Uint8 indexSize = RemoteIndexSizeFor(type); if (indexSize == 0) RefuseDrawByName(slot, "INDEX_TYPE"); - if (!bindings.ElementBufferBound) RefuseDrawByName(slot, "CLIENT_INDICES"); const auto n = static_cast(drawcount); MG_Pipe::MGPDrawInfo info = PlanDrawInfo(mode, indexSize, 1, 0, n, bindings); Vector& ranges = MultiDrawScratch(n); + Vector ownedIndices; for (Uint32 i = 0; i < n; ++i) { if (!PlanDrawRange(bindings, indexSize, indices[i], count[i], basevertex != nullptr ? basevertex[i] : 0, ranges[i])) { RefuseDrawByName(slot, "INDEX_OFFSET"); } + if (!bindings.ElementBufferBound) { + const Uint64 byteCount = static_cast(ranges[i].Count) * indexSize; + if (byteCount > std::numeric_limits::max() - ownedIndices.size() || + (byteCount != 0 && indices[i] == nullptr)) { + MG_State::pGLContext->RecordError(ErrorCode::InvalidOperation, + MakeUnique("MG_Remote/Client", slot, "invalid client index span")); + return; + } + ranges[i].Start = static_cast(ownedIndices.size() / indexSize); + const SizeT at = ownedIndices.size(); + ownedIndices.resize(at + static_cast(byteCount)); + if (byteCount != 0) std::memcpy(ownedIndices.data() + at, indices[i], byteCount); + } } - EmitDrawRecord(slot, info, ranges.data(), n, nullptr, 0, nullptr); + EmitDrawRecord(slot, info, ranges.data(), n, ownedIndices.empty() ? nullptr : ownedIndices.data(), + ownedIndices.size(), nullptr); } void EmitMultiDrawElements(GLenum mode, const GLsizei* count, GLenum type, const GLvoid* const* indices, GLsizei drawcount) {