swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-11-07 01:53:24 +09:00
Code Issues Packages Projects Releases Wiki Activity

[MG_Backend] (Disaggregated): P5e mv - the multi-draw path stops reading the frontend VAO

Browse Source 
- MultiDraw.cpp asked the bound element array buffer three questions (is one bound,
  what GL name is bound and how big is its store, give me its bytes on the CPU) and
  asked all three of MGB_CTX->GetBoundVertexArray() on the apply thread. That was the
  last unguarded read of that field in the backend: package vi retired the ordinary
  draw path's copy and this one survived only because these 9 entries used to die
  earlier, on the framebuffer row fb has since retired.
- ResolveBoundIndexBuffer(question, entry) answers all three from
  MGPipeApplier().IndexBuffer.Res on the record arm - EnsureBufferResourceForHandle
  for the driver name, ResourceWidthForHandle for the size, the server's staged
  shadow (hostBytes + RequireStagedCoverage) for the CPU bytes - and keeps the
  monolith body token for token below it. No wire row was added.
- The arm is STATED, not inferred (ID-81 / ID-110): BufferImpl::VertexInputReadsRecords(),
  the family's own selector, which reduces to Transport != Monolith AND the vertex-input
  bit. Nothing here decides an arm from a null pointer or a noted handle.
- A missing record on the handle arm is a named refusal:
  Fatal{RoleViolation, multidraw-index-buffer-arm}, the shape of
  RefuseNullFrontendTextureOffTheHandleArm. The frontend element slot is not a second
  opinion on this side - it answers for whatever the client has bound now.
- VertexInputReadsRecords() moved from DirectGLES.cpp to Managers.h, unchanged, so the
  two translation units on this draw path share ONE selector rather than a copy each.
- Strict census: GetBoundVertexArray@DrawArrays 9 -> 0. The same 9 entries now stop on
  GetProgramForDraw@DrawArrays, which is pa's row; no other row moved, no new row.
