From cf7366276ecc935d21eaef2d216d653537770c7e Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 20 Sep 2026 20:51:09 -0400 Subject: [PATCH] [MG_Backend, MG_Impl] (DirectGLES, Magma): the monolith arm's client fetch bounds DirectGLES: the client snapshot's baseInstance shift belongs to the backend's emulation, not to the element set. A driver that applies baseInstance itself (UseNativeBaseInstance) must not also receive a shifted pointer - doing both moved instance 0 from element 1 to element 2 and left the instance that reads element 1 outside the snapshot. EmulatedFetchBaseInstance is not declared this early in the file, so the wrapper states the same answer inline. Magma: ResolveIndirectCommandBytes reconciles a GPU-written command block before the callers read it. Every one of them reads the words on the CPU - the per-command expansion, the baseInstance the shader emulation is fed, and the range a client-memory vertex array's upload is bounded by - so a command a compute shader wrote this frame was expanded as the poison the shadow still held, which is no draw at all for glDrawArraysIndirect. Magma: a client-memory INSTANCE-RATE binding is bounded by its instances (baseInstance + (instanceCount - 1) / divisor), not by the vertex range; the upload now covers the union of the two, which is what instances shifted off the front of the array needed. --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 6 ++++- .../MG_Backend/DirectVulkan/DirectVulkan.cpp | 7 ++++++ .../DirectVulkan/Renderer/VulkanRenderer.cpp | 24 ++++++++++++++++++- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 94130347..80429193 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -2508,7 +2508,11 @@ namespace MobileGL::MG_Backend::DirectGLES { // baseInstance. The POINTER shift is not - it is this backend's emulation of what the // driver does natively when it can (EmulatedFetchBaseInstance is the same answer the // attribute walk uses), and applying both would shift the fetch twice. - return twin->SyncClientSideAttributesForDraw(currentVAO, plan, EmulatedFetchBaseInstance(baseInstance)); + // EmulatedFetchBaseInstance, which is declared with the draw entry points far below: + // a native-baseInstance driver applies the shift itself, so the pointer carries none. + const Uint32 fetchBaseInstance = + g_GLESCapabilities.SupportsBaseInstance ? 0u : static_cast(baseInstance); + return twin->SyncClientSideAttributesForDraw(currentVAO, plan, fetchBaseInstance); } // The indirect executors' form: the command's own words ARE the draw's fetch, and a diff --git a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp index 6f131962..54b2805f 100644 --- a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp @@ -356,6 +356,13 @@ namespace MobileGL::MG_Backend::DirectVulkan { auto drawBuffer = MGB_CTX->GetBufferBindingSlot(BufferTarget::DrawIndirect).GetBoundObject(); if (drawBuffer) { drawBuffer->SyncPersistentMappedRange(); + // A command block a compute shader wrote is the case this whole shape exists + // for, and every caller here reads the words on the CPU - the per-command + // expansion, the baseInstance the shader emulation is fed, and the fetch + // range a client-memory vertex array's upload is bounded by. Reconciling the + // shadow first is what keeps those readings the real ones (a no-op unless a + // GPU write is pending). + drawBuffer->SyncGpuWrites(); const SizeT commandOffset = reinterpret_cast(indirect); if (drawBuffer->MappedData() == nullptr || commandOffset + requiredBytes > drawBuffer->GetSize()) { MGLOG_E_ONCE("%s skipped: invalid GL_DRAW_INDIRECT_BUFFER binding or range", label); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 8a8f0ae8..b96cd78e 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -4028,7 +4028,29 @@ void main() { // real fetch range. For indexed draws that means scanning the index bytes: // the guessed vertexCount (indexCount + baseVertex) can both truncate draws // whose max index exceeds their index count and over-read below it. - const SizeT clientElementBound = resolveDrawElementBound(); + // AN INSTANCE-RATE BINDING IS BOUNDED BY ITS INSTANCES, not by the vertex range: + // it fetches one element per instance, shifted by the draw's baseInstance, so + // its last element is baseInstance + (instanceCount - 1) / divisor. The two + // bounds are independent and either can exceed the other (an indexed draw + // naming element 2^16, or baseInstance 1000 over a three-element array), so + // the upload covers their union. + SizeT clientElementBound = resolveDrawElementBound(); + if (vertexInputState.bindings[binding].inputRate == VK_VERTEX_INPUT_RATE_INSTANCE && + drawParams.instanceCount > 0) { + Uint32 divisor = 1; + for (const auto& divisorEntry : vertexInputState.bindingDivisors) { + if (divisorEntry.binding == vertexInputState.bindings[binding].binding) { + divisor = divisorEntry.divisor; + break; + } + } + if (divisor > 0) { + const Uint64 first = drawParams.firstInstance; + const Uint64 last = first + (drawParams.instanceCount - 1) / divisor; + clientElementBound = + std::max(clientElementBound, static_cast(last + 1)); + } + } BufferSlice slice{}; Bool uploaded = false; if (conversion == VertexInputStateFactory::VertexStreamConversion::None) {