Compare commits

..
6 Commits
Author SHA1 Message Date
swung0x48 9eae98581f [Test] (SelfTest, POST): probe upload/draw ordering in persistently mapped vertex arenas - native GLES controls distinguish mapped destination corruption from staging and synchronization failures
Add the persistent buffer ordering probe to the GLES POST's known-driver-bug inventory. Queue updates and draws into independent FBOs before reading them back, covering SubData and copies from both coherent persistent and ordinary staging buffers. Retry fresh mapped allocations for intermittent corruption.

Require passing never-mapped and fully serialized controls before reporting a finding. Include finish-before, map-then-unmap and barrier diagnostics, preserve caller GL state, and treat setup, allocation and GL errors as inconclusive. This adds detection and reporting only; no rendering workaround is enabled.

Validation: all 46 DriverBugProbesTest tests pass, including 11 new ordering, control, collector and cleanup cases. The Android API 26 / NDK 27 native probe detects all three upload paths on Mali-G1-Ultra r54p1 with clean controls and no GL errors. llvmpipe reports no finding after 240 mapped FBO checks per upload path.
2026-09-07 00:44:08 -04:00
swung0x48 d7655247f7 [Fix, Test] (DirectGLES, Integration): rebind VAOs when an adopted buffer is respecified - the immediate retire path forgot the buffer-id generation, so cached vertex and element bindings kept the deleted store
Advance the buffer-id generation when Ops_Respecify retires immutable storage on the context thread, matching the existing adoption and deferred-retirement paths.