This commit is contained in:
swung0x48
2026-09-18 11:38:28 -04:00
parent 7c6f6886ff
commit 98ea16042a
3 changed files with 211 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp
+4 -25
View File
@@ -522,31 +522,10 @@ namespace MobileGL::MG_Backend::DirectGLES {
// TODO: deletion for deleted objects
namespace BufferImpl {
// P5e (vi), CONTRACT-P5E §5.1 + §5.8 (ruling 1 / ID-81): THE ARM SELECTOR for this
// family, and it is a conjunction on purpose.
//
// Transport != Monolith the record arm exists because there is no frontend VAO on
// this side of a real split. Under Transport=monolith the
// push build keeps its frontend arms token for token - that
// is what the verify comparator compares against, and what
// makes MOBILEGL_IPC_RUN_AHEAD=0 a pure wait-rule A/B on
// identical server code rather than an arm swap.
// the family bit `0x0ff` (bit 7 on, bit 8 off) is a supported A/B and must
// keep running the legacy vertex-input walk; the bit is
// already the gate the rest of this family reads.
//
// It is NOT gated on run-ahead. The records carry the whole family either way, so a
// lockstep split session reads them too and the wait rule changes nothing here - which
// is the only reason ra can flip one constant at the end of the phase and change no
// backend code at all.
inline Bool VertexInputReadsRecords() {
#if MOBILEGL_BUILD_DISAGGREGATED
return MG_Config::Transport != MG_Config::TransportMode::Monolith &&
VertexInputSubsystemEnabled();
#else
return false;
#endif
}
// P5e (mv): THE ARM SELECTOR of this family - VertexInputReadsRecords() - moved to
// Managers.h, unchanged. MultiDraw.cpp is a second translation unit on the same draw
// path and has to select the same arm; a copy of the conjunction there would be two
// selectors for one family. Its rationale travelled with it.
#if MOBILEGL_BUILD_DISAGGREGATED
// P5e (vi): "does this draw fetch any attribute out of the application's own memory",
MobileGL/MG_Backend/DirectGLES/Managers.h
+32
View File
@@ -1094,6 +1094,38 @@ namespace MobileGL::MG_Backend::DirectGLES {
Uint CurrentBufferContextGeneration();
#endif
// P5e (vi), CONTRACT-P5E §5.1 + §5.8 (ruling 1 / ID-81): THE ARM SELECTOR for this
// family, and it is a conjunction on purpose.
//
// Transport != Monolith the record arm exists because there is no frontend VAO on
// this side of a real split. Under Transport=monolith the
// push build keeps its frontend arms token for token - that
// is what the verify comparator compares against, and what
// makes MOBILEGL_IPC_RUN_AHEAD=0 a pure wait-rule A/B on
// identical server code rather than an arm swap.
// the family bit `0x0ff` (bit 7 on, bit 8 off) is a supported A/B and must
// keep running the legacy vertex-input walk; the bit is
// already the gate the rest of this family reads.
//
// It is NOT gated on run-ahead. The records carry the whole family either way, so a
// lockstep split session reads them too and the wait rule changes nothing here - which
// is the only reason ra can flip one constant at the end of the phase and change no
// backend code at all.
//
// P5e (mv): it lives in the HEADER rather than in DirectGLES.cpp because the multi-draw
// path is a second translation unit that has to select the SAME arm - and the addendum's
// rule is that a site states the transport test it relies on, which a copy of the
// conjunction in MultiDraw.cpp would satisfy in letter while giving the family two
// selectors that can drift apart. One definition, spelled at every site that reads it.
inline Bool VertexInputReadsRecords() {
#if MOBILEGL_BUILD_DISAGGREGATED
return MG_Config::Transport != MG_Config::TransportMode::Monolith &&
VertexInputSubsystemEnabled();
#else
return false;
#endif
}
// Registered as the frontend's BufferBackendOps at backend init and on
// every MakeCurrent (the ES context can be destroyed and recreated, e.g.
// by the trace replayer's probe context).
MobileGL/MG_Backend/DirectGLES/MultiDraw.cpp
+175 -37
View File
@@ -11,6 +11,7 @@
#include <MG_State/GLState/Core.h>
#include <MG_Pipe/PipeInputsSwitch.h>
#include <MG_Util/Metrics/PipeStats.h>
#include <cstdlib>
#include <cstring>
#include <limits>
@@ -91,11 +92,151 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl {
return resource ? resource->id : 0;
}
const SharedPtr<MG_State::GLState::BufferObject>& BoundIndexBuffer() {
static const SharedPtr<MG_State::GLState::BufferObject> none;
// ---------------------------------------------------------------------------
// The bound index buffer, and WHICH SIDE ANSWERS FOR IT
//
// P5e (mv), CONTRACT-P5E §5.1 + §5.8 (ruling ID-81): this file asks the bound element
// array buffer exactly three questions - "is one bound at all", "what GL name did
// PrepareForDraw leave on GL_ELEMENT_ARRAY_BUFFER (and how big is its store)", and
// "give me its bytes on the CPU" - and until now it asked all three of the FRONTEND
// VAO, on the apply thread, once per indexed multi-draw. That was the last unguarded
// `MGB_CTX->GetBoundVertexArray()` in the backend: package vi retired the ordinary
// draw path's copy (DirectGLES.cpp's SyncCurrentVertexAttributeValues / PrepareForDraw)
// and this one survived only because these batches used to die earlier, on the
// framebuffer row fb has since retired.
//
// THE ARM IS STATED, NEVER INFERRED. BufferImpl::VertexInputReadsRecords() is the
// family's own selector and reduces to `Transport != Monolith && the vertex-input bit`;
// it is the same conjunction RefuseElementArrayBufferFromTheFrontend guards the
// single-draw path with. Nothing below decides an arm by noticing that a handle or a
// pointer happens to be null - conflating "a handle was noted" with "the record arm is
// selected" is what ID-107 cost this phase 137 scenarios.
// ---------------------------------------------------------------------------
// How much of the index buffer a caller needs. DELIBERATELY not nested levels: each
// value is exactly the work its original call site did, in its original order, so the
// monolith arm makes the same calls in the same sequence it made before this package.
enum class IndexBufferQuestion {
Presence, // is an element array buffer bound at all
DriverName, // + the GL name currently bound to GL_ELEMENT_ARRAY_BUFFER
DriverNameAndSize,// + the store's size in bytes (the compute tier's source)
HostBytes, // a CPU-readable copy of the whole store, and its size
};
struct BoundIndexBufferView {
Bool Present = false;
Uint Id = 0; // DriverName*: 0 when there is no backend store yet
SizeT Size = 0; // DriverNameAndSize / HostBytes
const Uint8* HostBytes = nullptr; // HostBytes: null when there is no CPU copy
};
#if MOBILEGL_BUILD_DISAGGREGATED
// A missing record on the handle arm is a NAMED refusal and never a quiet fall-back to
// the frontend (TASK-mv requirement 2; the shape is Managers.cpp's
// RefuseNullFrontendTextureOffTheHandleArm). The frontend element slot is not a second
// opinion on this side: it answers for whatever the CLIENT has bound NOW, which is a
// later draw than the one being applied. Drawing a batch from it would be a silently
// wrong picture, which is precisely what MultiDrawScenario's batch-matches-unrolled
// assertions exist to catch and what a fall-back would hide from them.
[[noreturn]] void RefuseMissingIndexBufferRecord(const char* entry, MG_Pipe::MGPipeHandle res,
const char* missing) {
MGLOG_F("MGPipe: Fatal{RoleViolation, \"multidraw-index-buffer-arm\"} - %s found no %s for "
"the index buffer {%u, %u} this batch's record named. The index buffer of this "
"family is MGPipeApplier().IndexBuffer.Res (CONTRACT-P5E §5.1) and the arm is "
"selected by Transport != Monolith AND the vertex-input subsystem bit (§5.8, "
"ID-81); falling back to the frontend VAO's element slot here would draw this "
"multi-draw from whatever the CLIENT has bound now",
entry, missing, res.Slot, res.Gen);
std::abort();
}
// Does the applier's record say the application SUPPLIED this store's content? The M-3
// rule ScopedRestartIndexSubstitution spells: for a supplied store a coverage gap in the
// server shadow is a missing record and Fatal by name; for one the application ORPHANED
// the gap is its own undefined content, and reading the shadow's zero-fill is exactly
// what the monolith arm's MappedData() hands back.
Bool IndexBufferRecordHasDefinedContent(const MG_Pipe::MGPipeApplierState& st,
MG_Pipe::MGPipeHandle res) {
if (res.Slot >= st.Resources.size()) return false;
const auto& candidate = st.Resources[res.Slot];
if (!candidate.Live || candidate.Gen != res.Gen) return false;
return candidate.Desc.HasDefinedContent != 0;
}
#endif
BoundIndexBufferView ResolveBoundIndexBuffer(IndexBufferQuestion question, const char* entry) {
BoundIndexBufferView view;
#if MOBILEGL_BUILD_DISAGGREGATED
if (BufferImpl::VertexInputReadsRecords()) {
const auto& st = MG_Pipe::MGPipeApplier();
const MG_Pipe::MGPipeHandle res = BufferImpl::ResolveDrawIndexBufferFromRecord(st).Res;
// A null Res is "no element array buffer bound", exactly as a null frontend slot
// is on the arm below - the batch's indices are a client array, and every caller
// already has an arm for that. It is NOT the arm test; the arm was decided above.
if (MG_Pipe::MGPipeHandleIsNull(res)) return view;
view.Present = true;
if (question == IndexBufferQuestion::Presence) return view;
if (question == IndexBufferQuestion::HostBytes) {
// No Sync* pair here and none is missing: on this side the applier has
// already consumed the persistent-map blocks and the shader writebacks for
// this resource before the draw verb replayed, so the server's staged shadow
// IS the synced copy. That is also why this arm cannot be expressed as "find
// the object and run the monolith body".
auto* resource = BufferImpl::FindBufferResourceForHandle(res);
if (resource == nullptr) RefuseMissingIndexBufferRecord(entry, res, "backend resource");
view.Size = BufferImpl::ResourceWidthForHandle(res);
view.HostBytes = resource->hostBytes;
if (view.HostBytes != nullptr && IndexBufferRecordHasDefinedContent(st, res)) {
BufferImpl::RequireStagedCoverage(*resource, view.HostBytes, 0, view.Size,
"multidraw_index_rebase");
}
// A null HostBytes is not a refusal: MappedData() on the monolith arm is
// equally allowed to be null (an adopted coherent map keeps its bytes
// elsewhere), and the one caller that asks declines the tier for it.
return view;
}
auto* resource = BufferImpl::EnsureBufferResourceForHandle(nullptr, res);
if (resource == nullptr) RefuseMissingIndexBufferRecord(entry, res, "backend resource");
view.Id = resource->id;
if (question == IndexBufferQuestion::DriverNameAndSize) {
view.Size = BufferImpl::ResourceWidthForHandle(res);
}
return view;
}
#endif
// MONOLITH GLUE from here down, token for token what each call site did before.
const auto& vao = MGB_CTX->GetBoundVertexArray();
if (!vao) return none;
return vao->GetIndexBufferBindingSlot().GetBoundObject();
if (!vao) return view;
const auto& ibo = vao->GetIndexBufferBindingSlot().GetBoundObject();
if (!ibo) return view;
view.Present = true;
switch (question) {
case IndexBufferQuestion::Presence:
break;
case IndexBufferQuestion::DriverName: {
const auto* resource = BufferImpl::EnsureBufferResource(ibo);
view.Id = resource ? resource->id : 0;
break;
}
case IndexBufferQuestion::DriverNameAndSize: {
const auto* resource = BufferImpl::EnsureBufferResource(ibo);
view.Id = resource ? resource->id : 0;
view.Size = ibo->GetSize();
break;
}
case IndexBufferQuestion::HostBytes:
// The shadow is the source of truth for CPU reads, but a persistent map or
// a shader write may have moved past it since the last sync.
ibo->SyncPersistentMappedRange();
ibo->SyncGpuWrites();
view.HostBytes = ibo->MappedData();
view.Size = ibo->GetSize();
break;
}
(void)entry;
return view;
}
// The GL name PrepareForDraw left on GL_ELEMENT_ARRAY_BUFFER, i.e. what a tier
@@ -103,10 +244,7 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl {
// matters beyond tidiness: the VAO twin memoises that it already synced this
// index binding and will not re-issue it on the next draw.
Uint BoundIndexBufferId() {
const auto& ibo = BoundIndexBuffer();
if (!ibo) return 0;
const auto* resource = BufferImpl::EnsureBufferResource(ibo);
return resource ? resource->id : 0;
return ResolveBoundIndexBuffer(IndexBufferQuestion::DriverName, "BoundIndexBufferId").Id;
}
// ---------------------------------------------------------------------------
@@ -361,13 +499,18 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl {
}
}
// CPU-readable bytes of one sub-draw's indices, from the frontend shadow of the
// bound index buffer or straight from the client array. Null when the sub-draw
// would read outside the buffer.
const Uint8* ResolveSubDrawIndices(const SharedPtr<MG_State::GLState::BufferObject>& indexBuffer,
const Uint8* indexBufferBytes, SizeT indexBufferSize, const void* indices,
SizeT indexCount, SizeT indexSize) {
if (!indexBuffer) {
// CPU-readable bytes of one sub-draw's indices, from the CPU copy of the bound index
// buffer (the frontend shadow on the monolith arm, the server's staged shadow on the
// record arm) or straight from the client array. Null when the sub-draw would read
// outside the buffer.
//
// P5e (mv): `hasIndexBuffer` is a Bool and not the frontend object it used to be,
// because presence is the only thing this function ever asked of it - and on the record
// arm there is no such object on this side to hand it.
const Uint8* ResolveSubDrawIndices(Bool hasIndexBuffer, const Uint8* indexBufferBytes,
SizeT indexBufferSize, const void* indices, SizeT indexCount,
SizeT indexSize) {
if (!hasIndexBuffer) {
return static_cast<const Uint8*>(indices);
}
if (!indexBufferBytes) return nullptr;
@@ -410,8 +553,7 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl {
if (indexSize == 0) return false;
// Indirect commands address indices as an element offset into the bound element
// array buffer, and an indirect draw is not defined without one.
const auto& indexBuffer = BoundIndexBuffer();
if (!indexBuffer) return false;
if (!ResolveBoundIndexBuffer(IndexBufferQuestion::Presence, "RunIndirect").Present) return false;
g_commandStaging.resize(static_cast<SizeT>(drawcount));
for (GLsizei i = 0; i < drawcount; ++i) {
@@ -502,17 +644,10 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl {
if (total == 0) return true;
if (total > kMaxFlattenedIndices) return false;
const auto& indexBuffer = BoundIndexBuffer();
const Uint8* indexBufferBytes = nullptr;
SizeT indexBufferSize = 0;
if (indexBuffer) {
// The shadow is the source of truth for CPU reads, but a persistent map or
// a shader write may have moved past it since the last sync.
indexBuffer->SyncPersistentMappedRange();
indexBuffer->SyncGpuWrites();
indexBufferBytes = indexBuffer->MappedData();
indexBufferSize = indexBuffer->GetSize();
}
const BoundIndexBufferView indexBuffer =
ResolveBoundIndexBuffer(IndexBufferQuestion::HostBytes, "RunRebasedDrawElements");
const Uint8* const indexBufferBytes = indexBuffer.HostBytes;
const SizeT indexBufferSize = indexBuffer.Size;
const Bool restartActive = RestartActive();
const Uint32 restartSentinel = RestartSentinelFor(type);
@@ -531,8 +666,8 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl {
for (GLsizei i = 0; i < drawcount; ++i) {
if (count[i] <= 0) continue;
const SizeT subDrawCount = static_cast<SizeT>(count[i]);
const Uint8* source = ResolveSubDrawIndices(indexBuffer, indexBufferBytes, indexBufferSize, indices[i],
subDrawCount, indexSize);
const Uint8* source = ResolveSubDrawIndices(indexBuffer.Present, indexBufferBytes, indexBufferSize,
indices[i], subDrawCount, indexSize);
if (!source) {
MGLOG_E_ONCE("DirectGLES multi-draw (drawelements tier): sub-draw %d reads outside the bound index "
"buffer; skipping the batch",
@@ -714,16 +849,18 @@ void main() {
// The shader reads the source indices as a storage buffer, so there has to be
// a real buffer to read - a client-memory index array has none.
const auto& indexBuffer = BoundIndexBuffer();
if (!indexBuffer) return;
if (!ResolveBoundIndexBuffer(IndexBufferQuestion::Presence, "FlattenWithCompute").Present) return;
// A dispatch inside an open capture span is not legal, and the span would also
// observe one merged draw rather than the batch it asked for.
if (XfbImpl::IsCaptureSpanOpen()) return;
auto* sourceResource = BufferImpl::EnsureBufferResource(indexBuffer);
if (!sourceResource || sourceResource->id == 0) return;
const SizeT sourceSize = indexBuffer->GetSize();
// Asked a second time, and one question later: the ensure below may do GL work, so
// it stays BEHIND the capture-span check exactly as it was before P5e (mv).
const BoundIndexBufferView source =
ResolveBoundIndexBuffer(IndexBufferQuestion::DriverNameAndSize, "FlattenWithCompute");
if (source.Id == 0) return;
const SizeT sourceSize = source.Size;
// std430 addresses the source as uint[]; a tail shorter than a word is not
// reachable, so a narrow index type needs a word-multiple buffer.
if (indexSize < 4 && (sourceSize % 4) != 0) return;
@@ -762,7 +899,7 @@ void main() {
return;
}
BufferImpl::BindBufferBaseCached(GL_SHADER_STORAGE_BUFFER, 0, sourceResource->id);
BufferImpl::BindBufferBaseCached(GL_SHADER_STORAGE_BUFFER, 0, source.Id);
BufferImpl::BindBufferBaseCached(GL_SHADER_STORAGE_BUFFER, 1, g_drawInfo.id);
BufferImpl::BindBufferBaseCached(GL_SHADER_STORAGE_BUFFER, 2, g_flattenedIndices.id);
@@ -906,7 +1043,8 @@ void main() {
const Bool arbitraryRestart = restartKind == RestartSubstitutionKind::RewriteIndices;
const ScopedSuppressedPrimitiveRestart restartCapOverride(restartKind);
const Bool hasIndexBuffer = BoundIndexBuffer() != nullptr;
const Bool hasIndexBuffer =
ResolveBoundIndexBuffer(IndexBufferQuestion::Presence, "DrawElementsBatch").Present;
// The compute tier dispatches BEFORE the draw state is established: doing it
// afterwards would mean unpicking the program, SSBO and index bindings