Add pixel regression coverage for unchanged VBO/IBO attachments across same-size redefinition, growth and shrinkage, including bound and unbound VAOs sharing a vertex arena and an index arena returning to shadow storage.
2026-09-06 22:55:06 -04:00
swung0x48 50fb13430f [Merge] (MG_State, ShaderTranspiler, tools/cts): land the write-map landing and open-ended fp64 storage block fixes that take KHR-Single-GL45.subgroups to 100% on Magma 2026-09-05 05:43:34 -04:00
swung0x48 d4f8adcf6d [Tools, Test] (tools/cts, MG_Test): make the CTS runner's chunk timeout idle-based so a healthy 20-minute invocation is no longer killed and its in-flight case mis-recorded as a crash, and link MSVC test executables with /WHOLEARCHIVE so the dllimport gl* references in GetProcAddress.cpp resolve 2026-09-05 05:36:50 -04:00
swung0x48 795e08f7e6 [Fix, Test] (ShaderTranspiler): flatten fp64 storage blocks whose last member is a runtime array - the pass declined them, so the fp64 demotion re-derived ArrayStride 4 over the application's 8/16/32-byte double buffer and every double/dvecN data[] SSBO read raw words 2026-09-05 05:36:49 -04:00
swung0x48 1e7ecab4db [Fix, Test] (MG_State, BufferObject): land a non-persistent write map's staged bytes into a GPU-resident store at unmap and explicit flush instead of dropping them - SSBO binding and large-store adoption make resident stores reachable through glMapBufferRange, so every per-draw re-initialisation was silently lost 2026-09-05 05:36:48 -04:00
19 changed files with 3052 additions and 2841 deletions
+1
View File
@@ -313,6 +313,7 @@ set(SOURCE_FILES
MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.cpp
MobileGL/MG_Util/SelfTest/DriverBugProbes.cpp
MobileGL/MG_Util/SelfTest/PersistentBufferOrderingProbe.cpp
MobileGL/MG_Util/SelfTest/DriverPost.cpp
MobileGL/MG_Util/SelfTest/DriverPostIterationRPWitness.cpp
MobileGL/MG_Util/SelfTest/PrimitivesGeneratedNoXfbProbe.cpp
+4 -1
View File
@@ -1081,6 +1081,9 @@ namespace MobileGL::MG_Backend::DirectGLES {
if (resource->id != 0 && CanTouchGLNow() &&
resource->contextGeneration == g_bufferContextGeneration) {
NoteBufferIdDeleted(resource->id);
// Frontend VAO bindings survive respecification; force their
// backend twins to bind the replacement buffer name.
++g_bufferBackendIdGeneration;
g_GLESFuncs.glDeleteBuffers(1, &resource->id);
resource->id = 0;
resource->immutableStorage = false;
@@ -1350,7 +1353,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
}
// See the declaration: re-mints of a live resource's driver id. Written only on
// the context thread (both re-mint sites run there), read only by the VAO sync.
// the context thread (all re-mint sites run there), read only by the VAO sync.
Uint64 g_bufferBackendIdGeneration = 0;
void RegisterBufferBackendOps() {
@@ -102,6 +102,12 @@ void main() { word = 0xC0FFEEu; }
// The NULL-data definition is the adoption point (and Minecraft's
// arena-creation idiom).
glBufferData(GL_ARRAY_BUFFER, kArenaBytes, nullptr, GL_DYNAMIC_DRAW);
ConfigureVertexArray(m_vao);
}
void ConfigureVertexArray(GLuint vao) {
glBindVertexArray(vao);
glBindBuffer(GL_ARRAY_BUFFER, m_arena);
glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, sizeof(Vertex),
reinterpret_cast<void*>(kVertexOffset));
glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex),
@@ -173,12 +179,12 @@ void main() { word = 0xC0FFEEu; }
GLsizeiptr(vertices.size() * sizeof(Vertex)), vertices.data());
}
void DrawQuad() {
void DrawQuad(GLuint vao = 0) {
glViewport(0, 0, Gl().Width(), Gl().Height());
glClearColor(0.f, 0.f, 0.f, 1.f);
glClear(GL_COLOR_BUFFER_BIT);
glUseProgram(m_program);
glBindVertexArray(m_vao);
glBindVertexArray(vao != 0 ? vao : m_vao);
glDrawArrays(GL_TRIANGLES, 0, 6);
}
@@ -222,6 +228,97 @@ void main() { word = 0xC0FFEEu; }
EXPECT_LT(px[0], 50) << "the draw still shows the previous frame's bytes";
}
// Respecifying a frontend buffer preserves its VAO attachments even when the
// backend replaces the adopted store's GL name. Keep every attribute binding
// unchanged so a stale backend VAO cannot be repaired by a frontend rebind.
TEST_F(LargeArenaAdoptionScenario, RespecifiedVertexArenaKeepsVaoBindings) {
if (!Ready() || IsSkipped()) return;
UploadQuad(1.f, 0.f, 0.f);
DrawQuad();
ASSERT_GT(CenterPixel()[0], 200);
ASSERT_EQ(FirstGLError(), 0u);
GLuint otherVao = 0;
glGenVertexArrays(1, &otherVao);
ConfigureVertexArray(otherVao);
DrawQuad(otherVao);
EXPECT_GT(CenterPixel()[0], 200);
EXPECT_EQ(FirstGLError(), 0u);
constexpr std::array<GLsizeiptr, 3> sizes = {
kArenaBytes, kArenaBytes + 4096, kArenaBytes - 4096,
};
constexpr std::array<std::array<float, 3>, 3> colors = {{
{0.f, 1.f, 0.f}, {0.f, 0.f, 1.f}, {1.f, 0.f, 0.f},
}};
for (std::size_t i = 0; i < sizes.size(); ++i) {
SCOPED_TRACE(sizes[i]);
glBindBuffer(GL_ARRAY_BUFFER, m_arena);
glBufferData(GL_ARRAY_BUFFER, sizes[i], nullptr, GL_DYNAMIC_DRAW);
UploadQuad(colors[i][0], colors[i][1], colors[i][2]);
// The unbound VAO can retain the deleted store; the current VAO's
// attachments can be cleared by deletion. Both must be repaired.
for (GLuint vao : {m_vao, otherVao}) {
SCOPED_TRACE(vao);
DrawQuad(vao);
const auto px = CenterPixel();
EXPECT_EQ(FirstGLError(), 0u);
for (std::size_t channel = 0; channel < 3; ++channel) {
if (colors[i][channel] != 0.f) {
EXPECT_GT(px[channel], 200) << "VAO did not fetch the replacement vertex store";
} else {
EXPECT_LT(px[channel], 50) << "VAO still fetched the previous vertex store";
}
}
}
}
glDeleteVertexArrays(1, &otherVao);
}
TEST_F(LargeArenaAdoptionScenario, RespecifiedIndexArenaKeepsVaoBinding) {
if (!Ready() || IsSkipped()) return;
auto vertices = QuadVertices(1.f, 0.f, 0.f);
const auto green = QuadVertices(0.f, 1.f, 0.f);
vertices.insert(vertices.end(), green.begin(), green.end());
glBindBuffer(GL_ARRAY_BUFFER, m_arena);
glBufferSubData(GL_ARRAY_BUFFER, kVertexOffset,
GLsizeiptr(vertices.size() * sizeof(Vertex)), vertices.data());
GLuint indices = 0;
glGenBuffers(1, &indices);
glBindVertexArray(m_vao);
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, indices);
// Redefine through COPY_WRITE_BUFFER so the element binding slot never
// changes. The small final store also exercises returning to shadow storage.
glBindBuffer(GL_COPY_WRITE_BUFFER, indices);
constexpr std::array<GLsizeiptr, 4> sizes = {
kArenaBytes, kArenaBytes, kArenaBytes + 4096, 4096,
};
for (std::size_t i = 0; i < sizes.size(); ++i) {
SCOPED_TRACE(sizes[i]);
const GLuint first = (i % 2) == 0 ? 0u : 6u;
const std::array<GLuint, 6> elements = {
first, first + 1, first + 2, first + 3, first + 4, first + 5,
};
glBufferData(GL_COPY_WRITE_BUFFER, sizes[i], nullptr, GL_DYNAMIC_DRAW);
glBufferSubData(GL_COPY_WRITE_BUFFER, 0, sizeof(elements), elements.data());
glViewport(0, 0, Gl().Width(), Gl().Height());
glClearColor(0.f, 0.f, 0.f, 1.f);
glClear(GL_COLOR_BUFFER_BIT);
glUseProgram(m_program);
glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_INT, nullptr);
const auto px = CenterPixel();
EXPECT_EQ(FirstGLError(), 0u);
EXPECT_GT(px[first == 0 ? 0 : 1], 200) << "VAO did not fetch the replacement index store";
EXPECT_LT(px[first == 0 ? 1 : 0], 50) << "VAO still fetched the previous index store";
}
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
glBindBuffer(GL_COPY_WRITE_BUFFER, 0);
glDeleteBuffers(1, &indices);
}
// The shadow IS the mapping: a readback straight after a CPU write must hand
// back exactly those bytes.
TEST_F(LargeArenaAdoptionScenario, ReadbackSeesTheLatestCpuWrite) {
@@ -67,6 +67,14 @@ namespace MobileGL::MG_State::GLState {
}
void BufferObject::NotifyContentWrite(SizeT offset, SizeT size) {
if (size == 0) {
// An empty write moves the serial and nothing else, exactly as NotifySubData
// and NotifyFlushMappedRange do: it wrote no byte, so it must not promote an
// undefined store to "has content" - that would cost the next orphaning
// respecification a full-size upload of bytes the application never wrote.
++m_changeSerial;
return;
}
m_hasDefinedContent = true;
if (m_resource.IsGpuResident()) {
// The write already landed in coherent GPU memory; the backend has no separate
@@ -111,7 +119,10 @@ namespace MobileGL::MG_State::GLState {
}
void BufferObject::Respecify(SizeT size, const void* data) {
ReleaseMemory();
// The store a live mapping wrote into is about to be replaced, so landing those
// bytes into it would copy a whole mapped range (an adopted arena's map is the
// arena) into storage the next line hands back.
ReleaseMemory(false);
RedefineStorage(size);
if (data && size > 0) {
Memcpy(m_resource.Bytes(), data, size);
@@ -136,7 +147,9 @@ namespace MobileGL::MG_State::GLState {
}
void BufferObject::AllocateImmutableStorage(SizeT size, const void* data, GLbitfield storageFlags) {
ReleaseMemory();
// Same as Respecify: the bytes a live mapping staged have nowhere to land, the
// store they belong to is being replaced.
ReleaseMemory(false);
RedefineStorage(size);
if (data) {
Memcpy(m_resource.Bytes(), data, size);
@@ -190,24 +203,45 @@ namespace MobileGL::MG_State::GLState {
m_usage = usage;
}
void BufferObject::ReleaseMemory() {
void BufferObject::ReleaseMemory(Bool landStagedWrites) {
if (!m_isMapped) return;
if (m_mappingAccess & BufferMappingAccessBit::Write) { // if we wrote to the buffer
// A persistent GPU-resident map wrote straight into coherent GPU memory, so
// there is nothing to copy back and no range to push down on unmap.
if (!m_resource.IsGpuResident() &&
!(m_mappingAccess & BufferMappingAccessBit::FlushExplicit)) { // if we didn't flush explicitly
if (!(m_mappingAccess & BufferMappingAccessBit::Persistent)) {
Memcpy(m_resource.Bytes() + m_mappedRange.start, m_stagingData.data() + m_stagingBias,
m_mappedRange.end - m_mappedRange.start);
if (landStagedWrites &&
(m_mappingAccess & BufferMappingAccessBit::Write)) { // if we wrote to the buffer
if (!(m_mappingAccess & BufferMappingAccessBit::FlushExplicit)) { // if we didn't flush explicitly
const SizeT mappedLength = m_mappedRange.end - m_mappedRange.start;
if (m_resource.IsGpuResident()) {
// A persistent map of an adopted store wrote straight into coherent
// GPU memory: nothing to copy back, no range to push down. A
// NON-persistent write map is a different thing: the application
// wrote a staging copy (glMapBuffer and glMapBufferRange hand one out
// regardless of where the store lives), and GL requires those bytes
// to be visible to every later command the moment glUnmapBuffer
// returns. Residency used to come only from a coherent persistent
// map, which never has a staging copy, so the copy-back was simply
// skipped for a resident store; residency now also comes from a
// shader storage binding (EnsureGpuResidentStorage at draw time) and
// from large-store adoption (TryAdoptLargeStorage), both of which an
// application then re-initialises through an ordinary map/write/unmap.
// Skipping the copy-back dropped every one of those writes. Land the
// staged bytes through the same route glBufferSubData takes into an
// adopted store - the backend's flush op is for stores it keeps a
// separate copy of and must not run here.
if (!(m_mappingAccess & BufferMappingAccessBit::Persistent)) {
LandBytesIntoResidentStore(m_mappedRange.start,
{m_stagingData.data() + m_stagingBias, mappedLength});
}
} else {
if (!(m_mappingAccess & BufferMappingAccessBit::Persistent)) {
Memcpy(m_resource.Bytes() + m_mappedRange.start, m_stagingData.data() + m_stagingBias,
mappedLength);
}
NotifyFlushMappedRange(m_mappedRange, m_mappingAccess);
}
NotifyFlushMappedRange(m_mappedRange, m_mappingAccess);
}
m_stagingData.clear();
}
m_stagingData.clear();
m_isMapped = false;
m_mappingAccess = BufferMappingAccessBit::Null;
m_mappedRange = {0, 0};
@@ -227,8 +261,21 @@ namespace MobileGL::MG_State::GLState {
MOBILEGL_ASSERT(end <= m_mappedRange.end, "Flush range out of bounds: mappedRange.end (%zu) < end (%zu)",
m_mappedRange.end, end);
// FLUSH_EXPLICIT maps are never GPU-resident (only coherent maps are adopted), so
// the staged bytes must be copied into the shadow before the backend reads them.
// A FLUSH_EXPLICIT map can sit on an adopted store: the map itself never adopts
// (only a coherent persistent one does), but a shader storage binding or
// large-store adoption may have made the buffer resident before the map. The
// flushed bytes then take the same landing as any other CPU write into an
// adopted store - a persistent map already wrote them in place and only has
// to publish the change, a non-persistent map staged them and has to land
// them. The backend's flush op is for stores it keeps a separate copy of.
if (m_resource.IsGpuResident()) {
if (m_mappingAccess & BufferMappingAccessBit::Persistent) {
NotifyContentWrite(start, length);
} else {
LandBytesIntoResidentStore(start, {m_stagingData.data() + m_stagingBias + offset, length});
}
return;
}
if (!(m_mappingAccess & BufferMappingAccessBit::Persistent)) {
Memcpy(m_resource.Bytes() + start, m_stagingData.data() + m_stagingBias + offset, length);
}
@@ -284,35 +331,48 @@ namespace MobileGL::MG_State::GLState {
data.size, m_size);
// An adopted store's Bytes() IS the memory in-flight frames are reading, and
// GL orders a glBufferSubData after those already-submitted reads. A backend
// that can land the bytes on the GPU timeline takes them here, untouched by
// the mapping - the in-place host write below tore the frames still reading
// the old bytes. The bytes are not current in the mapping until the backend's
// ordered copy executes, so reads reconcile through the same gate GPU-written
// buffers use.
if (m_resource.IsGpuResident() && data.size > 0 && g_bufferBackendOps &&
g_bufferBackendOps->ResidentSubData) {
g_bufferBackendOps->ResidentSubData(*this, atOffset, data);
// GL orders a glBufferSubData after those already-submitted reads: the write
// has to take the resident landing, never a plain host write into the mapping.
// Shadow-backed stores need none of this: the Memcpy below touches only the
// shadow, and the backend's SubData op does its own ordering against in-flight
// work.
if (m_resource.IsGpuResident()) {
LandBytesIntoResidentStore(atOffset, data);
return;
}
Memcpy(m_resource.Bytes() + atOffset, data.data, data.size);
NotifyContentWrite(atOffset, data.size);
}
// A backend that can land the bytes on the GPU timeline takes them here, untouched
// by the mapping - an in-place host write into coherent memory tore the frames
// still reading the old bytes (Minecraft patches LIVE chunk sections this way).
// The bytes are then not current in the mapping until the backend's ordered copy
// executes, so reads reconcile through the same gate GPU-written buffers use.
//
// Without that op the write lands in place, after retiring the GPU writes this store
// is known to be waiting on: a backend that defers work (DirectVulkan's frame command
// buffer) may still be holding a recorded-but-unsubmitted dispatch that GL orders this
// write AFTER, and writing the mapping now would land the bytes underneath that
// dispatch - its increments then execute on top of the newer data and invert the call
// order. That gate only knows about work that WROTE the store (MarkGpuWritten); work
// that merely READS it - a draw sourcing an adopted vertex arena - is not tracked here,
// so a backend without the op still owes the ordering against its own recorded reads.
// NotifyContentWrite on a resident store only bumps the serial: the backend has no
// separate copy to sync, so no transfer op runs.
void BufferObject::LandBytesIntoResidentStore(SizeT offset, DataPtr bytes) {
if (bytes.size > 0 && g_bufferBackendOps && g_bufferBackendOps->ResidentSubData) {
g_bufferBackendOps->ResidentSubData(*this, offset, bytes);
m_hasDefinedContent = true;
++m_changeSerial;
m_gpuWritePending = true;
return;
}
// An adopted store's Bytes() IS the memory the GPU reads, and a backend that
// defers work (DirectVulkan's frame command buffer) may still be holding a
// recorded-but-unsubmitted dispatch that GL orders this write AFTER. Writing
// the mapping now would land the bytes underneath that dispatch - its
// increments then execute on top of the newer data and invert the call order.
// Retire the pending GPU writes first, as FillSubData already does. Shadow-
// backed stores need none of this: the Memcpy below touches only the shadow,
// and the backend's SubData op does its own ordering against in-flight work.
if (m_resource.IsGpuResident()) {
SyncGpuWrites();
}
Memcpy(m_resource.Bytes() + atOffset, data.data, data.size);
NotifyContentWrite(atOffset, data.size);
SyncGpuWrites();
Memcpy(m_resource.Bytes() + offset, bytes.data, bytes.size);
NotifyContentWrite(offset, bytes.size);
}
void BufferObject::FillSubData(DataPtr pattern, SizeT atOffset, SizeT size) {
@@ -327,8 +387,13 @@ namespace MobileGL::MG_State::GLState {
"Cannot fill data while buffer is non-persistently mapped.");
if (size == 0) return;
// An adopted store takes the same GPU-timeline landing as UploadSubData: the
// in-place write below would tear in-flight readers of the mapping.
// An adopted store takes the same landing as UploadSubData: the in-place write
// below would tear in-flight readers of the mapping. The pattern is expanded
// first because the landing takes the final bytes, not a repeat rule - which is
// why only a backend that actually takes them comes through here. Without that
// op the landing would memcpy the expansion into the mapping the loop below
// fills in place anyway, so a whole-arena clear would allocate a whole arena
// for nothing.
if (m_resource.IsGpuResident() && g_bufferBackendOps && g_bufferBackendOps->ResidentSubData) {
Vector<Uint8> expanded(size);
if (pattern.size == 1) {
@@ -338,16 +403,13 @@ namespace MobileGL::MG_State::GLState {
Memcpy(expanded.data() + at, pattern.data, pattern.size);
}
}
g_bufferBackendOps->ResidentSubData(*this, atOffset, {expanded.data(), size});
m_hasDefinedContent = true;
++m_changeSerial;
m_gpuWritePending = true;
LandBytesIntoResidentStore(atOffset, {expanded.data(), size});
return;
}
// A clear is ordered after all earlier GPU writes. Partial clears additionally need the
// retained shadow bytes; whole-store clears need the same synchronization before writing
// an adopted persistent mapping that the GPU may still be accessing.
// A clear is ordered after all earlier GPU writes; partial clears additionally need
// the retained shadow bytes, and a resident store the backend cannot take the bytes
// for is written in place, which needs the same synchronization the landing does.
SyncGpuWrites();
Uint8* dst = m_resource.Bytes() + atOffset;
@@ -381,22 +443,13 @@ namespace MobileGL::MG_State::GLState {
size, m_size);
src->SyncGpuWrites();
// An adopted DESTINATION takes the same GPU-timeline landing as UploadSubData;
// the in-place write below would tear in-flight readers of the mapping.
if (m_resource.IsGpuResident() && size > 0 && g_bufferBackendOps &&
g_bufferBackendOps->ResidentSubData) {
g_bufferBackendOps->ResidentSubData(*this, dstOffset,
{src->m_resource.Bytes() + srcOffset, size});
m_hasDefinedContent = true;
++m_changeSerial;
m_gpuWritePending = true;
return;
}
// The DESTINATION needs the same ordering as UploadSubData: an adopted store is
// written in place, so pending recorded GPU writes to it must retire before the
// copy lands or they would execute on top of it.
// An adopted DESTINATION takes the same landing as UploadSubData: the in-place
// write below would tear in-flight readers of the mapping, and pending recorded
// GPU writes to it must retire before the copy lands or they would execute on
// top of it.
if (m_resource.IsGpuResident()) {
SyncGpuWrites();
LandBytesIntoResidentStore(dstOffset, {src->m_resource.Bytes() + srcOffset, size});
return;
}
Memcpy(m_resource.Bytes() + dstOffset, src->m_resource.Bytes() + srcOffset, size);
NotifyContentWrite(dstOffset, size);
@@ -433,6 +486,16 @@ namespace MobileGL::MG_State::GLState {
if (m_resource.IsGpuResident()) {
return true;
}
// Adoption releases the CPU shadow, and a live mapping may BE that shadow: a
// persistent map that did not itself adopt (a FLUSH_EXPLICIT one, or a read map)
// handed the application shadow + offset, and GL keeps that pointer valid while
// the buffer is drawn with - which is exactly when this runs, on the storage
// binding walk. Freeing it under the application is a use-after-free, so a mapped
// buffer keeps the shadow model until it is unmapped; the binding that follows
// adopts then. Same rule as TryAdoptLargeStorage.
if (m_isMapped) {
return false;
}
if (m_size == 0 || g_bufferBackendOps == nullptr || g_bufferBackendOps->AcquirePersistentMap == nullptr) {
return false;
}
@@ -451,7 +514,20 @@ namespace MobileGL::MG_State::GLState {
// The app is about to look at the bytes; a shader may have rewritten them since
// the shadow was last authoritative. Also needed for a write map without an
// invalidate bit, whose staging copy is seeded from the shadow.
SyncGpuWrites();
//
// One map shape looks at nothing: a non-persistent write map that discards the
// range it maps gets a staging copy the seeding below skips, so no reader of the
// store exists between here and the unmap. Reconciling an ADOPTED store would
// still cost the backend's full drain-and-wait (its queued landings are made
// visible to the CPU by finishing the pipeline), once per map, on exactly the
// streaming arena the adoption exists to keep cheap. The outstanding-write flag
// stays set, so the first read that DOES look at the bytes still pays for it.
const Bool discardsWhatItMaps =
(access & BufferMappingAccessBit::Write) && !(access & BufferMappingAccessBit::Persistent) &&
(access & (BufferMappingAccessBit::InvalidateRange | BufferMappingAccessBit::InvalidateBuffer));
if (!(m_resource.IsGpuResident() && discardsWhatItMaps)) {
SyncGpuWrites();
}
m_isMapped = true;
m_mappingAccess = access;
m_mappedRange = range;
@@ -81,7 +81,9 @@ namespace MobileGL {
// Contents update of [offset, offset + size) from the shadow.
void (*SubData)(BufferObject& bufferObject, SizeT offset, SizeT size) = nullptr;
// Contents update of an ADOPTED (GPU-resident) store. `data` holds the app's
// bytes; the frontend has NOT touched the resident mapping. GL orders a
// bytes, valid for the duration of the call only (a write map's staging
// store is freed the moment the unmap that lands it returns); the frontend
// has NOT touched the resident mapping. GL orders a
// glBufferSubData after already-submitted GPU reads of the store, and an
// in-place host write into the coherent mapping tears the frames still
// reading the old bytes (Minecraft patches LIVE chunk sections this way -
@@ -157,9 +159,14 @@ namespace MobileGL {
// Adopt backend host-visible coherent GPU storage as the source of truth
// (used for GPU-written targets like transform feedback capture, so
// MapBuffer/GetBufferSubData read real GPU results). No-op when already
// resident or when the backend declines.
// resident, while the buffer is mapped (adoption releases the shadow a
// mapping may have handed the application), or when the backend declines.
Bool EnsureGpuResidentStorage();
void ReleaseMemory();
// Unmap. A write map's staged bytes land in the store on the way out, unless
// the caller is about to replace that store (a respecification) and passes
// false - landing them there would copy a whole mapped range into storage
// being handed back on the next line.
void ReleaseMemory(Bool landStagedWrites = true);
void FlushMemoryRange(SizeT offset, SizeT length);
// Pushes the persistently-mapped write range to the backend; called by
@@ -234,6 +241,12 @@ namespace MobileGL {
// so this only bumps the change serial; otherwise it dispatches a backend
// SubData transfer to sync the backend's separate GPU copy.
void NotifyContentWrite(SizeT offset, SizeT size);
// The one route CPU-sourced bytes take into an ADOPTED (GPU-resident) store:
// glBufferSubData, a buffer clear, a buffer copy, and the landing of a
// non-persistent write map at unmap / explicit flush all go through it, so
// the routes cannot drift apart again. Carries no mapping asserts on
// purpose - the unmap landing runs while the buffer is still mapped.
void LandBytesIntoResidentStore(SizeT offset, DataPtr bytes);
static Uint64 AllocateLifetimeId();
+900
View File
@@ -1568,6 +1568,15 @@ namespace {
int respecifyCalls = 0;
int flushCalls = 0;
Bool provideMap = true; // false => backend declines, exercising the shadow fallback
// Only recorded by the variant of the ops table that offers ResidentSubData: the
// bytes a CPU write handed the backend for a GPU-ordered landing into an adopted
// store, held back from `gpu` until a readback "retires" them.
struct ResidentWrite {
SizeT offset = 0;
Vector<Uint8> bytes;
};
Vector<ResidentWrite> residentWrites;
int readbackCalls = 0;
};
ZeroCopyMockBackend* g_zeroCopyMock = nullptr;
@@ -1605,6 +1614,39 @@ namespace {
.AcquirePersistentMap = ZeroCopyMock_AcquirePersistentMap,
};
// The same backend with the GPU-ordered landing ops a staging-ring backend offers: a
// CPU write into an adopted store is queued (the mapping is NOT written through), and
// a readback is what lands the queue before the application reads.
void ZeroCopyMock_ResidentSubData(MG_State::GLState::BufferObject&, SizeT offset, DataPtr data) {
if (!g_zeroCopyMock) return;
auto& write = g_zeroCopyMock->residentWrites.emplace_back();
write.offset = offset;
const auto* bytes = static_cast<const Uint8*>(data.data);
write.bytes.assign(bytes, bytes + data.size);
}
void ZeroCopyMock_ReadbackFromGpu(MG_State::GLState::BufferObject&) {
if (!g_zeroCopyMock) return;
++g_zeroCopyMock->readbackCalls;
for (const auto& write : g_zeroCopyMock->residentWrites) {
// Reported, not asserted: an ASSERT here would return out of the readback and
// leave the remaining landings unapplied, which reads as a different failure.
EXPECT_LE(write.offset + write.bytes.size(), g_zeroCopyMock->gpu.size());
if (write.offset + write.bytes.size() > g_zeroCopyMock->gpu.size()) continue;
Memcpy(g_zeroCopyMock->gpu.data() + write.offset, write.bytes.data(), write.bytes.size());
}
g_zeroCopyMock->residentWrites.clear();
}
const MG_State::GLState::BufferBackendOps kResidentSubDataMockOps = {
.Respecify = ZeroCopyMock_Respecify,
.SubData = ZeroCopyMock_SubData,
.ResidentSubData = ZeroCopyMock_ResidentSubData,
.FlushMappedRange = ZeroCopyMock_Flush,
.OnDestroy = ZeroCopyMock_OnDestroy,
.AcquirePersistentMap = ZeroCopyMock_AcquirePersistentMap,
.ReadbackFromGpu = ZeroCopyMock_ReadbackFromGpu,
};
struct ScopedBackendOps {
explicit ScopedBackendOps(const MG_State::GLState::BufferBackendOps* ops) {
MG_State::GLState::SetBufferBackendOps(ops);
@@ -2074,3 +2116,861 @@ TEST_F(BufferTest, RedefiningANonAdoptedBufferIsUnchanged) {
g_zeroCopyMock = nullptr;
}
// ---------------------------------------------------------------------------
// A NON-persistent write map of an ADOPTED store. glMapBuffer / glMapBufferRange hand
// the application a staging copy regardless of where the store lives, and GL requires
// the bytes it wrote there to be visible to every later command once glUnmapBuffer
// returns. Residency used to come only from a coherent persistent map - which writes
// in place and never has a staging copy - so the unmap simply skipped the copy-back
// for a resident store. Residency now also comes from a shader storage binding
// (EnsureGpuResidentStorage at draw time) and from large-store adoption, both of which
// an application then re-initialises through an ordinary map/write/unmap: the
// conformance suite re-seeds every SSBO that way before each draw, and every re-seed
// after the first draw was dropped on the floor. These pin the landing for each map
// shape, on the backend that writes the coherent mapping in place and on the one that
// takes the bytes for a GPU-ordered landing, plus the shadow path as the control.
namespace {
constexpr SizeT kAdoptedInts = 16;
// A buffer of kAdoptedInts sequential ints, adopted by the mock backend exactly as an
// SSBO binding does at draw time. The per-write counters are zeroed afterwards so a
// test only sees the traffic of the map it makes.
SharedPtr<MG_State::GLState::BufferObject> MakeAdoptedBuffer(ZeroCopyMockBackend& mock, GLenum target,
GLuint& buffer) {
GenBuffers(1, &buffer);
BindBuffer(target, buffer);
Vector<GLint> initial(kAdoptedInts);
for (SizeT i = 0; i < kAdoptedInts; ++i) initial[i] = static_cast<GLint>(i);
BufferData(target, static_cast<GLsizeiptr>(kAdoptedInts * sizeof(GLint)), initial.data(),
GL_DYNAMIC_DRAW);
EXPECT_EQ(GetError(), GL_NO_ERROR);
auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer);
EXPECT_NE(bufferObject, nullptr);
if (bufferObject == nullptr) return nullptr;
EXPECT_TRUE(bufferObject->EnsureGpuResidentStorage());
EXPECT_TRUE(bufferObject->IsBackendPersistentMapped());
EXPECT_EQ(static_cast<const void*>(bufferObject->MappedData()), static_cast<const void*>(mock.gpu.data()));
mock.subDataCalls = 0;
mock.flushCalls = 0;
mock.respecifyCalls = 0;
return bufferObject;
}
const GLint* GpuInts(const ZeroCopyMockBackend& mock) {
return reinterpret_cast<const GLint*>(mock.gpu.data());
}
} // namespace
TEST_F(BufferTest, ANonPersistentReadWriteRangeMapOfAnAdoptedStoreLandsAtUnmap) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kZeroCopyMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_SHADER_STORAGE_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
const Uint64 baseSerial = bufferObject->GetChangeSerial();
// The conformance suite's shape: the whole store, READ|WRITE, then a full rewrite.
auto* mapped = static_cast<GLint*>(bufferObject->AcquireMemoryRange(
{0, kAdoptedInts * sizeof(GLint)}, BufferMappingAccessBit::Read | BufferMappingAccessBit::Write));
ASSERT_NE(mapped, nullptr);
// A non-persistent map is a staging copy, seeded from the adopted store...
EXPECT_NE(static_cast<void*>(mapped), static_cast<void*>(mock.gpu.data()));
for (SizeT i = 0; i < kAdoptedInts; ++i) EXPECT_EQ(mapped[i], static_cast<GLint>(i));
for (SizeT i = 0; i < kAdoptedInts; ++i) mapped[i] = 1000 + static_cast<GLint>(i);
// ...that the store does not see until the unmap.
EXPECT_EQ(GpuInts(mock)[0], 0);
bufferObject->ReleaseMemory();
EXPECT_FALSE(bufferObject->IsMapped());
EXPECT_TRUE(bufferObject->IsBackendPersistentMapped());
for (SizeT i = 0; i < kAdoptedInts; ++i) {
EXPECT_EQ(GpuInts(mock)[i], 1000 + static_cast<GLint>(i)) << "int " << i;
}
EXPECT_EQ(std::memcmp(bufferObject->MappedData(), mock.gpu.data(), mock.gpu.size()), 0);
// The landing publishes the change for cached consumers...
EXPECT_GT(bufferObject->GetChangeSerial(), baseSerial);
// ...but dispatches no transfer op: the backend keeps no separate copy of an
// adopted store, and its flush op would only upload the mapping onto itself.
EXPECT_EQ(mock.flushCalls, 0);
EXPECT_EQ(mock.subDataCalls, 0);
EXPECT_EQ(mock.acquireMapCalls, 1);
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
TEST_F(BufferTest, GlMapBufferWriteOnlyAndReadWriteOfAnAdoptedStoreLandAtUnmap) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kZeroCopyMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_SHADER_STORAGE_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
// glMapBuffer(GL_WRITE_ONLY): the staging copy is still seeded (no invalidate bit),
// so a partial write keeps the untouched ints.
Uint64 serial = bufferObject->GetChangeSerial();
auto* writeOnly = static_cast<GLint*>(bufferObject->AcquireMemory(true, false, true));
ASSERT_NE(writeOnly, nullptr);
EXPECT_NE(static_cast<void*>(writeOnly), static_cast<void*>(mock.gpu.data()));
writeOnly[0] = 100;
writeOnly[1] = 200;
bufferObject->ReleaseMemory();
EXPECT_EQ(GpuInts(mock)[0], 100);
EXPECT_EQ(GpuInts(mock)[1], 200);
EXPECT_EQ(GpuInts(mock)[2], 2);
EXPECT_EQ(GpuInts(mock)[kAdoptedInts - 1], static_cast<GLint>(kAdoptedInts - 1));
EXPECT_GT(bufferObject->GetChangeSerial(), serial);
// glMapBuffer(GL_READ_WRITE): reads see the previous landing, and the next one lands too.
serial = bufferObject->GetChangeSerial();
auto* readWrite = static_cast<GLint*>(bufferObject->AcquireMemory(true, true, true));
ASSERT_NE(readWrite, nullptr);
EXPECT_EQ(readWrite[0], 100);
EXPECT_EQ(readWrite[1], 200);
readWrite[2] = 300;
bufferObject->ReleaseMemory();
EXPECT_EQ(GpuInts(mock)[0], 100);
EXPECT_EQ(GpuInts(mock)[1], 200);
EXPECT_EQ(GpuInts(mock)[2], 300);
EXPECT_GT(bufferObject->GetChangeSerial(), serial);
EXPECT_EQ(mock.flushCalls, 0);
EXPECT_EQ(mock.subDataCalls, 0);
EXPECT_TRUE(bufferObject->IsBackendPersistentMapped());
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
TEST_F(BufferTest, AWriteMapInvalidatingAnAdoptedStoreLandsTheWholeRangeAtUnmap) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kZeroCopyMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_SHADER_STORAGE_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
const Uint64 baseSerial = bufferObject->GetChangeSerial();
auto* mapped = static_cast<GLint*>(bufferObject->AcquireMemoryRange(
{0, kAdoptedInts * sizeof(GLint)}, BufferMappingAccessBit::Write | BufferMappingAccessBit::InvalidateBuffer));
ASSERT_NE(mapped, nullptr);
EXPECT_NE(static_cast<void*>(mapped), static_cast<void*>(mock.gpu.data()));
// The whole range is undefined by contract, so the application rewrites all of it.
for (SizeT i = 0; i < kAdoptedInts; ++i) mapped[i] = -static_cast<GLint>(i) - 1;
bufferObject->ReleaseMemory();
for (SizeT i = 0; i < kAdoptedInts; ++i) {
EXPECT_EQ(GpuInts(mock)[i], -static_cast<GLint>(i) - 1) << "int " << i;
}
EXPECT_GT(bufferObject->GetChangeSerial(), baseSerial);
EXPECT_EQ(mock.flushCalls, 0);
EXPECT_EQ(mock.subDataCalls, 0);
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
// A range map at an offset off the alignment grid: the staging store is biased by the
// offset's phase (see AcquireMemoryRange), and the landing has to read from the biased
// start and write to the mapped offset - not from data(), not to 0.
TEST_F(BufferTest, ARangeMapAtAnUnalignedOffsetOfAnAdoptedStoreLandsInPlace) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kZeroCopyMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_ARRAY_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
const Uint64 baseSerial = bufferObject->GetChangeSerial();
// Ints 3..6, i.e. byte offset 12 - inside the first alignment, so the bias is non-zero.
constexpr SizeT kFirst = 3;
constexpr SizeT kCount = 4;
const Range1D range{kFirst * sizeof(GLint), (kFirst + kCount) * sizeof(GLint)};
ASSERT_NE(range.start % MG_State::GLState::MIN_MAP_BUFFER_ALIGNMENT, 0u);
auto* mapped = static_cast<GLint*>(bufferObject->AcquireMemoryRange(range, BufferMappingAccessBit::Write));
ASSERT_NE(mapped, nullptr);
// Seeded from the right place...
for (SizeT i = 0; i < kCount; ++i) EXPECT_EQ(mapped[i], static_cast<GLint>(kFirst + i));
for (SizeT i = 0; i < kCount; ++i) mapped[i] = 500 + static_cast<GLint>(i);
bufferObject->ReleaseMemory();
// ...and landed in the right place, with everything outside the range untouched.
for (SizeT i = 0; i < kAdoptedInts; ++i) {
const GLint expected = (i >= kFirst && i < kFirst + kCount) ? 500 + static_cast<GLint>(i - kFirst)
: static_cast<GLint>(i);
EXPECT_EQ(GpuInts(mock)[i], expected) << "int " << i;
}
EXPECT_GT(bufferObject->GetChangeSerial(), baseSerial);
EXPECT_EQ(mock.flushCalls, 0);
EXPECT_EQ(mock.subDataCalls, 0);
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
// FLUSH_EXPLICIT on an adopted store: only the flushed bytes land, at the flush, and the
// unmap lands nothing more - the application promised to flush what it wanted kept.
TEST_F(BufferTest, AnExplicitFlushOfAWriteMapOfAnAdoptedStoreLandsOnlyTheFlushedBytes) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kZeroCopyMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_SHADER_STORAGE_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
const Uint64 baseSerial = bufferObject->GetChangeSerial();
// Ints 2..13 mapped (offset 8, off the grid again), all of them rewritten...
constexpr SizeT kFirst = 2;
constexpr SizeT kCount = 12;
const Range1D range{kFirst * sizeof(GLint), (kFirst + kCount) * sizeof(GLint)};
auto* mapped = static_cast<GLint*>(bufferObject->AcquireMemoryRange(
range, BufferMappingAccessBit::Write | BufferMappingAccessBit::FlushExplicit));
ASSERT_NE(mapped, nullptr);
for (SizeT i = 0; i < kCount; ++i) mapped[i] = 700 + static_cast<GLint>(i);
// ...but only ints 5..8 (map-relative ints 3..6) flushed.
constexpr SizeT kFlushFirst = 3;
constexpr SizeT kFlushCount = 4;
bufferObject->FlushMemoryRange(kFlushFirst * sizeof(GLint), kFlushCount * sizeof(GLint));
const Uint64 flushSerial = bufferObject->GetChangeSerial();
EXPECT_GT(flushSerial, baseSerial);
EXPECT_EQ(mock.flushCalls, 0);
auto expectOnlyFlushedBytesLanded = [&](const char* when) {
for (SizeT i = 0; i < kAdoptedInts; ++i) {
const Bool flushed = i >= kFirst + kFlushFirst && i < kFirst + kFlushFirst + kFlushCount;
const GLint expected = flushed ? 700 + static_cast<GLint>(i - kFirst) : static_cast<GLint>(i);
EXPECT_EQ(GpuInts(mock)[i], expected) << when << ": int " << i;
}
};
expectOnlyFlushedBytesLanded("after the flush");
bufferObject->ReleaseMemory();
expectOnlyFlushedBytesLanded("after the unmap");
EXPECT_EQ(mock.flushCalls, 0);
EXPECT_EQ(mock.subDataCalls, 0);
EXPECT_TRUE(bufferObject->IsBackendPersistentMapped());
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
// The other kind of backend: one that takes the bytes for a GPU-ordered landing instead
// of letting the frontend write the coherent mapping in place. The unmap hands it the
// mapped offset and the bias-adjusted bytes, leaves the mapping alone, and marks a GPU
// write outstanding so the next read reconciles through the readback.
TEST_F(BufferTest, ABackendWithAResidentSubDataOpTakesTheUnmappedBytesForAGpuOrderedLanding) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kResidentSubDataMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_SHADER_STORAGE_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
const Uint64 baseSerial = bufferObject->GetChangeSerial();
constexpr SizeT kFirst = 3;
constexpr SizeT kCount = 5;
const Range1D range{kFirst * sizeof(GLint), (kFirst + kCount) * sizeof(GLint)};
auto* mapped = static_cast<GLint*>(bufferObject->AcquireMemoryRange(
range, BufferMappingAccessBit::Read | BufferMappingAccessBit::Write));
ASSERT_NE(mapped, nullptr);
for (SizeT i = 0; i < kCount; ++i) mapped[i] = 900 + static_cast<GLint>(i);
bufferObject->ReleaseMemory();
// The op got exactly the mapped range's bytes at the mapped offset...
ASSERT_EQ(mock.residentWrites.size(), 1u);
EXPECT_EQ(mock.residentWrites[0].offset, range.start);
ASSERT_EQ(mock.residentWrites[0].bytes.size(), kCount * sizeof(GLint));
const auto* handed = reinterpret_cast<const GLint*>(mock.residentWrites[0].bytes.data());
for (SizeT i = 0; i < kCount; ++i) EXPECT_EQ(handed[i], 900 + static_cast<GLint>(i)) << "int " << i;
// ...the mapping itself was not written through...
for (SizeT i = 0; i < kAdoptedInts; ++i) EXPECT_EQ(GpuInts(mock)[i], static_cast<GLint>(i)) << "int " << i;
EXPECT_GT(bufferObject->GetChangeSerial(), baseSerial);
EXPECT_EQ(mock.flushCalls, 0);
EXPECT_EQ(mock.subDataCalls, 0);
EXPECT_EQ(mock.readbackCalls, 0);
// ...and the pending flag makes the next read pull the landing back first.
const auto* readBack = static_cast<const GLint*>(bufferObject->AcquireMemory(false, true, false));
EXPECT_EQ(mock.readbackCalls, 1);
EXPECT_TRUE(mock.residentWrites.empty());
for (SizeT i = 0; i < kAdoptedInts; ++i) {
const GLint expected = (i >= kFirst && i < kFirst + kCount) ? 900 + static_cast<GLint>(i - kFirst)
: static_cast<GLint>(i);
EXPECT_EQ(readBack[i], expected) << "int " << i;
}
// A second read has nothing outstanding to reconcile.
bufferObject->AcquireMemory(false, true, false);
EXPECT_EQ(mock.readbackCalls, 1);
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
TEST_F(BufferTest, ABackendWithAResidentSubDataOpTakesAnExplicitlyFlushedRangeTheSameWay) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kResidentSubDataMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_SHADER_STORAGE_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
constexpr SizeT kFirst = 2;
constexpr SizeT kCount = 8;
const Range1D range{kFirst * sizeof(GLint), (kFirst + kCount) * sizeof(GLint)};
auto* mapped = static_cast<GLint*>(bufferObject->AcquireMemoryRange(
range, BufferMappingAccessBit::Write | BufferMappingAccessBit::FlushExplicit));
ASSERT_NE(mapped, nullptr);
for (SizeT i = 0; i < kCount; ++i) mapped[i] = 800 + static_cast<GLint>(i);
constexpr SizeT kFlushFirst = 5;
constexpr SizeT kFlushCount = 2;
bufferObject->FlushMemoryRange(kFlushFirst * sizeof(GLint), kFlushCount * sizeof(GLint));
ASSERT_EQ(mock.residentWrites.size(), 1u);
EXPECT_EQ(mock.residentWrites[0].offset, (kFirst + kFlushFirst) * sizeof(GLint));
ASSERT_EQ(mock.residentWrites[0].bytes.size(), kFlushCount * sizeof(GLint));
const auto* handed = reinterpret_cast<const GLint*>(mock.residentWrites[0].bytes.data());
EXPECT_EQ(handed[0], 800 + static_cast<GLint>(kFlushFirst));
EXPECT_EQ(handed[1], 800 + static_cast<GLint>(kFlushFirst + 1));
EXPECT_EQ(mock.flushCalls, 0);
// The unmap of a FLUSH_EXPLICIT map adds nothing.
bufferObject->ReleaseMemory();
EXPECT_EQ(mock.residentWrites.size(), 1u);
EXPECT_EQ(mock.flushCalls, 0);
EXPECT_EQ(mock.subDataCalls, 0);
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
// The CTS idiom end to end through the GL entry points: an SSBO made resident by a
// draw, re-seeded with glMapBufferRange(READ|WRITE) + glUnmapBuffer.
TEST_F(BufferTest, MapBufferRangeAndUnmapBufferReseedAnAdoptedShaderStorageBuffer) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kZeroCopyMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_SHADER_STORAGE_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
for (GLint pass = 1; pass <= 3; ++pass) {
auto* mapped = static_cast<GLint*>(
MapBufferRange(GL_SHADER_STORAGE_BUFFER, 0, static_cast<GLsizeiptr>(kAdoptedInts * sizeof(GLint)),
GL_MAP_READ_BIT | GL_MAP_WRITE_BIT));
ASSERT_NE(mapped, nullptr);
ASSERT_EQ(GetError(), GL_NO_ERROR);
for (SizeT i = 0; i < kAdoptedInts; ++i) mapped[i] = pass * 100 + static_cast<GLint>(i);
EXPECT_TRUE(UnmapBuffer(GL_SHADER_STORAGE_BUFFER));
ASSERT_EQ(GetError(), GL_NO_ERROR);
for (SizeT i = 0; i < kAdoptedInts; ++i) {
EXPECT_EQ(GpuInts(mock)[i], pass * 100 + static_cast<GLint>(i)) << "pass " << pass << " int " << i;
}
}
EXPECT_TRUE(bufferObject->IsBackendPersistentMapped());
EXPECT_EQ(mock.flushCalls, 0);
EXPECT_EQ(mock.subDataCalls, 0);
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
// The control: a store the backend declined to adopt keeps the shadow model exactly as
// before - the staging copy is written back into the shadow and the backend's flush op
// carries the range down.
TEST_F(BufferTest, ANonPersistentWriteMapOfAShadowBackedStoreStillFlushesThroughTheBackend) {
ZeroCopyMockBackend mock;
mock.provideMap = false;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kZeroCopyMockOps);
GLuint buffer = 0;
GenBuffers(1, &buffer);
BindBuffer(GL_SHADER_STORAGE_BUFFER, buffer);
Vector<GLint> initial(kAdoptedInts);
for (SizeT i = 0; i < kAdoptedInts; ++i) initial[i] = static_cast<GLint>(i);
BufferData(GL_SHADER_STORAGE_BUFFER, static_cast<GLsizeiptr>(kAdoptedInts * sizeof(GLint)), initial.data(),
GL_DYNAMIC_DRAW);
ASSERT_EQ(GetError(), GL_NO_ERROR);
auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer);
ASSERT_NE(bufferObject, nullptr);
EXPECT_FALSE(bufferObject->EnsureGpuResidentStorage());
EXPECT_FALSE(bufferObject->IsBackendPersistentMapped());
mock.flushCalls = 0;
mock.subDataCalls = 0;
const Uint64 baseSerial = bufferObject->GetChangeSerial();
constexpr SizeT kFirst = 3;
constexpr SizeT kCount = 4;
const Range1D range{kFirst * sizeof(GLint), (kFirst + kCount) * sizeof(GLint)};
auto* mapped = static_cast<GLint*>(bufferObject->AcquireMemoryRange(range, BufferMappingAccessBit::Write));
ASSERT_NE(mapped, nullptr);
for (SizeT i = 0; i < kCount; ++i) mapped[i] = 600 + static_cast<GLint>(i);
bufferObject->ReleaseMemory();
EXPECT_EQ(mock.flushCalls, 1);
EXPECT_EQ(mock.subDataCalls, 0);
EXPECT_GT(bufferObject->GetChangeSerial(), baseSerial);
const auto* shadow = reinterpret_cast<const GLint*>(bufferObject->MappedData());
for (SizeT i = 0; i < kAdoptedInts; ++i) {
const GLint expected = (i >= kFirst && i < kFirst + kCount) ? 600 + static_cast<GLint>(i - kFirst)
: static_cast<GLint>(i);
EXPECT_EQ(shadow[i], expected) << "int " << i;
}
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
// ---------------------------------------------------------------------------
// The other three CPU-sourced writes that share the unmap landing's route into an
// adopted store - glBufferSubData, a clear, and a copy - on both kinds of backend: the
// one that lets the frontend write the coherent mapping in place, and the one that takes
// the bytes for a GPU-ordered landing, where the offset it is handed is the only thing
// deciding where they end up.
TEST_F(BufferTest, GlBufferSubDataIntoAnAdoptedStoreLandsInPlaceWithoutABackendTransfer) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kZeroCopyMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_ARRAY_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
constexpr SizeT kFirst = 4;
const GLint updated[] = {70, 71, 72};
BufferSubData(GL_ARRAY_BUFFER, static_cast<GLintptr>(kFirst * sizeof(GLint)), sizeof(updated), updated);
ASSERT_EQ(GetError(), GL_NO_ERROR);
for (SizeT i = 0; i < kAdoptedInts; ++i) {
const GLint expected = (i >= kFirst && i < kFirst + 3) ? updated[i - kFirst] : static_cast<GLint>(i);
EXPECT_EQ(GpuInts(mock)[i], expected) << "int " << i;
}
EXPECT_EQ(mock.subDataCalls, 0);
EXPECT_EQ(mock.flushCalls, 0);
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
TEST_F(BufferTest, ABackendWithAResidentSubDataOpTakesAGlBufferSubDataAtItsOffset) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kResidentSubDataMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_ARRAY_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
constexpr SizeT kFirst = 4;
const GLint updated[] = {70, 71, 72};
BufferSubData(GL_ARRAY_BUFFER, static_cast<GLintptr>(kFirst * sizeof(GLint)), sizeof(updated), updated);
ASSERT_EQ(GetError(), GL_NO_ERROR);
ASSERT_EQ(mock.residentWrites.size(), 1u);
EXPECT_EQ(mock.residentWrites[0].offset, kFirst * sizeof(GLint));
ASSERT_EQ(mock.residentWrites[0].bytes.size(), sizeof(updated));
EXPECT_EQ(std::memcmp(mock.residentWrites[0].bytes.data(), updated, sizeof(updated)), 0);
// The mapping itself is left alone until the backend's ordered copy runs.
for (SizeT i = 0; i < kAdoptedInts; ++i) EXPECT_EQ(GpuInts(mock)[i], static_cast<GLint>(i)) << "int " << i;
const auto* readBack = static_cast<const GLint*>(bufferObject->AcquireMemory(false, true, false));
EXPECT_EQ(mock.readbackCalls, 1);
for (SizeT i = 0; i < kAdoptedInts; ++i) {
const GLint expected = (i >= kFirst && i < kFirst + 3) ? updated[i - kFirst] : static_cast<GLint>(i);
EXPECT_EQ(readBack[i], expected) << "int " << i;
}
EXPECT_EQ(mock.subDataCalls, 0);
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
TEST_F(BufferTest, GlClearBufferSubDataRepeatsItsPatternThroughAnAdoptedStoreInPlace) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kZeroCopyMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_SHADER_STORAGE_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
// A four-byte pattern, so the repeat - not a memset - is what fills the range.
constexpr SizeT kFirst = 5;
constexpr SizeT kCount = 6;
const GLint value = 0x0A0B0C0D;
ClearBufferSubData(GL_SHADER_STORAGE_BUFFER, GL_R32I, static_cast<GLintptr>(kFirst * sizeof(GLint)),
static_cast<GLsizeiptr>(kCount * sizeof(GLint)), GL_RED_INTEGER, GL_INT, &value);
ASSERT_EQ(GetError(), GL_NO_ERROR);
for (SizeT i = 0; i < kAdoptedInts; ++i) {
const GLint expected = (i >= kFirst && i < kFirst + kCount) ? value : static_cast<GLint>(i);
EXPECT_EQ(GpuInts(mock)[i], expected) << "int " << i;
}
EXPECT_EQ(mock.subDataCalls, 0);
EXPECT_EQ(mock.flushCalls, 0);
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
TEST_F(BufferTest, ABackendWithAResidentSubDataOpTakesTheExpandedClearPattern) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kResidentSubDataMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_SHADER_STORAGE_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
constexpr SizeT kFirst = 5;
constexpr SizeT kCount = 6;
const GLint value = 0x0A0B0C0D;
ClearBufferSubData(GL_SHADER_STORAGE_BUFFER, GL_R32I, static_cast<GLintptr>(kFirst * sizeof(GLint)),
static_cast<GLsizeiptr>(kCount * sizeof(GLint)), GL_RED_INTEGER, GL_INT, &value);
ASSERT_EQ(GetError(), GL_NO_ERROR);
// The backend takes the FINAL bytes, so the pattern arrives already repeated.
ASSERT_EQ(mock.residentWrites.size(), 1u);
EXPECT_EQ(mock.residentWrites[0].offset, kFirst * sizeof(GLint));
ASSERT_EQ(mock.residentWrites[0].bytes.size(), kCount * sizeof(GLint));
const auto* handed = reinterpret_cast<const GLint*>(mock.residentWrites[0].bytes.data());
for (SizeT i = 0; i < kCount; ++i) EXPECT_EQ(handed[i], value) << "int " << i;
for (SizeT i = 0; i < kAdoptedInts; ++i) EXPECT_EQ(GpuInts(mock)[i], static_cast<GLint>(i)) << "int " << i;
const auto* readBack = static_cast<const GLint*>(bufferObject->AcquireMemory(false, true, false));
EXPECT_EQ(mock.readbackCalls, 1);
for (SizeT i = 0; i < kAdoptedInts; ++i) {
const GLint expected = (i >= kFirst && i < kFirst + kCount) ? value : static_cast<GLint>(i);
EXPECT_EQ(readBack[i], expected) << "int " << i;
}
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
namespace {
// A plain (never adopted) buffer of kAdoptedInts ints, each `bias` above its index,
// bound to `target` as the source of a copy.
GLuint MakeCopySource(GLenum target, GLint bias) {
GLuint buffer = 0;
GenBuffers(1, &buffer);
BindBuffer(target, buffer);
Vector<GLint> bytes(kAdoptedInts);
for (SizeT i = 0; i < kAdoptedInts; ++i) bytes[i] = bias + static_cast<GLint>(i);
BufferData(target, static_cast<GLsizeiptr>(kAdoptedInts * sizeof(GLint)), bytes.data(), GL_STATIC_DRAW);
EXPECT_EQ(GetError(), GL_NO_ERROR);
return buffer;
}
} // namespace
TEST_F(BufferTest, GlCopyBufferSubDataIntoAnAdoptedStoreLandsAtTheDestinationOffset) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kZeroCopyMockOps);
GLuint destination = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_COPY_WRITE_BUFFER, destination);
ASSERT_NE(bufferObject, nullptr);
const GLuint source = MakeCopySource(GL_COPY_READ_BUFFER, 900);
// Deliberately different source and destination offsets: only the destination one
// may decide where the bytes land.
constexpr SizeT kSrcFirst = 1;
constexpr SizeT kDstFirst = 6;
constexpr SizeT kCount = 3;
CopyBufferSubData(GL_COPY_READ_BUFFER, GL_COPY_WRITE_BUFFER, static_cast<GLintptr>(kSrcFirst * sizeof(GLint)),
static_cast<GLintptr>(kDstFirst * sizeof(GLint)),
static_cast<GLsizeiptr>(kCount * sizeof(GLint)));
ASSERT_EQ(GetError(), GL_NO_ERROR);
for (SizeT i = 0; i < kAdoptedInts; ++i) {
const GLint expected = (i >= kDstFirst && i < kDstFirst + kCount)
? 900 + static_cast<GLint>(kSrcFirst + i - kDstFirst)
: static_cast<GLint>(i);
EXPECT_EQ(GpuInts(mock)[i], expected) << "int " << i;
}
EXPECT_EQ(mock.subDataCalls, 0);
EXPECT_EQ(mock.flushCalls, 0);
GLuint toDelete[] = {destination, source};
DeleteBuffers(2, toDelete);
g_zeroCopyMock = nullptr;
}
TEST_F(BufferTest, ABackendWithAResidentSubDataOpTakesACopyAtTheDestinationOffset) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kResidentSubDataMockOps);
GLuint destination = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_COPY_WRITE_BUFFER, destination);
ASSERT_NE(bufferObject, nullptr);
const GLuint source = MakeCopySource(GL_COPY_READ_BUFFER, 900);
constexpr SizeT kSrcFirst = 1;
constexpr SizeT kDstFirst = 6;
constexpr SizeT kCount = 3;
CopyBufferSubData(GL_COPY_READ_BUFFER, GL_COPY_WRITE_BUFFER, static_cast<GLintptr>(kSrcFirst * sizeof(GLint)),
static_cast<GLintptr>(kDstFirst * sizeof(GLint)),
static_cast<GLsizeiptr>(kCount * sizeof(GLint)));
ASSERT_EQ(GetError(), GL_NO_ERROR);
ASSERT_EQ(mock.residentWrites.size(), 1u);
EXPECT_EQ(mock.residentWrites[0].offset, kDstFirst * sizeof(GLint));
ASSERT_EQ(mock.residentWrites[0].bytes.size(), kCount * sizeof(GLint));
const auto* handed = reinterpret_cast<const GLint*>(mock.residentWrites[0].bytes.data());
for (SizeT i = 0; i < kCount; ++i) {
EXPECT_EQ(handed[i], 900 + static_cast<GLint>(kSrcFirst + i)) << "int " << i;
}
for (SizeT i = 0; i < kAdoptedInts; ++i) EXPECT_EQ(GpuInts(mock)[i], static_cast<GLint>(i)) << "int " << i;
GLuint toDelete[] = {destination, source};
DeleteBuffers(2, toDelete);
g_zeroCopyMock = nullptr;
}
// A PERSISTENT map of an adopted store is the one write shape that needs no landing at
// all: it wrote the coherent mapping in place. Its explicit flush therefore publishes the
// change and dispatches nothing - not the backend's flush op (whose upload would be the
// mapping onto itself) and not the resident landing op (whose bytes are already there).
TEST_F(BufferTest, AnExplicitFlushOfAPersistentMapOfAnAdoptedStoreOnlyPublishesTheChange) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kResidentSubDataMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_SHADER_STORAGE_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
const Uint64 baseSerial = bufferObject->GetChangeSerial();
constexpr SizeT kFirst = 2;
constexpr SizeT kCount = 8;
const Range1D range{kFirst * sizeof(GLint), (kFirst + kCount) * sizeof(GLint)};
auto* mapped = static_cast<GLint*>(bufferObject->AcquireMemoryRange(
range, BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent |
BufferMappingAccessBit::FlushExplicit));
ASSERT_NE(mapped, nullptr);
// The application writes the store itself: the map IS the adopted memory.
EXPECT_EQ(static_cast<void*>(mapped), static_cast<void*>(mock.gpu.data() + range.start));
for (SizeT i = 0; i < kCount; ++i) mapped[i] = 400 + static_cast<GLint>(i);
constexpr SizeT kFlushFirst = 3;
constexpr SizeT kFlushCount = 2;
bufferObject->FlushMemoryRange(kFlushFirst * sizeof(GLint), kFlushCount * sizeof(GLint));
EXPECT_GT(bufferObject->GetChangeSerial(), baseSerial);
EXPECT_EQ(mock.flushCalls, 0);
EXPECT_EQ(mock.subDataCalls, 0);
EXPECT_TRUE(mock.residentWrites.empty());
EXPECT_TRUE(bufferObject->HasDefinedContent());
// Every byte the map wrote is in the store, flushed or not - it was written there.
for (SizeT i = 0; i < kCount; ++i) {
EXPECT_EQ(GpuInts(mock)[kFirst + i], 400 + static_cast<GLint>(i)) << "int " << i;
}
const Uint64 flushSerial = bufferObject->GetChangeSerial();
bufferObject->ReleaseMemory();
EXPECT_EQ(bufferObject->GetChangeSerial(), flushSerial); // the unmap adds nothing
EXPECT_EQ(mock.flushCalls, 0);
EXPECT_EQ(mock.subDataCalls, 0);
EXPECT_TRUE(mock.residentWrites.empty());
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
// A flush of nothing wrote no byte, so it may not report the store as written: an
// orphaning respecification prices a "has content" store as a full-size upload.
TEST_F(BufferTest, AZeroLengthExplicitFlushOfAnAdoptedStoreLeavesItUndefined) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kZeroCopyMockOps);
GLuint buffer = 0;
GenBuffers(1, &buffer);
BindBuffer(GL_SHADER_STORAGE_BUFFER, buffer);
BufferData(GL_SHADER_STORAGE_BUFFER, static_cast<GLsizeiptr>(kAdoptedInts * sizeof(GLint)), nullptr,
GL_DYNAMIC_DRAW);
ASSERT_EQ(GetError(), GL_NO_ERROR);
auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer);
ASSERT_NE(bufferObject, nullptr);
ASSERT_FALSE(bufferObject->HasDefinedContent());
ASSERT_TRUE(bufferObject->EnsureGpuResidentStorage());
const Uint64 baseSerial = bufferObject->GetChangeSerial();
auto* mapped = bufferObject->AcquireMemoryRange({0, kAdoptedInts * sizeof(GLint)},
BufferMappingAccessBit::Write |
BufferMappingAccessBit::Persistent |
BufferMappingAccessBit::FlushExplicit);
ASSERT_NE(mapped, nullptr);
bufferObject->FlushMemoryRange(0, 0);
EXPECT_GT(bufferObject->GetChangeSerial(), baseSerial);
EXPECT_FALSE(bufferObject->HasDefinedContent());
EXPECT_EQ(mock.flushCalls, 0);
EXPECT_EQ(mock.subDataCalls, 0);
bufferObject->ReleaseMemory();
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
// A write map that discards the range it maps reads nothing of the store: its staging
// copy is not seeded from it. Reconciling an adopted store at map time would run the
// backend's drain-and-wait for no reader, once per map, on the streaming arena the
// adoption exists to keep cheap - so it is deferred, not dropped: the first map that DOES
// read the bytes still pays for it, and every queued landing is still applied, in order.
TEST_F(BufferTest, AWriteMapThatDiscardsWhatItMapsDoesNotReconcileAnAdoptedStoreAtMapTime) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kResidentSubDataMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_ARRAY_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
// An earlier write is queued for its GPU-ordered landing...
const GLint firstInt = 55;
BufferSubData(GL_ARRAY_BUFFER, 0, sizeof(firstInt), &firstInt);
ASSERT_EQ(GetError(), GL_NO_ERROR);
ASSERT_EQ(mock.residentWrites.size(), 1u);
EXPECT_EQ(mock.readbackCalls, 0);
// ...and the map that discards its range does not wait for it.
constexpr SizeT kFirst = 8;
constexpr SizeT kCount = 4;
const Range1D range{kFirst * sizeof(GLint), (kFirst + kCount) * sizeof(GLint)};
auto* mapped = static_cast<GLint*>(bufferObject->AcquireMemoryRange(
range, BufferMappingAccessBit::Write | BufferMappingAccessBit::InvalidateRange));
ASSERT_NE(mapped, nullptr);
EXPECT_EQ(mock.readbackCalls, 0);
for (SizeT i = 0; i < kCount; ++i) mapped[i] = 300 + static_cast<GLint>(i);
bufferObject->ReleaseMemory();
ASSERT_EQ(mock.residentWrites.size(), 2u);
// The first read reconciles both landings, oldest first.
const auto* readBack = static_cast<const GLint*>(bufferObject->AcquireMemory(false, true, false));
EXPECT_EQ(mock.readbackCalls, 1);
EXPECT_EQ(readBack[0], firstInt);
for (SizeT i = 0; i < kCount; ++i) EXPECT_EQ(readBack[kFirst + i], 300 + static_cast<GLint>(i)) << "int " << i;
// The control: a map that keeps what it maps still reconciles before seeding.
BufferSubData(GL_ARRAY_BUFFER, 0, sizeof(firstInt), &firstInt);
ASSERT_EQ(GetError(), GL_NO_ERROR);
auto* seeded = static_cast<GLint*>(
bufferObject->AcquireMemoryRange(range, BufferMappingAccessBit::Read | BufferMappingAccessBit::Write));
ASSERT_NE(seeded, nullptr);
EXPECT_EQ(mock.readbackCalls, 2);
for (SizeT i = 0; i < kCount; ++i) EXPECT_EQ(seeded[i], 300 + static_cast<GLint>(i)) << "int " << i;
bufferObject->ReleaseMemory();
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
// Adoption releases the CPU shadow, and a persistent map that did not itself adopt
// (FLUSH_EXPLICIT is excluded from adoption) handed the application a pointer into that
// shadow which GL keeps valid while the buffer is drawn with - which is exactly when a
// storage binding asks for residency. So a mapped buffer keeps the shadow model.
TEST_F(BufferTest, AStorageBindingDoesNotAdoptTheStoreWhileTheApplicationHoldsAMapping) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kZeroCopyMockOps);
GLuint buffer = 0;
GenBuffers(1, &buffer);
BindBuffer(GL_SHADER_STORAGE_BUFFER, buffer);
Vector<GLint> initial(kAdoptedInts);
for (SizeT i = 0; i < kAdoptedInts; ++i) initial[i] = static_cast<GLint>(i);
BufferData(GL_SHADER_STORAGE_BUFFER, static_cast<GLsizeiptr>(kAdoptedInts * sizeof(GLint)), initial.data(),
GL_DYNAMIC_DRAW);
ASSERT_EQ(GetError(), GL_NO_ERROR);
auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer);
ASSERT_NE(bufferObject, nullptr);
const auto* shadowBase = bufferObject->MappedData();
constexpr SizeT kFirst = 2;
constexpr SizeT kCount = 4;
const Range1D range{kFirst * sizeof(GLint), (kFirst + kCount) * sizeof(GLint)};
auto* mapped = static_cast<GLint*>(bufferObject->AcquireMemoryRange(
range, BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent |
BufferMappingAccessBit::FlushExplicit));
ASSERT_NE(mapped, nullptr);
ASSERT_EQ(static_cast<const void*>(mapped), static_cast<const void*>(shadowBase + range.start));
EXPECT_FALSE(bufferObject->EnsureGpuResidentStorage());
EXPECT_FALSE(bufferObject->IsBackendPersistentMapped());
EXPECT_EQ(mock.acquireMapCalls, 0);
// The application's pointer is still the store's: it survived the binding.
EXPECT_EQ(static_cast<const void*>(bufferObject->MappedData()), static_cast<const void*>(shadowBase));
for (SizeT i = 0; i < kCount; ++i) mapped[i] = 250 + static_cast<GLint>(i);
bufferObject->FlushMemoryRange(0, kCount * sizeof(GLint));
EXPECT_EQ(mock.flushCalls, 1);
const auto* shadowInts = reinterpret_cast<const GLint*>(bufferObject->MappedData());
for (SizeT i = 0; i < kCount; ++i) EXPECT_EQ(shadowInts[kFirst + i], 250 + static_cast<GLint>(i));
// Unmapped, the next binding adopts as usual.
bufferObject->ReleaseMemory();
EXPECT_TRUE(bufferObject->EnsureGpuResidentStorage());
EXPECT_TRUE(bufferObject->IsBackendPersistentMapped());
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
// Respecifying a store hands any adoption back and replaces the bytes, so the staged
// bytes of a map that is still live have nowhere to land: copying a whole mapped range
// into storage that is released on the next line is pure waste.
TEST_F(BufferTest, RespecifyingAStoreWhileItIsMappedDoesNotLandTheStagedBytesIntoIt) {
ZeroCopyMockBackend mock;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kResidentSubDataMockOps);
GLuint buffer = 0;
auto bufferObject = MakeAdoptedBuffer(mock, GL_ARRAY_BUFFER, buffer);
ASSERT_NE(bufferObject, nullptr);
auto* mapped = static_cast<GLint*>(
bufferObject->AcquireMemoryRange({0, kAdoptedInts * sizeof(GLint)}, BufferMappingAccessBit::Write));
ASSERT_NE(mapped, nullptr);
for (SizeT i = 0; i < kAdoptedInts; ++i) mapped[i] = 1234;
bufferObject->Respecify(kAdoptedInts * sizeof(GLint), nullptr);
EXPECT_TRUE(mock.residentWrites.empty());
EXPECT_EQ(mock.subDataCalls, 0);
EXPECT_EQ(mock.flushCalls, 0);
EXPECT_EQ(mock.respecifyCalls, 1);
EXPECT_FALSE(bufferObject->IsMapped());
EXPECT_FALSE(bufferObject->IsBackendPersistentMapped());
EXPECT_FALSE(bufferObject->HasDefinedContent());
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
TEST_F(BufferTest, RespecifyingAShadowBackedStoreWhileItIsMappedPushesNoRangeDown) {
ZeroCopyMockBackend mock;
mock.provideMap = false;
g_zeroCopyMock = &mock;
ScopedBackendOps scopedOps(&kZeroCopyMockOps);
GLuint buffer = 0;
GenBuffers(1, &buffer);
BindBuffer(GL_ARRAY_BUFFER, buffer);
Vector<GLint> initial(kAdoptedInts, 7);
BufferData(GL_ARRAY_BUFFER, static_cast<GLsizeiptr>(kAdoptedInts * sizeof(GLint)), initial.data(),
GL_DYNAMIC_DRAW);
ASSERT_EQ(GetError(), GL_NO_ERROR);
auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer);
ASSERT_NE(bufferObject, nullptr);
mock.flushCalls = 0;
mock.subDataCalls = 0;
mock.respecifyCalls = 0;
auto* mapped = static_cast<GLint*>(
bufferObject->AcquireMemoryRange({0, kAdoptedInts * sizeof(GLint)}, BufferMappingAccessBit::Write));
ASSERT_NE(mapped, nullptr);
for (SizeT i = 0; i < kAdoptedInts; ++i) mapped[i] = 1234;
bufferObject->Respecify(kAdoptedInts * sizeof(GLint), nullptr);
EXPECT_EQ(mock.flushCalls, 0);
EXPECT_EQ(mock.subDataCalls, 0);
EXPECT_EQ(mock.respecifyCalls, 1);
EXPECT_FALSE(bufferObject->IsMapped());
EXPECT_FALSE(bufferObject->HasDefinedContent());
DeleteBuffers(1, &buffer);
g_zeroCopyMock = nullptr;
}
+9
View File
@@ -64,6 +64,15 @@ set(LINK_LIBRARIES
include(GoogleTest)
gtest_discover_tests(SanityTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit)
if (MSVC)
# The GL headers declare gl* as dllimport on Windows, so any test that pulls
# GetProcAddress.cpp out of the static library references __imp_gl*, which only
# resolves when the in-library entry-point definitions are part of the link.
# Applies to every test executable below, the way the DirectVulkan and
# integration test targets already do it for themselves.
add_link_options(/WHOLEARCHIVE:MobileGL_s)
endif()
add_subdirectory(BackendLoader)
add_subdirectory(Buffer)
# The heap-address-is-not-an-identity invariant the backends' per-object memos
+11
View File
@@ -23,6 +23,17 @@ target_link_libraries(
${LINK_LIBRARIES}
)
if (MSVC)
# This test compiles library sources of its own; pulling the whole static
# library in as well (the directory-wide MG_Test link option) would define
# them twice, so that option is dropped for this one target.
get_target_property(_program_util_link_options ProgramUtilTest LINK_OPTIONS)
if (_program_util_link_options)
list(REMOVE_ITEM _program_util_link_options /WHOLEARCHIVE:MobileGL_s)
set_target_properties(ProgramUtilTest PROPERTIES LINK_OPTIONS "${_program_util_link_options}")
endif()
endif()
add_executable(
ProgramTest
ProgramTest.cpp
+1
View File
@@ -18,6 +18,7 @@ target_link_libraries(DriverPostIterationRPWitnessTest PRIVATE
add_executable(
DriverBugProbesTest
DriverBugProbesTest.cpp
PersistentBufferOrderingProbeTest.cpp
)
target_include_directories(DriverBugProbesTest PRIVATE
@@ -0,0 +1,326 @@
// MobileGL - MobileGL/MG_Test/SelfTest/PersistentBufferOrderingProbeTest.cpp
// Copyright (c) 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
#include <gtest/gtest.h>
#include <MG_Util/SelfTest/PersistentBufferOrderingProbe.h>
#include <map>
#include <set>
using namespace MobileGL;
using namespace MobileGL::MG_Util::SelfTest;
namespace {
// Deferred vertex fetch, not canned ReadPixels answers: a broken mapped destination
// reads its current bytes at Finish instead of the bytes at DrawArrays. ReadPixels also
// drains these jobs, so inserting an early readback into the probe hides the bug here too.
struct FakeDriver {
struct Buffer {
Bool mapped = false;
Bool arena = false;
Bool copied = false;
Int channel = 0;
Vector<Uint8> staging;
};
struct Draw { GLuint fbo, buffer; Int channel; Bool late; };
std::map<GLuint, Buffer> buffers;
std::map<GLuint, GLuint> vaoBuffers;
std::map<GLuint, Int> colors;
Vector<Draw> draws;
std::set<GLuint> live;
std::map<GLenum, GLint> state = {
{GL_CURRENT_PROGRAM, 1}, {GL_VERTEX_ARRAY_BINDING, 2}, {GL_ARRAY_BUFFER, 3},
{GL_COPY_READ_BUFFER, 4}, {GL_COPY_WRITE_BUFFER, 5},
{GL_DRAW_FRAMEBUFFER_BINDING, 6}, {GL_READ_FRAMEBUFFER_BINDING, 7},
{GL_TEXTURE_BINDING_2D, 8}, {GL_PIXEL_PACK_BUFFER, 9},
{GL_PACK_ALIGNMENT, 8}, {GL_PACK_ROW_LENGTH, 31},
{GL_PACK_SKIP_PIXELS, 4}, {GL_PACK_SKIP_ROWS, 5}};
std::map<GLenum, GLboolean> enabled = {{GL_BLEND, GL_TRUE}, {GL_SCISSOR_TEST, GL_TRUE},
{GL_SAMPLE_MASK, GL_TRUE}, {GL_RASTERIZER_DISCARD, GL_TRUE}};
std::array<GLint, 4> viewport = {3, 4, 5, 6};
std::array<GLfloat, 4> clear = {.25f, .5f, .75f, 0};
std::array<GLboolean, 4> mask = {GL_FALSE, GL_TRUE, GL_FALSE, GL_TRUE};
GLuint next = 100;
GLenum error = GL_NO_ERROR;
Bool extension = true, corruptSubData = false, corruptCopy = false;
Bool corruptUnmapped = false, corruptSerialized = false;
Bool failMap = false, failAllocation = false, failFramebuffer = false;
Int failReadbackAt = 0, readbacks = 0;
Uint arenaAllocations = 0;
Uint8 pointerSentinel = 0;
GLuint Create() { live.insert(next); return next++; }
void Generate(GLsizei count, GLuint* ids) { for (Int i = 0; i < count; ++i) ids[i] = Create(); }
void Delete(GLsizei count, const GLuint* ids) {
for (Int i = 0; i < count; ++i) {
live.erase(ids[i]);
buffers.erase(ids[i]);
}
}
GLint Get(GLenum name) const {
const auto found = state.find(name);
return found == state.end() ? 0 : found->second;
}
static Int Channel(const void* data) {
GLfloat color[3];
std::memcpy(color, static_cast<const Uint8*>(data) + 2 * sizeof(GLfloat), sizeof(color));
return color[0] > .5f ? 0 : color[1] > .5f ? 1 : 2;
}
void Finish() {
for (const auto& draw : draws) {
const auto& buffer = buffers.at(draw.buffer);
colors[draw.fbo] = draw.late ? buffer.channel : draw.channel;
if (buffer.mapped && corruptSerialized) colors[draw.fbo] = (draw.channel + 1) % 3;
}
draws.clear();
}
} driver;
MG_External::GLESFunctionsTable Table() {
MG_External::GLESFunctionsTable gl{};
gl.glGetIntegerv = [](GLenum name, GLint* out) {
if (name == GL_MAJOR_VERSION) *out = 3;
else if (name == GL_MINOR_VERSION) *out = 2;
else if (name == GL_NUM_EXTENSIONS) *out = driver.extension ? 1 : 0;
else if (name == GL_VIEWPORT) std::copy(driver.viewport.begin(), driver.viewport.end(), out);
else if (name == GL_ARRAY_BUFFER_BINDING) *out = driver.Get(GL_ARRAY_BUFFER);
else if (name == GL_PIXEL_PACK_BUFFER_BINDING) *out = driver.Get(GL_PIXEL_PACK_BUFFER);
else *out = driver.Get(name);
};
gl.glGetBooleanv = [](GLenum, GLboolean* out) { std::copy(driver.mask.begin(), driver.mask.end(), out); };
gl.glGetFloatv = [](GLenum, GLfloat* out) { std::copy(driver.clear.begin(), driver.clear.end(), out); };
gl.glGetStringi = [](GLenum, GLuint) { return reinterpret_cast<const GLubyte*>("GL_EXT_buffer_storage"); };
gl.glGetError = []() { return std::exchange(driver.error, GL_NO_ERROR); };
gl.glIsEnabled = [](GLenum name) -> GLboolean { return driver.enabled[name]; };
gl.glEnable = [](GLenum name) { driver.enabled[name] = GL_TRUE; };
gl.glDisable = [](GLenum name) { driver.enabled[name] = GL_FALSE; };
gl.glCreateShader = [](GLenum) { return driver.Create(); };
gl.glShaderSource = [](GLuint, GLsizei, const GLchar* const*, const GLint*) {};
gl.glCompileShader = [](GLuint) {};
gl.glGetShaderiv = [](GLuint, GLenum, GLint* out) { *out = GL_TRUE; };
gl.glGetShaderInfoLog = [](GLuint, GLsizei, GLsizei*, GLchar* out) { *out = 0; };
gl.glDeleteShader = [](GLuint id) { driver.Delete(1, &id); };
gl.glCreateProgram = []() { return driver.Create(); };
gl.glAttachShader = [](GLuint, GLuint) {};
gl.glLinkProgram = [](GLuint) {};
gl.glGetProgramiv = [](GLuint, GLenum, GLint* out) { *out = GL_TRUE; };
gl.glGetProgramInfoLog = gl.glGetShaderInfoLog;
gl.glDeleteProgram = gl.glDeleteShader;
gl.glUseProgram = [](GLuint id) { driver.state[GL_CURRENT_PROGRAM] = id; };
gl.glGenBuffers = [](GLsizei count, GLuint* ids) { driver.Generate(count, ids); };
gl.glBindBuffer = [](GLenum target, GLuint id) { driver.state[target] = id; };
gl.glBufferStorageEXT = [](GLenum target, GLsizeiptr size, const void*, GLbitfield) {
auto& buffer = driver.buffers[driver.Get(target)];
buffer.arena = size >= 16 * 1024 * 1024;
if (buffer.arena) ++driver.arenaAllocations;
if (driver.failAllocation) driver.error = GL_OUT_OF_MEMORY;
if (!buffer.arena) buffer.staging.resize(size);
};
gl.glBufferData = [](GLenum target, GLsizeiptr size, const void*, GLenum) {
driver.buffers[driver.Get(target)].staging.resize(size);
};
gl.glMapBufferRange = [](GLenum target, GLintptr offset, GLsizeiptr, GLbitfield) -> void* {
if (driver.failMap) return nullptr;
auto& buffer = driver.buffers[driver.Get(target)];
buffer.mapped = true;
return buffer.arena ? &driver.pointerSentinel : buffer.staging.data() + offset;
};
gl.glUnmapBuffer = [](GLenum) -> GLboolean { return GL_TRUE; }; // Preserve allocation history.
gl.glBufferSubData = [](GLenum target, GLintptr offset, GLsizeiptr size, const void* data) {
auto& buffer = driver.buffers[driver.Get(target)];
if (buffer.arena) {
buffer.channel = FakeDriver::Channel(data);
buffer.copied = false;
} else std::memcpy(buffer.staging.data() + offset, data, size);
};
gl.glCopyBufferSubData = [](GLenum read, GLenum write, GLintptr offset, GLintptr, GLsizeiptr) {
auto& source = driver.buffers[driver.Get(read)];
auto& dest = driver.buffers[driver.Get(write)];
dest.channel = FakeDriver::Channel(source.staging.data() + offset);
dest.copied = true;
};
gl.glDeleteBuffers = [](GLsizei count, const GLuint* ids) { driver.Delete(count, ids); };
gl.glGenVertexArrays = gl.glGenBuffers;
gl.glBindVertexArray = [](GLuint id) { driver.state[GL_VERTEX_ARRAY_BINDING] = id; };
gl.glVertexAttribPointer = [](GLuint, GLint, GLenum, GLboolean, GLsizei, const void*) {
driver.vaoBuffers[driver.Get(GL_VERTEX_ARRAY_BINDING)] = driver.Get(GL_ARRAY_BUFFER);
};
gl.glEnableVertexAttribArray = [](GLuint) {};
gl.glDeleteVertexArrays = gl.glDeleteBuffers;
gl.glGenTextures = gl.glGenBuffers;
gl.glBindTexture = [](GLenum, GLuint id) { driver.state[GL_TEXTURE_BINDING_2D] = id; };
gl.glTexStorage2D = [](GLenum, GLsizei, GLenum, GLsizei, GLsizei) {};
gl.glDeleteTextures = gl.glDeleteBuffers;
gl.glGenFramebuffers = gl.glGenBuffers;
gl.glBindFramebuffer = [](GLenum target, GLuint id) {
if (target != GL_READ_FRAMEBUFFER) driver.state[GL_DRAW_FRAMEBUFFER_BINDING] = id;
if (target != GL_DRAW_FRAMEBUFFER) driver.state[GL_READ_FRAMEBUFFER_BINDING] = id;
};
gl.glFramebufferTexture2D = [](GLenum, GLenum, GLenum, GLuint, GLint) {};
gl.glCheckFramebufferStatus = [](GLenum) -> GLenum {
return driver.failFramebuffer ? GL_FRAMEBUFFER_UNSUPPORTED : GL_FRAMEBUFFER_COMPLETE;
};
gl.glDeleteFramebuffers = gl.glDeleteBuffers;
gl.glViewport = [](GLint x, GLint y, GLsizei w, GLsizei h) { driver.viewport = {x, y, w, h}; };
gl.glColorMask = [](GLboolean r, GLboolean g, GLboolean b, GLboolean a) { driver.mask = {r, g, b, a}; };
gl.glClearColor = [](GLfloat r, GLfloat g, GLfloat b, GLfloat a) { driver.clear = {r, g, b, a}; };
gl.glClear = [](GLbitfield) {};
gl.glDrawArrays = [](GLenum, GLint, GLsizei) {
const GLuint id = driver.vaoBuffers.at(driver.Get(GL_VERTEX_ARRAY_BINDING));
const auto& buffer = driver.buffers.at(id);
const Bool late = buffer.mapped ? (buffer.copied ? driver.corruptCopy : driver.corruptSubData)
: driver.corruptUnmapped;
driver.draws.push_back({GLuint(driver.Get(GL_DRAW_FRAMEBUFFER_BINDING)), id, buffer.channel, late});
};
gl.glFinish = []() { driver.Finish(); };
gl.glMemoryBarrier = [](GLbitfield) {};
gl.glPixelStorei = [](GLenum name, GLint value) { driver.state[name] = value; };
gl.glReadPixels = [](GLint, GLint, GLsizei width, GLsizei height, GLenum, GLenum, void* data) {
driver.Finish(); // Models the implicit wait that must NOT occur between subject draws.
if (++driver.readbacks == driver.failReadbackAt) {
driver.error = GL_INVALID_OPERATION;
return;
}
EXPECT_EQ(driver.Get(GL_PIXEL_PACK_BUFFER), 0);
EXPECT_EQ(driver.Get(GL_PACK_ROW_LENGTH), 0);
const Int channel = driver.colors.at(driver.Get(GL_READ_FRAMEBUFFER_BINDING));
auto* pixels = static_cast<Uint8*>(data);
for (Int i = 0; i < width * height; ++i)
for (Int c = 0; c < 4; ++c) pixels[4 * i + c] = c == channel || c == 3 ? 255 : 0;
};
return gl;
}
class PersistentBufferOrderingProbeTest : public ::testing::Test {
protected:
void SetUp() override { driver = FakeDriver{}; }
void TearDown() override { EXPECT_TRUE(driver.live.empty()); EXPECT_TRUE(driver.draws.empty()); }
};
}
TEST_F(PersistentBufferOrderingProbeTest, RequiresExtensionAndCompleteDispatchBeforeAllocating) {
auto gl = Table();
driver.extension = false;
EXPECT_FALSE(ProbePersistentBufferUpdateOrdering(gl).supported);
driver.extension = true;
gl.glCopyBufferSubData = nullptr;
EXPECT_FALSE(ProbePersistentBufferUpdateOrdering(gl).supported);
EXPECT_EQ(driver.arenaAllocations, 0u);
}
TEST_F(PersistentBufferOrderingProbeTest, OrderedDriverPassesAllUploadsAndRestoresCallerState) {
const auto saved = driver;
const auto measurement = ProbePersistentBufferUpdateOrdering(Table());
ASSERT_TRUE(measurement.supported);
for (const auto& row : measurement.uploads) {
EXPECT_TRUE(row.unmapped.Passed());
EXPECT_TRUE(row.mapped.Passed());
EXPECT_EQ(row.mapped.frames, 240u); // Three fresh attempts before a negative result.
EXPECT_EQ(row.finishBoth.status, BufferOrderingProbeStatus::NotRun);
}
EXPECT_FALSE(DescribePersistentBufferOrderingBug(measurement));
EXPECT_EQ(driver.state, saved.state);
EXPECT_EQ(driver.viewport, saved.viewport);
EXPECT_EQ(driver.clear, saved.clear);
EXPECT_EQ(driver.mask, saved.mask);
for (const auto& [cap, value] : driver.enabled) {
const auto found = saved.enabled.find(cap);
EXPECT_EQ(value, found == saved.enabled.end() ? GL_FALSE : found->second);
}
}
TEST_F(PersistentBufferOrderingProbeTest, DeferredMappedSubDataFetchIsDetectedWithPassingControls) {
driver.corruptSubData = true;
const auto measurement = ProbePersistentBufferUpdateOrdering(Table());
EXPECT_TRUE(measurement.uploads[0].Detected());
EXPECT_TRUE(measurement.uploads[0].finishBefore.Passed());
EXPECT_GT(measurement.uploads[0].mapThenUnmap.badFrames, 0u);
EXPECT_GT(measurement.uploads[0].barrierBefore.badFrames, 0u);
EXPECT_FALSE(measurement.uploads[1].Detected());
EXPECT_FALSE(measurement.uploads[2].Detected());
const auto finding = DescribePersistentBufferOrderingBug(measurement);
ASSERT_TRUE(finding);
EXPECT_EQ(finding->verdict, DriverBugVerdict::Unfixable);
EXPECT_NE(finding->detail.find("SubData:"), String::npos);
EXPECT_NE(finding->detail.find("MOBILEGL_DISABLE_LARGE_BUFFER_ADOPTION=1"), String::npos);
}
TEST_F(PersistentBufferOrderingProbeTest, DeferredCopyFetchIsDetectedWithBothStagingSources) {
driver.corruptCopy = true;
const auto measurement = ProbePersistentBufferUpdateOrdering(Table());
EXPECT_FALSE(measurement.uploads[0].Detected());
EXPECT_TRUE(measurement.uploads[1].Detected());
EXPECT_TRUE(measurement.uploads[2].Detected());
}
TEST_F(PersistentBufferOrderingProbeTest, PostCollectorIncludesTheMeasuredFinding) {
driver.corruptSubData = true;
const auto findings = CollectGlesKnownDriverBugs(Table());
const auto found = std::find_if(findings.begin(), findings.end(), [](const auto& finding) {
return finding.name == "Persistent-mapped vertex buffers lose upload/draw ordering";
});
ASSERT_NE(found, findings.end());
EXPECT_NE(found->detail.find("never-mapped 0/80"), String::npos);
EXPECT_EQ(found->verdict, DriverBugVerdict::Unfixable);
}
TEST_F(PersistentBufferOrderingProbeTest, CorruptNeverMappedControlCannotAccusePersistentMapping) {
driver.corruptSubData = driver.corruptCopy = driver.corruptUnmapped = true;
const auto measurement = ProbePersistentBufferUpdateOrdering(Table());
for (const auto& row : measurement.uploads) {
EXPECT_GT(row.unmapped.badFrames, 0u);
EXPECT_EQ(row.mapped.status, BufferOrderingProbeStatus::NotRun);
}
EXPECT_FALSE(DescribePersistentBufferOrderingBug(measurement));
}
TEST_F(PersistentBufferOrderingProbeTest, CorruptSerializedControlCannotConfirmOrderingDefect) {
driver.corruptSubData = driver.corruptCopy = driver.corruptSerialized = true;
const auto measurement = ProbePersistentBufferUpdateOrdering(Table());
for (const auto& row : measurement.uploads) EXPECT_GT(row.finishBoth.badFrames, 0u);
EXPECT_FALSE(DescribePersistentBufferOrderingBug(measurement));
}
TEST_F(PersistentBufferOrderingProbeTest, FailedMappingIsInconclusiveAndReleasesResources) {
driver.failMap = true;
const auto measurement = ProbePersistentBufferUpdateOrdering(Table());
EXPECT_EQ(measurement.uploads[0].mapped.status, BufferOrderingProbeStatus::Failed);
EXPECT_EQ(measurement.uploads[1].unmapped.status, BufferOrderingProbeStatus::Failed);
EXPECT_FALSE(DescribePersistentBufferOrderingBug(measurement));
}
TEST_F(PersistentBufferOrderingProbeTest, AllocationFailureIsInconclusiveAndRestoresBindings) {
driver.failAllocation = true;
const auto saved = driver.state;
const auto measurement = ProbePersistentBufferUpdateOrdering(Table());
for (const auto& row : measurement.uploads) {
EXPECT_EQ(row.unmapped.status, BufferOrderingProbeStatus::Failed);
EXPECT_EQ(row.unmapped.error, GLenum(GL_OUT_OF_MEMORY));
}
EXPECT_FALSE(DescribePersistentBufferOrderingBug(measurement));
EXPECT_EQ(driver.state, saved);
}
TEST_F(PersistentBufferOrderingProbeTest, ReadbackErrorAfterAMismatchDoesNotProduceAFinding) {
driver.corruptSubData = true;
driver.failReadbackAt = 82; // Eighty clean control readbacks, then one corrupt subject FBO.
const auto measurement = ProbePersistentBufferUpdateOrdering(Table());
EXPECT_GT(measurement.uploads[0].mapped.badFrames, 0u);
EXPECT_EQ(measurement.uploads[0].mapped.status, BufferOrderingProbeStatus::Failed);
EXPECT_EQ(measurement.uploads[0].mapped.error, GLenum(GL_INVALID_OPERATION));
EXPECT_FALSE(DescribePersistentBufferOrderingBug(measurement));
}
TEST_F(PersistentBufferOrderingProbeTest, IncompleteFramebufferIsInconclusiveAndRestoresBindings) {
driver.failFramebuffer = true;
const auto saved = driver.state;
const auto measurement = ProbePersistentBufferUpdateOrdering(Table());
EXPECT_FALSE(DescribePersistentBufferOrderingBug(measurement));
EXPECT_EQ(driver.arenaAllocations, 0u);
EXPECT_EQ(driver.state, saved);
}
@@ -22,6 +22,8 @@
#include "Includes.h"
#include "Init.h"
#include <MG_Util/ShaderTranspiler/ShaderCompiler.h>
#include <MG_Util/ShaderTranspiler/SpvcSession.h>
#include <MG_Util/ShaderTranspiler/Types.h>
#include <spirv-tools/libspirv.hpp>
@@ -38,6 +40,7 @@ namespace {
constexpr Uint32 kOpTypeInt = 21;
constexpr Uint32 kOpTypeFloat = 22;
constexpr Uint32 kOpTypeArray = 28;
constexpr Uint32 kOpTypeRuntimeArray = 29;
constexpr Uint32 kOpTypeStruct = 30;
constexpr Uint32 kOpConstant = 43;
constexpr Uint32 kDecorationArrayStride = 6;
@@ -116,6 +119,18 @@ namespace {
return {elementTypeId, length};
}
// The element type id of OpTypeRuntimeArray <arrayId>, or 0 when it is not one - which is
// what a BOUNDED flattened member (an OpTypeArray) answers too, so the two shapes can be told
// apart by the pair of helpers.
Uint32 RuntimeArrayElementOf(const Vector<Uint32>& spirv, Uint32 arrayId) {
Uint32 elementTypeId = 0;
ForEachInstruction(spirv, [&](Uint32 opcode, const Uint32* words, Uint32 wordCount) {
if (opcode != kOpTypeRuntimeArray || wordCount < 3 || words[1] != arrayId) return;
elementTypeId = words[2];
});
return elementTypeId;
}
Bool IsUint32Type(const Vector<Uint32>& spirv, Uint32 typeId) {
Bool isUint = false;
ForEachInstruction(spirv, [&](Uint32 opcode, const Uint32* words, Uint32 wordCount) {
@@ -140,6 +155,61 @@ namespace {
return text;
}
// How many lines of a disassembly hold BOTH fragments - "OpIMul %uint" and "%uint_8", say -
// which is how the index arithmetic the pass emits is pinned without a host that could run it.
Uint32 CountLinesWith(const String& text, const String& first, const String& second) {
Uint32 count = 0;
SizeT lineStart = 0;
while (lineStart < text.size()) {
SizeT lineEnd = text.find('\n', lineStart);
if (lineEnd == String::npos) lineEnd = text.size();
const String line = text.substr(lineStart, lineEnd - lineStart);
if (line.find(first) != String::npos && line.find(second) != String::npos) ++count;
lineStart = lineEnd + 1;
}
return count;
}
// What every test of the open-ended shape asserts: the block collapsed to ONE member, which
// is a `uint[]` RUNTIME array of stride 4 rather than a bounded one, and nothing 64-bit is
// left for the demotion to find. Returns the disassembly for the arithmetic checks.
String ExpectOpenEndedWordArray(const Vector<Uint32>& output, const String& blockName) {
const String text = Disassemble(output);
const Uint32 structId = StructIdNamed(output, blockName);
EXPECT_NE(structId, 0u) << text;
if (structId == 0) return text;
const Vector<Uint32> members = MemberTypesOf(output, structId);
EXPECT_EQ(members.size(), 1u) << "the block should have collapsed to one member\n" << text;
if (members.size() != 1) return text;
EXPECT_EQ(MemberOffsetsOf(output, structId), (Vector<Uint32>{0}));
const Uint32 elementTypeId = RuntimeArrayElementOf(output, members[0]);
EXPECT_NE(elementTypeId, 0u) << "member 0 is not a runtime array\n" << text;
EXPECT_EQ(ArrayShapeOf(output, members[0]).first, 0u)
<< "an open-ended block must not be given a bounded length\n"
<< text;
EXPECT_TRUE(IsUint32Type(output, elementTypeId)) << text;
EXPECT_EQ(DecorationValueOf(output, members[0], kDecorationArrayStride), 4u) << text;
EXPECT_EQ(CountFloatTypesOfWidth(output, 64), 0u) << text;
return text;
}
// The compute shape every failing KHR-Single-GL45.subgroups fp64 case binds: one runtime
// array of doubles, indexed by an invocation id, read whole-element.
String OpenEndedComputeSource(const String& elementType) {
return String(R"(#version 430 core
layout(local_size_x = 16) in;
layout(std430, binding = 0) buffer Sink { uint result[]; };
layout(std430, binding = 1) buffer Data { )") +
elementType + R"( data[]; };
void main() {
)" + elementType +
R"( value = data[gl_LocalInvocationID.x] * data[0];
result[gl_GlobalInvocationID.x] = uint(value)" +
(elementType == "double" ? String{} : String(".x")) + R"();
}
)";
}
Vector<Uint32> CompileToSpirv(GLenum stage, const String& source) {
using namespace MG_Util::ShaderTranspiler;
ShaderAttrib shaderAttrib{.shaderType = stage, .sourceStr = source};
@@ -345,3 +415,679 @@ TEST_F(FlattenFloat64StorageBlockTest, TheDemotedPathIsUnchangedByTheCapabilityA
EXPECT_EQ(explicitlyDemoted, defaulted);
EXPECT_EQ(CountFloatTypesOfWidth(defaulted, 64), 0u) << Disassemble(defaulted);
}
// ---------------------------------------------------------------------------
// The open-ended shape: a block whose last member is a runtime array. Before this was accepted
// the pass declined it and the demotion re-derived ArrayStride 4 for the now-float element, so
// `double data[]` read the application's 8-byte-stride buffer as 32-bit words - every fp64
// KHR-Single-GL45.subgroups case failed on exactly that.
// ---------------------------------------------------------------------------
TEST_F(FlattenFloat64StorageBlockTest, AnOpenEndedBlockOfDoublesBecomesAWordRuntimeArray) {
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, OpenEndedComputeSource("double"));
ASSERT_FALSE(input.empty());
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
const String text = ExpectOpenEndedWordArray(output, "Data");
// Element i of the original array starts at word 2i, so the dynamic index is scaled by 2 ...
EXPECT_EQ(CountLinesWith(text, "OpIMul %uint", "%uint_2"), 1u) << text;
// ... and the constant `data[0]` is the pair of words at 0 and 1, reached through the one
// member the block has left.
EXPECT_GE(CountLinesWith(text, "OpAccessChain %_ptr_StorageBuffer_uint", "%uint_0 %uint_0"), 1u) << text;
}
TEST_F(FlattenFloat64StorageBlockTest, EachDoubleVectorWidthStepsByItsOwnStride) {
struct Shape {
const char* element;
// std430 strides: dvec2 16 bytes, dvec3 and dvec4 32 bytes - i.e. 4, 8 and 8 words.
const char* strideWords;
// The last component's word offset inside one element, and the first one past it.
const char* lastComponentWords;
const char* firstWordPastIt;
};
const Shape shapes[] = {{"dvec2", "%uint_4", "%uint_2", "%uint_4"},
{"dvec3", "%uint_8", "%uint_4", "%uint_6"},
{"dvec4", "%uint_8", "%uint_6", "%uint_8"}};
for (const Shape& shape : shapes) {
SCOPED_TRACE(shape.element);
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, OpenEndedComputeSource(shape.element));
ASSERT_FALSE(input.empty());
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
const String text = ExpectOpenEndedWordArray(output, "Data");
EXPECT_EQ(CountLinesWith(text, "OpIMul %uint", shape.strideWords), 1u) << text;
EXPECT_GE(CountLinesWith(text, "OpIAdd %uint", shape.lastComponentWords), 1u) << text;
// A dvec3 is six words in a stride of eight: nothing may be read from the padding.
EXPECT_EQ(CountLinesWith(text, "OpIAdd %uint", shape.firstWordPastIt), 0u) << text;
}
}
TEST_F(FlattenFloat64StorageBlockTest, AFixedPrefixBeforeTheRuntimeArrayIsAddedToEveryIndex) {
const String source = R"(#version 430 core
layout(local_size_x = 16) in;
layout(std430, binding = 0) buffer Sink { uint result[]; };
layout(std430, binding = 1) buffer Data {
uvec4 head;
double data[];
};
void main() {
result[gl_GlobalInvocationID.x] = head.x + uint(data[gl_LocalInvocationID.x]);
}
)";
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, source);
ASSERT_FALSE(input.empty());
const Uint32 inputStructId = StructIdNamed(input, "Data");
ASSERT_NE(inputStructId, 0u);
EXPECT_EQ(MemberOffsetsOf(input, inputStructId), (Vector<Uint32>{0, 16})) << Disassemble(input);
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
const String text = ExpectOpenEndedWordArray(output, "Data");
// The 16-byte prefix is 4 words: element i is at word 4 + 2i.
EXPECT_EQ(CountLinesWith(text, "OpIMul %uint", "%uint_2"), 1u) << text;
EXPECT_EQ(CountLinesWith(text, "OpIAdd %uint", "%uint_4"), 1u) << text;
// And the prefix member itself is still word 0.
EXPECT_GE(CountLinesWith(text, "OpAccessChain %_ptr_StorageBuffer_uint", "%uint_0 %uint_0"), 1u) << text;
}
// OpArrayLength on the flattened member counts WORDS. GL's `.length()` is the number of whole
// elements the bound range holds past the array's offset, so the count has to be rebased and
// divided - in unsigned arithmetic, and clamped rather than wrapped when the range is shorter
// than the prefix.
namespace {
// A prefix, an open-ended array of doubles, and a `.length()` of it - the one shape whose
// rewrite is an instruction SPIRV-Cross has to spell rather than plain arithmetic.
constexpr const char* kOpenEndedLengthSource = R"(#version 430 core
layout(local_size_x = 16) in;
layout(std430, binding = 0) buffer Sink { uint result[]; };
layout(std430, binding = 1) buffer Data {
uvec4 head;
double data[];
};
void main() {
result[gl_GlobalInvocationID.x] = uint(data.length()) + head.y;
}
)";
} // namespace
TEST_F(FlattenFloat64StorageBlockTest, TheLengthOfAnOpenEndedBlockIsRewrittenToAnElementCount) {
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, kOpenEndedLengthSource);
ASSERT_FALSE(input.empty());
// glslang asks for member 1's length and signs the answer.
EXPECT_EQ(CountLinesWith(Disassemble(input), "OpArrayLength %uint", " 1"), 1u) << Disassemble(input);
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
const String text = ExpectOpenEndedWordArray(output, "Data");
// Re-aimed at the one member left, ...
EXPECT_EQ(CountLinesWith(text, "OpArrayLength %uint", " 0"), 1u) << text;
EXPECT_EQ(CountLinesWith(text, "OpArrayLength %uint", " 1"), 0u) << text;
// ... rebased past the 4-word prefix, clamped at zero when the range does not reach it, ...
EXPECT_EQ(CountLinesWith(text, "OpISub %uint", "%uint_4"), 1u) << text;
EXPECT_EQ(CountLinesWith(text, "OpULessThan %bool", "%uint_4"), 1u) << text;
EXPECT_EQ(CountLinesWith(text, "OpSelect %uint", "%uint_0"), 1u) << text;
// ... and divided by the 2-word stride, with glslang's own sign conversion still downstream.
EXPECT_EQ(CountLinesWith(text, "OpUDiv %uint", "%uint_2"), 1u) << text;
EXPECT_EQ(CountLinesWith(text, "OpBitcast %int", ""), 1u) << text;
}
TEST_F(FlattenFloat64StorageBlockTest, TheLengthOfABlockWithNoPrefixNeedsNoClamp) {
const String source = R"(#version 430 core
layout(local_size_x = 16) in;
layout(std430, binding = 0) buffer Sink { uint result[]; };
layout(std430, binding = 1) buffer Data { dvec2 data[]; };
void main() {
result[gl_GlobalInvocationID.x] = uint(data.length());
}
)";
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, source);
ASSERT_FALSE(input.empty());
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
const String text = ExpectOpenEndedWordArray(output, "Data");
EXPECT_EQ(CountLinesWith(text, "OpArrayLength %uint", " 0"), 1u) << text;
// Nothing to subtract, so nothing to clamp: the word count over the 4-word stride is it.
EXPECT_EQ(CountLinesWith(text, "OpISub", ""), 0u) << text;
EXPECT_EQ(CountLinesWith(text, "OpSelect", ""), 0u) << text;
EXPECT_EQ(CountLinesWith(text, "OpUDiv %uint", "%uint_4"), 1u) << text;
}
// The graphics shape of the same CTS group: a fragment stage reading a `readonly` block. The
// NonWritable the qualifier became is a promise about the whole block, and has to be on the one
// member the flattened block keeps.
TEST_F(FlattenFloat64StorageBlockTest, AReadOnlyOpenEndedBlockKeepsNonWritable) {
const String source = R"(#version 450 core
layout(binding = 4, std430) readonly buffer Buffer4 { dvec3 data[]; };
layout(location = 0) out vec4 o_color;
void main() {
uint index = uint(gl_FragCoord.x);
o_color = vec4(float(data[index].z));
}
)";
const Vector<Uint32> input = CompileToSpirv(GL_FRAGMENT_SHADER, source);
ASSERT_FALSE(input.empty());
EXPECT_EQ(CountLinesWith(Disassemble(input), "OpMemberDecorate %Buffer4 0 NonWritable", ""), 1u)
<< Disassemble(input);
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
const String text = ExpectOpenEndedWordArray(output, "Buffer4");
EXPECT_EQ(CountLinesWith(text, "OpMemberDecorate %Buffer4 0 NonWritable", ""), 1u) << text;
// dvec3: stride 8 words, .z at +4.
EXPECT_EQ(CountLinesWith(text, "OpIMul %uint", "%uint_8"), 1u) << text;
EXPECT_GE(CountLinesWith(text, "OpIAdd %uint", "%uint_4"), 1u) << text;
}
// Writing through an open-ended block, which no CTS case does but any shader may: the store
// is decomposed into the same words the load would have read, so the bytes the application
// gets back are the ones GL says it wrote.
TEST_F(FlattenFloat64StorageBlockTest, AnOpenEndedBlockIsWrittenThroughTheSameWords) {
const String source = R"(#version 430 core
layout(local_size_x = 16) in;
layout(std430, binding = 1) buffer Data { double data[]; };
void main() {
data[gl_LocalInvocationID.x] = double(gl_LocalInvocationID.y);
}
)";
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, source);
ASSERT_FALSE(input.empty());
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
const String text = ExpectOpenEndedWordArray(output, "Data");
// One dynamic index, scaled to the 2-word element ...
EXPECT_EQ(CountLinesWith(text, "OpIMul %uint", "%uint_2"), 1u) << text;
// ... and the double left as exactly two word stores, nothing wider.
EXPECT_EQ(CountLinesWith(text, "OpStore", ""), 2u) << text;
EXPECT_EQ(CountLinesWith(text, "OpAccessChain %_ptr_StorageBuffer_uint", ""), 2u) << text;
}
// The exact compute shader KHR-Single-GL45.subgroups.arithmetic.compute.subgroupmul_double
// generates, so the CTS shape is pinned as it is and not as a paraphrase of it.
TEST_F(FlattenFloat64StorageBlockTest, TheSubgroupMulDoubleComputeShaderIsFlattened) {
const String source = R"(#version 450
#extension GL_KHR_shader_subgroup_arithmetic: enable
#extension GL_KHR_shader_subgroup_ballot: enable
layout (local_size_x = 16, local_size_y = 1, local_size_z = 1) in;
layout(binding = 0, std430) buffer Buffer0
{
uint result[];
};
layout(binding = 1, std430) buffer Buffer1
{
double data[];
};
void main (void)
{
uvec3 globalSize = gl_NumWorkGroups * gl_WorkGroupSize;
highp uint offset = globalSize.x * ((globalSize.y * gl_GlobalInvocationID.z) + gl_GlobalInvocationID.y) + gl_GlobalInvocationID.x;
uvec4 mask = subgroupBallot(true);
uint start = 0u, end = gl_SubgroupSize;
double ref = double(1);
uint tempResult = 0u;
for (uint index = start; index < end; index++)
{
if (subgroupBallotBitExtract(mask, index))
{
ref = ref * data[index];
}
}
tempResult = (abs(ref - subgroupMul(data[gl_SubgroupInvocationID])) < 0.00001) ? 0x1u : 0u;
if (1u == (gl_SubgroupInvocationID % 2u))
{
mask = subgroupBallot(true);
ref = double(1);
for (uint index = start; index < end; index++)
{
if (subgroupBallotBitExtract(mask, index))
{
ref = ref * data[index];
}
}
tempResult |= (abs(ref - subgroupMul(data[gl_SubgroupInvocationID])) < 0.00001) ? 0x2u : 0u;
}
else
{
tempResult |= 0x2u;
}
result[offset] = tempResult;
}
)";
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, source);
ASSERT_FALSE(input.empty());
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
const String text = ExpectOpenEndedWordArray(output, "Buffer1");
// Four reads of the array, each scaled to the 2-word element.
EXPECT_EQ(CountLinesWith(text, "OpIMul %uint", "%uint_2"), 4u) << text;
// The result block holds no double and is not the pass's business.
const Uint32 resultStructId = StructIdNamed(output, "Buffer0");
ASSERT_NE(resultStructId, 0u) << text;
const Vector<Uint32> resultMembers = MemberTypesOf(output, resultStructId);
ASSERT_EQ(resultMembers.size(), 1u);
EXPECT_TRUE(IsUint32Type(output, RuntimeArrayElementOf(output, resultMembers[0]))) << text;
}
// A runtime array whose element is a MATRIX. The member's own MatrixStride and RowMajor
// decorations describe those elements, so a row-major one has to be declined - its columns are
// not contiguous, and addressing it in column order against a row-major buffer would be silently
// wrong bytes rather than a refusal. The column-major twin must flatten, stepping by the
// element's stride and then by the column's.
TEST_F(FlattenFloat64StorageBlockTest, ARowMajorMatrixRuntimeArrayIsLeftToTheDemotion) {
const String source = R"(#version 430 core
layout(local_size_x = 16) in;
layout(std430, binding = 0) buffer Sink { uint result[]; };
layout(std430, binding = 1, row_major) buffer Data { dmat4 data[]; };
void main() {
result[gl_GlobalInvocationID.x] = uint(data[gl_LocalInvocationID.x][1][2]);
}
)";
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, source);
ASSERT_FALSE(input.empty());
// The premise: glslang really did mark the member row-major.
EXPECT_EQ(CountLinesWith(Disassemble(input), "OpMemberDecorate %Data 0 RowMajor", ""), 1u)
<< Disassemble(input);
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
const String text = Disassemble(output);
const Uint32 structId = StructIdNamed(output, "Data");
ASSERT_NE(structId, 0u) << text;
const Vector<Uint32> members = MemberTypesOf(output, structId);
ASSERT_EQ(members.size(), 1u) << text;
// Still a runtime array of matrices - narrowed to fp32 by the demotion, not re-addressed.
EXPECT_NE(DecorationValueOf(output, members[0], kDecorationArrayStride), 4u)
<< "a row-major matrix element must not have been flattened into words\n"
<< text;
EXPECT_EQ(CountLinesWith(text, "OpIMul %uint", ""), 0u)
<< "nothing should have been re-addressed\n"
<< text;
}
TEST_F(FlattenFloat64StorageBlockTest, AColumnMajorMatrixRuntimeArrayStepsByItsColumnStride) {
const String source = R"(#version 430 core
layout(local_size_x = 16) in;
layout(std430, binding = 0) buffer Sink { uint result[]; };
layout(std430, binding = 1) buffer Data { dmat2x4 data[]; };
void main() {
dvec4 column = data[gl_LocalInvocationID.x][1];
result[gl_GlobalInvocationID.x] = uint(column.w);
}
)";
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, source);
ASSERT_FALSE(input.empty());
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
const String text = ExpectOpenEndedWordArray(output, "Data");
// dmat2x4: two columns of dvec4, column stride 32 bytes, so one element is 64 bytes -
// 16 words - and column 1 starts 8 words into it.
EXPECT_EQ(CountLinesWith(text, "OpIMul %uint", "%uint_16"), 1u) << text;
// Exactly one +8: the column's own offset inside the element. A second would mean a word
// past the column was being addressed off that same base.
EXPECT_EQ(CountLinesWith(text, "OpIAdd %uint", "%uint_8"), 1u) << text;
// All eight words of that column are read - the last of its four doubles ends at +7 ...
EXPECT_EQ(CountLinesWith(text, "OpIAdd %uint", "%uint_7"), 1u) << text;
// ... and the column that was not asked for is not touched: nothing is read at +9 or past.
EXPECT_EQ(CountLinesWith(text, "OpIAdd %uint", "%uint_9"), 0u) << text;
EXPECT_EQ(CountLinesWith(text, "OpIAdd %uint", "%uint_10"), 0u) << text;
}
// A runtime array whose element is a STRUCT: the same walk, and the same decline test, as a
// bounded array of them - a shape no other open-ended case reaches.
TEST_F(FlattenFloat64StorageBlockTest, AStructRuntimeArrayStepsByItsElementStride) {
const String source = R"(#version 430 core
layout(local_size_x = 16) in;
struct Pair { double a; float b; };
layout(std430, binding = 0) buffer Sink { uint result[]; };
layout(std430, binding = 1) buffer Data { Pair data[]; };
void main() {
result[gl_GlobalInvocationID.x] = uint(data[gl_LocalInvocationID.x].a) +
uint(data[gl_LocalInvocationID.x].b);
}
)";
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, source);
ASSERT_FALSE(input.empty());
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
const String text = ExpectOpenEndedWordArray(output, "Data");
// std430 rounds `{ double a; float b; }` up to its 8-byte alignment: 16 bytes, 4 words,
// with `b` two words in.
EXPECT_EQ(CountLinesWith(text, "OpIMul %uint", "%uint_4"), 2u) << text;
EXPECT_GE(CountLinesWith(text, "OpIAdd %uint", "%uint_2"), 1u) << text;
}
// The leaf cap bounds ONE load or store, not a member's size: a block whose element is far too
// big to expand whole is still flattened while every access to it names a scalar. Declining it
// would leave the application's 8-byte-stride doubles to the demotion's re-derived stride 4 -
// the exact defect the open-ended shape exists to avoid.
TEST_F(FlattenFloat64StorageBlockTest, AHugeRuntimeArrayElementIsStillFlattenedWhenAccessesAreSmall) {
const String source = R"(#version 430 core
layout(local_size_x = 16) in;
struct Big { dvec4 v[300]; };
layout(std430, binding = 0) buffer Sink { uint result[]; };
layout(std430, binding = 1) buffer Data { Big data[]; };
void main() {
result[gl_GlobalInvocationID.x] = uint(data[gl_LocalInvocationID.x].v[3].y);
}
)";
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, source);
ASSERT_FALSE(input.empty());
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
const String text = ExpectOpenEndedWordArray(output, "Data");
// 300 dvec4 of 32 bytes each: 9600 bytes, 2400 words per element - 1200 scalars, well past
// the per-access cap that a whole-element load would have to respect and this never does.
EXPECT_EQ(CountLinesWith(text, "OpIMul %uint", "%uint_2400"), 1u) << text;
// v[3].y is 3 * 8 + 2 = 26 words into the element.
EXPECT_GE(CountLinesWith(text, "OpIAdd %uint", "%uint_26"), 1u) << text;
}
// The flatten preserves a byte layout ACROSS a narrowing; where the backend consumes 64-bit
// floats itself there is nothing to preserve, and the open-ended block has to keep its runtime
// array of doubles exactly as the driver would lay it out.
TEST_F(FlattenFloat64StorageBlockTest, TheNativePathLeavesAnOpenEndedBlockAndItsDoublesAlone) {
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, OpenEndedComputeSource("double"));
ASSERT_FALSE(input.empty());
Vector<Uint32> output;
ASSERT_TRUE(ShaderCompiler::SanitizeAndOptimizeBinary(input, output, true, true, true));
ASSERT_FALSE(output.empty());
const String text = Disassemble(output);
const Uint32 structId = StructIdNamed(output, "Data");
ASSERT_NE(structId, 0u) << text;
const Vector<Uint32> members = MemberTypesOf(output, structId);
ASSERT_EQ(members.size(), 1u) << text;
EXPECT_NE(RuntimeArrayElementOf(output, members[0]), 0u)
<< "the member should still be a runtime array\n"
<< text;
EXPECT_EQ(DecorationValueOf(output, members[0], kDecorationArrayStride), 8u)
<< "the array must keep the 8-byte stride the application bound\n"
<< text;
EXPECT_GT(CountFloatTypesOfWidth(output, 64), 0u)
<< "nothing narrows here, so the doubles must survive\n"
<< text;
}
// The other backend prints the flattened module through SPIRV-Cross: an open-ended `uint[]`
// member has to come out as ESSL that names no 64-bit type. The `.length()` shape is here too,
// because the OpArrayLength the rewrite re-issues is the one instruction in it whose ESSL
// spelling is not plain arithmetic - if that backend ever refused it on the flattened member,
// a DirectGLES shader asking an fp64 buffer its length would fail at link and nowhere else.
namespace {
String TranspileToEssl(const Vector<Uint32>& spirv) {
using namespace MG_Util::ShaderTranspiler;
SpvcSession session(spirv, SessionUsageBit::Transpile);
spvc_compiler_options options;
EXPECT_EQ(session.CreateOptions(&options), SPVC_SUCCESS);
spvc_compiler_options_set_uint(options, SPVC_COMPILER_OPTION_GLSL_VERSION, 320);
spvc_compiler_options_set_bool(options, SPVC_COMPILER_OPTION_GLSL_ES, SPVC_TRUE);
spvc_compiler_options_set_bool(options, SPVC_COMPILER_OPTION_GLSL_VULKAN_SEMANTICS, SPVC_FALSE);
EXPECT_EQ(session.SetOptions(options), SPVC_SUCCESS);
auto essl = ShaderCompiler::DecompileShader(session);
EXPECT_TRUE(essl) << (essl ? String{} : essl.error().log);
return essl ? *essl : String{};
}
} // namespace
TEST_F(FlattenFloat64StorageBlockTest, AnOpenEndedBlockCanBeEmittedAsEssl) {
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, OpenEndedComputeSource("dvec4"));
ASSERT_FALSE(input.empty());
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
ExpectOpenEndedWordArray(output, "Data");
const String essl = TranspileToEssl(output);
ASSERT_FALSE(essl.empty());
EXPECT_EQ(essl.find("double"), String::npos) << essl;
EXPECT_EQ(essl.find("dvec"), String::npos) << essl;
EXPECT_NE(essl.find("uint"), String::npos) << essl;
}
TEST_F(FlattenFloat64StorageBlockTest, TheRewrittenLengthCanBeEmittedAsEssl) {
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, kOpenEndedLengthSource);
ASSERT_FALSE(input.empty());
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
ExpectOpenEndedWordArray(output, "Data");
const String essl = TranspileToEssl(output);
ASSERT_FALSE(essl.empty());
EXPECT_EQ(essl.find("double"), String::npos) << essl;
EXPECT_EQ(essl.find("dvec"), String::npos) << essl;
// The length survived as a length - it was not folded away or dropped on the floor.
EXPECT_NE(essl.find(".length()"), String::npos) << essl;
}
// ---------------------------------------------------------------------------
// The gate from the other side: a runtime array anywhere but the block's own last member is a
// shape GLSL cannot spell and this pass does not describe. SPIR-V can spell it, so both are
// hand-written, and both are invalid Vulkan SPIR-V - the chain runs without its validator here,
// which is also why neither can be a validation-failure count.
// ---------------------------------------------------------------------------
namespace {
// `buffer Odd { double data[]; uint tail; }`, the runtime array FIRST.
const char* kRuntimeArrayNotLastAsm = R"(
OpCapability Shader
OpCapability Float64
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %main "main"
OpExecutionMode %main LocalSize 1 1 1
OpName %Odd "Odd"
OpName %var ""
OpDecorate %_runtimearr_double ArrayStride 8
OpDecorate %Odd Block
OpMemberDecorate %Odd 0 Offset 0
OpMemberDecorate %Odd 1 Offset 8
OpDecorate %var Binding 0
OpDecorate %var DescriptorSet 0
%void = OpTypeVoid
%3 = OpTypeFunction %void
%uint = OpTypeInt 32 0
%int = OpTypeInt 32 1
%int_0 = OpConstant %int 0
%int_1 = OpConstant %int 1
%double = OpTypeFloat 64
%double_2 = OpConstant %double 2
%_runtimearr_double = OpTypeRuntimeArray %double
%Odd = OpTypeStruct %_runtimearr_double %uint
%_ptr_StorageBuffer_Odd = OpTypePointer StorageBuffer %Odd
%var = OpVariable %_ptr_StorageBuffer_Odd StorageBuffer
%_ptr_StorageBuffer_double = OpTypePointer StorageBuffer %double
%main = OpFunction %void None %3
%5 = OpLabel
%6 = OpAccessChain %_ptr_StorageBuffer_double %var %int_0 %int_1
OpStore %6 %double_2
OpReturn
OpFunctionEnd
)";
// `struct Inner { double data[]; }; buffer Outer { uint head; Inner inner; }`: the runtime
// array IS last, but of a member rather than of the block.
const char* kRuntimeArrayNestedAsm = R"(
OpCapability Shader
OpCapability Float64
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %main "main"
OpExecutionMode %main LocalSize 1 1 1
OpName %Outer "Outer"
OpName %Inner "Inner"
OpName %var ""
OpDecorate %_runtimearr_double ArrayStride 8
OpMemberDecorate %Inner 0 Offset 0
OpDecorate %Outer Block
OpMemberDecorate %Outer 0 Offset 0
OpMemberDecorate %Outer 1 Offset 8
OpDecorate %var Binding 0
OpDecorate %var DescriptorSet 0
%void = OpTypeVoid
%3 = OpTypeFunction %void
%uint = OpTypeInt 32 0
%int = OpTypeInt 32 1
%int_0 = OpConstant %int 0
%int_1 = OpConstant %int 1
%double = OpTypeFloat 64
%double_2 = OpConstant %double 2
%_runtimearr_double = OpTypeRuntimeArray %double
%Inner = OpTypeStruct %_runtimearr_double
%Outer = OpTypeStruct %uint %Inner
%_ptr_StorageBuffer_Outer = OpTypePointer StorageBuffer %Outer
%var = OpVariable %_ptr_StorageBuffer_Outer StorageBuffer
%_ptr_StorageBuffer_double = OpTypePointer StorageBuffer %double
%main = OpFunction %void None %3
%5 = OpLabel
%6 = OpAccessChain %_ptr_StorageBuffer_double %var %int_1 %int_0 %int_1
OpStore %6 %double_2
OpReturn
OpFunctionEnd
)";
Vector<Uint32> AssembleUnchecked(const char* asmText) {
spvtools::SpirvTools tools(SPV_ENV_VULKAN_1_1);
Vector<Uint32> module;
EXPECT_TRUE(tools.Assemble(asmText, &module));
return module;
}
} // namespace
TEST_F(FlattenFloat64StorageBlockTest, ARuntimeArrayThatIsNotTheBlocksLastMemberIsLeftToTheDemotion) {
struct Shape {
const char* asmText;
const char* blockName;
};
const Shape shapes[] = {{kRuntimeArrayNotLastAsm, "Odd"}, {kRuntimeArrayNestedAsm, "Outer"}};
for (const Shape& shape : shapes) {
SCOPED_TRACE(shape.blockName);
const Vector<Uint32> input = AssembleUnchecked(shape.asmText);
ASSERT_FALSE(input.empty());
Vector<Uint32> output;
ASSERT_TRUE(ShaderCompiler::SanitizeAndOptimizeBinary(input, output, false, false));
ASSERT_FALSE(output.empty());
const String text = Disassemble(output);
// Declined: both members are still there, and the demotion narrowed them the old way.
const Uint32 structId = StructIdNamed(output, shape.blockName);
ASSERT_NE(structId, 0u) << text;
EXPECT_EQ(MemberTypesOf(output, structId).size(), 2u) << text;
EXPECT_EQ(CountFloatTypesOfWidth(output, 64), 0u) << text;
EXPECT_EQ(CountLinesWith(text, "OpIMul %uint", ""), 0u)
<< "nothing should have been re-addressed\n"
<< text;
}
}
// ---------------------------------------------------------------------------
// The front end declares types in first-use order, so a block that is the first thing the
// shader touches is declared before the module's `uint` - and the flattened member is an array
// OF `uint`. For an OPEN-ENDED block the pass moves that operand-less type up in front of the
// block rather than declining, so that where a buffer of doubles stands in the shader does not
// decide whether its bytes survive. A BOUNDED block in the same position keeps the decline it
// has always had: widening that is a change to a path this fix does not need, and the pair below
// pins both halves.
// ---------------------------------------------------------------------------
namespace {
// The position of <id>'s declaration in instruction order, or 0 when it has none.
Uint32 DeclarationIndexOf(const Vector<Uint32>& spirv, Uint32 id) {
Uint32 index = 0;
Uint32 found = 0;
ForEachInstruction(spirv, [&](Uint32 opcode, const Uint32* words, Uint32 wordCount) {
++index;
if (found != 0 || wordCount < 2) return;
// Every OpType* has its result id in word 1; that is all this is asked about.
if (opcode >= kOpTypeInt && opcode <= kOpTypeStruct && words[1] == id) found = index;
});
return found;
}
Uint32 Uint32TypeIdOf(const Vector<Uint32>& spirv) {
Uint32 typeId = 0;
ForEachInstruction(spirv, [&](Uint32 opcode, const Uint32* words, Uint32 wordCount) {
if (opcode == kOpTypeInt && wordCount >= 4 && words[2] == 32u && words[3] == 0u) typeId = words[1];
});
return typeId;
}
} // namespace
TEST_F(FlattenFloat64StorageBlockTest, AnOpenEndedBlockDeclaredBeforeTheModulesUintIsStillFlattened) {
// The block is the first thing main touches, and nothing before it needs a uint - not even
// an array length, which is a uint constant and would declare one.
const String source = R"(#version 430 core
layout(local_size_x = 1) in;
layout(std430, binding = 0) buffer Data { double data[]; };
layout(std430, binding = 1) buffer Sink { float result[]; };
void main() {
result[0] = float(data[0] + data[1]);
}
)";
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, source);
ASSERT_FALSE(input.empty());
const Uint32 inputStructId = StructIdNamed(input, "Data");
ASSERT_NE(inputStructId, 0u);
const Uint32 inputUintId = Uint32TypeIdOf(input);
// The premise: the module's uint really is declared after the block (or not at all).
ASSERT_TRUE(inputUintId == 0 ||
DeclarationIndexOf(input, inputUintId) > DeclarationIndexOf(input, inputStructId))
<< "this shader was meant to declare the block before any uint\n"
<< Disassemble(input);
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
const String text = ExpectOpenEndedWordArray(output, "Data");
const Uint32 structId = StructIdNamed(output, "Data");
ASSERT_NE(structId, 0u) << text;
const Vector<Uint32> members = MemberTypesOf(output, structId);
ASSERT_EQ(members.size(), 1u) << text;
// And the uint now stands in front of the block it is an element of.
EXPECT_LT(DeclarationIndexOf(output, RuntimeArrayElementOf(output, members[0])),
DeclarationIndexOf(output, structId))
<< text;
}
TEST_F(FlattenFloat64StorageBlockTest, ABoundedBlockDeclaredBeforeTheModulesUintIsLeftToTheDemotion) {
// The same position, a bounded block: this is the shape that has always been declined, and
// it stays declined - its members and the demotion's own repacking come through untouched.
const String source = R"(#version 430 core
layout(local_size_x = 1) in;
layout(std430, binding = 0) buffer Wide {
double data0;
dvec2 data1;
} g_wide;
layout(std430, binding = 1) buffer Sink { float result[]; };
void main() {
double sum = g_wide.data0 + g_wide.data1.y;
result[0] = float(sum);
}
)";
const Vector<Uint32> input = CompileToSpirv(GL_COMPUTE_SHADER, source);
ASSERT_FALSE(input.empty());
const Uint32 inputStructId = StructIdNamed(input, "Wide");
ASSERT_NE(inputStructId, 0u);
const Uint32 inputUintId = Uint32TypeIdOf(input);
ASSERT_TRUE(inputUintId == 0 ||
DeclarationIndexOf(input, inputUintId) > DeclarationIndexOf(input, inputStructId))
<< "this shader was meant to declare the block before any uint\n"
<< Disassemble(input);
const Vector<Uint32> output = Sanitize(input);
ASSERT_FALSE(output.empty());
const String text = Disassemble(output);
const Uint32 structId = StructIdNamed(output, "Wide");
ASSERT_NE(structId, 0u) << text;
EXPECT_EQ(MemberTypesOf(output, structId).size(), 2u)
<< "a bounded block in this position must keep the decline it shipped with\n"
<< text;
EXPECT_EQ(CountLinesWith(text, "OpIMul %uint", ""), 0u)
<< "nothing should have been re-addressed\n"
<< text;
}
@@ -7,6 +7,7 @@
// End of Source File Header
#include "DriverBugProbes.h"
#include "PersistentBufferOrderingProbe.h"
#include <Config.h>
#include <MG_Util/Debug/Log.h>
@@ -2445,6 +2446,10 @@ namespace MobileGL::MG_Util::SelfTest {
DriverBugVerdict::Unfixable, detail};
}
Optional<DriverBugFinding> ProbePersistentBufferOrderingBug(const GLESFunctionsTable& gl) {
return DescribePersistentBufferOrderingBug(ProbePersistentBufferUpdateOrdering(gl));
}
// The table. One row per known driver bug; see the header for how to add a sibling.
using DriverBugProbeFn = Optional<DriverBugFinding> (*)(const GLESFunctionsTable&);
constexpr DriverBugProbeFn kGlesDriverBugProbes[] = {
@@ -2457,6 +2462,7 @@ namespace MobileGL::MG_Util::SelfTest {
&ProbeLayeredBlitDestinationBug,
&ProbeLocatedIoBlockPayloadBug,
&ProbeCopyImagePacked16FieldOrderBug,
&ProbePersistentBufferOrderingBug,
};
} // namespace
@@ -0,0 +1,413 @@
// MobileGL - MobileGL/MG_Util/SelfTest/PersistentBufferOrderingProbe.cpp
// Copyright (c) 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
#include "PersistentBufferOrderingProbe.h"
#include <MG_Util/Debug/Log.h>
#include <array>
#include <cmath>
#include <cstring>
#include <new>
namespace MobileGL::MG_Util::SelfTest {
namespace {
using MG_External::GLESFunctionsTable;
constexpr GLbitfield kPersistent = 0x0040;
constexpr GLbitfield kCoherent = 0x0080;
constexpr GLbitfield kDynamicStorage = 0x0100;
constexpr GLbitfield kMapFlags = GL_MAP_WRITE_BIT | kPersistent | kCoherent;
constexpr GLsizeiptr kArenaSize = 128 * 1024 * 1024;
constexpr GLintptr kOffset = 96 * 1024 * 1024 + 28;
constexpr GLsizei kSide = 128;
constexpr GLsizei kSlots = 8;
constexpr Int kBatches = 10;
constexpr Int kDraws = 32;
constexpr GLsizei kQuads = 64 * 32;
constexpr Int kAttempts = 3;
constexpr std::array<const char*, 3> kUploadNames = {"SubData", "Copy/persistent staging",
"Copy/SubData staging"};
enum class Shape { Unmapped, Mapped, FinishBefore, FinishBoth, MapThenUnmap, BarrierBefore };
struct Vertex { GLfloat x, y, r, g, b; };
constexpr GLsizeiptr kPayloadSize = kQuads * 6 * sizeof(Vertex);
static_assert(kOffset + kPayloadSize <= kArenaSize);
void DrainErrors(const GLESFunctionsTable& gl) {
for (Int i = 0; i < 32 && gl.glGetError() != GL_NO_ERROR; ++i) {}
}
Bool CanProbe(const GLESFunctionsTable& gl) {
if (!(gl.glGetIntegerv && gl.glGetBooleanv && gl.glGetFloatv && gl.glGetError &&
gl.glGetStringi && gl.glIsEnabled && gl.glEnable && gl.glDisable &&
gl.glCreateShader && gl.glShaderSource && gl.glCompileShader && gl.glGetShaderiv &&
gl.glGetShaderInfoLog && gl.glDeleteShader && gl.glCreateProgram && gl.glAttachShader &&
gl.glLinkProgram && gl.glGetProgramiv && gl.glGetProgramInfoLog && gl.glDeleteProgram &&
gl.glUseProgram && gl.glGenBuffers && gl.glBindBuffer && gl.glBufferStorageEXT &&
gl.glMapBufferRange && gl.glUnmapBuffer && gl.glBufferData && gl.glBufferSubData &&
gl.glCopyBufferSubData && gl.glDeleteBuffers && gl.glGenVertexArrays &&
gl.glBindVertexArray && gl.glVertexAttribPointer && gl.glEnableVertexAttribArray &&
gl.glDeleteVertexArrays && gl.glGenTextures && gl.glBindTexture && gl.glTexStorage2D &&
gl.glDeleteTextures && gl.glGenFramebuffers && gl.glBindFramebuffer &&
gl.glFramebufferTexture2D && gl.glCheckFramebufferStatus && gl.glDeleteFramebuffers &&
gl.glViewport && gl.glColorMask && gl.glClearColor && gl.glClear && gl.glDrawArrays &&
gl.glFinish && gl.glMemoryBarrier && gl.glPixelStorei && gl.glReadPixels)) return false;
DrainErrors(gl);
GLint major = 0, minor = 0, count = 0;
gl.glGetIntegerv(GL_MAJOR_VERSION, &major);
gl.glGetIntegerv(GL_MINOR_VERSION, &minor);
gl.glGetIntegerv(GL_NUM_EXTENSIONS, &count);
if (gl.glGetError() != GL_NO_ERROR || major < 3 || (major == 3 && minor < 1)) return false;
for (GLint i = 0; i < count; ++i) {
const auto* extension = gl.glGetStringi(GL_EXTENSIONS, i);
if (extension && std::strcmp(reinterpret_cast<const char*>(extension),
"GL_EXT_buffer_storage") == 0) return true;
}
return false;
}
// This probe touches no images/SSBO bindings. Keep its scope independent from the
// other POST probes, including pack state and the caller's currently active texture unit.
struct StateScope {
const GLESFunctionsTable& gl;
GLint program = 0, vao = 0, array = 0, copyRead = 0, copyWrite = 0;
GLint drawFbo = 0, readFbo = 0, texture = 0, packBuffer = 0;
GLint viewport[4]{};
GLfloat clear[4]{};
GLboolean colorMask[4]{};
static constexpr std::array<GLenum, 10> enables = {
GL_BLEND, GL_DEPTH_TEST, GL_STENCIL_TEST, GL_CULL_FACE, GL_SCISSOR_TEST,
GL_RASTERIZER_DISCARD, GL_DITHER, GL_SAMPLE_ALPHA_TO_COVERAGE,
GL_SAMPLE_COVERAGE, GL_SAMPLE_MASK};
static constexpr std::array<GLenum, 4> packNames = {
GL_PACK_ALIGNMENT, GL_PACK_ROW_LENGTH, GL_PACK_SKIP_PIXELS, GL_PACK_SKIP_ROWS};
std::array<GLboolean, enables.size()> enabled{};
std::array<GLint, packNames.size()> pack{};
explicit StateScope(const GLESFunctionsTable& api) : gl(api) {
gl.glGetIntegerv(GL_CURRENT_PROGRAM, &program);
gl.glGetIntegerv(GL_VERTEX_ARRAY_BINDING, &vao);
gl.glGetIntegerv(GL_ARRAY_BUFFER_BINDING, &array);
gl.glGetIntegerv(GL_COPY_READ_BUFFER_BINDING, &copyRead);
gl.glGetIntegerv(GL_COPY_WRITE_BUFFER_BINDING, &copyWrite);
gl.glGetIntegerv(GL_DRAW_FRAMEBUFFER_BINDING, &drawFbo);
gl.glGetIntegerv(GL_READ_FRAMEBUFFER_BINDING, &readFbo);
gl.glGetIntegerv(GL_TEXTURE_BINDING_2D, &texture);
gl.glGetIntegerv(GL_PIXEL_PACK_BUFFER_BINDING, &packBuffer);
gl.glGetIntegerv(GL_VIEWPORT, viewport);
gl.glGetFloatv(GL_COLOR_CLEAR_VALUE, clear);
gl.glGetBooleanv(GL_COLOR_WRITEMASK, colorMask);
for (SizeT i = 0; i < enables.size(); ++i) enabled[i] = gl.glIsEnabled(enables[i]);
for (SizeT i = 0; i < packNames.size(); ++i) gl.glGetIntegerv(packNames[i], &pack[i]);
}
void Prepare() {
for (auto cap : enables) gl.glDisable(cap);
gl.glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
gl.glBindBuffer(GL_PIXEL_PACK_BUFFER, 0);
for (auto name : packNames) gl.glPixelStorei(name, name == GL_PACK_ALIGNMENT ? 1 : 0);
gl.glViewport(0, 0, kSide, kSide);
gl.glClearColor(0, 0, 0, 1);
}
~StateScope() {
gl.glUseProgram(program);
gl.glBindVertexArray(vao);
gl.glBindBuffer(GL_ARRAY_BUFFER, array);
gl.glBindBuffer(GL_COPY_READ_BUFFER, copyRead);
gl.glBindBuffer(GL_COPY_WRITE_BUFFER, copyWrite);
gl.glBindBuffer(GL_PIXEL_PACK_BUFFER, packBuffer);
gl.glBindFramebuffer(GL_DRAW_FRAMEBUFFER, drawFbo);
gl.glBindFramebuffer(GL_READ_FRAMEBUFFER, readFbo);
gl.glBindTexture(GL_TEXTURE_2D, texture);
gl.glViewport(viewport[0], viewport[1], viewport[2], viewport[3]);
gl.glClearColor(clear[0], clear[1], clear[2], clear[3]);
gl.glColorMask(colorMask[0], colorMask[1], colorMask[2], colorMask[3]);
for (SizeT i = 0; i < enables.size(); ++i) {
if (enabled[i]) gl.glEnable(enables[i]); else gl.glDisable(enables[i]);
}
for (SizeT i = 0; i < packNames.size(); ++i) gl.glPixelStorei(packNames[i], pack[i]);
}
};
struct Resources {
const GLESFunctionsTable& gl;
GLuint program = 0, vao = 0;
std::array<GLuint, kSlots> fbos{}, textures{};
explicit Resources(const GLESFunctionsTable& api) : gl(api) {}
~Resources() {
gl.glDeleteFramebuffers(kSlots, fbos.data());
gl.glDeleteTextures(kSlots, textures.data());
gl.glDeleteVertexArrays(1, &vao);
if (program) gl.glDeleteProgram(program);
}
GLuint Compile(GLenum type, const char* source) {
GLuint shader = gl.glCreateShader(type);
if (!shader) return 0;
gl.glShaderSource(shader, 1, &source, nullptr);
gl.glCompileShader(shader);
GLint compiled = 0;
gl.glGetShaderiv(shader, GL_COMPILE_STATUS, &compiled);
if (!compiled) {
char log[512]{};
gl.glGetShaderInfoLog(shader, sizeof(log), nullptr, log);
MGLOG_I("[driver-bug] persistent buffer ordering: shader failed: %s", log);
gl.glDeleteShader(shader);
return 0;
}
return shader;
}
Bool Setup() {
const GLuint vs = Compile(GL_VERTEX_SHADER,
"#version 310 es\nlayout(location=0) in vec2 pos; layout(location=1) in vec3 color;\n"
"out highp vec3 vColor; void main(){gl_Position=vec4(pos,0,1);vColor=color;}\n");
const GLuint fs = Compile(GL_FRAGMENT_SHADER,
"#version 310 es\nprecision highp float; in highp vec3 vColor;\n"
"layout(location=0) out vec4 outColor; void main(){outColor=vec4(vColor,1);}\n");
if (vs && fs) {
program = gl.glCreateProgram();
if (program) {
gl.glAttachShader(program, vs);
gl.glAttachShader(program, fs);
gl.glLinkProgram(program);
}
}
if (vs) gl.glDeleteShader(vs);
if (fs) gl.glDeleteShader(fs);
if (!program) return false;
GLint linked = 0;
gl.glGetProgramiv(program, GL_LINK_STATUS, &linked);
if (!linked) {
char log[512]{};
gl.glGetProgramInfoLog(program, sizeof(log), nullptr, log);
MGLOG_I("[driver-bug] persistent buffer ordering: link failed: %s", log);
return false;
}
gl.glUseProgram(program);
gl.glGenVertexArrays(1, &vao);
gl.glBindVertexArray(vao);
gl.glGenFramebuffers(kSlots, fbos.data());
gl.glGenTextures(kSlots, textures.data());
for (Int i = 0; i < kSlots; ++i) {
if (!vao || !fbos[i] || !textures[i]) return false;
gl.glBindTexture(GL_TEXTURE_2D, textures[i]);
gl.glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, kSide, kSide);
gl.glBindFramebuffer(GL_FRAMEBUFFER, fbos[i]);
gl.glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D,
textures[i], 0);
if (gl.glCheckFramebufferStatus(GL_FRAMEBUFFER) != GL_FRAMEBUFFER_COMPLETE) return false;
}
return gl.glGetError() == GL_NO_ERROR;
}
};
struct Buffers {
const GLESFunctionsTable& gl;
GLuint arena = 0, staging = 0;
explicit Buffers(const GLESFunctionsTable& api) : gl(api) {}
~Buffers() {
// All normal batches finish before cleanup; also retire a partially queued
// batch on an error path before destroying a mapped staging source.
gl.glFinish();
gl.glDeleteBuffers(1, &arena);
gl.glDeleteBuffers(1, &staging);
}
};
void FillVertices(Vector<Vertex>& vertices, Int channel) {
constexpr std::array<Vertex, 6> quad = {{{-1,-1,0,0,0}, {1,-1,0,0,0}, {1,1,0,0,0},
{-1,-1,0,0,0}, {1,1,0,0,0}, {-1,1,0,0,0}}};
for (SizeT k = 0; k < vertices.size(); ++k) {
auto& v = vertices[k];
v = quad[k % 6];
const SizeT q = k / 6;
v.x = v.x / 64.f - 1.f + (2 * (q % 64) + 1) / 64.f;
v.y = v.y / 32.f - 1.f + (2 * (q / 64) + 1) / 32.f;
v.r = channel == 0 ? 1.f : 0.f;
v.g = channel == 1 ? 1.f : 0.f;
v.b = channel == 2 ? 1.f : 0.f;
}
}
BufferOrderingSample Run(const GLESFunctionsTable& gl, const Resources& resources,
const Vector<Uint8>& seed, Int upload, Shape shape) {
BufferOrderingSample sample;
sample.status = BufferOrderingProbeStatus::Failed;
Buffers buffers(gl);
Vector<Vertex> payload(kQuads * 6);
Vector<Uint8> pixels(kSide * kSide * 4);
DrainErrors(gl);
do {
gl.glGenBuffers(1, &buffers.arena);
if (!buffers.arena) break;
gl.glBindBuffer(GL_ARRAY_BUFFER, buffers.arena);
gl.glBufferStorageEXT(GL_ARRAY_BUFFER, kArenaSize, seed.data(), kMapFlags | kDynamicStorage);
sample.error = gl.glGetError();
if (sample.error != GL_NO_ERROR) break;
if (shape != Shape::Unmapped) {
// Deliberately never dereference the destination pointer. All destination
// writes below are ordered GL commands, with no client mapping accesses.
if (!gl.glMapBufferRange(GL_ARRAY_BUFFER, 0, kArenaSize, kMapFlags)) break;
if (shape == Shape::MapThenUnmap && !gl.glUnmapBuffer(GL_ARRAY_BUFFER)) break;
}
gl.glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, sizeof(Vertex),
reinterpret_cast<const void*>(kOffset));
gl.glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex),
reinterpret_cast<const void*>(kOffset + 2 * sizeof(GLfloat)));
gl.glEnableVertexAttribArray(0);
gl.glEnableVertexAttribArray(1);
void* sourceMap = nullptr;
if (upload != 0) {
gl.glGenBuffers(1, &buffers.staging);
if (!buffers.staging) break;
gl.glBindBuffer(GL_COPY_READ_BUFFER, buffers.staging);
if (upload == 1) {
gl.glBufferStorageEXT(GL_COPY_READ_BUFFER, kSlots * kPayloadSize, nullptr, kMapFlags);
sourceMap = gl.glMapBufferRange(GL_COPY_READ_BUFFER, 0, kSlots * kPayloadSize, kMapFlags);
if (!sourceMap) break;
} else {
gl.glBufferData(GL_COPY_READ_BUFFER, kSlots * kPayloadSize, nullptr, GL_STREAM_DRAW);
}
gl.glBindBuffer(GL_COPY_WRITE_BUFFER, buffers.arena);
}
sample.error = gl.glGetError();
if (sample.error != GL_NO_ERROR) break;
for (Int batch = 0; batch < kBatches; ++batch) {
for (Int slot = 0; slot < kSlots; ++slot) {
FillVertices(payload, (batch * kSlots + slot) % 3);
if (shape == Shape::FinishBefore || shape == Shape::FinishBoth) gl.glFinish();
if (shape == Shape::BarrierBefore) gl.glMemoryBarrier(GL_ALL_BARRIER_BITS);
if (upload == 0) {
gl.glBufferSubData(GL_ARRAY_BUFFER, kOffset, kPayloadSize, payload.data());
} else {
// No slot is reused until the entire batch has finished on the GPU.
if (upload == 1) {
std::memcpy(static_cast<Uint8*>(sourceMap) + slot * kPayloadSize,
payload.data(), kPayloadSize);
} else {
gl.glBufferSubData(GL_COPY_READ_BUFFER, slot * kPayloadSize,
kPayloadSize, payload.data());
}
gl.glCopyBufferSubData(GL_COPY_READ_BUFFER, GL_COPY_WRITE_BUFFER,
slot * kPayloadSize, kOffset, kPayloadSize);
}
if (shape == Shape::FinishBoth) gl.glFinish();
gl.glBindFramebuffer(GL_FRAMEBUFFER, resources.fbos[slot]);
gl.glClear(GL_COLOR_BUFFER_BIT);
for (Int draw = 0; draw < kDraws; ++draw) gl.glDrawArrays(GL_TRIANGLES, 0, kQuads * 6);
}
// No readback/Finish between subject update/draw pairs. Early readback
// would hide precisely the old-reader/new-writer overlap being tested.
gl.glFinish();
sample.error = gl.glGetError();
if (sample.error != GL_NO_ERROR) break;
for (Int slot = 0; slot < kSlots; ++slot) {
gl.glBindFramebuffer(GL_FRAMEBUFFER, resources.fbos[slot]);
gl.glReadPixels(0, 0, kSide, kSide, GL_RGBA, GL_UNSIGNED_BYTE, pixels.data());
sample.error = gl.glGetError();
if (sample.error != GL_NO_ERROR) break;
const Int channel = (batch * kSlots + slot) % 3;
Uint bad = 0;
for (Int pixel = 0; pixel < kSide * kSide; ++pixel) {
for (Int c = 0; c < 3; ++c) {
const Int expected = c == channel ? 255 : 0;
if (std::abs(Int(pixels[pixel * 4 + c]) - expected) > 8) ++bad;
}
}
++sample.frames;
if (bad != 0) ++sample.badFrames;
sample.badComponents += bad;
}
if (sample.error != GL_NO_ERROR) break;
}
if (sample.error == GL_NO_ERROR && sample.frames == kBatches * kSlots)
sample.status = BufferOrderingProbeStatus::Complete;
} while (false);
if (sample.error == GL_NO_ERROR) sample.error = gl.glGetError();
return sample;
}
String Describe(const BufferOrderingSample& sample) {
if (sample.status == BufferOrderingProbeStatus::NotRun) return "not run";
if (sample.status == BufferOrderingProbeStatus::Failed)
return format("inconclusive (GL error 0x{:x}, {} readbacks)", sample.error, sample.frames);
return format("{}/{} bad FBOs ({} components)", sample.badFrames, sample.frames, sample.badComponents);
}
String DescribeUpload(const BufferOrderingUploadMeasurement& row, Int upload) {
return format("{}: mapped {}, never-mapped {}, Finish-before {}, Finish-both {}, "
"map-then-unmap {}, barrier-before {}", kUploadNames[upload], Describe(row.mapped),
Describe(row.unmapped), Describe(row.finishBefore), Describe(row.finishBoth),
Describe(row.mapThenUnmap), Describe(row.barrierBefore));
}
} // namespace
PersistentBufferOrderingMeasurement ProbePersistentBufferUpdateOrdering(const GLESFunctionsTable& gl) try {
PersistentBufferOrderingMeasurement measurement;
if (!CanProbe(gl)) return measurement;
measurement.supported = true;
StateScope state(gl);
state.Prepare();
Resources resources(gl);
if (gl.glGetError() != GL_NO_ERROR || !resources.Setup()) {
for (auto& row : measurement.uploads) row.unmapped.status = BufferOrderingProbeStatus::Failed;
MGLOG_I("[driver-bug] persistent buffer ordering: setup failed; inconclusive");
return measurement;
}
Vector<Uint8> seed(kArenaSize, 0);
for (Int upload = 0; upload < Int(measurement.uploads.size()); ++upload) {
auto& row = measurement.uploads[upload];
row.unmapped = Run(gl, resources, seed, upload, Shape::Unmapped);
if (row.unmapped.Passed()) {
// A single allocation can miss on Mali. Stop once a mismatch is measured,
// otherwise retry with fresh storage rather than treating one pass as proof.
for (Int attempt = 0; attempt < kAttempts; ++attempt) {
const auto sample = Run(gl, resources, seed, upload, Shape::Mapped);
row.mapped.status = sample.status;
row.mapped.error = sample.error;
row.mapped.frames += sample.frames;
row.mapped.badFrames += sample.badFrames;
row.mapped.badComponents += sample.badComponents;
if (sample.status != BufferOrderingProbeStatus::Complete || sample.badFrames) break;
}
if (row.mapped.status == BufferOrderingProbeStatus::Complete && row.mapped.badFrames) {
row.finishBoth = Run(gl, resources, seed, upload, Shape::FinishBoth);
row.finishBefore = Run(gl, resources, seed, upload, Shape::FinishBefore);
row.mapThenUnmap = Run(gl, resources, seed, upload, Shape::MapThenUnmap);
row.barrierBefore = Run(gl, resources, seed, upload, Shape::BarrierBefore);
}
}
MGLOG_I("[driver-bug] persistent buffer ordering: %s; %s", DescribeUpload(row, upload).c_str(),
row.Detected() ? "detected" : "not detected or inconclusive");
}
return measurement;
} catch (const std::bad_alloc&) {
// The CPU initializer is arena-sized too. An allocation failure must not discard
// the rest of the POST report or turn a partially sampled case into a finding.
MGLOG_I("[driver-bug] persistent buffer ordering: host allocation failed; inconclusive");
PersistentBufferOrderingMeasurement measurement;
measurement.supported = true;
for (auto& row : measurement.uploads) row.unmapped.status = BufferOrderingProbeStatus::Failed;
return measurement;
}
Optional<DriverBugFinding> DescribePersistentBufferOrderingBug(
const PersistentBufferOrderingMeasurement& measurement) {
String detail;
for (Int upload = 0; upload < Int(measurement.uploads.size()); ++upload) {
if (!measurement.uploads[upload].Detected()) continue;
if (!detail.empty()) detail += "; ";
detail += DescribeUpload(measurement.uploads[upload], upload);
}
if (detail.empty()) return std::nullopt;
detail += ". A 128 MiB immutable vertex destination was mapped WRITE|PERSISTENT|COHERENT, "
"but never accessed through its client pointer. Queued uploads/draws corrupt vertex data; "
"identical never-mapped and Finish-before-and-after controls pass. "
"This POST does not enable a workaround. MOBILEGL_DISABLE_LARGE_BUFFER_ADOPTION=1 "
"avoids automatic arena adoption; explicit application mappings remain separate. "
"FBO counts describe this bounded stress probe, not application flicker frequency.";
return DriverBugFinding{"Persistent-mapped vertex buffers lose upload/draw ordering",
DriverBugVerdict::Unfixable, Move(detail)};
}
} // namespace MobileGL::MG_Util::SelfTest
@@ -0,0 +1,66 @@
// MobileGL - MobileGL/MG_Util/SelfTest/PersistentBufferOrderingProbe.h
// Copyright (c) 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
#pragma once
#include "DriverBugProbes.h"
#include <array>
namespace MobileGL::MG_Util::SelfTest {
enum class BufferOrderingProbeStatus : Uint8 { NotRun, Complete, Failed };
struct BufferOrderingSample {
BufferOrderingProbeStatus status = BufferOrderingProbeStatus::NotRun;
Uint frames = 0; // Independent FBO readbacks, not draw calls or application frames.
Uint badFrames = 0;
Uint badComponents = 0;
GLenum error = GL_NO_ERROR;
Bool Passed() const { return status == BufferOrderingProbeStatus::Complete && badFrames == 0; }
};
struct BufferOrderingUploadMeasurement {
BufferOrderingSample unmapped;
BufferOrderingSample mapped;
BufferOrderingSample finishBefore;
BufferOrderingSample finishBoth;
BufferOrderingSample mapThenUnmap;
BufferOrderingSample barrierBefore;
Bool Detected() const {
return unmapped.Passed() && finishBoth.Passed() &&
mapped.status == BufferOrderingProbeStatus::Complete && mapped.badFrames != 0;
}
};
struct PersistentBufferOrderingMeasurement {
Bool supported = false;
// SubData; CopyBufferSubData from coherent persistent staging; CopyBufferSubData
// from ordinary SubData staging. Each has its OWN otherwise-identical controls.
std::array<BufferOrderingUploadMeasurement, 3> uploads;
};
// POST-only: native GLES calls, no MobileGL buffers, renderer-name rules or config changes.
// The Mali r54p1 finding: updating an immutable vertex arena that has been persistently
// mapped can corrupt queued draws even when the application never accesses that mapping.
// Queue eight update/draw pairs into separate FBOs BEFORE any Finish/readback, then check
// every pixel of both old and new draws. Staging slots never overlap while in flight.
//
// Each upload runs a never-mapped control with identical storage flags. Try up to three
// fresh mapped allocations to catch intermittent failures. On corruption, measure explicit
// waits, map-then-unmap and a barrier as diagnostics. Only a passing never-mapped AND
// Finish-before-and-after control permits a finding. Setup/GL failures are inconclusive.
// Explicit allocations are one 128 MiB arena, its initializer, and small staging/FBOs;
// allocations, batches and draws are bounded. Every touched GL state is restored.
PersistentBufferOrderingMeasurement ProbePersistentBufferUpdateOrdering(
const MG_External::GLESFunctionsTable& gl);
// Used by the POST collector. A report never labels an inconclusive sample as a bug.
Optional<DriverBugFinding> DescribePersistentBufferOrderingBug(
const PersistentBufferOrderingMeasurement& measurement);
} // namespace MobileGL::MG_Util::SelfTest
@@ -84,8 +84,18 @@ namespace MobileGL {
struct BlockPlan {
Instruction* structType = nullptr;
uint32_t storageClass = 0;
// A bounded block's length in words. For an open-ended block - one whose
// last member is a runtime array - the FIXED PREFIX in words, i.e. the
// runtime array's own offset, which is where its element 0 starts.
uint32_t wordCount = 0;
bool openEnded = false;
// The original runtime array's stride in words; what one element of it
// steps by, and what its word count divides by to become a length.
uint32_t tailStrideWords = 0;
std::vector<ChainPlan> chains;
// The OpArrayLength users of an open-ended block's variables, which count
// WORDS once the member is a `uint[]` and so have to be rewritten too.
std::vector<Instruction*> arrayLengths;
};
bool IsDoubleType(const Instruction* type) {
@@ -178,8 +188,9 @@ namespace MobileGL {
}
// Byte size of a type as it is laid out INSIDE a block, or 0 when this pass
// cannot describe it (a runtime array, a width it does not carry, a matrix with
// no stride or a row-major one).
// cannot describe it (a runtime array - the one place a block may have one is
// its last member, which MeasureBlock handles above this - a width it does not
// carry, a matrix with no stride or a row-major one).
uint32_t LaidOutByteSize(IRContext* context, const TypeCursor& cursor) {
const Instruction* type = context->get_def_use_mgr()->GetDef(cursor.typeId);
if (type == nullptr) return 0;
@@ -231,8 +242,17 @@ namespace MobileGL {
}
// Whether this type decomposes into scalars the rewrite can move one word at a
// time, counting them so a whole-aggregate access can be refused before it is
// expanded.
// time. |leafCount| counts them, so a whole-aggregate access can be refused
// before it is expanded; passing NULL asks the SHAPE question alone - is this
// type addressable at all - and then identical array elements and vector
// components are walked once instead of once each, because the answer cannot
// differ between them and the walk of a big one would not be free.
//
// The two questions are separate because only a LOAD or a STORE expands into
// leaves, and the cap bounds one of those. How large a runtime array's element
// is says nothing about how many scalars a single access to it moves, so
// MeasureBlock asks for the shape and BuildPlans applies the cap where it
// belongs - per chain, to the type that chain actually names.
bool CanDecompose(IRContext* context, const TypeCursor& cursor, uint32_t* leafCount) {
const Instruction* type = context->get_def_use_mgr()->GetDef(cursor.typeId);
if (type == nullptr) return false;
@@ -240,12 +260,15 @@ namespace MobileGL {
case spv::Op::OpTypeInt:
case spv::Op::OpTypeFloat:
if (ScalarByteSize(type) == 0) return false;
if (leafCount == nullptr) return true;
++*leafCount;
return *leafCount <= kMaxLeavesPerAccess;
case spv::Op::OpTypeVector: {
TypeCursor component;
component.typeId = type->GetSingleWordInOperand(0);
for (uint32_t i = 0; i < type->GetSingleWordInOperand(1); ++i) {
const uint32_t repeats =
leafCount == nullptr ? 1u : type->GetSingleWordInOperand(1);
for (uint32_t i = 0; i < repeats; ++i) {
if (!CanDecompose(context, component, leafCount)) return false;
}
return true;
@@ -257,7 +280,9 @@ namespace MobileGL {
}
TypeCursor column;
column.typeId = type->GetSingleWordInOperand(0);
for (uint32_t i = 0; i < type->GetSingleWordInOperand(1); ++i) {
const uint32_t repeats =
leafCount == nullptr ? 1u : type->GetSingleWordInOperand(1);
for (uint32_t i = 0; i < repeats; ++i) {
if (!CanDecompose(context, column, leafCount)) return false;
}
return true;
@@ -273,10 +298,12 @@ namespace MobileGL {
context->get_constant_mgr()->FindDeclaredConstant(type->GetSingleWordInOperand(1));
if (length == nullptr || length->AsIntConstant() == nullptr) return false;
const uint32_t count = length->AsIntConstant()->GetU32BitValue();
if (count == 0 || count > kMaxLeavesPerAccess) return false;
if (count == 0) return false;
if (leafCount != nullptr && count > kMaxLeavesPerAccess) return false;
TypeCursor element = cursor;
element.typeId = type->GetSingleWordInOperand(0);
for (uint32_t i = 0; i < count; ++i) {
const uint32_t repeats = leafCount == nullptr ? 1u : count;
for (uint32_t i = 0; i < repeats; ++i) {
if (!CanDecompose(context, element, leafCount)) return false;
}
return true;
@@ -300,6 +327,63 @@ namespace MobileGL {
}
}
// Measures the block struct itself. A bounded block reports its laid-out byte
// size; a block whose LAST member is a runtime array - the only place GLSL lets
// one stand, and the only place SPIR-V lets a Block have one - reports the byte
// offset that array starts at and says so through |openEnded|, with the array's
// stride alongside. A runtime array anywhere else, one without a stride the
// words can step by, or one whose element the rewrite could not take apart is a
// shape this pass does not describe, and so is a bounded block it cannot size.
bool MeasureBlock(IRContext* context, const Instruction* structType, uint32_t* bytes,
bool* openEnded, uint32_t* tailStrideBytes) {
*bytes = 0;
*openEnded = false;
*tailStrideBytes = 0;
const uint32_t structId = structType->result_id();
const uint32_t memberCount = structType->NumInOperands();
uint64_t end = 0;
for (uint32_t member = 0; member < memberCount; ++member) {
uint32_t offset = 0;
if (!TryGetMemberDecorationLiteral(context, structId, member, spv::Decoration::Offset,
&offset)) {
return false;
}
const TypeCursor cursor = MemberCursor(context, structType, member);
const Instruction* type = context->get_def_use_mgr()->GetDef(cursor.typeId);
if (type == nullptr) return false;
if (type->opcode() == spv::Op::OpTypeRuntimeArray) {
if (member + 1 != memberCount) return false;
uint32_t stride = 0;
if (!TryGetDecorationLiteral(context, cursor.typeId, spv::Decoration::ArrayStride,
&stride) ||
stride == 0 || stride % kWordBytes != 0) {
return false;
}
// The member's own matrix decorations describe the array's ELEMENTS,
// exactly as they do for a bounded array of matrices. Only the shape
// is asked for: how big one element is decides nothing about how
// many scalars one access moves, and a leaf cap here would decline a
// block over a member the shader may never read whole.
TypeCursor element = cursor;
element.typeId = type->GetSingleWordInOperand(0);
if (!CanDecompose(context, element, nullptr)) return false;
// Element 0 has to start past every fixed member, or the words the
// prefix owns and the words the array owns would overlap.
if (offset < end) return false;
end = offset;
*openEnded = true;
*tailStrideBytes = stride;
break;
}
const uint32_t size = LaidOutByteSize(context, cursor);
if (size == 0) return false;
end = std::max<uint64_t>(end, static_cast<uint64_t>(offset) + size);
}
if (end > kMaxBlockBytes) return false;
*bytes = static_cast<uint32_t>(end);
return true;
}
bool TypeContainsFloat64(IRContext* context, uint32_t typeId,
std::unordered_set<uint32_t>& visiting) {
const Instruction* type = context->get_def_use_mgr()->GetDef(typeId);
@@ -366,6 +450,9 @@ namespace MobileGL {
switch (type->opcode()) {
case spv::Op::OpTypeArray:
// A runtime array steps exactly like a bounded one; only its end is
// unknown, and a chain never needs that.
case spv::Op::OpTypeRuntimeArray:
if (!TryGetDecorationLiteral(context, cursor.typeId, spv::Decoration::ArrayStride,
&stride)) {
return false;
@@ -611,6 +698,39 @@ namespace MobileGL {
}
}
// Replaces an OpArrayLength of an open-ended block with the element count
// of the ORIGINAL runtime array. The instruction now counts the words of
// the flattened `uint[]`, so the length is `(words - prefix) / stride`, in
// unsigned arithmetic and clamped at zero when the bound range does not
// even reach the array's offset - a wrapped subtraction would otherwise
// report a few billion elements. The division floors, which is what GL
// defines `.length()` as for a range that is not a whole number of
// elements. A fresh OpArrayLength is issued rather than the old one re-aimed,
// so the uses being redirected are never the ones the arithmetic just made.
void RewriteArrayLength(Instruction* arrayLength, uint32_t prefixWords, uint32_t strideWords) {
InstructionBuilder builder(m_context, arrayLength, kPreservedAnalyses);
const uint32_t variableId = arrayLength->GetSingleWordInOperand(0);
const uint32_t wordsId = m_context->TakeNextId();
builder.AddInstruction(MakeUnique<Instruction>(
m_context, spv::Op::OpArrayLength, m_uintTypeId, wordsId,
std::initializer_list<Operand>{{SPV_OPERAND_TYPE_ID, {variableId}},
{SPV_OPERAND_TYPE_LITERAL_INTEGER, {0u}}}));
uint32_t count = wordsId;
if (prefixWords != 0) {
const uint32_t prefixId = UintConstant(prefixWords);
const uint32_t past = Binary(builder, spv::Op::OpISub, m_uintTypeId, count, prefixId);
const uint32_t tooShort =
Binary(builder, spv::Op::OpULessThan, m_boolTypeId, count, prefixId);
count = Select(builder, tooShort, UintConstant(0), past);
}
if (strideWords != 1) {
count = Binary(builder, spv::Op::OpUDiv, m_uintTypeId, count,
UintConstant(strideWords));
}
m_context->ReplaceAllUsesWith(arrayLength->result_id(), count);
m_context->KillInst(arrayLength);
}
private:
uint32_t ComponentWords(uint32_t componentTypeId) {
return ScalarByteSize(m_context->get_def_use_mgr()->GetDef(componentTypeId)) /
@@ -788,38 +908,72 @@ namespace MobileGL {
return false;
}
// A fresh `uint[length]` with ArrayStride 4, spliced in immediately BEFORE the
// block that will name it - SPIR-V has no forward references between types, so
// appending it at the end of the section would make the module invalid. A
// duplicate OpTypeArray is legal (SPIR-V 2.8 exempts aggregates from the
// uniqueness rule, and so does spirv-val), so no search for an existing one is
// needed; the LENGTH CONSTANT is not exempt, and if the module already declares
// it after the block there is nowhere legal to put the array - the block is then
// declined and keeps today's behaviour. Returns 0 for that, and for a uint type
// that is itself declared too late.
// A fresh `uint[length]` with ArrayStride 4 - or, for an open-ended block, a
// `uint[]` runtime array with the same stride and no length at all - spliced in
// immediately BEFORE the block that will name it: SPIR-V has no forward
// references between types, so appending it at the end of the section would make
// the module invalid. A duplicate OpTypeArray or OpTypeRuntimeArray is legal
// (SPIR-V 2.8 exempts aggregates from the uniqueness rule, and so does
// spirv-val), so no search for an existing one is needed; the LENGTH CONSTANT is
// not exempt, and if the module already declares it after the block there is
// nowhere legal to put the array - the block is then declined and keeps today's
// behaviour. Returns 0 for that; an open-ended block has no length constant to
// place, so that reason cannot reach it.
//
// The `uint` element type is a different matter, and only for an OPEN-ENDED
// block. The front end declares types in first-use order, so a block that is the
// first thing a shader touches sits BEFORE the module's `uint` (or the module has
// none, and the one the pass asked for was appended at the end). Declining there
// would send exactly the buffers this rewrite exists for back to the demotion on
// nothing but where they stand in the source. OpTypeInt has no operands, and
// nothing that names it can precede where it was, so moving it up in front of the
// block is always legal. A BOUNDED block keeps declining instead: that is what it
// has always done, and widening it is a change to a path this one does not need.
//
// NOTHING IS WRITTEN until every reason to decline has been ruled out, so a block
// this returns 0 for leaves the module as it found it - which is what lets
// Process() truthfully report SuccessWithoutChange for a module of only those.
uint32_t CreateWordArrayTypeBefore(IRContext* context, Instruction* structType,
uint32_t uintTypeId, uint32_t length) {
if (!DeclaredBefore(context, uintTypeId, structType->result_id())) return 0;
uint32_t uintTypeId, uint32_t length, bool openEnded) {
Instruction* uintType = context->get_def_use_mgr()->GetDef(uintTypeId);
if (uintType == nullptr || uintType->opcode() != spv::Op::OpTypeInt) return 0;
const bool hoistUint = !DeclaredBefore(context, uintTypeId, structType->result_id());
if (hoistUint && !openEnded) return 0;
auto* constantMgr = context->get_constant_mgr();
const spvtools::opt::analysis::Type* uintType = context->get_type_mgr()->GetType(uintTypeId);
if (uintType == nullptr) return 0;
const spvtools::opt::analysis::Constant* lengthConstant =
constantMgr->GetConstant(uintType, {length});
if (lengthConstant == nullptr) return 0;
uint32_t lengthConstantId = 0;
if (!openEnded) {
auto* constantMgr = context->get_constant_mgr();
const spvtools::opt::analysis::Type* uintDescriptor =
context->get_type_mgr()->GetType(uintTypeId);
if (uintDescriptor == nullptr) return 0;
const spvtools::opt::analysis::Constant* lengthConstant =
constantMgr->GetConstant(uintDescriptor, {length});
if (lengthConstant == nullptr) return 0;
Module::inst_iterator position = PositionOf(context, structType);
if (position == context->types_values_end()) return 0;
Instruction* lengthInst = constantMgr->GetDefiningInstruction(lengthConstant, 0, &position);
if (lengthInst == nullptr) return 0;
if (!DeclaredBefore(context, lengthInst->result_id(), structType->result_id())) return 0;
Module::inst_iterator position = PositionOf(context, structType);
if (position == context->types_values_end()) return 0;
// Created in front of the block when it is not there yet, so the only way
// this declines is a constant the module already declares after it.
Instruction* lengthInst =
constantMgr->GetDefiningInstruction(lengthConstant, 0, &position);
if (lengthInst == nullptr) return 0;
if (!DeclaredBefore(context, lengthInst->result_id(), structType->result_id())) return 0;
lengthConstantId = lengthInst->result_id();
}
const uint32_t arrayTypeId = context->TakeNextId();
if (arrayTypeId == 0) return 0;
auto arrayType = MakeUnique<Instruction>(
context, spv::Op::OpTypeArray, 0, arrayTypeId,
std::initializer_list<Operand>{{SPV_OPERAND_TYPE_ID, {uintTypeId}},
{SPV_OPERAND_TYPE_ID, {lengthInst->result_id()}}});
if (hoistUint) uintType->InsertBefore(structType);
std::unique_ptr<Instruction> arrayType =
openEnded
? MakeUnique<Instruction>(
context, spv::Op::OpTypeRuntimeArray, 0, arrayTypeId,
std::initializer_list<Operand>{{SPV_OPERAND_TYPE_ID, {uintTypeId}}})
: MakeUnique<Instruction>(
context, spv::Op::OpTypeArray, 0, arrayTypeId,
std::initializer_list<Operand>{{SPV_OPERAND_TYPE_ID, {uintTypeId}},
{SPV_OPERAND_TYPE_ID, {lengthConstantId}}});
Instruction* inserted = structType->InsertBefore(std::move(arrayType));
context->AnalyzeDefUse(inserted);
context->get_decoration_mgr()->AddDecorationVal(
@@ -948,8 +1102,14 @@ namespace MobileGL {
Instruction* structType = defUseMgr->GetDef(structId);
TypeCursor blockCursor;
blockCursor.typeId = structId;
const uint32_t blockBytes = LaidOutByteSize(context, blockCursor);
if (blockBytes == 0 || blockBytes % kWordBytes != 0) {
uint32_t blockBytes = 0;
bool openEnded = false;
uint32_t tailStrideBytes = 0;
// An open-ended block whose runtime array is its only member measures a
// prefix of 0 bytes and is perfectly describable; only a BOUNDED block of
// no bytes is not, and MeasureBlock already refuses to size one of those.
if (!MeasureBlock(context, structType, &blockBytes, &openEnded, &tailStrideBytes) ||
blockBytes % kWordBytes != 0 || (!openEnded && blockBytes == 0)) {
MGLOG_D("[spirv] storage block %%%u holds a double but its byte layout cannot be "
"described exactly; left to the fp64 demotion",
structId);
@@ -960,11 +1120,15 @@ namespace MobileGL {
plan.structType = structType;
plan.storageClass = storageClassByStruct[structId];
plan.wordCount = blockBytes / kWordBytes;
plan.openEnded = openEnded;
plan.tailStrideWords = tailStrideBytes / kWordBytes;
const uint32_t lastMember = structType->NumInOperands() - 1;
bool expressible = true;
for (Instruction* variable : variablesByStruct[structId]) {
std::vector<Instruction*> chains;
std::unordered_set<uint32_t> seenChains;
std::unordered_set<uint32_t> seenLengths;
defUseMgr->ForEachUser(variable, [&](Instruction* user) {
if (!expressible) return;
switch (user->opcode()) {
@@ -982,6 +1146,27 @@ namespace MobileGL {
}
expressible = false;
return;
case spv::Op::OpArrayLength: {
// Only an open-ended block has a length to ask for, and
// only of its last member; the result has to be the 32-bit
// uint the rewrite's arithmetic is typed in, which is the
// only result type the instruction allows anyway.
const Instruction* resultType = defUseMgr->GetDef(user->type_id());
const bool isUint = resultType != nullptr &&
resultType->opcode() == spv::Op::OpTypeInt &&
resultType->GetSingleWordInOperand(0) == 32u &&
resultType->GetSingleWordInOperand(1) == 0u;
if (openEnded && isUint && user->NumInOperands() >= 2 &&
user->GetSingleWordInOperand(0) == variable->result_id() &&
user->GetSingleWordInOperand(1) == lastMember) {
if (seenLengths.insert(user->result_id()).second) {
plan.arrayLengths.push_back(user);
}
return;
}
expressible = false;
return;
}
default:
expressible = false;
return;
@@ -1058,16 +1243,25 @@ namespace MobileGL {
Emitter emitter(irContext, uintTypeId, boolTypeId, floatTypeId);
bool modified = false;
// Every block declines before anything is written for it, so |touched| only ever
// parts company with |modified| on a shape that cannot happen without the module
// running out of ids - and even then the status must not claim the bytes are
// untouched, because the caller relies on that to skip invalidating its analyses.
bool touched = false;
for (BlockPlan& plan : plans) {
const uint32_t structId = plan.structType->result_id();
const uint32_t arrayTypeId =
CreateWordArrayTypeBefore(irContext, plan.structType, uintTypeId, plan.wordCount);
const uint32_t arrayTypeId = CreateWordArrayTypeBefore(
irContext, plan.structType, uintTypeId, plan.wordCount, plan.openEnded);
if (arrayTypeId == 0) {
// Nothing was written for it, so the module is still the one that came in.
MGLOG_D("[spirv] storage block %%%u: no legal place for the flattened word array; "
"left to the fp64 demotion",
structId);
continue;
}
// Past this point the module HAS been written to, so an abandoned block would
// leave a dead type behind - the status has to say so even then.
touched = true;
const uint32_t wordPointerTypeId = irContext->get_type_mgr()->FindPointerToType(
uintTypeId, static_cast<spv::StorageClass>(plan.storageClass));
if (wordPointerTypeId == 0) continue;
@@ -1095,6 +1289,9 @@ namespace MobileGL {
}
irContext->KillInst(chainPlan.chain);
}
for (Instruction* arrayLength : plan.arrayLengths) {
emitter.RewriteArrayLength(arrayLength, plan.wordCount, plan.tailStrideWords);
}
const std::vector<spv::Decoration> surviving = SurvivingAccessQualifiers(
irContext, structId, plan.structType->NumInOperands());
@@ -1113,12 +1310,19 @@ namespace MobileGL {
{SPV_OPERAND_TYPE_DECORATION, {static_cast<uint32_t>(kind)}}});
}
modified = true;
MGLOG_D("[spirv] storage block %%%u: flattened into %u words so its 64-bit members keep "
"the byte layout the application bound",
structId, plan.wordCount);
if (plan.openEnded) {
MGLOG_D("[spirv] storage block %%%u: flattened into an open-ended word array (%u-word "
"prefix, %u-word elements) so its 64-bit members keep the byte layout the "
"application bound",
structId, plan.wordCount, plan.tailStrideWords);
} else {
MGLOG_D("[spirv] storage block %%%u: flattened into %u words so its 64-bit members "
"keep the byte layout the application bound",
structId, plan.wordCount);
}
}
if (!modified) {
if (!modified && !touched) {
return Status::SuccessWithoutChange;
}
irContext->InvalidateAnalysesExceptFor(IRContext::kAnalysisNone);
@@ -66,17 +66,32 @@ namespace MobileGL {
// fp32 promise DemoteFloat64Pass already makes - what changes is only that the
// BYTES around the value stay where the application put them.
//
// AN OPEN-ENDED BLOCK - one whose last member is a runtime array, the
// `buffer B { double data[]; }` every unsized storage buffer is spelled as - is
// flattened the same way: the members before the array are the fixed prefix, and
// the flattened member is itself a `uint[]` runtime array, ArrayStride 4, with no
// length for the driver to re-derive. Element i of the original array lives at
// word `prefix + i * stride` of it, which is where the application put it. The
// block's `.length()` is rewritten too, because OpArrayLength on the flattened
// member counts WORDS: it becomes `(words - prefix) / stride` in unsigned
// arithmetic, clamped at zero when the bound range is shorter than the prefix,
// which is the floor GL defines `.length()` as.
//
// DECLINES, leaving the block exactly as it was for DemoteFloat64Pass to handle the
// old way, whenever it meets something it cannot rewrite exactly:
// - a block whose variable is used as anything but an access-chain base (loaded
// whole, handed to a function, asked its OpArrayLength);
// whole, handed to a function), or asked an OpArrayLength it is not open-ended
// for;
// - an access chain that is not rooted at the variable, or whose result feeds
// anything but a plain OpLoad / OpStore (an atomic, OpCopyMemory, a further
// chain);
// - a non-constant index into a struct, a runtime array anywhere in the block, a
// RowMajor matrix (its columns are not contiguous, so a whole-column access is
// not one range), a member width other than 32 or 64 bits, or an offset or
// stride that is not a multiple of 4;
// chain), or one that names a whole runtime array rather than an element of it;
// - a non-constant index into a struct, a runtime array that is not the last
// member of the block itself (nested in a member, or followed by another -
// shapes GLSL cannot spell but SPIR-V can), a runtime array without an
// ArrayStride or whose element the pass cannot decompose, a RowMajor matrix (its
// columns are not contiguous, so a whole-column access is not one range), a
// member width other than 32 or 64 bits, or an offset or stride that is not a
// multiple of 4;
// - a load or store whose type decomposes into more scalars than the cap below,
// so legalizing a block can never explode the module.
//
File diff suppressed because it is too large Load Diff
-302
View File
@@ -1,302 +0,0 @@
# 拆分设计评审记录(MGPipe
> 生成于 2026-09-05,配合同目录 `PLAN.md` 阅读。这一轮的前提是用户的方向修正:backend 应拥有贴近后端 API 的状态机并暴露 gallium 式显式接口;memo/`SharedPtr`/版本计数器无 wire 对应物是要解决的工程问题,不是否定薄后端的理由。
## 1. 候选方案与评分
三个独立方案,三位评审按 5 项加权打分(边界清晰度/架构价值 0.25、改造成本与风险 0.20、性能 0.15、语义完整性 0.20、可增量/monolith 保留/可测试 0.20)。
| 方案 | 角度 | 三位评审加权分 |
|---|---|---|
| MGPipe: a split-first explicit backend interface (server owns its state machine, no MG_State replica) | SPLIT-FIRST PRAGMATIC. Keep PLAN.md's transport/data-plane/sync/present/threading/platform/build design essentially verbatim, and replace on | 8.2 / 8.8 / 8.4 |
| MGPipe: a gallium-faithful explicit interface for MobileGL | GALLIUM-FAITHFUL. Introduce MGPipe — an MGPipeScreen/MGPipeContext pair modelled directly on pipe_screen/pipe_context (CSOs with create/bind | 7.3 / 7.65 / 7.7 |
| MGPipe: a twin-derived explicit backend interface for MobileGL | Backend-native state machine first. The interface is not designed top-down from gallium; it is read off the memo/snapshot/twin structures Di | 8.45 / 8.6 / 8.25 |
### 评审指出的致命缺陷(已在综合稿中处理)
- Design 1 — internal schedule contradiction, and it is the axis this review weighs hardest. Its comparison section claims 'the earliest honest IPC frame on a trivial workload is day ~45-55, and a Minecraft frame ~day 120+'. Its own phase list places the first IPC frame in P11, which follows P0-P10 (8-11 + 10-14 + 8-11 + 12-16 + 12-16 + 9-12 + 35-44 + 24-30 + 26-33 + 8-12 + 10-14 = 217-283 days). The phase list is the binding artifact, so the real first frame is ~day 220. A plan that asks for 260-340 engineer-days with zero IPC value for ten months, against a verified 77-day alternative (PLAN.md P0..P9 sums to exactly 77), will be rejected on schedule regardless of its architectural merit — and its own comparison text obscures that rather than confronting it.
- Design 1 — it takes the one gallium deviation the tree argues against, and takes it on the hottest path. Decomposing RenderStateParameters into blend/depth_stencil/rasterizer CSOs discards a documented layout invariant (ScissorBoxWrittenMask at RenderState.h:363 and ClipDistanceEnabledMask at :369 were deliberately placed in the tail span after LogicOp so DirectGLES's three-span memcmp at :2035-2046 catches them) and turns one 8-byte version compare into three hash computations plus three lookups per state transition. Content-addressing answers the correctness half but not the cost half, and DirectGLES still needs the blob per CSO anyway to diff against the driver and emit only changed GL calls — so the decomposition buys the server a handle compare while the client pays three hashes. Not fatal to the architecture; fatal to the claim that this is the cheapest shape.
- Design 3 — the residual value block is a live semantic hole during the P5-P8 split window with only half a guard. The poison mask catches UNFILLED fields; it does not catch a block whose layout differs between the emitting client and the applying server, which is exactly the failure a union of heterogeneous PODs invites across a compiler/ABI boundary. The design specifies static_assert on sizeof but not on member offsets. Without per-member offsetof asserts (or serializing the block field-wise rather than memcpying it), a padding difference produces silently wrong render state in split mode that the monolith verify harness cannot see, because in monolith mode both sides are the same translation unit.
- Design 3 — P7 (DirectVulkan, 48 days) is roughly half the independent 85-111 estimate for the same work, and it sits on the critical path for the second backend's split support. The design names this honestly and makes P3a the falsification point, which is the right response, but the 192-day total should be read as 192-260 and the plan should state that a P3a overrun by more than 50% re-baselines the whole schedule before P4a starts — which it says, but only in the risk list, not in the headline number.
- All three — the central performance claim is unfalsified and cannot be settled from the tree. Every design argues the per-draw reachability traversal MOVES to the client rather than doubling (as the replica plan's does), and therefore that net CPU is <= monolith. Nothing in the tree measures per-frame bytes or calls: MG_Util/Metrics is format arithmetic and Tracy has zones but no plots. All three correctly put TracyPlot counters in P0, and all three correctly nominate per-thread CPU time rather than wall-clock frame time as the metric. But until those land, every ring size, every batching threshold, the render-state wire granularity decision and the headline CPU argument are estimates. Any adopted plan must treat the P0 counters as a hard prerequisite, not a nice-to-have.
- All three — the server-initiated texture re-mint pull is a genuinely new stall class that the replica plan does not have, and its rate on the real corpus is unmeasured by all three. imageBindableHint pre-empts RequireImageBindableStorage (Managers.cpp:2813), but full format regeneration (:3950-4195) fires on ordinary glTexImage format changes and is not pre-emptible. All three ship the same three mitigations (hint, asynchronous park-and-re-emit so the stall lands on the apply thread, bounded retention LRU) and all three gate it with a scenario plus a published per-case pull counter, which is the right shape. The residual risk is identical across designs and should be tracked as a portfolio risk, not scored against any one of them.
- Design 1 — the render-state CSO decomposition is wrong and its justification is internally inconsistent. I verified both halves of the counter-evidence: DirectGLES.cpp:2025-2050 does a three-span head/blend/tail memcmp guarded by static_assert(is_trivially_copyable_v<RenderStateParameters>), and RenderState.h:355-370 states verbatim that ScissorBoxWrittenMask and ClipDistanceEnabledMask were placed 'Deliberately beside ScissorBoxes so it shares their tail span (after LogicOp) and DirectGLES' span memcmp picks a transition up like any other state.' Design 1 §5.4 then proposes hashing 'the three spans DirectGLES already memcmps' to obtain three CSO handles — but head/blend/tail is not the blend/depth-stencil/rasterizer partition, so the proposed mechanism cannot produce the proposed handles. Beyond the inconsistency, decomposition introduces a hand-maintained field→CSO partition over a ~150-field struct with no completeness tripwire: a field added to RenderStateParameters and not assigned to a CSO is silently never pushed, whereas under the blob it rides along and a sizeof static_assert catches schema drift. Not fatal to the design as a whole — replace this one entry with Design 3's create/bind_render_state and Design 1 becomes competitive.
- Design 2 — handle/data-structure mismatch. MGHandle is defined as the monotone, never-reused GetLifetimeId() (8 B), and the design then claims the six StateBackendObjectRegistry instances and thirteen Magma caches become 'arrays indexed by handle' and that this is what deletes TwinLookupMemo/OwnerEquals/g_fbSlotCache. A sparse monotone u64 cannot index an array; without a dense per-kind slot allocator the server keeps a hash map and retains most of the lookup cost the design books as deleted. The fix is Design 3's PipeHandle{slot, gen} with per-kind dense slots plus reserved bands — same 8 bytes, same ABA guarantee, and it actually delivers the array.
- Design 2 — an asserted factual correction that is itself wrong. It opens by 'correcting' the evidence to 'exactly 71 function pointers plus one capability bool, GLFunctionsTable BackendObject.h:117-278 … not 67, not 73.' Measured: 67 function pointers in that range. Minor in substance, non-trivial in credibility for a design whose entire method is 'I re-measured the tree where the reports disagree.'
- Design 3 — the day-62 milestone is narrower than it reads. Emulations (client vertex/index arrays, primitive-restart rewrite, indirect-count resolve, CopyImage mirror) are deliberately Fatal in split mode until P8, so 'first cross-process frame' means OpenRA on a reduced path. That is a legitimate engineering choice but it must be labelled at the go/no-go, or a stakeholder will read it as 'the split works' when the answer is 'the transport and five object classes work.'
- Design 3 — the 192-day total is the least defensible number in the set, against a refactor-cost evidence range of 202-266 days for the backend work alone plus ~68 for IPC. The design concedes this and names a falsification (P3a overrun >50% ⇒ re-baseline before P4a), which is the right response, but the headline figure should be presented as a range with the P3a checkpoint attached.
- All three — the central performance claim (the per-draw reachability traversal MOVES to the client and gets cheaper rather than doubling) is unmeasured, because the tree has no per-frame byte or call metric at all (MG_Util/Metrics is format arithmetic; Tracy has zones and no plots). All three correctly schedule TracyPlot counters in P0/M0 and all three correctly insist the metric be per-thread CPU time rather than wall clock. No design should be believed on CPU until that lands, and the first real datapoint (render state on both backends) must be a hard go/no-go, not a report.
- All three — loss of PLAN.md's byte-identity monolith gate (nm --defined-only plus stripped .text equality) is unavoidable and all three say so explicitly. This is a shared cost, not a flaw of any one design, and the five-part replacement (purity grep + nm, per-draw field-wise MOBILEGL_PIPE_VERIFY, behavioural A/B across {monolith-pull, monolith-push, split}, per-thread CPU non-regression, coverage/poison/no-raw-pointer-memo asserts) is stronger semantically than what it replaces. It must be written down as a cost in the final doc, not buried.
- DESIGN 1 — MAJOR, not strictly fatal but must be reversed before P0 freezes the header: decomposing RenderStateParameters into blend/depth_stencil/rasterizer CSOs (§1.2 D3, §3.2). Its own evidence contradicts it — RenderState.h:359-368 records that ScissorBoxWrittenMask and ClipDistanceEnabledMask were deliberately placed in the tail span so DirectGLES' three-span memcmp (DirectGLES.cpp:2035-2046, guarded by a static_assert(is_trivially_copyable_v) at :2033) picks a transition up like any other state. Espryt keeps a byte-for-byte value mirror precisely so it can emit only the changed GL calls, so the server must retain the blob per CSO regardless; the decomposition therefore buys a handle compare the versioned blob already provides and adds a span re-hash plus three cache lookups on every GetPipelineStateVersion move. Fix: adopt Design 2/3's versioned blob with a dirty-span mask (Design 3's client LRU makes a repeat cost 12 bytes), and let the server derive whatever CSOs it wants internally.
- DESIGN 2 — CREDIBILITY, not architecture: the opening Verification note asserts 'GLFunctionsTable has exactly 71 function pointers plus one capability bool ... with Present/SetSwapInterval that is 74 members — not 67, not 73' and explicitly overrides the other reports. Measured at dev@81b17c0b: 67 function pointers + 1 Bool = 68 members, 70 with GlobalBackendFunctionsTable. It also states '50 include lines over 18 distinct MG_State headers' where I measure 50 lines over 15 distinct MG_State paths, and carries 169 DirectVulkan pGLContext reads where the actual count is 166 (VulkanRenderer 126 + DirectVulkan 18 + UniformManager 14 + VkRenderPassManager 3 + VkTextureManager 2 + BackendObject_DirectVulkan 2 + VkClearManager 1). A design whose central methodological claim is 'I re-derived this from the tree rather than copying the brief' cannot afford to be wrong in the one place it says so loudest. None of this invalidates the design, but every other unverified number in it now needs an independent check before it is used for sizing.
- DESIGN 3 — SCHEDULE, acknowledged but under-absorbed: P7 (DirectVulkan, all subsystems) is priced at 48 days against the refactor-cost reader's 85-111 for the same scope, and the 192-day total sits below the reader's 202-266 for the backend refactor ALONE. Design 3 names this as a risk and supplies a falsification trigger (re-baseline if P3a overruns >50%), which is the right instinct, but the trigger fires on Espryt's wave-1 and cannot detect a Magma-specific overrun until P7 is already the critical path. Fix: add a second explicit re-baseline gate at P7 midpoint, and price the CTS turnaround (gl44to46 is ~56,271 cases) as a separate line rather than folding it into the phase estimates.
- ALL THREE — completeness gap in the migration mechanism, shared and unaddressed: MG_Backend has 348 pGLContext mentions of which only 290 are arrow uses. All three designs propose a mechanical sed of 'MG_State::pGLContext->' to a macro/alias over '293 sites' and none accounts for the 58 non-arrow uses — the null-guards (Managers.cpp:3608, 3737, 3808, 4663, 8678; BackendObject_DirectVulkan.cpp:388, 788), the MOBILEGL_ASSERT truth tests, the raw-pointer capture at DirectGLES.cpp:146 (MG_State::GLState::GLContext* ctx = MG_State::pGLContext.get()), and the patch-parameter ternaries at Managers.cpp:7120-7131 that sit inside the transpile path. The patch reads are semantically covered by set_patch_state in all three catalogues, but the mechanical step is under-specified and the raw .get() capture defeats an accessor-shaped alias entirely. Whichever design is chosen must enumerate and convert those 58 sites explicitly, and the interface-purity gate must grep for 'pGLContext' (not 'pGLContext->').
- NONE OF THE THREE is fatally incomplete on semantics. Each satisfies all 290 backend reads, both texture-byte channels, the 26 reverse pulls, XFB (CPU accounting client-side, capture writeback as a reply), queries and fences (client-minted, two-valued contract preserved), persistent maps (explicitly quarantined from the refactor, decided by a POST-probed tier), GPU-written buffer reads (conservative client pending set narrowed by an EvGpuWritten reply), share groups (one flat handle space in v1, screen/context split declared in the header from day one), and the composite pipeline program (never crosses; resolved by Core.cpp:592-744 as today). All three correctly identify the server-initiated texture re-mint pull as the one genuinely NEW stall class and mitigate it three ways with a dedicated gate and a per-trace-case counter.
### 评审建议嫁接的要点
- From Design 3 — the Track V / Track H accessor split. Roughly 55% of the class-B reads are value-typed (RenderStateParameters, PixelStoreParameters, IsCapabilityEnabled, GetStencilState, GetColorMaskIndexed, the ~22 Magma singletons) and need no reshaping whatsoever: the client memcpys, the server hands the backend a reference to its own copy. Only the 167 SharedPtr<MG_State...> points need real work. This is the decomposition that makes migration granularity one accessor rather than one subsystem, and it is the load-bearing premise under any split-first schedule. Neither Design 1 nor Design 2 states it.
- From Design 3 — the residual value block with a compile-error retirement. One temporary set_residual_value_state carrying the union of not-yet-migrated value accessors, guarded by static_assert(sizeof(ResidualValueBlock) == MGL_RESIDUAL_BLOCK_SIZE) with the constant bumped DOWN each phase, ending at static_assert(sizeof(...) == 0). This is what lets the split run subsystem by subsystem instead of after a finished refactor, and it is the only temporary in any of the three designs with a mechanical (not procedural) retirement. Add the layout static_assert it omits: the block must be byte-identically laid out on both sides, so assert offsetof for every member, not only sizeof.
- From Design 3 — the PipeInputs::m_filledMask poison. In debug and disaggregated builds, reading a field the tracker never pushed is Fatal{UnmigratedPipeInput, "GetStencilState"} on the first draw. Design 2's G5 written-once bitmask is the same idea, but Design 3's runtime-fatal formulation is the one that cannot be rendered past, and it works during the split window where Design 2's generated comparer needs both models live in one address space.
- From Design 3 — the ordering rule that identity handle-ification precedes the first frame while memo re-keying follows it (P3a/P4a before P5/P6; P3b/P4b after). The wire needs handles; the 28 days of memo re-keying, dirty-flag inversion and program-staleness rework are optimizations that can land behind a working split. This single reordering is worth ~5 weeks of time-to-first-frame and neither other design exploits it.
- From Design 3 — the explicit day-21 hedge: run PLAN.md's P0 verbatim (its hygiene, skeleton, spikes and byte counters are state-model-independent), then MGPipe P1+P2 (15 days), then decide. At day 21 you hold the verify harness proving push works, render state pushed on both backends, a measured monolith per-thread CPU delta on two devices, and the per-accessor cost of Track H sampled. That is a genuine, cheap decision point, and it is the only one offered in the set.
- From Design 1 — the client-side content-addressed CSO cache modelled on Mesa's cso_context/cso_cache, with per-kind caps and LRU eviction issuing delete_*_state. Design 2's render-state LRU is the same idea applied to one blob; Design 1 generalizes it to vertex-elements, samplers and sampler views, and the property that two different programs setting identical state produce ZERO server-side transitions is a real per-draw win worth keeping even while shipping the render-state blob rather than three CSOs.
- From Design 1 — the framing that inproc IS u_threaded_context: a push-only interface recorded into batches and applied on the server thread. Mesa proved this shape can be transparently threaded, and it reframes the monolith render-thread deliverable from 'an IPC side effect' to 'the interface's second consumer'. Worth stating explicitly in whatever plan is adopted, because it is the argument that the interface pays for itself even if the process split never ships.
- From Design 1 — homing each emulation by gallium's own rule (state-tracker side when caps say the driver cannot, driver side when it is a driver lowering) with a named cap bit per decision: kCapPrimitiveRestart, kCapMultiDrawIndirectCount, kCapFloat64VertexAttrib, kCapNeedsHostIndexBytes. That turns the per-backend asymmetry (Magma's deliberately null ResidentSubData, the 8 null slots, PrefersCpuXfbPrimitiveAccounting) from a wart into the mechanism, and it replaces today's implicit slot-nullness capability probes at GL_Query.cpp:471/545/768.
- From Design 2 — PipeCalls.def as one X-macro consumed by five generators (function table, monolith thunks, wire records with per-kind static_assert plus generated runtime bounds checks, the shadow-compare comparer, the written-once mask). Design 3 has the coverage generator but not the comparer/mask generators; generating the semantic gate from the same source as the call table is what stops the gate going stale as the catalogue grows.
- From Design 2 — the D18 exception. Its D-class table is the only one that marks VkRenderPassManager::m_renderbufferResources / VkTextureManager::m_textureResources as UNCHANGED, with the reason (callers cache Resource* across further lookups; a table grow once relocated a cached &layout and BlitFramebuffer silently bailed at 'source image layout undefined'; ska's erase-shift makes it worse, not historical). Whichever plan is adopted must carry that postmortem verbatim into the review checklist, because converting those to slot arrays is exactly the change a refactor makes without reading the comment.
- From Design 2 — the DERIVATION METHOD, adopted as the doc's opening chapter: build the call catalogue by inverting the backends' own key structures (SetupDrawSnapshot VulkanRenderer.h:948-1042, BackendTextureObject::IsDrawSyncClean Managers.h:1003-1020, ResolvedDrawBuffers Managers.h:697-717, ResolvedVertexBindings VulkanRenderer.h:1153-1218, g_syncedRenderStateParameters DirectGLES.cpp:1956, BufferBackendOps BufferObject.h:76-120), not top-down from gallium. This is both the honest justification for every entry and the reason the interface is complete: the inputs to those structures ARE the interface.
- From Design 2 — PipeCalls.def as single source of truth with FIVE generators: function tables, monolith thunks, wire records with per-kind static_assert plus generated runtime bounds checks, the MOBILEGL_PIPE_VERIFY field-wise comparer, and the written-once bitmask. Generating both tripwires removes the hand-maintenance risk that is the design's own biggest exposure. Graft over Design 3's hand-written verify.
- From Design 2 — the explicit two-kinds-of-generation statement: client-owned identity vs the twelve server-only epochs (g_bufferMutationEpoch, g_bufferBackendIdGeneration, g_attachmentBackendIdGeneration, g_backendContextGeneration, m_textureImageEpoch, m_resourceEraseEpoch, m_renderbufferImageEpoch, m_sliceEpochCounter, m_cacheStructureEpoch, m_evictionEpoch, m_recordingGeneration, m_frameSerial) that the client must never be asked about. Write this as a normative interface rule, not prose.
- From Design 2 — D18 marked UNCHANGED with a review-checklist note: VkRenderPassManager::m_renderbufferResources and VkTextureManager::m_textureResources are deliberately node-based std::unordered_map, not the project's open-addressed UnorderedMap, because callers cache Resource* across further lookups (postmortem at VkRenderPassManager.h:375-397, a BlitFramebuffer silently bailing at 'source image layout undefined' after a table grow relocated a cached &layout). It is the only design that explicitly flags 'do not optimise this container back during the refactor.'
- From Design 2 — the dirtySpanMask on the render-state wire. Compose with Design 3's CSO: on a CSO cache MISS ship only the changed spans of the blob plus the previous CSO handle as a base, rather than the full ~1.1 KiB. Cheapest of all three encodings.
- From Design 1 — CAPS-GATED emulation homing, replacing fixed client/server assignment. MGPipeCaps carries kCapPrimitiveRestart, kCapPrimitiveRestartFixedIndex, kCapMultiDraw, kCapMultiDrawIndirectCount, kCapFloat64VertexAttrib, kCapResidentSubData, kCapCpuXfbPrimitiveAccounting, kCapNeedsHostIndexBytes, and each lowering (u_primconvert-style restart rewrite, indirect-count fallback, client-array upload) runs client-side only when the cap says the server cannot. This replaces today's implicit null-slot capability probes at GL_Query.cpp:471/545/768 and makes per-backend asymmetry (Magma's deliberately absent ResidentSubData, VkBufferManager.cpp:104-111) the mechanism rather than a wart.
- From Design 1 — kCapNeedsHostIndexBytes specifically: it prices the monolith/split asymmetry of MGHostSpan honestly (a free pointer in-process, a copy on the wire) so a backend that never needs host index bytes does not pay.
- From Design 1 — the explicit deviations-from-gallium table with a tree citation per row. Keep the format; replace only the render-state row with Design 3's blob-CSO.
- From Design 3 — the render-state shape itself: create_render_state(cso, blob) + bind_render_state(cso, v, pipeV) with a client LRU. Graft into whichever design wins.
- From Design 3 — PipeFramebufferState with a CLIENT-RESOLVED readSurface and inline attachment internalFormats. Two defect classes and one lookup deleted by struct shape alone.
- From Design 3 — Track V / Track H accessor split, per-accessor migration granularity, and MOBILEGL_PIPE_PUSH as a per-subsystem bitmask latched at init like MOBILEGL_BACKEND_TYPE (ConfigLoader.cpp:212-225), so every commit has a same-binary A/B on either backend.
- From Design 3 — every temporary gets a compile-error retirement: PipeInputs::m_filledMask poison giving Fatal{UnmigratedPipeInput, fieldName}, and static_assert(sizeof(ResidualValueBlock) == 0) before the pull path may be deleted. Adopt this rule wholesale; it is the difference between a strangler that finishes and one that ossifies.
- From all three, unchanged — the EvLogLine severity split (level <= WARN lossy, level >= ERROR lossless plus a per-second rate limiter emitting 'N suppressed'), because backend program link failure is surfaced ONLY as MGLOG_E plus a bind-program-0 no-op (Managers.cpp:8091-8126, 8357-8372) and PLAN.md §7.4's uniform lossy policy would silently drop the system's most valuable diagnostic.
- FROM DESIGN 2 — derive the interface from the backends' own key structures, not from gallium top-down. SetupDrawSnapshot (VulkanRenderer.h:948-1042) is a 40-field enumeration of everything Magma must have pinned for a draw; DrawTextureSyncKeys + IsDrawSyncClean (Managers.h:1003-1020) is the same for Espryt's textures; ResolvedDrawBuffers/ResolvedVertexBindings are the vertex-input statement; g_syncedRenderStateParameters is the render-state statement verbatim. This is a stronger completeness argument than any coverage table, and it is what produces the correct blob-not-CSO answer on render state. Design 3 should adopt this as the explicit derivation rationale for its call catalogue.
- FROM DESIGN 2 — PipeCalls.def with five generators from one file: function table, monolith thunks, wire records + per-kind static_assert + generated runtime bounds checks, the MOBILEGL_PIPE_VERIFY field-wise comparer, and the written-once bitmask. Generating the verify comparer and the completeness tripwire from the same declaration as the call list means the gates cannot drift from the interface. Design 3 hand-writes both; it should generate them.
- FROM DESIGN 2 — keying PipeInputs on MEMO KEYS rather than read sites. That is why the pushed block stays ~20 KB with a field set stable across the migration, and it is the reason per-accessor granularity actually works. Design 3's PipeInputs is described per-accessor, which is a larger and less stable field set.
- FROM DESIGN 2 — D18 explicitly marked UNCHANGED with the VkRenderPassManager.h:375-397 postmortem carried verbatim into the review checklist, so nobody 'optimises' m_renderbufferResources/m_textureResources back to the project's open-addressed UnorderedMap. The ska erase-shift behaviour makes that hazard worse, not historical. Neither other design guards this.
- FROM DESIGN 2 — MGHostSpan: one 32-byte accessor for the four host-byte classes (client vertex arrays, client index arrays, indirect/parameter command blocks, index bytes) whose fill policy differs by build. Zero monolith cost (one pointer load), and it is the abstraction that makes the disappearance of the 26 SyncPersistentMappedRange/SyncGpuWrites reverse pulls a mechanical consequence rather than a per-site argument.
- FROM DESIGN 1 — the emulation-homing RULE (gallium's own: state-tracker lowering when a cap says the driver cannot, driver lowering when the driver forces it), with each emulation gated on a named capability bit — kCapPrimitiveRestart, kCapMultiDrawIndirectCount, kCapFloat64VertexAttrib, kCapNeedsHostIndexBytes. Designs 2 and 3 assign emulation ownership case by case; Design 1's rule generalises to a third backend and makes the assignment auditable.
- FROM DESIGN 1 — kCapNeedsHostIndexBytes specifically: it prices the monolith-vs-split asymmetry (a shadow pointer costs nothing in-process, a copy in split) into the interface as a capability, so a backend that never needs host index bytes never pays.
- FROM DESIGN 1 — the explicit 8-deviation ledger (each deviation from gallium named, justified by a file:line or a measured cliff, and numbered). This is the right way to document an interface that will outlive its authors; Designs 2 and 3 justify their deviations inline and less traceably.
- FROM DESIGN 1 — MGPipeCallbacks as a single named struct of 8 reply/event kinds installed at context_create, rather than an ad-hoc event list. In the monolith they are direct calls; in split they are records. This makes the reverse channel a first-class part of the interface rather than an appendix.
- FROM DESIGN 3 (keep) — dense per-kind slots in an 8-byte PipeHandle{slot, gen}. Designs 1 and 2 use sparse 64-bit lifetime ids as the wire handle, which keeps the server on a hash table; dense slots make the server's object tables literal arrays, which is what actually deletes the hashing/ABA layer rather than merely re-keying it. The lifetime id stays client-side as the tracker's own identity.
- FROM DESIGN 3 (keep) — client-resolved readSurface in the framebuffer payload, and static_assert(sizeof(ResidualValueBlock)==0) as the retirement device for a deliberate temporary.
## 2. 对抗性审查(三个视角)
### GL 语义正确性(refuted=False12 条)
- **[major] The headline per-draw cost comparison (§10.2, §5.1) is a static-site-count vs dynamic-call-count category error; the baseline is overstated by roughly an order of magnitude**
- 问题:§10.2's table and §5.1 price today's per-draw state acquisition as "Espryt 124 / Magma 169 accessor calls + version compares + a ~1.2KB three-span memcmp + CurrentUnitBindingsEpoch's per-unit owner walk + Magma's two lossy version sums + ~40 payload accessor walks". 124/169 are STATIC `pGLContext->` call sites (§2.1's own definition), not dynamic per-draw calls. Every one of those costs is already memo-gated in the tree: - `SyncRenderState` returns at the top on a single Uint16 compare (`MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp:2016-2018`: `if (!forceFullPush && !colorMaskWidenDirty && g_hasSyncedRenderState && currentRenderStateVersion == g_syncedRenderStateVersion) return;`). The three memcmps run only when the version moved. - `SyncNeccessaryTextures` steady state is a 6-value key compare plus `PairingsIntact` and a per-entry `IsDrawSyncClean` word compare (`DirectGLES.cpp:1537-1560`); the unit walk runs only on a miss. - `CurrentUnitBindingsEpoch` has a three-value fast gate and only walks owners when the bind generation moved (`DirectGLES.cpp:1421-1426`). - Magma's `TrySetupDrawFastPath` steady state is ~10 accessor calls and ~20 word compares (`MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp:6002-6300`), not 169. - `GetOrCreatePipeline` recomputes the pipeline-state hash only when `GetPipelineStateVersion()` moved (`VulkanRenderer.cpp:4982-4993`), and the "~40 payload accessor walk" at :5155-5200 runs only on a pipeline memo MISS. - `ApplyDynamicDrawStateTail` has a two-level gate: one version compare, then a value key built from one bulk fetch (`VulkanRenderer.cpp:5888-5893`). So the real steady-state pull cost is on the order of 10-25 accessor calls and a few dozen word compares per draw per backend. Comparing that against "1 dirty word test + N set_*" is a much narrower margin than the plan's table implies, and the plan's entire business case (B-R2, the day-24 GO/NO-GO in §0.6/P2, the "traversal is moved, not doubled" claim) is built on the inflated figure.
- 修法:Restate §10.2's table in DYNAMIC terms and stop citing 124/169 as a per-draw cost anywhere in the document (they belong only in §2.1's coupling-surface argument). Add a per-draw dynamic counter (accessor calls executed, memo hit/miss per gate) to P0's TracyPlot deliverable list alongside the byte counters — the plan currently lands byte counters but no call counters, so it will still be guessing at P2. Then make the day-24 GO/NO-GO threshold an ABSOLUTE number (ns/draw of tracker cost measured on both devices) rather than "within the noise of monolith-pull", because relative-to-noise passes trivially when the true baseline is 20 calls, not 124.
- **[major] The tracker is specified as a poll of existing counters, which is the same traversal it claims to eliminate — §5.2 and §10.2 are mutually inconsistent**
- 问题:§1.1/§5.2 state "MG_State 零新增记账" and map every dirty bit onto an existing version counter; §5.4-2 explicitly requires the two high-water-mark walks (`TouchBindPoint`/`GetTouchedBindPointCount`, `NoteUnitTouched`/`GetMaxTouchedUnit`) to stay "in the tracker's walk". That means `m_dirty` is COMPUTED by polling, not SET by the mutators. But §10.2 and §5.1 price the steady state as "one 64-bit dirty word test + N set_* calls". These cannot both be true. `MGPIPE_NEW_SAMPLER_VIEWS` alone is mapped in §5.2 onto `GetContentVersion` + `GetShapeVersion` + `GetTextureParamsVersion` + `GetTextureBindGeneration` + `GetSamplingResolutionGeneration`. The first three are PER-TEXTURE, so computing that one bit requires walking the touched units and reading three counters per bound texture — which is exactly `SetupDrawSnapshot`'s `sampledContentSum`/`sampledParamsSum` walk (`VulkanRenderer.cpp:6253-6254`) that §4.7.3-D14 claims collapses to "one compare", and exactly Espryt's unit list walk. Same for `NEW_VERTEX_BUFFERS` (per-attribute `VertexAttributeVersion` triples) and `NEW_FRAMEBUFFER` (`Array<Uint16,40>` attachment versions). Gallium does not work this way: `st_invalidate_*` sets dirty bits from the GL entry points; `st_validate_state` never polls object versions. The plan adopts gallium's validate-time push but not gallium's dirty-marking, and then quotes gallium's cost.
- 修法:Choose explicitly, in the design document, and price the choice. The correct answer is dirty-MARKING: have MG_Impl's mutating entry points call `MGPipeTracker::MarkDirty(group)` so validate is genuinely O(dirty groups). Then delete the "zero new bookkeeping in MG_State" claim, add the marking-site audit to B-R6 (it is the same completeness obligation as the reconciler, on a larger surface — every GL setter, not every backend read), and let the G5 written-once bitmask plus MOBILEGL_PIPE_VERIFY cover it. If instead polling is kept, §10.2 and §5.1 must be rewritten to say the tracker performs the same per-object walk as today's backend, and the net win reduces to the server-side memo deletions only.
- **[major] The ~115-line unit-bindings epoch machinery is booked as deleted, but it cannot be deleted — only moved to the client**
- 问题:§2.5, §4.7.3-D3 ("结构性删除") and §10.4-1 count `UnitBindingsSnapshot`/`CaptureUnitBindings`/`UnitBindingsUnchanged`/`CurrentUnitBindingsEpoch`/`UnitTextureSyncEntry`/`PairingsIntact` (~115 lines, `DirectGLES.cpp:1372-1489`) as a structural deletion, on the ground that "the push call IS the change signal". That is only true if the client can cheaply decide WHETHER to push. It cannot, for exactly the reason the machinery exists: `GetTextureBindGeneration()` bumps on REDUNDANT rebinds — the comment at `DirectGLES.cpp:1414-1420` records that MC 26.2 rebinds the same sampler around every texture-unit switch. If the tracker keys `set_sampler_views` on the bind generation it will push a full resolved view array on every redundant `glBindSampler`, which in the workload that motivated the machinery is per-batch. To avoid that it must do the same owner-comparison walk — i.e. the code moves to `MG_Impl/Pipe/Tracker.cpp`, it does not disappear. Worse, in split mode a spurious push is not just CPU: `set_sampler_views` is a `kVarTail` record carrying an `MGPSamplerView`-shaped entry per sampled unit, so a redundant push costs hundreds of ring bytes per draw. The same argument applies to `g_fboTextureSyncList` (D8) and, in weaker form, to `ResolvedTextureBindingMemo` (D9): the client needs its own memo keyed on the same epoch to avoid re-resolving completeness (`IsMipmapCompleteForFilter` / `SamplesAsIncompleteTexture` / `IsUndefinedDefaultTexture`) per draw, since §5.5 puts view resolution on the client.
- 修法:Move these rows from "deleted" to "relocated" in §2.5, §4.7.3 and §10.4-1, and subtract them from the "~550 lines deleted" ledger (which then drops to roughly 350-400, of which the genuinely-deleted parts are TwinLookupMemo×3 + OwnerEquals, the six registry GC sweeps, `sourcePin`, and the placeholder-texture puppetry). Add the client-side epoch memo and its key to §5.5 as an explicit deliverable of P3b/P4b, and add a `set_sampler_views` push-count-per-frame counter to the P0 counter list so a regression to per-batch pushing is visible immediately.
- **[major] D-B1's whole-block RenderStateCso re-creates the exact regression the two version counters exist to prevent**
- 问题:`RenderState.h:519-528` documents why there are two counters: "Viewport, scissor, depth range, blend colour, line width, polygon offset, stencil write mask, the clear values, hints and the point-size family are all either dynamic pipeline state or not pipeline state at all, so changing one of them must not evict a cached pipeline. Keeping one counter for both made a glViewport call knock the next draw off the pipeline memo AND the draw fast path." Verified: `RenderState.cpp:639-640, 702-735` and neighbours bump only `++m_version` for those setters, never `BumpVersions()`. D-B1 makes the CSO identity the CONTENT of the whole `RenderStateParameters` block. Therefore `glViewport`, `glScissor`, `glBlendColor`, `glClearColor`, `glLineWidth`, `glStencilMask` and `glPolygonOffset` each produce a different content hash, hence a different CSO handle. Consequences: (a) a 64-entry client LRU (§4.5.2/§4.1) keyed on a block containing 16 viewports + 16 scissor boxes + 16 depth ranges + clear values will thrash under Iris shader packs and shadow-cascade rendering, which change viewport/scissor many times per frame; (b) each LRU miss re-sends a ~1.2 KB `create_render_state` blob; (c) a new CSO handle invalidates any per-CSO pipeline-hash memo the server keeps, which is the very thing §4.5.2 promises ("Magma 每 CSO 算一次 pipeline hash"). D-B1 and D3 ("CSO 边界跟 Vulkan 动态状态走") therefore contradict each other inside the same document.
- 修法:Key the CSO on the pipeline-relevant subset only — the same field set `ComputePipelineStateHash` already enumerates (`VulkanRenderer.cpp:4826-4906`) and the same subset `m_pipelineStateVersion` guards — and carry viewport/scissor/depth-range/blend-colour/line-width/polygon-offset/stencil-ref-and-write-mask as a separate `set_dynamic_state` payload, mirroring `DynamicStateShadow` and `ApplyDynamicDrawStateTail`. Accept and state that this breaks the "reuse the existing head/blend/tail span division" argument (the head span starts with `Viewports` and also contains `LineWidth`/`PointSize`/`PolygonOffset*`, so the existing spans do not align with the pipeline/dynamic split); the span-memcmp layout invariant then applies inside the pipeline-subset blob and must be re-derived, which is cheaper than paying a CSO per glViewport.
- **[major] Content-addressed CSOs make the single path the code names as hottest more expensive, not cheaper**
- 问题:`DirectGLES.cpp:2029-2032` names the target: "a per-draw blend toggle used to re-diff all ~40 pieces of state field by field on every draw (Blaze3D brackets every batch with glEnable/glDisable(GL_BLEND), making this the hottest thing mc_state_toggle did)". Verified that a real toggle does move the version — `SET_CAPABILITY` short-circuits only on a REDUNDANT set (`RenderState.cpp:311-313`), and enable/disable pairs are not redundant. Today's cost on that path: three memcmps over ~1.2 KB, server-side, once per draw whose version moved. Under the plan the client must find the CSO by hashing, and it cannot shortcut via the version: `m_version` is monotonic (`++m_version`), so a version value never repeats and no version→CSO memo can ever hit on the alternating-content pattern. So the client pays an xxHash over the same ~1.2 KB plus a `ska::flat_hash_map` probe on every such draw. Then, because the handle changed, Espryt's 693-line body still runs its span memcmp — P2's deliverable explicitly keeps it "一行不动". Net: a full-block hash and a map probe ADDED, nothing removed. For Magma it is worse in a subtler way: `ComputePipelineStateHash` folds roughly 25-30 words out of one bulk fetch (`VulkanRenderer.cpp:4826-4906`) — far cheaper than an xxHash of the full 1.2 KB block. Moving pipeline-hash computation behind a CSO handle therefore trades a cheap server-side hash for an expensive client-side one on precisely the toggle pattern §4.5.2 cites as the justification.
- 修法:Do not content-address on the full block. Derive the CSO key from the pipeline-subset field list (reuse `ComputePipelineStateHash`'s enumeration verbatim so the two can never disagree) plus the two version counters, and let the CSO cache hold the small key. Alternatively drop content addressing on the hot path entirely: mint a CSO per distinct `m_pipelineStateVersion` value and run a dedupe/coalesce pass off the draw path at frame boundaries. Either way, P2's acceptance must include a dedicated microbenchmark of the Blaze3D toggle pattern (enable/draw/disable/draw at MC batch rates) on both devices, because that single pattern decides whether §10.2's central claim survives.
- **[major] §5.8.1's blanket reconcile rule adds a per-frame round trip on the *IndirectCount path that the monolith does not pay, on a named trace fixture**
- 问题:§5.8.1 asserts that "every client-side scan/rewrite in the table above immediately follows `SyncPersistentMappedRange()` + `SyncGpuWrites()` in the monolith" and mandates "publish → wait for appliedSeq → drain events" at each. That is true for the restart rewrite and multi-draw flattening (`DirectGLES.cpp:4412-4413`, `MultiDraw.cpp:498-499`, `VulkanRenderer.cpp:3431, 4159`), but it is NOT true for the `*IndirectCount` CPU fallback, which §5.8's table also assigns to the client. Verified: `MultiDrawElementsIndirectCount` (`DirectGLES.cpp:4667-4668`) calls only `drawBuffer->SyncPersistentMappedRange(); parameterBuffer->SyncPersistentMappedRange();` and then reads the count and the command block straight out of `MappedData()` (`:4690-4694`). There is no `SyncGpuWrites()` and therefore no stall today. `SyncGpuWrites` is what triggers `ReadbackFromGpu` (`BufferObject.cpp:265-274`). If the plan applies its blanket rule here, every `glMultiDrawElementsIndirectCount` acquires a publish-and-wait round trip. The trace corpus contains `minecraft-1.21.1-neoforge-create-indirect-in-world` — a Create/Flywheel fixture whose indirect and parameter buffers are compute-written each frame — so this would be a per-frame, per-batch synchronous round trip on a named acceptance fixture, and the plan's §9.2 #10 dismisses it as "常见情况不 pending,代价为零".
- 修法:Replace the blanket rule with a per-site table that reproduces the monolith's reconcile set exactly: `SyncPersistentMappedRange` only where the monolith calls only that, `SyncPersistentMappedRange + SyncGpuWrites` where the monolith calls both. Add the round-trip counter for the indirect-count path to the P8 acceptance and require it to read zero on `create-indirect`. Separately, note that the monolith's omission of `SyncGpuWrites` there may itself be a latent correctness gap — but that is a `dev` question, not something the split should silently fix by adding a stall.
- **[major] The day-24 GO/NO-GO measures the one subsystem where push's benefit is smallest and its overhead is largest**
- 问题:§0.5 and P2's acceptance make the day-24 decision on "monolith-push within monolith-pull's noise on p50 and p99 per-thread CPU" after converting only render state. But render state is the subsystem where push helps LEAST and the plan's CSO design costs MOST: - Espryt already holds a byte-exact value mirror with a version early-out and a span memcmp (`DirectGLES.cpp:2016-2047`) — there is almost nothing to save. - Magma already caches the pipeline-state hash under the version (`VulkanRenderer.cpp:4982-4993`) and gates the dynamic tail twice (`:5888-5893`). - The CSO overheads identified above (full-block hash on the client, CSO churn on glViewport) land squarely and only on this subsystem. So a GREEN P2 does not validate the claim it gates (that Track H handle-ization pays for itself across 200+ days), and a RED P2 is more likely to indict the CSO design than the push model. Either way the decision the gate is supposed to inform is not the decision it measures. §0.6 also asserts the fallback cost is "only 16 of the 24 days", which understates it: P1's 293-site sed plus the 58 hand-converted non-arrow sites plus the G4/G5 generators are not reusable by the earlier (since-dropped) design.
- 修法:Extend the day-24 gate to require both (a) the render-state conversion and (b) one Track H slice — the plan already prices the cheapest ones: 0d handle infrastructure (5-7 days, §6.4) and Magma's `VertexInputStateFactory`/`VaoDrawMemo` re-key (2-3 days, §6.5-4, explicitly "低(纯结构性收益)"). That yields a real Track H unit cost, which is what B-R14's re-baselining actually needs. Add an explicit exit criterion that separates "push is slower" from "the CSO design is slower" by running P2 with content addressing disabled (a `MOBILEGL_PIPE_PUSH` sub-bit) as a negative control.
- **[major] The interface-purity gate's shared-value-header allowlist is not achievable as written, and the nm gate cannot detect the failure**
- 问题:§4.7.2 and §10.3-① define the purity gate as: `MG_Backend` may include only "a shared VALUE header allowlist (`RenderStateParameters` from RenderState.h, `SamplerParameters` from SamplerObject.h, `PixelStoreParameters`, `VertexAttribute`, texture/format enums)", plus `nm --undefined-only libMobileGLServer.so | grep -E 'MG_State::GLState::|glslang'` empty. Verified that the allowlist is not a leaf set: `MobileGL/MG_State/GLState/RenderState/RenderState.h:12` includes `MG_State/GLState/FramebufferState/FramebufferObject.h`, which at `:12-13` includes `MG_State/GLState/TextureState/TextureObject.h` and `MG_State/GLState/RenderbufferState/RenderbufferObject.h`. The dependency is structural: `RenderStateParameters` sizes two of its arrays with `MG_State::GLState::FramebufferObject::MAX_DRAW_BUFFERS` (`RenderState.h:263, 273`). So shipping `RenderStateParameters` to a "pure" MG_Backend drags the entire framebuffer/texture/renderbuffer class graph in with it. And the nm gate is blind to this: header inclusion of classes whose members are never called emits no undefined symbols, so `nm --undefined-only | grep MG_State::GLState::` can be empty while the include graph is fully coupled. The plan prices this cleanup inside P13's 6 days ("MG_Backend 的 MG_State include 收缩到共享值头白名单") as if it were a mechanical trim.
- 修法:Make header extraction an explicit P0/P1 deliverable, not a P13 trim: move `MAX_DRAW_BUFFERS`, `PerBufferBlendState`, `StencilFaceState`, `PixelStoreParameters` and `RenderStateParameters` into a dependency-free `MG_Pipe/MGPipeValueTypes.h` that includes nothing from `MG_State/GLState`, and have `RenderState.h` include that instead. Then replace the nm gate with an INCLUDE-GRAPH gate — compile `MG_Backend` in the disaggregated configuration with `MG_State/GLState` removed from the include search path (or assert on `-H` output), which is the only check that can actually go red for the reason the gate exists.
- **[minor] draw_vbo's payload construction is priced at parity with today's 3-scalar call, and mandates fields that are currently computed only where needed**
- 问题:§10.2's first table row reads "每 verb 的分发: 1 次间接调用 (已经在付) → 1 次间接调用", implying parity. But today's entry is `DrawArrays(GLenum mode, GLint first, GLsizei count)` — three scalars in registers (`MG_Backend/BackendObject.h:117`). The replacement is `draw_vbo(const MGPDrawInfo*, Uint32, const MGPDrawIndirect*, const MGPDrawRange*, Uint)`, and `MGPDrawInfo` as specified in §4.5.7 is ~80 bytes (mode, indexSize, flags, pad, instanceCount, startInstance, restartIndex, minIndex, maxIndex, an 8-byte handle, a 32-byte `MGHostSpan`, and an 8-byte `xfbCpuCapturedVertices`) plus a 12-byte `MGPDrawRange`. That is ~90 bytes of stores constructed per draw where there were three register moves. Two of those fields are new work, not just new stores: `minIndex`/`maxIndex` come from an index scan that today runs only for client-memory arrays (`TryComputeMaxIndexFromHostBytes`, `VulkanRenderer.cpp:3407-3470`, used at `:3599`), and `xfbCpuCapturedVertices` is a `GetTransformFeedbackCapturedVertices()` read that today happens only inside the XFB scatter path (`DirectGLES.cpp:~900`). At MC draw rates this is small but not nothing, and §10.2 accounts for none of it.
- 修法:State the payload cost explicitly in §10.2, gate `minIndex`/`maxIndex` and `xfbCpuCapturedVertices` behind `MGPDrawInfo::flags` so they are only computed when a consumer asked for them, and add per-draw payload bytes to the P0 counter set (`cmd-records` is per-frame; a per-draw histogram is what sizes SEG_CMD).
- **[minor] The +50-60 MiB memory figure omits the retention LRU the same document introduces, and that LRU is probably unnecessary**
- 问题:§7.11 (formerly the removed comparison table) gives the plan's memory as "transport segments (~48MiB) + POD slot records + an optional bounded ≤32MiB texel-retention LRU ≈ +50-60MiB". The arithmetic does not include the LRU it just described: §8.1's segment defaults are SEG_CMD 8 + SEG_STAGE 32 + SEG_REPLY 8 + SEG_EVENT 0.25 = 48.25 MiB, and `MOBILEGL_PIPE_TEXEL_RETAIN_MB` defaults to 32 (附 B). That is 80 MiB before §8.2's mandated SEG_STAGE growth for the four new byte classes. Separately, the retention LRU appears to be unnecessary. `MipmapStorage` keeps `Vector<Vector<Uint8>> m_data` — a complete CPU shadow of every level (`MobileGL/MG_State/GLState/TextureState/MipmapStorage.h:117`) — so a server-initiated pull (§7.5) can always be serviced from bytes the client already holds. The LRU therefore buys latency, not correctness, and its cost lands on the metric (memory) that §0.4 uses as the plan's strongest argument against the earlier (since-dropped) design in a project whose headline result was saving ~400 MB.
- 修法:Correct the arithmetic to 48 MiB + SEG_STAGE headroom + POD records, and default `MOBILEGL_PIPE_TEXEL_RETAIN_MB=0`. Turn it on only if §7.5(d)'s measured per-trace pull rate justifies it — which is exactly the discipline §7.5 already commits to for the pull count itself.
- **[minor] §9.1's "glGetTexImage = 0 round trips on DirectGLES" does not survive the plan's own generated-mipmap ownership split**
- 问题:§9.1 claims zero round trips for `glGetTexImage`/`glGetTextureImage` on DirectGLES because the client shadow answers. Verified that MG_Impl routes to the backend only when the backend is DirectVulkan (`MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp:6453-6459`), otherwise calling `CopyTextureImageToClientOrPBO_State`. But §5.8's row for generated mipmaps splits ownership: "client 分配 level 存储 … server 生成". A GPU-generated mip level therefore has allocated-but-empty client storage. `CopyTextureImageToClientOrPBO_State` will happily answer from that empty shadow. The plan's answer is `on_mip_levels_generated` (§7.1), but that callback as specified carries only `{res, base, count}` — no texels — so it can only mark the levels as needing a pull, which converts the query into a blocking round trip (the same class as §9.2 #9), or the design must instead eagerly write back every generated level (potentially megabytes per `glGenerateMipmap` on an atlas). The plan never says which, and §9.1 books it as zero.
- 修法:Decide explicitly in §5.8/§7.2 between eager `on_texture_writeback` of generated levels and lazy pull-on-query, and move the DirectGLES `glGetTexImage` row from §9.1 (zero) to §9.2 (conditional blocking) with the condition named. Add the generated-level case to `TextureRemintPullScenario` so the chosen path has a gate.
- **[minor] Two smaller round-trip accountings are optimistic: map_persistent is per-respecify not per-object-lifetime, and MGHostSpan is not free**
- 问题:(a) §9.2 #8 prices `map_persistent` under tier T1 as "每 store 生命桥期一次,不是每次使用". But storage respecification re-mints the store, and the plan's own P3a acceptance lists `StorageBufferRegrowScenario`. `TryAdoptLargeStorage` fires at storage-definition time, so a buffer that grows N times costs N blocking round trips, not one. For a workload that grows chunk arenas during world load this is a burst of stalls at exactly the moment the user perceives them. (b) §4.5.7 states "monolith 代价为零(一次指针加载)" for `MGHostSpan`. It is a 32-byte struct embedded in every `MGPDrawInfo` and read through `MGPipeHostBytes` which the same section describes as "一次分支,每次使用解析一次". That is a branch plus 32 bytes of payload on every draw record, whether or not the draw uses host bytes — which for VBO-based workloads (all of MC/Sodium) is every draw.
- 修法:(a) Reword §9.2 #8 to "once per storage definition" and add a `map-persistent-roundtrips` counter to the P0/P11 counter set, with `StorageBufferRegrowScenario` publishing it. (b) Reword §4.5.7's cost line to "one predictable branch plus 32 bytes on the draw record", and consider moving `userIndices` out of `MGPDrawInfo` into the `kHostSpan` var-tail so draws that carry no host bytes do not pay for the field.
已验证的优点:
- Push at draw-validate time rather than at GL-setter time (推论 1 / §5.1) is the right call and is directly supported by the tree: `RenderState::SetCapability` short-circuits redundant sets (`RenderState.cpp:311-313`) but a real enable/disable pair does bump the version, and `DirectGLES.cpp:2029-2032` names the Blaze3D per-batch blend toggle as the hottest path. A per-setter push would have turned that into an interface call plus a server CSO lookup per toggle. The plan identifies this as its most-likely-to-be-implemented-wrong decision and writes it as a spec clause (B-R15).
- The A/B/C/D/E read classification (§2.3) and the conclusion that the interface must push VALUES not invalidation is correct and load-bearing. Verified: Magma keeps no render-state mirror and rebuilds its payload from ~40 direct field reads on a pipeline miss (`VulkanRenderer.cpp:5155-5200` region) while Espryt keeps a byte mirror and diffs it (`DirectGLES.cpp:1956`, `:2035-2047`). A bump-a-version-and-let-the-server-pull interface would indeed regress to today's model.
- `MOBILEGL_PIPE_VERIFY` (§10.3-②) is a genuine semantic gate that exists only because the interface lands in the monolith first, and the plan is right to require FIELD-WISE comparison rather than memcmp — `DirectGLES.cpp:2029-2032` documents that a `RenderStateParameters` memcmp can false-DIFFER on padding but never false-match, so a byte comparer would produce false positives in the verify harness. This is the specific defect prior candidate designs were judged on, and it is answered.
- D-B5 is honest about the cost: the plan states plainly that the earlier byte-identity monolith gate dies by construction and puts the loss in the design document rather than hiding it. Verified that no configuration can preserve it — the backend stops reading `pGLContext`, memos re-key, and MG_Impl gains validate calls.
- Keeping `resource_subdata` carrying BOTH the union box and the rect list with the shape decision server-side (§4.5.6, §7.3) correctly preserves a measured hardware cliff. `MipmapStorage.h:60-83` documents the 96-slot rationale and the ~100-sprites/frame Minecraft pattern that motivated it; putting the decision on the side that pays the GPU cost is the right call.
- PBO readback becoming fire-and-forget (§9.1) is strictly better than the monolith, verified: `DirectGLES.cpp:9191-9204` maps the pack PBO with `GL_MAP_READ_BIT` and copies back synchronously inside `ReadPixels`, which stalls on the read regardless of whether the application ever touches the PBO. Likewise `glFinish`/`glFlush` are genuine no-ops today (`MG_Impl/GLImpl/Exporting/Definitions.cpp:111-112`), so the requirement that they stay free is achievable rather than aspirational.
- Per-backend optionality as a first-class interface property (§4.4.4, B-R9) is faithful to the existing contract: `BackendObject.h:212-215` and `:265-269` already document null table entries as "not implemented, frontend falls back", DirectVulkan already leaves 8 entries null, and Magma's deliberate omission of `ResidentSubData` (`VkBufferManager.cpp:104-111`) is preserved rather than papered over. Choosing a function-pointer struct over a virtual base is correctly justified by this, not by dispatch cost.
- The composite pipeline-program answer (§5.6.3) is correct and cost-free: `GLContext::GetProgramForDraw` (`Core.cpp:592`) already resolves and links the composite entirely frontend-side, so the client pushes one handle and the blocking `JoinLinkAndSpirv()` leaves the server draw path. This closes the objection that killed the prior thin-server design without adding machinery.
- P0 landing per-frame byte and call counters BEFORE any migration, and clearing the uncommitted per-draw `fprintf` instrumentation first, is the right sequencing — the tree genuinely has no per-frame byte or call metrics today, so every ring size, batching threshold and wire-granularity decision would otherwise be a guess.
- The identity model is sound where it matters: verified that the ABA hazards the re-key table addresses are real and documented in-tree (`TwinLookupMemo`'s owner-equality at `DirectGLES.cpp:83-90` exists precisely because a recycled heap address would otherwise hit a memo slot), and that a dense `{slot, gen}` array index genuinely replaces a Fibonacci-hashed probe plus two `owner_before` calls that touch a control block — a real per-draw win on three lookups per draw.
### 改造可行性与估时(refuted=False13 条)
- **[major] Stage-A snapshot is filled at 2 sites, but 48 of 70 backend entry points read pGLContext outside them**
- 问题:§6.2.1 and §11 P1 place `SnapshotFromGLContext()` at exactly two points: the top of `PrepareForDraw` (DirectGLES.cpp:2916) and `SetupDraw` (VulkanRenderer.cpp:6371). §5.1's tracker has exactly four validate entry points (ValidateForDraw/Dispatch/Clear/BlitOrCopy). Both are far too few. Of the 70 distinct `gBackendFunctionsTable.GL.*` entries reached from MG_Impl (89 call sites), 48 are neither draw nor dispatch, and many read pGLContext on their own: `UpdateTextureBindingAtTarget` reads `GetActiveTextureUnit()`/`GetTextureUnitObject()` at DirectGLES.cpp:6051-6052 and is reached from CopyTexImage2D/CopyTexSubImage2D; `GenerateMipmap` reads them at :6876-6877; `GetTexImage` at :9254-9257; `BlitFramebuffer` reads both FBO slots at :5988-5989; `Clear` reads `GetRenderStateParameters().ClearColor` at :4106 and the draw FBO at :4165; the readback family reads pack state at :6129/:7614/:9101/:9480 and the pack PBO at :7622/:8604/:8834/:9144/:9570; DSA-by-name reads at :4038-4043 and :7417-7418. The code says so explicitly: the comment at DirectGLES.cpp:1501-1502 states the no-arg `CaptureDrawTextureSyncKeys` wrappers exist "for every non-draw call site (Clear, readbacks)". The G5 poison mask does not save this: it fires only on a field that was NEVER filled; a field filled by an earlier draw reads STALE, not poisoned.
- 修法:Enumerate a validate/fill hook per non-draw backend entry class (texture-op, readback, blit, clear, xfb-span, query, DSA-by-name) in `PipeCalls.def` alongside the verbs, and make G5's written-once bitmask assert per CALL rather than per draw (a field written by draw N must not satisfy the read in the glTexSubImage that follows it). Alternatively make `PipeInputs` accessors lazily filled with a per-call fill generation. Until this is fixed P1's acceptance criterion ("40 traces green under MOBILEGL_PIPE_VERIFY") is unreachable, and §11's day-16 milestone should not be scheduled against the two-site design.
- **[major] Pushing texture resource_subdata at GL-call time destroys the dirty-rect coalescing the plan's own +6 ms/frame evidence rests on**
- 问题:§5.1 states the rule "only resource mutations push at GL-call time — which is exactly what BufferBackendOps does today". That is true for buffers and false for textures. `glTexSubImage*` never calls the backend table at all: MG_Impl/GLImpl/Texture/GL_Texture.cpp:1817, :1937, :2004 only call `MarkStorageDirtyRegion`. Espryt coalesces the ACCUMULATED region at sync time (Managers.cpp:4274-4311), where MipmapStorage's 96-rect cascade merge and the `summedArea*4 >= unionArea*3` union-box fallback run, and then deliberately collapses the rect list to one box when the unpack ring is live (`if (BufferImpl::UnpackRingAvailable()) dirtyRectCount = 0;`, :4321) with the in-tree measurement "~100 sprite rects become ~100 jobs ... measured +6 ms/frame of GPU time in MC's animated-atlas ticks. One box, one job." Emitting one `resource_subdata` per glTexSubImage call reproduces exactly the ~100-job shape. §7.3 gestures at a deferred "emission cursor" but never resolves the contradiction with §5.1, and §5.1 is the section an implementer will follow because it is written as the design's most emphatic rule.
- 修法:Amend §5.1 to say the GL-call-time rule applies only to the ops that already dispatch at GL-call time today (the seven BufferBackendOps hooks). State that texture subdata is accumulated in the client's existing MipmapStorage rect model and emitted at the next validate/flush point, so the merge heuristic keeps running before anything crosses the interface. Add a MOBILEGL_PIPE_STATS counter for `resource_subdata` emits per frame with an explicit ceiling on the MC animated-atlas fixture.
- **[major] Sub-rect texture upload is gated on pointer identity and whole-level stride arithmetic that no MGPBlobRef can satisfy in split mode**
- 问题:§5.4 prices subsystem 5's repack family as "unchanged in place, only the input changes from a pulled shadow pointer to an MGPBlobRef (the same pointer in monolith)". The code does not permit that. Managers.cpp:4278-4283 gates the whole sub-rect path on `uploadData == mipData` — literally "the upload source IS the whole level shadow" — and :4288-4293 computes `regionPtr = uploadData + z*levelSliceBytes + y*levelRowBytes + x*bpp`, striding into the FULL level with UNPACK_ROW_LENGTH; `rectShadowPtr` (:4321-4326) does the same per rect. The comment at :4270-4273 says conversion fallbacks "rewrite the whole level into a fresh buffer, so they stay on the full-level path" — i.e. the moment the source is not the level shadow, sub-rect upload is disabled by design. In split mode the client can stage (a) the whole level every time, which destroys the bandwidth benefit and contradicts §0.4's "零副本 / +50-60MiB" headline claim, (b) tightly-packed regions, which makes `uploadData == mipData` false and silently forces full-level uploads, or (c) nothing — requiring a server-side whole-level mirror, which IS the duplicated MipmapStorage the plan's strongest argument against the earlier (since-dropped) design says it avoids. §4.5.6's "carry both box and rect list, server picks the shape" does not address the stride source at all.
- 修法:Redefine MGPSubData so each region carries {dstBox, srcRowStride, srcSliceStride, blob} and rework Managers.cpp:4274-4326 to take a strided-source descriptor instead of comparing pointers, so the server can set UNPACK_ROW_LENGTH from the descriptor over a tightly-packed staged region. Move this out of "原地不动" and into subsystem 5's day estimate, and add a Mali-device gate that publishes the box-vs-rect job count and frame-time delta at P3b/P4b exit — the plan already names this as B-R5's cliff but assigns it no work.
- **[major] The XFB scatter path is a read-modify-write of the client's buffer shadow, and MGPipeCallbacks has no buffer pull**
- 问题:§7.2 assigns all 8 `WritebackFromBackend` sites to `MGPReplySlot` (readback) plus `on_buffer_writeback` (XFB capture, PBO readback) — all one-way server→client. But `ScatterCapturedRecords` (DirectGLES.cpp:928) does `Memcpy(staged.data(), target.buffer->MappedData() + target.start, rangeBytes)`: it STARTS from the application's existing bytes so that the holes `gl_SkipComponents` asks for keep whatever the application had put there (the comment at :891-895 says this is "the whole point of the feature"), patches only the captured varyings in, then writes back and re-uploads. The server has no `MappedData()`, and §7.1's callback table has `on_texture_pull_request` but no buffer equivalent. As specified the scatter either zero-fills the skip holes — a conformance break; DirectGLES.cpp:882-883 names `KHR-GL46.transform_feedback.capture_special_interleaved_test` as the case that reaches this path — or needs an unnamed synchronous reverse buffer read at glEndTransformFeedback, a stall class the plan's §9.2 roundtrip table does not list.
- 修法:Move the scatter to the client: the server pushes the packed scratch bytes via `on_buffer_writeback`, and the client — which owns the destination shadow and already has `GetTransformFeedbackVaryings()`/`GetTransformFeedbackStride()`/`GetTransformFeedbackPackedStride()` from the reflection archive — performs the patch and re-emits the range as an ordinary `resource_subdata`. If the scatter must stay server-side, add an explicit `resource_read_host(res, off, size)` reverse request to §7.1 and price its stall in §9.2 next to the texture pull.
- **[major] The unit-bindings debouncer is deleted while its dirty signal is replaced by the very counter it exists to filter**
- 问题:§2.5, §10.4-1, and §4.7.3 D3/D9 book ~115 lines at DirectGLES.cpp:1372-1489 as deleted because "the push call IS the change signal". But the comment at DirectGLES.cpp:1412-1421 states why `CurrentUnitBindingsEpoch` exists: `GetTextureBindGeneration()` bumps on REDUNDANT re-binds (26.2 re-binds the same sampler around every texture-unit switch), so the counter is untrustworthy and the epoch is built to "move exactly when WHAT is bound changes, never on a redundant re-bind". §5.2 then names `GetTextureBindGeneration()` as a dirty-bit input for NEW_SAMPLER_VIEWS. The tracker therefore re-emits `set_sampler_views` on every redundant re-bind, and D9's replacement (`viewSetSerial` bumped by the server inside `set_sampler_views`) invalidates the server's resolved-binding and sampler-pass memos on every batch — a per-batch regression on the exact workload the project optimises for, concealed inside a claimed 115-line deletion. `set_sampler_views` is a kVarTail `set_*`, not a CSO, so §4.2.3's "content addressing gives N=0 for repeated state" does not cover it; the same holds for `set_shader_images` and `set_shader_buffers`.
- 修法:State that the debounce MOVES to the client rather than disappearing: the tracker must hash the resolved view/image/buffer sets and suppress the emit on an unchanged hash (`MGPFramebufferState::contentHash` already demonstrates the pattern — extend it to the other var-tail set_* calls and use it client-side as an emit suppressor, not only as the server's memo key). Re-charge ~115 lines to MG_Impl/Pipe/Tracker.cpp and correct §10.2's per-draw arithmetic and §10.4's deletion count accordingly.
- **[major] Multi-draw cannot be split by a static screen cap: tier selection is per-batch and depends on backend-only program facts**
- 问题:§5.8 assigns "CPU tier on the client (!kCapMultiDraw); compute tier stays server-side". `ResolveTierForBatch` (MultiDraw.cpp:282-320) chooses among five tiers PER BATCH using `programReadsDrawID` — a property of the transpiled ESSL, which exists only on the server — plus `perSubDrawBaseVertex` and the batch's index totals against `kMaxFlattenedIndices` (MultiDraw.cpp:72, 1<<24) and `kMaxComputeFlattenedIndices` (:82). The auto ladder is Ext → BaseVertex → MultiIndirect → Indirect → DrawElements (:241-243), so the CPU-flatten `DrawElements` tier is a FALLBACK reached only after the batched tiers decline for reasons the client cannot evaluate. A client that flattens whenever `!kCapMultiDraw` bypasses the BaseVertex and compute tiers; a client that does not flatten leaves the server-side fallback with no index bytes in split mode. `kCapMultiDraw*` as a lowering-ownership switch is therefore not expressible.
- 修法:Keep all five tiers server-side. Carry what they need through the interface instead: `draw_vbo(info, indirect, MGPDrawRange[], numDraws)` plus a `kCapNeedsHostIndexBytes`-gated `MGHostSpan` for the index data, with the server deciding the tier. Delete `kCapMultiDraw`/`kCapMultiDrawIndirect`/`kCapMultiDrawIndirectCount` from §5.8's ownership table and replace them with a single rule: the server always owns multi-draw tiering; the client supplies index bytes when the caps say the server may need them.
- **[major] on_texture_pull_request can park a twin forever: there is no negative completion**
- 问题:§7.5(b) says the server marks the twin not-ready and the client re-emits on its next publish, and §9.2-9 says the resulting stall lands on mgl-srv-apply. But the client may have nothing to send. `RequireImageBindableStorage` (Managers.cpp:2789-2822) re-dirties every level of every upload target, and the replay reads the shadow — while :2810-2812 already skips levels whose `GetMipmapByteSize(...)` is 0, and a level whose content came from rendering, from a `glCopyTexSubImage` into a shape `CanMirrorCopyImageShadow` declines (DirectGLES.cpp:7068-7073), or from a GPU-side mip generation has no client bytes at all. With no negative completion the apply thread blocks on a twin that never becomes ready. B-R4 and the `TextureRemintPullScenario` gate address the RATE of pulls, never the unanswerable pull.
- 修法:Make the pull a request/response pair terminated by an explicit `resource_subdata_complete(res, target, firstLevel, levelCount)` that may carry zero regions, and specify that the server proceeds with allocated-and-empty storage on an empty answer (matching today's monolith behaviour) with a logged diagnostic. Add the unanswerable case — a texture whose only content came from rendering, then image-bound — to TextureRemintPullScenario, and require the scenario to be red before the terminator lands.
- **[major] MOBILEGL_PIPE_VERIFY is the plan's only semantic gate, and P13 deletes the code that produces its reference**
- 问题:§13.3-② calls the per-draw per-field shadow compare "the decisive one" and §0.4 D-B5 makes it the whole justification for abandoning the earlier byte-identity monolith gate. Verify computes its reference by calling `SnapshotFromGLContext()` (§6.2.1 stage B). §6.7 and §11 P13 then say: "delete SnapshotFromGLContext(), the MGB_CTX macro, MOBILEGL_PIPE_PUSH ... KEEP the MOBILEGL_PIPE_VERIFY harness for later work." With the snapshot gone, verify has nothing to compare against; after P13 the design has no semantic tripwire at all. Open question 11 half-acknowledges the same hole for split-only diagnosis ("the plan's server has no MG_Impl, so a split-only rendering bug has no second opinion") without connecting it to the loss of verify.
- 修法:Decide this before P0 freezes the gate list, because it changes what P13's purity gate may assert. Either keep SnapshotFromGLContext() compiled only under MOBILEGL_PIPE_VERIFY past P13 and scope the purity gate's `grep -c 'pGLContext' MG_Backend/` to the non-verify build, or replace it at P13 with the recorded-golden mode the plan already sketches at §10.4-9: turn MG_Test's mock backend into an MGPipe recorder, capture pushed state per draw on a set of fixtures, and diff future builds against the stored trace.
- **[minor] Texture parameters are modelled only on sampler-view CSOs, but they are per-texture-object state that non-sampled textures still need**
- 问题:§4.7.1 maps the "TexParam / SamplerParam" delta class (9 read points) entirely onto `create_sampler_view` (base/max level, swizzle, dsMode) plus `create_sampler_state`. But Espryt calls `SyncTextureParamsToBackend` for every touched unit binding AND every draw-FBO attachment texture (DirectGLES.cpp:1548-1560 for the unit list, :1580-1601 for the attachment list), and `RequireImageBindableStorage` sets `m_forceTextureParamsResync` precisely because a channel-widened carrier needs a swizzle override the frontend params version never moves (Managers.cpp:2815-2821). A texture that is only an FBO attachment, only an image-unit binding, or only a `glCopyImageSubData` endpoint has no sampler view, so under §4.7.1 its `glTexParameter` state has no carrier across the interface.
- 修法:Put base/max level, swizzle, depth-stencil mode and the LOD clamps on `MGPResourceDesc` or a dedicated `set_texture_params(res, ...)` call, and let `MGPSamplerView` carry only the view restriction (min/num level, min/num layer, alias format). This also keeps `glTextureView` modellable as what it actually is — a real texture object with its own parameters that can itself be an FBO attachment and a glTexSubImage destination (TextureObjectView.cpp:281, :290) — rather than the "ordinary view CSO" §4.5.4 reduces it to.
- **[minor] The client's per-(texture, uploadTarget, level) emission cursor aliases across glTextureView and its storage owner**
- 问题:§7.3 inverts dirty ownership and gives the client a cursor keyed on `(texture, uploadTarget, level)` that it clears on emit. But `TextureObjectView` forwards `IsStorageDirty`, `MapMipmapData` and `GetStorageDirtyRegion` to the storage OWNER's mipmap with index remapping (TextureObjectView.cpp:290-322, and :281 writes into the owner's data). A view and its owner therefore share one underlying dirty state while carrying two independent cursors: whichever emits first clears the flag the other still needed, or both emit the same texels. The plan's own §4.7.3-D18 discipline about not "optimising" a documented hazard away applies here too, but the aliasing is never mentioned.
- 修法:Key the emission cursor on `(storageOwner, ownerUploadTarget, ownerLevel)` — resolve through `GetViewStorageOwner()` and the view's `ToOwnerUploadTarget()`/`ToOwnerLevel()` mapping before consulting or clearing. Add a scenario that uploads through a view and samples through the owner (and the reverse) across a draw boundary.
- **[minor] The OOM-ack story names entry points that never reach the backend**
- 问题:§7.4 and §9.2-7 mark "glRenderbufferStorage*, the failure-capable forms of glTexImage*/glTexStorage*/glCopyTexImage*, and glBufferStorage" as kNeedsAck so the OOM-probe idiom works. The texture family never calls the backend table at all: MG_Impl/GLImpl/Texture/GL_Texture.cpp only calls `MarkStorageDirty(..., true)` at :2515, :2671, :2755, and Espryt allocates lazily at sync time. `RecordGLError` (DirectGLES.cpp:6309-6324) — the texture-side error reporter — has exactly one caller, glGenerateMipmap at :6916. Even the one genuine synchronous allocation, `glRenderbufferStorage*`, runs its OOM check inside `BackendRenderbufferObject::SyncToBackend` (Managers.cpp:8674-8684), i.e. also lazily. So kNeedsAck as specified has no producer for the texture family, and the renderbuffer case would need a forced sync at the GL call to be ackable at all.
- 修法:Enumerate the actual synchronous allocation points rather than the GL entry points that look like them. State plainly that texture allocation OOM is already deferred to sync time in the monolith so the split changes nothing observable, and restrict kNeedsAck to the one case that can be made synchronous (renderbuffer storage, if forced to sync at the GL call) plus glBufferStorage. Otherwise §9.2-7's "rare and already expensive, so the ack is nearly free" is pricing a mechanism that does not fire.
- **[minor] SEG_STAGE sizing omits the largest single-call payload the plan itself moves to the client**
- 问题:§8.2 lists four new byte classes for SEG_STAGE (client vertex arrays, client index arrays, multi-draw argument blocks, client-resolved indirect command blocks) and claims "byte volume unchanged — they are re-uploaded per draw today". The whole-EBO primitive-restart rewrite that §5.8 moves to the client is not among them, and it is bounded at `kMaxRestartRewriteBytes = SizeT{1} << 26` — 64 MiB (DirectGLES.cpp:4218) — twice the default `MOBILEGL_IPC_STAGE_MB=32` in Appendix B. Unlike client vertex arrays these bytes are not re-uploaded per draw today: the rewrite lands in a backend scratch buffer the driver keeps. The multi-draw flattened index stream (kMaxFlattenedIndices = 1<<24 indices, MultiDraw.cpp:72) is in the same class.
- 修法:Add the restart-rewrite blob and the multi-draw flattened index stream to §8.2's list, size SEG_STAGE against them or specify the grow/decline path for a single record larger than the segment, and keep the ceiling check with its `m_valid=false` decline and MGLOG_E_ONCE on the client (DirectGLES.cpp:4401-4409) so the diagnostic still fires on the thread that issued the draw.
- **[minor] The fixed validate order puts set_shader_images after set_draw_program, contradicting D-B3's own argument**
- 问题:§5.3's order is 1 framebuffer, 2 program, 3 sampler views / images / buffers / global constants, 4 render state, 5 vertex. D-B3 (§0.5) and §5.3 both claim the fixed order is what retires `ImageUnitFormatsStillMatch` (Managers.cpp:6545-6573, whose comment says it is "not expressible as a monotone version") by telling the server the image formats before the program build — but images are pushed at step 3, after the program at step 2. It only works because D-B2 defers specialization to draw time. And once specialization is deferred to `draw_vbo`, the framebuffer-before-program ordering argument carries no weight either: what actually retires the fragColor-broadcast workaround at DirectGLES.cpp:2712-2732 is LATE specialization, not call order. An implementer who takes §5.3 literally will build ordering assumptions the design does not need and does not honour.
- 修法:Replace the numbered order with the invariant that actually holds: all set_* for a command complete before the verb, and the server specializes the shader at the verb from whatever has been pushed. Then §5.3's list is a convenience, and D-B3's claim should be restated as "late specialization plus complete state at the verb" rather than "framebuffer strictly first".
已验证的优点:
- The dead-capability finding is real and independently verified: CapabilityInput::FramebufferSrgb and DepthClamp exist as enum values (RenderState.h:165, :168) but SetCapability falls to `default: // not supported currently` (RenderState.cpp:380) and IsCapabilityEnabled returns false at the `default:` arm (:428-429). All six backend consumers therefore read a constant false today. §10.4-6 is right to demand an answer before the render-state blob is frozen; writing the interface down genuinely surfaced this.
- The dirty-ownership inversion (§7.3) is sound and rests on a fact I verified: `grep -rn 'IsStorageDirty|GetStorageDirtyRects|GetStorageDirtyRegion' MG_Impl/` returns exactly 0 hits — the frontend never reads its own texture dirty state, only sets and clears it. Deleting PLAN.md §5.6a's ack protocol and risk R6 is therefore justified.
- The backend-memo-writeback asymmetry is exactly as claimed: DirectGLES writes zero Set*Memo calls into frontend objects (0 grep hits under MG_Backend/DirectGLES/), while DirectVulkan writes four — ProgramFactory.cpp:3448 and VertexInputStateFactory.cpp:60/78/83, with :78 storing a raw backend-heap pointer (`vao.SetBackendStateMemo(&entry, m_evictionEpoch)`). D12's verdict of "delete outright, do not translate" is the right call and the D13 VaoDrawMemo replacement really does already exist.
- D21 is a genuine latent bug, verified: `VulkanRenderer::CurrentXfbCounterSlot` (VulkanRenderer.cpp:11136-11146) keys `m_xfbCounterSlotByObject` on `GetBoundTransformFeedbackName()` — a raw, LIFO-recycled GL name with no generation — so a deleted-and-regenerated XFB object inherits the predecessor's counter slot. Landing this on `dev` independently at P0 is correct sequencing.
- The composite-pipeline-program answer ("nothing to do") is correct. GLContext::GetProgramForDraw (Core.cpp:592-660) already performs the whole flattening frontend-side, including both J1 join sites, `ComputeDrawProgramSignature()`, and `MakeShared<ProgramObject>(0u)` at :644 with the in-code rationale "deliberately not a named program ... backend registries key on the object, not the name". Deleting PLAN.md's proposed `SetReplicaResolvedDrawProgram` hook is justified, and this answers the prior judges' "unpriced composite" objection.
- Moving the CopyImage shadow mirror to the client is correct and does delete a whole reverse byte channel. `MirrorCopyImageIntoDestinationShadow` (DirectGLES.cpp:7085-7148) is a pure shadow→shadow row memcpy whose eligibility (`CanMirrorCopyImageShadow`, :7068-7073 — single upload target, not 1D-array) and whose bounds/texel-size checks are all decidable from frontend data alone, and it deliberately does not mark dirty.
- `RecProgramLinkOp` really is impossible, not merely undesirable: ProgramObject.h:11 includes ShaderObject.h, which at :12 includes ShaderCompileTask.h and at :145 returns `const SharedPtr<glslang::TShader>&`; ProgramObject.h:14 pulls SpvcSession.h. Collapsing PLAN.md's two program tiers to one, deleting phase P5, and promoting `nm -D | grep glslang` to a P7 acceptance criterion all follow correctly.
- §2.4's catalogue of the 58 non-arrow `pGLContext` uses is a real gap no prior design caught, and DirectGLES.cpp:146 (`MG_State::GLState::GLContext* ctx = MG_State::pGLContext.get();`) is verified as sed-invisible. The adjacent `using FbBindingSlot = std::remove_reference_t<decltype(MG_State::pGLContext->GetFramebufferBindingSlot(...))>` at :142 is a second wrinkle in the same family. Making the purity gate grep `pGLContext` rather than `pGLContext->` is the right response.
- The interface-purity gate (§4.7.2) is a genuinely stronger completeness argument than the prior branch's 477-row read inventory: making `MG_State::pGLContext` undeclared in the MGPipe build turns every unsatisfied read into a named compile error rather than a catalogue entry that can go stale. Keeping the inventory only as a G6 coverage checklist is the right demotion.
- Carrying the CPU-modelled XFB vertex count on MGPDrawInfo is correct on the point I expected to be wrong: `AccountTransformFeedbackPrimitives(mode, count)` runs BEFORE the backend draw call (GL_Drawing.cpp:1132-1133, :1140-1141), so the value pushed with a draw already includes that draw's contribution.
- The function-pointer-struct-not-vtable decision (§4.1) is well grounded in this codebase: the boundary already is a function-pointer struct installed at one hook point, null entries already mean "not implemented, frontend falls back", and that is the natural expression of a partially migrated subsystem during the strangler. A pure-virtual class would need stub overrides that lie.
- D18 being the single identity row marked UNCHANGED — the deliberate node-based `std::unordered_map` for VkTextureManager/VkRenderPassManager resources, with the BlitFramebuffer "layout undefined" postmortem carried verbatim into the review checklist — is exactly the right instinct for a refactor of this size, and B-R8 names the failure mode (someone "optimising" it back) correctly.
- The plan is honest about the two things that most threaten it: D-B5 states in the open that the earlier byte-identity monolith gate dies by construction and is a cost of this design, and B-R2 states that the central performance claim (the reachability traversal moves rather than doubles) is unmeasured and that the tree has no per-frame byte or call metric today. Landing TracyPlot counters and clearing the working-tree per-draw fprintf in P0, before any migration, is the correct ordering.
### 性能(refuted=False14 条)
- **[major] Program reflection payload cannot be decoded without linking glslang — the plan's own enforcement gate is unreachable and the fix is unbudgeted**
- 问题:§4.5.5 defines MGPProgramDesc.reflection as "Visit() 归档的 LinkArtifacts + SpirvArtifacts(全结构体)", and §5.7/§11-P7 make `nm -D libMobileGLServer.so | grep glslang` empty the "整个论点的强制执行点". But all five payload types are declared INSIDE ProgramObject.h: TypeFacts at MG_State/GLState/ProgramState/ProgramObject.h:44, ResourceReflection :76, XfbVarying :1146, LinkArtifacts :1210, SpirvArtifacts :1409. ProgramObject.h:11 includes ShaderObject.h (which exposes `SharedPtr<glslang::TShader>` at ShaderObject.h:146 and at :12 includes ShaderCompileTask.h, which itself pulls MG_Util/Async/JobNode.h, MG_Util/ShaderTranspiler/CompileEnv.h and MG_State/GLState/BufferState/BufferState.h), and ProgramObject.h:14 includes MG_Util/ShaderTranspiler/SpvcSession.h, which at :11 includes spirv_reflect.h. The server must have the *definitions* of LinkArtifacts/SpirvArtifacts to deserialize into, so it must include the exact header the gate forbids. ProgramObject.h is 1803 lines with 10 in-tree includers. The plan never budgets this extraction in any phase, and open question 5 concedes the MG_Util/MG_State seam "没有审计过" — while P7 acceptance depends on it.
- 修法:Insert an explicit phase (before P4a, ~5-8 days) that extracts TypeFacts/ResourceReflection/XfbVarying/LinkArtifacts/SpirvArtifacts into a standalone MG_State/GLState/ProgramState/ProgramArtifacts.h with no ShaderObject.h/SpvcSession.h dependency, update the 10 includers, and add a CI assert that ProgramArtifacts.h's transitive include closure contains no glslang, no SPIRV-Cross and no spirv_reflect header. Only then is `nm -D | grep glslang` a gate rather than a wish.
- **[major] Per-draw named-uniform-block bytes have no MGPipe call — the "all 26 reverse pulls disappear" claim is false and SEG_STAGE is under-sized**
- 问题:§7.2 asserts the 20 SyncPersistentMappedRange sites "作为反向调用彻底消失" because "每一处都紧挨着一次对客户端字节的 CPU 读,而那些读全部搬到了 client(§5.8". Verified counter-example: UniformManager::ResolveUniformBufferPayload calls bufferObject->SyncPersistentMappedRange() at MG_Backend/DirectVulkan/Renderer/UniformManager.cpp:2022 and then reads `outData = bufferObject->MappedData() + rangeStart` at :2052 (with a zero-padding copy at :2053-2057) to pack the block into Magma's own UBO ring — a per-draw read whose consumer is server-side, so it cannot move to the client. §5.8's ownership table does not list it; §4.4.3 and 附A define set_shader_buffers(cls, start, count, const MGPBufferRange*, writableMask) with flags V only, no kHasBlob and no MGHostSpan. §5.7/D6's set_global_constants covers only the DEFAULT uniform block (SpirvArtifacts::globalUboScratch), not named blocks. So every Iris/MC draw with a named UBO has an uncarried data dependency, and §8.2's SEG_STAGE sizing list (client vertex arrays, client index arrays, multi-draw args, resolved indirect blocks) omits it.
- 修法:Either (a) add kHasBlob/MGHostSpan to set_shader_buffers for cls==Uniform and price the per-draw byte volume with the P0 counters before freezing the payload, or (b) land a separate dev PR making Magma descriptor-bind the resident VkBuffer range instead of ring-packing it, with its own perf gate on the Iris traces. Then re-audit all 26 sites individually (they are 20+6 and enumerable) and publish the per-site disposition rather than a blanket claim.
- **[major] Phase days contradict the plan's own per-subsystem tables; P3a's re-baseline checkpoint fires by construction**
- 问题:§11-P3a is "slot 基建、buffer、VAO12 天)" and its deliverable list is exactly §6.4 rows 0b (handle infra, 5-7 d), 2 (buffer + 7 BufferBackendOps, 10-13 d) and 3 (VAO/vertex elements, 7-9 d) = 22-29 days. The phase then declares "⚠ 再基线检查点 1:若 P3a 超期 >50%>18 天)… 必须重定基线" — i.e. the plan's own subsystem table already predicts the checkpoint trips. Same shape at P4a: 16 days for §6.4 row 4 (7-9) plus the identity halves of rows 5 (20-26) and 6 (14-18). P7 is stated 48-85 against §6.5's own total of 85-111, and B-R14 admits "P7 的 48 天下界明显低于同口径的 85-111" yet the headline 199-236/200-260 still uses 48. Espryt subsystem 7 (XFB, 5-7 d) has no phase home at all — it appears only in P9's split acceptance list. Summing §6.4 (89-120) + §6.5 (85-111) + shared infra + the 51 days of IPC phases (P5 12 + P6 5 + P9 10 + P10 6 + P11 8 + P12 10) gives ~245-310 excluding CTS, versus the advertised 200-260 including IPC.
- 修法:Rebuild §11's day column by summing §6.4/§6.5 rows per phase rather than assigning budgets independently; publish the arithmetic. Set P3a's checkpoint at the subsystem-derived number (e.g. >36 days) and give Espryt XFB an explicit phase. Restate the headline as ~245-310 person-days excluding CTS turnaround, or split P3a into P3a-i (handle infra) / P3a-ii (buffer) / P3a-iii (VAO) so each has a checkpoint that can actually fire early.
- **[major] The verify harness — the plan's decisive replacement for the byte gate — is structurally blind in the subsystem the plan calls most dangerous**
- 问题:§10.3-② and §6.2.1 stage B make MOBILEGL_PIPE_VERIFY (tracker fills a second PipeInputs via SnapshotFromGLContext, G4 compares field-wise per draw) the mechanism that "在语义上严格强于任何符号 diff" and the answer to every prior review. But §7.3 inverts texture dirty ownership: the client keeps the MipmapStorage rect model, maintains a per-(texture, uploadTarget, level) emission cursor, and "在发射后清自己的标志". Once the client has cleared the flags, a from-scratch snapshot recompute cannot reconstruct the dirty rect set, so the comparator has no independent second opinion for resource_subdata payloads — precisely subsystem 5, which §6.4 and B-R5 both single out as "全表最危险" because of the measured +6 ms/frame box-vs-rects cliff (Managers.cpp:4311-4319) and the 7 fallback-repack paths whose eligibility test requires uploadData == mipData. The same blindness applies to any group where the push path consumes-and-clears rather than reads.
- 修法:Add a verify-only retention mode: under MOBILEGL_PIPE_VERIFY the tracker keeps the pre-clear dirty set for the draw and G4 compares emitted (box, rectCount, rects[]) against a snapshot recompute. Additionally record the pull-mode upload shape per texture per frame into a golden and compare it in a TextureUploadShapeScenario, so the +6 ms cliff is gated by shape equality, not only by SSIM.
- **[major] After stage C the MOBILEGL_PIPE_PUSH knob is no longer an A/B against the old backend, and the plan claims otherwise**
- 问题:§6.7 states "任何一次提交都能在同一份二进制上按子系统 A/B" and "设备回归可以二分到'哪个子系统'", and §12-B-R1/B-R3 lean on this as the migration-risk mitigation. But stage C (§6.2.1) changes the PipeInputs field TYPE from SharedPtr<FrontendObject> to MGPipeHandle + POD descriptor, rekeys the backend memos to {slot,gen}, and (P3a) replaces the six StateBackendObjectRegistry hash tables (Managers.h:270-390, instances at :806/:1123/:1216/:1731/:1830/:1858) with slot arrays while deleting TwinLookupMemo x3 and OwnerEquals. With the bit cleared, SnapshotFromGLContext must still synthesise the handle from the client slot map and the backend still executes the rekeyed memo code — so both arms run the same new code. A rekeying bug (exactly the D1/D2/D3/D11/D13 hazard class the plan is trying to close) is present in both arms and cannot be bisected by the knob. The plan never states this narrowing.
- 修法:State in §6.7 that the bitmask A/B is scoped to stage-B value fields. For P3a and P4a add a second, compile-time switch (e.g. MOBILEGL_PIPE_LEGACY_MEMOS) that keeps the registry/TwinLookupMemo implementations alive behind the same PipeInputs surface, so the first two handle waves retain a true old-vs-new arm on device; retire it at P13 with the pull path.
- **[major] P2's day-24 GO/NO-GO measures the one face where the pull model is already nearly free, so a green result does not de-risk the central claim**
- 问题:§0.6 and §11-P2 make day 24 the GO/NO-GO for "可达性遍历是搬走了而不是翻倍", on monolith-push per-thread CPU after only render state, pack state, patch state and attrib defaults have moved. But Espryt's render-state pull already early-outs on a single Uint16 compare before ever touching the block: DirectGLES.cpp:2007 reads GetRenderStateParametersVersion(), :2016-2018 returns when it matches g_syncedRenderStateVersion, and only then is GetRenderStateParameters() read at :2021 and the three-span memcmp run at :2042-2047. The tracker replaces that with an xxHash over the same ~1.2 KB plus a 64-entry CSO LRU probe — roughly neutral for Espryt, a clear win for Magma (~55 reads), and in neither case representative. The costs the claim actually rests on are the ones P2 does not move and that become NEW client work at P3a/P4a: the touched-unit sampler walk over Array<TextureUnit,192> (TextureState.h:41,128), the 84-per-target buffer binding-point walk, the 32-attribute VAO walk, and the per-texture content/params version reads. §3's own table concedes "这是主张,不是测量".
- 修法:Move one object-valued group into the GO/NO-GO — set_sampler_views over the GetMaxTouchedUnit prefix is the cheapest honest candidate — and measure that. Otherwise relabel day 24 as "mechanism proven, zero product risk" and place the real GO/NO-GO at the P3a exit, where the first Track-H walk exists; adjust B-R1's "退回the earlier (since-dropped) design 只损失 16 天" accordingly (it becomes ~36 days).
- **[major] "Zero new bookkeeping in MG_State" and "one 64-bit dirty word test" cannot both hold for object-valued groups; the mutator-enumeration obligation plan A had is not deleted, only renamed**
- 问题:§5.2 promises the dirty bits come entirely from existing counters with "MG_State 零新增记账"; §5.1 and §10.2 price steady state at "一次 64 位 dirty word 测试 + N 次 set_*". For NEW_SAMPLER_VIEWS the listed sources are per-object and per-slot — ITextureObject::GetContentVersion/GetShapeVersion/GetTextureParamsVersion plus GetTextureBindGeneration()/GetSamplingResolutionGeneration() — and there is no aggregate covering "did any bound texture's content move". That is exactly why Magma resorts to the lossy sampledContentSum/sampledParamsSum (VulkanRenderer.h:975-1000). So the tracker must either walk the touched units at every validate (not O(1), and it is new client work the backend's ResolvedTextureBindingMemo currently skips), or add aggregate generations to TextureState (new bookkeeping), or set dirty bits from every MG_Impl mutator entry point — MobileGL implements desktop GL 4.6 and MG_Impl/GLImpl alone references 181 distinct gl* names. §0.4-4 claims plan A's "第七个面" and gen_impl_mutation_surface.py vanish because there is no replica to replay into; but plan A enumerated MG_Impl mutations to REPLAY them and plan B must enumerate them to MARK them dirty. The generator is deleted; the enumeration is not, and no phase budgets it. B-R6 names the risk but its three mitigations (written-once bitmap, poison, verify) all detect omissions, none enumerate the surface.
- 修法:Decide per group and write it down: for value groups use the existing counter; for object groups either add an explicit aggregate generation to TextureState/BufferState/VertexArrayState (and price it as MG_State work), or keep gen_impl_mutation_surface.py in a repurposed form that enumerates the MG_Impl mutators which must set each MGPIPE_NEW_* bit and fails CI on an unmapped mutator. Then correct §10.2's steady-state cost row to show the per-group walk that survives.
- **[minor] P1's byte-identity acceptance is contradicted by P1's own deliverables**
- 问题:§11-P1 acceptance: "pull 构建里 nm --defined-only + 剥调试信息 .text size 与替换前完全一致——本阶段可证明是一次替换(这是最后一次这条等式成立)". But P1's deliverables include the §2.4 conversion list, of which the ~22 real null guards generate code: 7 `if (MG_State::pGLContext)` (e.g. Managers.cpp:3608, verified: the guard wraps three assignments in BackendTextureObject::StampViewSyncKeys), 14 `!= nullptr` and 1 `== nullptr`. Deleting or unconditionalising those changes .text in RelWithDebInfo. Only the 34 MOBILEGL_ASSERT sites are genuinely free — Defines.h:114 defines the macro as empty outside debug builds (verified). P1 also installs SnapshotFromGLContext() at the top of PrepareForDraw (DirectGLES.cpp:2916) and SetupDraw (VulkanRenderer.cpp:6371) with no stated #if guard, which adds a call in the pull build.
- 修法:Guard SnapshotFromGLContext and the G4/G5 machinery behind MOBILEGL_PIPE_PUSH/_VERIFY/debug, defer the null-guard and ternary rewrites to P2 (where the fields are genuinely always-valid), and restate P1's acceptance as "nm --defined-only unchanged; .text within N bytes with the delta attributable line-by-line" rather than exact equality.
- **[minor] P1 snapshots only at the two draw-prepare sites, but a large share of the pull reads are in non-draw verbs — the poison mask will Fatal on the first glGenerateMipmap/glReadPixels**
- 问题:§11-P1 places SnapshotFromGLContext() at PrepareForDraw and SetupDraw only, while arming G5's poison mask so that reading an unfilled field is Fatal{UnmigratedPipeInput} "发生在第一个 draw 上", and then requires "全部 40 个 trace 与 367 个集成测试在 MOBILEGL_PIPE_VERIFY=1 下零分歧". Verified non-draw reads that would be unfilled: DirectGLES.cpp:6051-6052 (GetActiveTextureUnit + GetTextureUnitObject inside the GenerateMipmap path), :6129 and :7614 (GetPixelStoreParameters(false) in readback paths), :6643-6644, :6738-6739, :6876-6877 (texture verbs resolving the active unit), :6319 (RecordError). §5.1 does declare ValidateForClear/ValidateForBlitOrCopy/ValidateForDispatch, but P1's deliverable list does not enumerate them or the texture/readback verbs.
- 修法:Make the per-verb snapshot points an explicit P1 deliverable derived from PipeCalls.def: generate, per kCtxVerb/kCtxObject call, the set of PipeInputs fields it may read, and emit the snapshot/validate call at each of the ~89 MG_Impl boundary sites accordingly. This also converts G5 from "catches an omission at some draw" into "catches it at the specific verb that needed it".
- **[minor] §4.5.7 and §5.8 disagree on where primitive-restart rewrite and indirect-count resolve live; either answer moves the A/B baseline a second time**
- 问题:§4.5.7's MGHostSpan consumer table says for restart rewrite / multi-draw flattening: "monolith 填法: ptr 指向 shadow" (server does it) / "split 填法: 暂存,或 client 已重写". §5.8's ownership table says client, gated on !kCapPrimitiveRestart. Both backends actually perform the rewrite — DirectGLES.cpp:4283 RewriteRestartIndices, :4377 ScopedRestartIndexSubstitution, whole-EBO bounded by kMaxRestartRewriteBytes = 1<<26 at :4218; VulkanRenderer.cpp:3990/:4089/:4161 — so the cap is false on both and the client always does it, i.e. a monolith behaviour change scheduled at P8 (day ~97-111), long after §10.3-③'s name-for-name integration baseline was taken at P2. If instead it is split-only, monolith and split run different implementations of a whole-buffer correctness-critical transform and the name-for-name gate compares two different programs. Open question 12 flags the diagnostic-thread change but not the baseline problem.
- 修法:Choose client-side unconditionally, land it as an independent dev PR before P2 together with the decline-diagnostic relocation (resolving open question 12), so the monolith baseline moves exactly once and before any comparison is taken. Delete the conflicting row from §4.5.7's table.
- **[minor] set_sampler_views/bind_sampler_states import a per-stage slot space that MobileGL's state model does not have**
- 问题:§4.4.3 defines set_sampler_views(stage, start, count, const MGPBoundView*) and bind_sampler_states(stage, start, count, const MGPipeHandle*). Verified model: TextureState::m_textureUnits is Array<TextureUnit, MAX_TEXTURE_IMAGE_UNITS> with MAX_TEXTURE_IMAGE_UNITS = 192 (TextureState.h:41, :128) — one COMBINED unit space, with the per-stage limit only an advertised number (:42). TextureUnit holds Array<BindingSlot<ITextureObject>, TextureTargetCount> plus a single sampler (TextureUnit.h:20, :24-25). The same combined unit can be sampled by two stages, and both backends bind by combined unit (g_boundTexturesCache[192][TargetCount]). A stage parameter forces the client either to duplicate views under each stage or to invent a stage attribution GL does not define, and it adds a dimension the server must collapse again.
- 修法:Drop the stage parameter from both calls and address the combined unit space directly — which is also what LinkArtifacts::uniformSamplerOrImageUnitIndex already yields for the client-side resolution described in §5.5. Keep stage only where the target API genuinely needs it (Magma's descriptor stage flags), derived server-side from the reflection archive.
- **[minor] The monolith benefit is argued on ~550 deleted lines with no accounting of the code added**
- 问题:§2.5, §3's comparison table and §10.4-1 lead the monolith case with "~550 行 per-draw 失效发现机制删除". Nowhere does the plan estimate the permanent additions: PipeCalls.def plus six generators (G1-G6), MG_Impl/Pipe/{Tracker, SlotAllocator, CsoCache, HostResolve, CompositeResolver}, MG_Pipe/{MGPipeTypes, MGPipeHandles, MGPipeCallbacks, MGPipeHostSpan}, MG_Backend/MGPipe/{PipeInputs, two impl files}, plus MG_Remote's emitter and PipeApplier/PipeObjectTables. For a ~72-call interface with ~14 POD payloads across two backends that is plainly an order of magnitude more than 550 lines, all permanently maintained, and it is added to a codebase where MG_Backend is already 68k lines and MG_Impl 37k.
- 修法:Publish a net-LOC estimate and, more importantly, a net per-draw instruction/cache-line estimate next to the deletion list, and make §10.3-④'s per-thread CPU number — not the deletion count — the stated monolith case. This also gives B-R2 a falsifiable prediction rather than a qualitative claim.
- **[minor] A block of SamplerObject.h citations point at lines that do not exist in the file**
- 问题:The document header asserts "全部 file:line 引用针对工作树 dev@81b17c0b". MG_State/GLState/SamplerState/SamplerObject.h is 160 lines at 81b17c0b (identical at HEAD): BorderColorForm is at :66-70 and struct SamplerParameters at :72-96. But §4.5.4 cites ":468-492" for SamplerParameters, ":462-466" for BorderColorForm and ":455-461" for its rationale; §5.2 cites ":532, 551" for GetVersion/m_version; §4.2.1 cites ":533-537" for GetLifetimeId. All are past end-of-file. The substance is correct and is in the file (borderColorForm is mandatory because all three representations are always populated, :60-66; BumpVersion also bumps the context-wide sampling-resolution generation, :152-158), so this is an inherited transcription error rather than an invented fact — but the plan is meant to be an implementation spec, and every other citation I sampled was exact (293 arrow / 58 non-arrow pGLContext, 89 gBackendFunctionsTable.GL. sites, 40 pActiveBackendObject-> sites, 354/709 MG_State:: mentions, 50 include lines over 18 headers, DirectGLES.cpp:2035 static_assert, :2042-2047 three-span memcmp, RenderState.h:363/:369/:522/:529 all verified).
- 修法:Re-verify the SamplerObject.h block and anything else inherited from the same reader report before P0 freezes MGPipeTypes.h, and add a cheap CI lint that every file:line in docs/Disaggregated/*.md resolves to a line that exists at the referenced baseline.
- **[minor] The day-64 "first inproc IPC frame" milestone is unfalsifiable as specified**
- 问题:§11-P5 delivers InProcessTransport and claims the milestone "★ 第 64 天 — 首个 IPC 帧(inproc", honestly flagged as a reduced path. But nothing in §11-P5 or §8.1 says whether inproc goes through the same G3-generated encode/decode as spawn or short-circuits it. If it passes PipeInputs by pointer inside one address space, the subsystems not yet handle-ified at P5 (Espryt XFB, which has no phase at all; readback beyond the single blocking read_pixels) keep working via SharedPtr and the milestone proves nothing about wire completeness — while P6 (spawn, day 69) would then discover the gap five days later, on the critical path.
- 修法:Specify that InProcessTransport uses the identical G3 serialization and differs only in the doorbell/copy mechanism, and add a debug assertion in PipeApplier that no SharedPtr or raw frontend pointer crosses the applier boundary in any transport. Then day 64 and day 69 differ only by process boundary, which is what the milestone is meant to assert.
已验证的优点:
- The pull-surface accounting is exact and better than every prior design's. Verified at dev@81b17c0b: 293 `pGLContext->` occurrences and 58 lines using pGLContext without the arrow, with the plan's §2.4 breakdown reproducing precisely (34 MOBILEGL_ASSERT truth tests, 14 `!= nullptr`, 7 `if (`, 1 `== nullptr`, 1 `.get()` at DirectGLES.cpp:146, 1 comment at VertexInputStateFactory.h:133). Identifying the `.get()` capture as invisible to sed, and specifying that the purity gate greps `pGLContext` rather than `pGLContext->`, closes a real hole the three earlier candidate designs all left open.
- The function-pointer-table-over-vtable decision is correctly argued from this codebase rather than from gallium. Verified: GLFunctionsTable + GlobalBackendFunctionsTable contain 69 function pointers (BackendObject.h:117-285), reached from 89 `gBackendFunctionsTable.GL.` sites and 40 `pActiveBackendObject->` sites in MG_Impl, installed at the single hook point MG_Backend/Init.cpp, and null entries already mean "not implemented, frontend falls back" (documented at BackendObject.h:212-215, 265-269). A null `set_*` is a native expression of "this subsystem is not migrated"; a pure-virtual class would need stub overrides that lie.
- D-B1 (ship RenderStateParameters as one blob, not three gallium CSOs) is grounded in verified in-tree evidence rather than preference: `static_assert(std::is_trivially_copyable_v<RenderStateParameters>)` at DirectGLES.cpp:2035, the head/blend/tail memcmp at :2042-2047 keyed on offsetof(...,BlendStates)/offsetof(...,LogicOp), and the load-bearing field placement of ScissorBoxWrittenMask (RenderState.h:363) and ClipDistanceEnabledMask (:369). Carrying both m_version (:522) and m_pipelineStateVersion (:529) on the wire is likewise correct and correctly justified by the glViewport-evicts-pipeline-memo regression recorded at :523-528.
- The texture dirty-ownership inversion rests on a fact I confirmed independently: MG_Impl contains zero `IsStorageDirty(`, `GetStorageDirtyRects(` and `GetStorageDirtyRegion(` call sites while calling `MarkStorageDirty(` 14 times. Deleting plan A's §5.6a ack protocol and risk R6 on that basis is sound, and keeping the box-vs-rects upload-shape decision server-side (MGPSubData carrying both payloads) correctly leaves the choice on the side that paid for the +6 ms/frame measurement at Managers.cpp:4311-4319.
- D-B4 — leave AcquirePersistentMap completely untouched through the entire monolith refactor and isolate it to the IPC step behind a week-one POST spike — is the right structural call. It is already an explicit call returning a pointer (BufferObject.h), so it genuinely passes through unchanged, and refusing to let one platform unknown gate ~200 days of interface work is exactly the right sequencing judgement.
- The two backend-internal MG_State usages that the previous review round priced at zero are correctly identified and costed. Verified: UniformManager::MakePlaceholderTextureObject at UniformManager.cpp:161-181 with the real construction at :1417-1424, :1479-1496 (including SetSamples(2) for VUID-RuntimeSpirv-samples-08726 and TruncateMipmapLevels at :1496) and :1620; and the two internal shaders at VulkanRenderer.cpp:4211 and :4287 building MakeShared<MG_State::GLState::ShaderObject> (:4214, :4222, :4290, :4300), a ProgramObject (:4230) and calling Link(false) (:4233). Preferring checked-in SPIR-V guarded by an in-tree-glslang byte-compare MG_Test over a host-tool build step is the right trade for this repo's four build lanes.
- VertexInputStateFactory's backend-heap-pointer write-back into the frontend VAO is correctly classified D12 "delete, do not translate", and D18 (VkRenderPassManager/VkTextureManager's deliberate node-based std::unordered_map) is correctly the single UNCHANGED row with a mandate to carry its postmortem comment verbatim into the P7 review checklist. Naming the one thing a large refactor must not "optimise back" is exactly the discipline these reviews usually find missing.
- The milestone labelling is honest where a weaker plan would have overclaimed: P5/P6 are explicitly marked 缩减路径 with emulation Fatal in split until P8; §3 concedes plan A wins first-frame time by 4-5x; D-B5 states outright that the byte-identity gate dies by construction and calls it a cost that must be written down rather than hidden; and §9.3 refuses a blanket zero-round-trip claim in favour of published per-trace-case round-trip and texture-pull counters.
- The design surfaced two genuine in-tree defects as by-products and routed them correctly: D21, m_xfbCounterSlotByObject keyed on the raw GL name (VulkanRenderer.cpp:11136-11146), so a deleted-and-regenerated XFB object resumes a capture that should restart — scheduled as an independent dev PR in P0; and the dead CapabilityInput::FramebufferSrgb/DepthClamp with no storage (RenderState.cpp:380, :428-429) feeding six constant-false backend reads, correctly made a blocking question before the render-state blob is frozen.
- Ordering the strangler so framebuffer precedes textures and programs (D-B3, §6.6 step 4) is right and well-evidenced: the four cross-object masks are derived from attachment formats at Managers.cpp:5616-5619 and consumed by the render-state push (DirectGLES.cpp:2014) and the program staleness test (:2769-2770), and inlining internalFormat into MGPSurface lets them be derived at push time with no lookup — which genuinely retires the fragColor re-derivation workaround at :2712-2732 rather than porting it.
## 3. 综合稿的关键决定
- Wrote 5 files (part2 split into 2a/2b): part1=§0-3, part2a=§4, part2b=§5-6, part3=§7-10, part4=§11-14+附. Single title in part1 only; §0-§14+附 headings in required order; each file ~35-49KB UTF-8 ≈ 12-16K Chinese chars, well under the cap.
- Base = winning Design 3 (split-first) phase plan, grafted with Design 2's twin-derived interface derivation (SetupDrawSnapshot / IsDrawSyncClean / ResolvedDrawBuffers / g_syncedRenderStateParameters / BufferBackendOps as the source of the call catalogue), its PipeCalls.def six-generator toolchain, its two-kinds-of-generation split (client identity vs 12 server-only MGGen epochs), its D18-UNCHANGED node-container discipline, and its MGHostSpan; plus Design 1's caps-gated emulation-homing rule, its numbered gallium-deviation ledger, and MGPipeCallbacks as a named struct.
- Resolved Design 1's fatal flaw: render state ships as ONE versioned blob behind a content-addressed CSO handle (create_render_state(blob) + bind_render_state 12B, client 64-entry LRU keyed on the three existing memcmp spans), never decomposed into blend/depth-stencil/rasterizer CSOs — cited RenderState.h:359-368 (field order load-bearing), DirectGLES.cpp:2035 static_assert + :2042-2047 three-span memcmp, and the :523-528 two-counter regression.
- Resolved Design 2's fatal flaw: MGPipeHandle is {slot:Uint32, gen:Uint32} with CLIENT-ALLOCATED DENSE PER-KIND SLOTS (not a sparse 64-bit lifetimeId), which is what actually turns the 6 StateBackendObjectRegistry hash tables and 13 Magma caches into arrays; GetLifetimeId() stays client-side as the tracker's own identity; 2^32 slot-reuse wrap documented and asserted.
- Re-measured every contested count against the working tree rather than inheriting any report: GLFunctionsTable = 67 function pointers + 1 Bool (BackendObject.h:117-278), 69 fps with GlobalBackendFunctionsTable (not 73 or 71); 293 pGLContext-> occurrences over 290 lines + 58 non-arrow lines; 50 MG_State include lines over 18 distinct headers; 95 backend->frontend mutator sites over 17 methods; 7 BufferBackendOps hooks; 89 MG_Impl table sites + 40 pActiveBackendObject->; 1494 MG_Impl pGLContext->; 367 TEST_F / 428 TEST( / 40 trace cases at SSIM 0.99; PLAN.md phases sum to exactly 77 days.
- Closed the shared migration gap all three designs missed: the 58 non-arrow pGLContext uses (≈40 MOBILEGL_ASSERT truth tests, ~10 null guards, 3 patch-param ternaries, the DirectGLES.cpp:146 .get() raw capture that sed cannot catch, 2 != nullptr conditions, 1 comment) are enumerated by form in §2.4, made an explicit P1 deliverable, and the purity gate greps 'pGLContext' not 'pGLContext->'.
- Hardened the residual value block (the split-first accelerant): per-member offsetof static_asserts in addition to sizeof, AND field-wise serialization in split mode instead of a bulk memcpy — because the monolith verify harness cannot see a layout mismatch when both sides are the same TU; retirement is a compile error via static_assert(sizeof(ResidualValueBlock)==0) at P13.
- Priced the schedule honestly: 200-260 engineer-days (single track 199-236, P7/Magma 48-85), first inproc IPC frame day 64 and first cross-process frame day 69 — both explicitly labelled REDUCED PATH (emulations Fatal in split until P8, full function at day 111) — against PLAN.md's verified 77 days and day-15 cross-process frame; added TWO re-baseline checkpoints (P3a overrun >50%, P7 midpoint <40% complete) and priced CTS turnaround (~56,271 cases) as a separate tiered-gating line, not folded into phase estimates.
- Stated D-B5 as an explicit cost in the TL;DR: PLAN.md's byte-identity monolith gate dies by construction, replaced by a five-part gate (purity grep+nm, per-draw field-wise MOBILEGL_PIPE_VERIFY shadow-compare, behavioural A/B across {monolith-pull, monolith-push, split}, per-thread-CPU non-regression, coverage+poison+handle-recycle asserts) with two surviving nm equalities kept as assertions and .text drift published as informational.
- Kept the texture re-mint pull as a named NEW stall class with all three mitigations shipping together (imageBindableHint pre-emption, asynchronous park-and-re-emit so the stall lands on mgl-srv-apply not the app thread, bounded 32MiB retention LRU), a dedicated TextureRemintPullScenario, and a per-trace-case pull counter that is PUBLISHED rather than asserted to zero.
- Corrected PLAN.md §7.4 with evidence: backend program link/compile failure is surfaced ONLY as MGLOG_E plus a bind-program-0 no-op (Managers.cpp:8091-8126, 8357-8372, rationale at :7098/:7247-7249/:6478/:7827), so on_log must split by severity — <=WARN lossy, >=ERROR lossless with a per-second rate limiter emitting 'N errors suppressed' — with a log-flood fault-injection gate.
- Quarantined AcquirePersistentMap from the refactor entirely (it is already an explicit pointer-returning call and survives P0-P13 untouched; only IPC breaks it), deferring it to PLAN.md §6.8's three POST-probed tiers with spike B in week one, so no platform unknown blocks 200 days of interface work.
- Inherited PLAN.md §6-§13 essentially verbatim with a per-section table in §8.1 (no re-derivation), and listed every delete/change/add against it in §8.2 and §14.1 — including that the copy account drops to 3/2 (PLAN.md's own 'the plan' target) and inproc isolation drops from four process globals to two, which makes PLAN.md's earliest falsification gate cheap.
## 4. 修订记录(综合稿 v1 → 定稿 v2)
- [stage-A fill sites] Verified only ~22 of the 70 table entries MG_Impl uses are draw/dispatch; confirmed non-draw entries read pGLContext themselves (DirectGLES.cpp:6051-6052 GenerateMipmap path, :6129 pack state, :4106/:4165 Clear, :5988-5989 Blit, :1501-1502 comment). Replaced the 2-site SnapshotFromGLContext with G5-generated per-verb-class fill/validate points at the ~93 MG_Impl boundary sites; Tracker grows from 4 to 8 validate entries (§5.1, §6.2.1, P1).
- [poison granularity] Upgraded G5's written-once bitmask to a per-verb generation (m_filledGen[f] == m_currentVerbSerial, sticky fields listed explicitly), so a field filled by draw N no longer satisfies the read in the following glTexSubImage; poison now fires on the verb that needed it (§6.2.2).
- [texture push timing] Verified glTexSubImage* never calls the backend table (GL_Texture.cpp has 3 MarkStorageDirtyRegion sites only) and that Espryt coalesces at sync time with the union-box collapse at Managers.cpp:4386-4390 (+6 ms/frame). Rewrote 推论 1 and added §5.1.1: the GL-call-time push rule applies only to the seven BufferBackendOps hooks; texture subdata accumulates in the client's rect model and is emitted as one resource_subdata at the next validate/flush point, with a per-frame emit counter and an MC animated-atlas ceiling.
- [sub-rect upload] Verified the `uploadData == mipData` gate (Managers.cpp:4278-4283) and whole-level stride arithmetic (:4288-4293, :4321-4326), and that the unpack-ring path already uses a strided source descriptor (UnpackStagingBlock, :4340-4390, tightly repacked). Redefined MGPSubData to carry MGPSubRegion{dstBox, srcRowStride, srcSliceStride, srcOffset} plus sourceIsVerbatimLevelShadow, reworked Managers.cpp:4274-4326 to read strides from the descriptor, moved this out of 原地不动 and priced it into Espryt subsystem 5 (+3-4 days).
- [XFB scatter] Verified ScatterCapturedRecords does a read-modify-write of the client shadow (DirectGLES.cpp:928, rationale :889-892, case KHR-GL46.transform_feedback.capture_special_interleaved_test). Moved the scatter to the client: server pushes packed scratch bytes via on_buffer_writeback + new on_xfb_scatter_ready{packedStride, vertices}; client patches and re-emits an ordinary resource_subdata. No new reverse read is introduced (§7.2.1).
- [unit-bindings debouncer] Confirmed GetTextureBindGeneration bumps on redundant re-binds (DirectGLES.cpp:1414-1420). Reclassified the ~115 lines from 'deleted' to 'relocated': the debounce becomes a client-side resolved-set xxHash emit suppressor (m_lastSetHash[]) covering every kVarTail set_*, and D9's viewSetSerial now has that as an explicit precondition. §2.5 split into ~372 lines truly deleted vs ~175 relocated; §3, §10.2 and §10.4 ledgers corrected.
- [multi-draw / restart ownership] Verified ResolveTierForBatch (MultiDraw.cpp:282-320) selects per batch using programReadsDrawID (a server-only ESSL fact) and that both backends perform the restart rewrite. Deleted kCapPrimitiveRestart/kCapPrimitiveRestartFixedIndex/kCapMultiDraw/kCapMultiDrawIndirect/kCapMultiDrawIndirectCount as ownership switches (D-B7); all five tiers and the restart rewrite stay server-side, fed in split mode by a new incrementally-maintained Server/IndexHostMirror gated on kCapNeedsHostIndexBytes (budgeted, counted, with a per-draw shipping fallback). Resolves the §4.5.7-vs-§5.8 contradiction and closes open question 12.
- [texture pull terminator] Added resource_subdata_complete(res, target, firstLevel, levelCount, pullSerial) which may carry zero regions; server proceeds with allocated-and-empty storage (matching monolith EnsureGenerateMipmapStorageAllocated at DirectGLES.cpp:6270-6271) plus a logged diagnostic. TextureRemintPullScenario must include the unanswerable case (render-only texture later image-bound) and be red before the terminator lands (§7.5e, P9).
- [verify survives P13] SnapshotFromGLContext and its MG_State includes are now kept behind #if MOBILEGL_PIPE_VERIFY past P13; the three purity gates run only on the non-verify build; P13 additionally delivers the MGPipe recorder golden mode as a long-term MG_State-free semantic gate and as the answer to open question 11 (D-B5, B-R17).
- [texture params] Verified SyncTextureParamsToBackend runs for FBO attachment textures (DirectGLES.cpp:1580-1601) and that RequireImageBindableStorage sets m_forceTextureParamsResync (Managers.cpp:2815-2821). Added set_texture_params(res, ...) carrying base/max level, swizzle, depth-stencil mode, LOD clamps and forceResync; MGPSamplerView reduced to view restriction only (new gallium deviation D10, plus a gate for attachment-only / image-only / CopyImage-endpoint textures).
- [emission cursor aliasing] Verified TextureObjectView forwards IsStorageDirty/MapMipmapData/MarkStorageDirty(Region)/GetStorageDirtyRegion to the storage owner with index remapping (TextureObjectView.cpp:281, 290-322). Keyed the client emission cursor on (storageOwnerHandle, ownerUploadTarget, ownerLevel) and added a view/owner aliasing scenario.
- [OOM ack] Verified the texture family never reaches the backend table and that even glRenderbufferStorage allocates lazily in SyncToBackend (Managers.cpp:8674-8684). Narrowed kNeedsAck to glBufferStorage plus, conditionally, glRenderbufferStorage*; P0 must answer whether the corpus actually contains a glRenderbufferStorage OOM probe. Stated plainly that texture allocation OOM is already deferred in the monolith so the split changes nothing observable (§7.4, §9.2-7).
- [SEG_STAGE sizing] Rewrote the new-byte-class list to six items including named-UBO host payloads and tightly repacked texture regions; removed the 64 MiB restart rewrite and the multi-draw flattened stream from SEG_STAGE entirely (they are served by the index host mirror), and required G3 to define a chunking/degradation path for a single record larger than the segment (§8.2, open question 9).
- [validate order] Replaced the numbered order contract with the invariant 'all set_* for a command complete before the verb; the server specializes at the verb'. D-B3 restated: what retires the fragColor workaround and ImageUnitFormatsStillMatch is late specialization, not framebuffer-first ordering (§5.3, D-B3).
- [reflection payload / glslang gate] Verified TypeFacts/ResourceReflection/XfbVarying/LinkArtifacts/SpirvArtifacts all live in ProgramObject.h, which includes ShaderObject.h (glslang) and SpvcSession.h (spirv_reflect), with 7 in-tree includers. Added a new prerequisite phase P0.5 that extracts them into ProgramArtifacts.h with a CI include-closure assertion, without which P7's `nm -D | grep glslang` criterion is unreachable (§0.4, §4.5.5, P0.5).
- [named UBO bytes] Verified UniformManager::ResolveUniformBufferPayload syncs at UniformManager.cpp:2022 and reads MappedData()+rangeStart at :2052 into Magma's own UBO ring - a server-side consumer that cannot move. Added an optional MGHostSpan payload to set_shader_buffers(cls==Uniform) gated by a new kCapNeedsHostUboBytes, plus a stage-ubo-named counter, and forbade freezing the payload shape before P0 gives byte volumes (D-B8, §5.7, §7.2).
- [phase arithmetic] Rebuilt every phase day count as the sum of the §6.4/§6.5 rows it contains and published the arithmetic; total changed from 200-260 to 267-337 person-days excluding CTS turnaround; milestones moved to days 25 / 43 / 99 / 104 / 145 / 187 / 267; re-baseline checkpoints set at the summed upper bound +50% (P3a >27d, P4a >39d); Espryt XFB given an explicit phase home in P3b/P4b (§11.5, B-R14).
- [verify blind spot] Added a verify-only retention mode: under MOBILEGL_PIPE_VERIFY the tracker keeps the pre-clear dirty set and G4 compares the emitted (unionBox, regionCount, regions[]) against a snapshot recompute; added TextureUploadShapeScenario recording upload shape and job count as a golden, because SSIM is insensitive to the +6 ms/frame box-vs-rect cliff (§7.3, §10.3-②, P3b/P4b).
- [stage-C A/B narrowing] Stated in §6.7 that MOBILEGL_PIPE_PUSH stops being an old-vs-new arm after stage C (both arms run the rekeyed memo code), and added a compile-time MOBILEGL_PIPE_LEGACY_MEMOS switch keeping the registry/TwinLookupMemo implementations alive through P3a/P4a, retired with the pull path at P13 (+1 day per phase, costed; new risk B-R16).
- [GO/NO-GO scope] Extended P2 to include one Track H slice per backend (Espryt 0b handle infrastructure, Magma subsystem 4) plus a Blaze3D blend-toggle microbenchmark and a CSO-content-addressing negative control, so day 43 measures the decision it gates; fallback cost restated honestly as 28-39 days rather than 16 (§0.6, P2, B-R1).
- [dirty marking vs polling] Verified no aggregate exists for 'did any bound texture's content move' (which is why Magma uses lossy sampledContentSum/sampledParamsSum). Added 推论 4: value groups keep the polling model with zero new bookkeeping; object groups get 5 new aggregate generations in MG_State (~20 lines at existing bump points), and gen_impl_mutation_surface.py is repurposed as gen_pipe_dirty_surface.py enumerating MG_Impl mutators to aggregate generations with a CI failure on any unmapped mutator (§0.3, §5.2, §10.3-⑤, B-R6 layer 4).
- [P1 byte identity] Verified MOBILEGL_ASSERT compiles away outside debug (Defines.h:114) but that the 7 null guards, 14 != nullptr conditions and 3 ternaries do generate code. Deferred those rewrites to P2, guarded SnapshotFromGLContext/G4/G5 behind build switches, and restated P1's acceptance as 'nm unchanged; .text delta attributable line by line' (P1).
- [restart/indirect ownership conflict] Resolved the §4.5.7-vs-§5.8 contradiction by keeping restart rewrite and multi-draw tiering server-side (D-B7), which also means the monolith's behaviour and diagnostic thread do not change and the name-for-name baseline moves only once (open question 12 closed).
- [stage parameter] Verified MobileGL has one combined 192-unit texture space (TextureState.h:41,128; TextureUnit.h:20,24-25) with the per-stage 32 being an advertised number only. Dropped the stage parameter from set_sampler_views and bind_sampler_states; stage flags are derived server-side from the reflection archive where the target API needs them (§4.4.3).
- [net LOC honesty] Added §2.7 estimating MGPipe's permanent additions (~6,650 hand-written + ~4,000 generated in the monolith, excluding MG_Remote) against ~372 lines truly deleted, demoted the deletion ledger to supporting evidence, and made §10.3-④'s per-thread CPU number the primary monolith argument (new risk B-R18).
- [citations] Verified SamplerObject.h is 160 lines and corrected every reference (BorderColorForm :60-70, SamplerParameters :72-96, GetLifetimeId :141, BumpVersion :151, m_version :155); added scripts/check_doc_citations.py as a P0 CI lint that every file:line in the docs resolves at the baseline commit.
- [per-draw cost口径] Verified the dynamic early-outs (SyncRenderState :2016-2018, SyncNeccessaryTextures, CurrentUnitBindingsEpoch :1418-1436, TrySetupDrawFastPath, GetOrCreatePipeline :4982-4993, ApplyDynamicDrawStateTail :5888-5893) and added §2.3.1: the real steady-state pull is ~10-25 accessor calls per backend per draw, not 124/169. Rewrote §10.2 in dynamic terms, added dynamic call/memo-hit counters to P0's deliverables, and required an absolute ns/draw threshold at the GO/NO-GO instead of a relative-to-noise one.
- [render-state CSO] Verified the two-counter rationale (RenderState.h:519-528) and that viewport/scissor/line-width setters bump only ++m_version while SET_CAPABILITY bumps BumpVersions (RenderState.cpp:312). Rewrote D-B1: the blob still travels whole for Espryt's span memcmp, but the CSO identity is the pipeline subset only (MGPipeComputePipelineSubsetHash moved verbatim out of VulkanRenderer.cpp:4826-4906 into MG_Pipe/), the dynamic subset goes through a new set_dynamic_state, the server keeps one working RenderStateParameters, and G7 generates a setter-consistency test asserting pipelineSubsetHash changes iff m_pipelineStateVersion changes. Client gates the hash on m_pipelineStateVersion so glViewport costs zero hashing and never evicts Magma's pipeline memo.
- [reconcile discipline] Verified MultiDrawElementsIndirectCount calls only SyncPersistentMappedRange (DirectGLES.cpp:4666-4667), never SyncGpuWrites. Replaced §5.8.1's blanket publish/wait/drain rule with a per-site table reproducing the monolith's set exactly, and added a P8 acceptance requiring roundtrips-per-frame to read zero on the create-indirect fixture; flagged the monolith's own omission as a separate dev question the split must not silently fix (open question 15).
- [purity gate] Verified RenderState.h:12 includes FramebufferObject.h which includes TextureObject.h/RenderbufferObject.h, and that RenderStateParameters sizes arrays with FramebufferObject::MAX_DRAW_BUFFERS (:263, :273), so the value-header allowlist is not a leaf set and nm --undefined-only is blind to include coupling. Split the purity gate into three: an include-graph gate (compile MG_Backend with MG_State/GLState off the search path) backed by a new MGPipeValueTypes.h extracted in P0.5, the symbol gate, and the undeclared gate - all run only on the non-verify build.
- [draw payload cost] Stated MGPDrawInfo's real cost against today's three-register DrawArrays, flag-gated minIndex/maxIndex and xfbCpuCapturedVertices (computed only where a consumer asked), moved the 32-byte MGHostSpan out of the fixed header into the var-tail, and added a per-draw payload-byte histogram to P0's counters (§4.5.7, §10.2).
- [memory arithmetic] Corrected §0.4-1 to a full table: 48.25 MiB transport + 0-32 MiB SEG_STAGE headroom + 0-64 MiB index host mirror (split only) + ~1-2 MiB records, with MOBILEGL_PIPE_TEXEL_RETAIN_MB defaulted to 0 because MipmapStorage keeps a complete CPU shadow so retention buys latency, not correctness. Typical +50-60 MiB, worst case ~+145 MiB.
- [generated mipmaps] Verified EnsureGenerateMipmapStorageAllocated does AllocateStorage + MarkStorageDirty(false) with no content (DirectGLES.cpp:6270-6271), so GPU-generated levels are allocated-and-zero in the monolith too. Decided explicitly that on_mip_levels_generated carries shape only, glGetTexImage stays 0 round trips on DirectGLES, and only the CPU fallback path produces texels via on_texture_writeback (§9.1).
- [map_persistent frequency] Corrected 'once per store lifetime' to 'once per storage definition' (TryAdoptLargeStorage fires at storage-definition time, so a regrowing arena pays N times) and required StorageBufferRegrowScenario to publish a map-persistent-roundtrips counter (D-B4, §8.3, §9.2-8).
- [MGHostSpan cost] Restated the monolith cost as one predictable branch plus 32 bytes carried only when kHasUserIndices is set, rather than 'zero'.
- [P5 inproc honesty] Added a specification clause that InProcessTransport uses the identical G3 serialization and differs only in doorbell/copy mechanism, plus a PipeApplier debug assertion that no SharedPtr or raw frontend pointer crosses the applier boundary in any transport, so the day-99 milestone actually proves wire completeness (P5).
- [P2 baseline definition] Defined the name-for-name functional baseline as 'the refactored monolith at P1 exit' (itself proven equivalent to 81b17c0b by verify), with 81b17c0b retained only as the performance anchor (§10.3-③, B-R3).
- [gate list] Added HandleRecycleScenario / TextureRemintPullScenario (with the unanswerable case) / TextureUploadShapeScenario / view-owner cursor aliasing scenario / attachment-only glTexParameter scenario / ClientArrayAfterComputeWriteScenario, each with an explicit statement of what must make it red before the corresponding fix lands.
- [callbacks] MGPipeCallbacks grew from 9 to 10 (added on_xfb_scatter_ready) plus the forward terminator resource_subdata_complete; set_* grew from 14 to 17 (set_dynamic_state, set_texture_params, and set_shader_buffers gaining kHostSpan); appendix A and the call-count totals updated throughout.
## 5. 被驳回或部分驳回的审查意见
- [performance #11, partial] 'glGetTexImage = 0 round trips does not survive the generated-mipmap ownership split' - the demand for an explicit decision was accepted, but the implied conclusion (it must become a blocking round trip or an eager multi-megabyte writeback) is refuted. EnsureGenerateMipmapStorageAllocated (DirectGLES.cpp:6270-6271) does AllocateStorage + MarkStorageDirty(false) with no content, so a GPU-generated level's shadow is allocated-and-zero in the monolith too; CopyTextureImageToClientOrPBO_State answers from it identically in both modes. on_mip_levels_generated therefore carries shape only and the row stays in §9.1 at zero round trips; only the CPU fallback path (RGB16F/RGB32F, :6811-6861) needs on_texture_writeback. Documented as an explicit decision in §9.1 rather than a fix.
- [skeptic framing on §0.4-4] The claim that gen_impl_mutation_surface.py 'vanishes' was corrected rather than accepted as-is: the replay obligation genuinely disappears (there is no replica), but the enumeration obligation reappears as dirty-marking, so the generator is repurposed (gen_pipe_dirty_surface.py) rather than deleted. Listing it as a pure deletion in §0.4-4 was the error; listing the enumeration obligation as unbudgeted was also inaccurate once the generator is repurposed - it is now a P2 deliverable.
- [correctness #6, partial] The proposed fix 'delete kCapMultiDraw* and let the client supply index bytes when caps say the server may need them' was accepted for tiering ownership but rejected in its transport form: shipping index bytes per draw through MGHostSpan would put up to 1<<24 indices on the ring per batch. Replaced with an incrementally-maintained server-side index host mirror (D-B7) that costs zero per-draw wire traffic, at the price of a budgeted, counted memory duplication limited to element-array-bound buffers in split mode only - stated openly in the §0.4-1 memory table as the design's one data copy.
+47 -4
View File
@@ -51,6 +51,48 @@ def wait_for_device(serial, attempts=20, delay=15):
return False
def device_file_size(serial, path):
r = adb(serial, "shell", f"stat -c %s {path} 2>/dev/null || echo 0", timeout=30)
m = re.search(r"(\d+)", r.stdout or "")
return int(m.group(1)) if m else 0
def run_chunk(serial, cmd, dev_qpa, dev_list, idle_timeout, poll_interval=15):
"""Run one glcts invocation; give up only when the log stops growing.
A chunk is thousands of cases and legitimately runs for an hour, so a fixed
wall-clock cap would kill healthy invocations and record whichever case was
in flight as a crash. A GPU hang, by contrast, stops the .qpa from growing.
The timeout is therefore measured from the last observed growth of the
device-side log. On expiry the device-side glcts is killed (matched by the
caselist path this runner alone uses, so other processes are left alone) and
returncode 124 is reported, the same signal a hard timeout used to give.
"""
proc = subprocess.Popen(["adb", "-s", serial, "shell", cmd],
stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
last_size = -1
last_growth = time.time()
while True:
try:
out, err = proc.communicate(timeout=poll_interval)
return subprocess.CompletedProcess(proc.args, proc.returncode, out, err)
except subprocess.TimeoutExpired:
pass
size = device_file_size(serial, dev_qpa)
now = time.time()
if size != last_size:
last_size = size
last_growth = now
elif now - last_growth > idle_timeout:
adb(serial, "shell", f"pkill -f {dev_list}", timeout=30)
proc.kill()
try:
proc.communicate(timeout=30)
except subprocess.TimeoutExpired:
pass
return subprocess.CompletedProcess(proc.args, 124, "", "idle timeout")
def mem_available_kb(serial):
r = adb(serial, "shell", "grep MemAvailable /proc/meminfo", timeout=30)
m = re.search(r"(\d+)", r.stdout or "")
@@ -152,7 +194,8 @@ def main():
ap.add_argument("--min-mem-kb", type=int, default=400000,
help="pause when the device drops below this much available memory")
ap.add_argument("--chunk-timeout", type=int, default=900,
help="seconds before giving up on one glcts invocation (a GPU hang never returns)")
help="seconds without any growth of the device-side .qpa before the glcts "
"invocation is declared hung and killed (a GPU hang never returns)")
ap.add_argument("--skip-file", default=None,
help="file of case names to exclude, e.g. cases known to hang the device")
ap.add_argument("--env", action="append", default=[], metavar="K=V",
@@ -239,10 +282,10 @@ def main():
f"--deqp-log-images=disable --deqp-log-shader-sources=disable "
f"--deqp-log-filename={dev_qpa} > /dev/null 2>&1; rc=$?; sync; echo RC=$rc"
)
run = adb(args.serial, "shell", cmd, timeout=args.chunk_timeout)
run = run_chunk(args.serial, cmd, dev_qpa, dev_list, args.chunk_timeout)
if run.returncode == 124:
print(f"[run_cts] chunk {chunk:04d} timed out after {args.chunk_timeout}s "
f"(likely a GPU hang)", file=sys.stderr)
print(f"[run_cts] chunk {chunk:04d}: no log growth for {args.chunk_timeout}s "
f"(likely a GPU hang); killed glcts", file=sys.stderr)
# Some cases hang the GPU hard enough to reboot the device. The log on
# /data/local/tmp survives that, so wait for the device to come back and