From 05d627ba2d6368546734538aca1620cbaeaee51a Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 27 Aug 2026 10:02:07 -0400 Subject: [PATCH] [Fix] (Xfb): never issue a transform feedback capture-point bind the application did not ask for, and error-check the driver span instead of losing it silently --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 255 ++++++- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 92 +++ MobileGL/MG_Backend/DirectGLES/Managers.h | 15 +- .../MG_Impl/GLImpl/Drawing/GL_Drawing.cpp | 34 +- MobileGL/MG_IntegrationTest/CMakeLists.txt | 1 + .../Scenarios/XfbRepeatedCaptureScenario.cpp | 657 ++++++++++++++++++ 6 files changed, 1025 insertions(+), 29 deletions(-) create mode 100644 MobileGL/MG_IntegrationTest/Scenarios/XfbRepeatedCaptureScenario.cpp diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 34ba243b..8df6f184 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -393,6 +393,65 @@ namespace MobileGL::MG_Backend::DirectGLES { } } + // The capture points the CAPTURE PROGRAM uses, and nothing else. + // + // This used to go through SyncBufferBindingPoints, which walks the application's + // GLOBAL touched-binding-point high-water mark and binds 0 to every point with no + // frontend buffer. deqp/glcts permanently raises that mark to + // GL_MAX_TRANSFORM_FEEDBACK_SEPARATE_ATTRIBS by clearing all of them after each test + // case, so every capture using fewer points than that - i.e. every INTERLEAVED_ATTRIBS + // capture - had glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, i, 0) issued for the + // unused tail immediately before glBeginTransformFeedback. The Mali G1-Ultra driver + // then recorded NOTHING: no GL error, GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN 0, the + // application's buffer left holding its pre-draw bytes. Confirmed on device - the + // separate/interleaved split in KHR-GL46.transform_feedback follows exactly whether + // all four points were left bound. + // + // Those binds were never needed for correctness either. A capture only writes the + // points the program's buffer mode uses (GL 4.6 core 13.2.2), so a point past + // bufferCount cannot be written whatever is left bound there, and a point the program + // DOES use with no buffer bound is already an error the frontend raised at + // glBeginTransformFeedback. The rule this encodes: never issue a capture-point bind + // the application did not ask for. + // + // Scoping it to the program (rather than skipping redundant binds behind the shadow) + // is what makes it ORDER-INDEPENDENT: the shadow has to drop to unknown whenever a + // transform feedback OBJECT is bound, since the points belong to the object, and the + // clears came straight back for the next capture in the process. + void SyncTransformFeedbackBindingPoints(SizeT bufferCount) { +#ifdef TRACY_ENABLE + ZoneScopedC(TRACY_ZONECOLOR_BACKEND); +#endif + const SizeT pointCount = std::min( + bufferCount, MG_State::GLState::GLContext::MAX_TRANSFORM_FEEDBACK_BUFFERS); + for (SizeT i = 0; i < pointCount; ++i) { + auto& point = MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::TransformFeedback, i); + const auto& obj = point.GetBoundObject(); + // A stride-0 slot (two consecutive gl_NextBuffer entries) captures nothing and + // needs no binding; anything else with no buffer never got past the frontend. + if (!obj) continue; + + auto* backendResource = EnsureBufferResource(obj); + if (!backendResource || backendResource->id == 0) { + MGLOG_E_ONCE("No backend buffer for GL_TRANSFORM_FEEDBACK_BUFFER capture point %zu; the capture " + "will not reach the application's buffer.", + i); + continue; + } + + const auto& range = point.GetRange(); + const auto backendBufferId = backendResource->id; + if (range.start == 0 && range.end >= obj->GetSize()) { + BindBufferBaseCached(GL_TRANSFORM_FEEDBACK_BUFFER, static_cast(i), backendBufferId); + } else { + const auto start = std::min(range.start, obj->GetSize()); + const auto end = std::min(range.end, obj->GetSize()); + BindBufferRangeCached(GL_TRANSFORM_FEEDBACK_BUFFER, static_cast(i), backendBufferId, + static_cast(start), static_cast(end - start)); + } + } + } + // Called once the storage-buffer points are bound and the draw/dispatch is about to // go out: whatever the shader writes there lands in the ES driver's buffers, behind // the frontend's CPU shadow. Flagging them makes the next MapBuffer/GetBufferSubData @@ -654,6 +713,14 @@ namespace MobileGL::MG_Backend::DirectGLES { Uint backendId = 0; SizeT start = 0; SizeT end = 0; + // WHICH capture buffer of the program this is. The list is COMPACTED - a + // capture buffer with no bound buffer object contributes no entry - so the + // position in the vector is not the program's buffer index, and everything + // that asks the program about a target (its stride, which varyings land in + // it) has to ask about this index instead. A capture list beginning with + // gl_NextBuffer is the shape that makes them differ: buffer 0 has stride 0 + // and nothing bound, so target 0 describes buffer 1. + SizeT bufferIndex = 0; }; // Per frontend transform feedback object. The default object (name 0) maps to @@ -698,6 +765,28 @@ namespace MobileGL::MG_Backend::DirectGLES { return *g_currentXfbState; } + // EVERY way this path can lose a capture used to be silent: three unlogged early + // returns before the driver Begin, an unchecked glBeginTransformFeedback, and two + // `continue`s in the readback. The application sees a buffer that kept its + // pre-draw bytes, GL_NO_ERROR, and GL_LINK_STATUS true - which is how one defect + // reached ~320 conformance bodies across four families before anyone could say + // which of the branches fired. Nothing below changes what MobileGL DOES on a + // healthy capture; it only makes a lost one name itself in /sdcard/MG/latest.log. + // + // MGLOG_E_ONCE (not _D) on purpose: these have to be readable in an INFO-level + // artifact, the same reason the backend link failure at Managers.cpp is MGLOG_E. + constexpr Int kMaxDrainedXfbErrors = 32; + + // The ES error raised by the call just issued, GL_NO_ERROR if it succeeded. Drains + // the rest of the queue so the next probe cannot read this one as its own. + GLenum TakeXfbDriverError() { + const GLenum first = g_GLESFuncs.glGetError(); + if (first == GL_NO_ERROR) return GL_NO_ERROR; + for (Int i = 0; i < kMaxDrainedXfbErrors && g_GLESFuncs.glGetError() != GL_NO_ERROR; ++i) { + } + return first; + } + Bool AreTransformFeedbackObjectsSupported() { return g_GLESFuncs.glGenTransformFeedbacks != nullptr && g_GLESFuncs.glBindTransformFeedback != nullptr && @@ -712,6 +801,16 @@ namespace MobileGL::MG_Backend::DirectGLES { // the backend already owns (coherent persistent map) need nothing: reads resolve // against that storage directly. void ReadbackCapturedRanges(Vector& targets) { + if (g_GLESFuncs.glMapBufferRange == nullptr || g_GLESFuncs.glUnmapBuffer == nullptr) { + MGLOG_E_ONCE("EndTransformFeedback: the ES driver exposes no glMapBufferRange/glUnmapBuffer, so " + "captured data can never reach the application's buffers"); + } + if (targets.empty()) { + // The span closed with nothing to mirror back. Either the deferred Begin + // never ran (a span with no draw - legal) or it ran and found no bound + // capture buffer, which is not. + MGLOG_D("EndTransformFeedback: capture span closed with no recorded targets"); + } if (g_GLESFuncs.glMapBufferRange != nullptr && g_GLESFuncs.glUnmapBuffer != nullptr) { for (const auto& target : targets) { if (!target.buffer || target.buffer->IsBackendPersistentMapped()) continue; @@ -721,8 +820,14 @@ namespace MobileGL::MG_Backend::DirectGLES { static_cast(target.start), static_cast(size), GL_MAP_READ_BIT); if (mapped == nullptr) { - MGLOG_E_ONCE("EndTransformFeedback: failed to map backend buffer %u for capture readback", - target.backendId); + // Silent before: the capture landed in the ES buffer and the + // application's next glMapBuffer read the untouched shadow, which + // is indistinguishable from "the draw wrote nothing". + MGLOG_E_ONCE("EndTransformFeedback: failed to map backend buffer %u [%zu, %zu) for " + "capture readback (ES error %s); the captured data will NOT be visible to " + "the application", + target.backendId, target.start, target.end, + MG_Util::ConvertGLEnumToString(TakeXfbDriverError()).c_str()); continue; } target.buffer->WritebackFromBackend({mapped, size}, target.start); @@ -755,15 +860,20 @@ namespace MobileGL::MG_Backend::DirectGLES { GL_DYNAMIC_COPY); g_scatterBufferSize = required; } - // Point 0 carries every captured varying (the ES capture is INTERLEAVED); the - // other points must be cleared or the driver would still write the app's buffers. + // Point 0 carries every captured varying: the gl_NextBuffer / gl_SkipComponents + // entries are consumed at link time and never reach the driver, so the ES + // program is declared INTERLEAVED over a single buffer and point 0 is the only + // point it can write (GL 4.6 core 13.2.2). + // + // The other points are therefore left exactly as they are. Clearing them - which + // this used to do, across the application's whole touched high-water mark - is + // both unnecessary (the ES program cannot write an unused point) and the precise + // trigger for the Mali G1-Ultra capture loss: see + // SyncTransformFeedbackBindingPoints for the mechanism and the device evidence. + // KHR-GL46.transform_feedback.capture_special_interleaved_test is the case that + // reaches this path. BufferImpl::BindBufferRangeCached(GL_TRANSFORM_FEEDBACK_BUFFER, 0, g_scatterBufferId, 0, static_cast(required)); - const SizeT pointCount = - MG_State::pGLContext->GetTouchedBufferBindingPointCount(BufferTarget::TransformFeedback); - for (SizeT i = 1; i < pointCount; ++i) { - BufferImpl::BindBufferBaseCached(GL_TRANSFORM_FEEDBACK_BUFFER, static_cast(i), 0); - } return true; } @@ -777,10 +887,22 @@ namespace MobileGL::MG_Backend::DirectGLES { if (g_GLESFuncs.glMapBufferRange == nullptr || g_GLESFuncs.glUnmapBuffer == nullptr) return; const SizeT packedStride = program->GetTransformFeedbackPackedStride(); - const SizeT vertices = std::min( - static_cast(MG_State::pGLContext->GetTransformFeedbackCapturedVertices()), - xfb.scatterCapacityVertices); - if (packedStride == 0 || vertices == 0) return; + const SizeT modelledVertices = + static_cast(MG_State::pGLContext->GetTransformFeedbackCapturedVertices()); + const SizeT vertices = std::min(modelledVertices, xfb.scatterCapacityVertices); + if (packedStride == 0 || vertices == 0) { + // The scatter path redirected the DRIVER's capture into the scratch buffer, + // so bailing here leaves the application's buffers holding their pre-draw + // bytes - a total data loss, not a no-op. The vertex count is the CPU model + // (AccountTransformFeedbackPrimitives), which is 0 for any draw mode + // CountPrimitivesForDraw does not know and for the instanced/indirect entry + // points that never call it. + MGLOG_E_ONCE("EndTransformFeedback: scattered capture discarded - packedStride=%zu, " + "CPU-modelled captured vertices=%zu, scratch capacity=%zu. The capture buffers keep " + "their pre-draw contents.", + packedStride, modelledVertices, xfb.scatterCapacityVertices); + return; + } BufferImpl::BindBufferId(BufferImpl::TempBufferTarget, g_scatterBufferId); const void* packed = g_GLESFuncs.glMapBufferRange(BufferImpl::TempBufferTarget, 0, @@ -798,14 +920,15 @@ namespace MobileGL::MG_Backend::DirectGLES { for (SizeT targetIndex = 0; targetIndex < xfb.targets.size(); ++targetIndex) { const auto& target = xfb.targets[targetIndex]; if (!target.buffer) continue; - const SizeT stride = program->GetTransformFeedbackStride(static_cast(targetIndex)); + // By BUFFER index, not by position in the compacted list - see XfbCaptureTarget. + const SizeT stride = program->GetTransformFeedbackStride(static_cast(target.bufferIndex)); if (stride == 0) continue; const SizeT rangeBytes = target.end - target.start; Vector staged(rangeBytes); Memcpy(staged.data(), target.buffer->MappedData() + target.start, rangeBytes); for (const auto& varying : program->GetTransformFeedbackVaryings()) { - if (varying.bufferIndex != targetIndex) continue; + if (varying.bufferIndex != target.bufferIndex) continue; for (SizeT v = 0; v < vertices; ++v) { const SizeT dstOffset = v * stride + varying.offsetBytes; if (dstOffset + varying.byteSize > rangeBytes) break; @@ -860,9 +983,20 @@ namespace MobileGL::MG_Backend::DirectGLES { // not captured, and opening the span would also subject it to the capture // primitive-mode rule the paused draw is exempt from. if (!xfb.pending || xfb.paused) return; - xfb.pending = false; const auto& program = MG_State::pGLContext->GetTransformFeedbackProgram(); - if (!program) return; + if (!program) { + // The pending flag is deliberately NOT consumed here. It used to be cleared + // before this check, so a single draw that could not see the capture program + // retired the span permanently: every later draw of the same span found + // pending==false, the driver Begin never happened, and End found started==false + // and skipped the readback - a whole capture lost with no GL error anywhere. + // The frontend only reaches a draw with an active span after glBeginTransformFeedback + // stored a program, so this is a "cannot happen" that must stay recoverable. + MGLOG_E_ONCE("StartPendingTransformFeedback: an active capture span has no capture program; the " + "driver span stays closed and this draw is not captured"); + return; + } + xfb.pending = false; // Snapshot what the driver is about to capture into. GL forbids rebinding the // capture buffers while the span is open, so this stays valid until End, and @@ -879,10 +1013,10 @@ namespace MobileGL::MG_Backend::DirectGLES { const SizeT start = std::min(range.start, bufferObject->GetSize()); const SizeT end = std::min(range.end, bufferObject->GetSize()); if (end <= start) continue; - xfb.targets.push_back({bufferObject, backendResource->id, start, end}); + xfb.targets.push_back({bufferObject, backendResource->id, start, end, i}); } - BufferImpl::SyncBufferBindingPoints(BufferTarget::TransformFeedback, GL_TRANSFORM_FEEDBACK_BUFFER); + BufferImpl::SyncTransformFeedbackBindingPoints(bufferCount); // A layout with holes or several interleaved buffers is not expressible on ES: // capture gap-free into scratch storage and place the records at End instead. @@ -891,31 +1025,99 @@ namespace MobileGL::MG_Backend::DirectGLES { xfb.scatterCapacityVertices = 0; if (program->NeedsScatteredTransformFeedbackCapture()) { SizeT capacityVertices = ~SizeT(0); - for (SizeT i = 0; i < xfb.targets.size(); ++i) { - const SizeT stride = program->GetTransformFeedbackStride(static_cast(i)); + for (const auto& target : xfb.targets) { + // By BUFFER index. Reading the stride at the target's POSITION made a + // capture list beginning with gl_NextBuffer - buffer 0 has stride 0 and + // nothing bound, so target 0 describes buffer 1 - read stride 0, skip every + // target, and leave the capacity at zero. + const SizeT stride = program->GetTransformFeedbackStride(static_cast(target.bufferIndex)); if (stride == 0) continue; - capacityVertices = - std::min(capacityVertices, (xfb.targets[i].end - xfb.targets[i].start) / stride); + capacityVertices = std::min(capacityVertices, (target.end - target.start) / stride); } if (capacityVertices == ~SizeT(0)) capacityVertices = 0; if (BindScatterCaptureBuffer(program->GetTransformFeedbackPackedStride(), capacityVertices)) { xfb.scattered = true; xfb.scatterProgram = program; xfb.scatterCapacityVertices = capacityVertices; + } else { + // NO SPAN RATHER THAN A SPAN THAT WRITES SOMEWHERE ELSE. The ES program for a + // scattered capture is a single-buffer INTERLEAVED one (the gl_NextBuffer / + // gl_SkipComponents entries are consumed at link time and never reach the + // driver), so it writes capture point 0 and nothing else. Point 0 here is + // either unbound or - the dangerous case - still holds whatever an earlier + // capture in this process bound there, because the frontend's own + // glBindBufferBase is state-only and nothing else in the backend touches the + // indexed points. Opening the span would then have the driver capture over an + // application buffer that has nothing to do with this draw, and the frontend + // shadow would never learn of it. + // + // Leaving the span closed reproduces exactly what the old high-water clear + // loop achieved by binding 0 here and letting the driver refuse the Begin - + // the capture records nothing - without issuing a capture-point bind the + // application did not ask for, which is the thing that loses captures whole + // on Mali (see SyncTransformFeedbackBindingPoints). + MGLOG_E_ONCE("StartPendingTransformFeedback: no scratch storage for a scattered capture " + "(packed stride %zu, capacity %zu vertices); leaving the driver span CLOSED so the " + "capture cannot land in a stale binding. Nothing will be captured.", + program->GetTransformFeedbackPackedStride(), capacityVertices); + xfb.targets.clear(); + return; } } + // A capture program with buffers bound must have produced at least one target; + // an empty list means End has nothing to mirror back and the application will + // read its buffer's pre-draw bytes however well the GPU captured. + if (xfb.targets.empty()) { + MGLOG_E_ONCE("StartPendingTransformFeedback: opening a capture span with NO capture targets " + "(program declares %zu capture buffer(s), none of them resolved to a bound backend " + "buffer with a non-empty range); nothing will be read back", + bufferCount); + } + g_GLESFuncs.glBeginTransformFeedback(xfb.primitiveMode); + // Unchecked before. Every ES error condition here (already active, a current + // program with no capture set, a capture point the program uses with no buffer) + // ends the same way: the driver records nothing, GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN + // reads 0 and the application sees no error at all - MobileGL's own error state is + // separate from the driver's, so a driver rejection here is invisible to it. + if (const GLenum beginError = TakeXfbDriverError(); beginError != GL_NO_ERROR) { + // The mode is printed as a number as well as a name: GL_POINTS is 0, which the + // enum converter spells "GL_FALSE", and a reader chasing a lost capture should + // not have to know that. + MGLOG_E_ONCE("StartPendingTransformFeedback: the ES driver REJECTED " + "glBeginTransformFeedback(%s / 0x%04x) with %s - nothing will be captured. Backend " + "program %u, %zu capture buffer(s), %zu target(s), mode=%s.", + MG_Util::ConvertGLEnumToString(xfb.primitiveMode).c_str(), + static_cast(xfb.primitiveMode), + MG_Util::ConvertGLEnumToString(beginError).c_str(), + PrgramImpl::g_lastUsedBackendProgramId, bufferCount, + xfb.targets.size(), + MG_Util::ConvertGLEnumToString(program->GetTransformFeedbackBufferMode()).c_str()); + } xfb.started = true; } void EndTransformFeedback() { auto& xfb = CurrentXfb(); + const Bool wasPending = xfb.pending; xfb.pending = false; xfb.paused = false; - if (!xfb.started) return; + if (!xfb.started) { + // A span that never drew is legal and captures nothing by definition; one that + // is STILL pending here drew nothing the backend saw, which for a span the + // application expected data from is the whole bug in one line. + MGLOG_D("EndTransformFeedback: closing a span the driver never opened (pending=%d)", + wasPending ? 1 : 0); + return; + } xfb.started = false; g_GLESFuncs.glEndTransformFeedback(); + if (const GLenum endError = TakeXfbDriverError(); endError != GL_NO_ERROR) { + MGLOG_E_ONCE("EndTransformFeedback: the ES driver rejected glEndTransformFeedback with %s - the " + "driver's capture state and MobileGL's have diverged", + MG_Util::ConvertGLEnumToString(endError).c_str()); + } if (xfb.scattered) { ScatterCapturedRecords(xfb); xfb.scattered = false; @@ -943,6 +1145,10 @@ namespace MobileGL::MG_Backend::DirectGLES { void BindTransformFeedback(GLuint name) { g_currentXfbState = nullptr; // name changes; operator[] below may also rehash + // The capture buffer bindings are the OBJECT's, not the context's: the bind below + // swaps all of them for whatever the target object holds, which the redundant-bind + // shadow has never seen. + BufferImpl::InvalidateTransformFeedbackBindingShadows(); if (!AreTransformFeedbackObjectsSupported()) { // Without driver objects there is only the default span; keep the frontend // name so the bookkeeping below stays consistent. @@ -979,6 +1185,7 @@ namespace MobileGL::MG_Backend::DirectGLES { g_currentXfbName = 0; g_scatterBufferId = 0; g_scatterBufferSize = 0; + BufferImpl::InvalidateTransformFeedbackBindingShadows(); } } // namespace XfbImpl diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 92616ba7..d53426a1 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -1383,10 +1383,24 @@ namespace MobileGL::MG_Backend::DirectGLES { constexpr SizeT kMaxIndexedBufferBindings = 64; IndexedBufferBinding g_indexedUBOBindings[kMaxIndexedBufferBindings]; IndexedBufferBinding g_indexedSSBOBindings[kMaxIndexedBufferBindings]; + // Transform feedback gets a shadow for a reason the other two do not have: the + // capture points are synced from the application's TOUCHED high-water mark, which + // deqp/glcts permanently raises to GL_MAX_TRANSFORM_FEEDBACK_SEPARATE_ATTRIBS by + // clearing every point after each test case. Without a shadow every capture that + // uses fewer points than that (i.e. every INTERLEAVED_ATTRIBS capture) re-issued a + // redundant glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, i, 0) for the unused + // tail immediately before glBeginTransformFeedback - calls a plain GL application + // never makes there, and the only thing MobileGL does differently from one. + // + // Unlike the UBO/SSBO points these are NOT context state: they belong to the bound + // transform feedback OBJECT, so XfbImpl::BindTransformFeedback drops the whole + // shadow to unknown on every object switch (InvalidateTransformFeedbackBindingShadows). + IndexedBufferBinding g_indexedXFBBindings[kMaxIndexedBufferBindings]; IndexedBufferBinding* IndexedBindingShadow(GLenum glTarget, Uint index) { if (index >= kMaxIndexedBufferBindings) return nullptr; // out of range: never cache if (glTarget == GL_UNIFORM_BUFFER) return &g_indexedUBOBindings[index]; if (glTarget == GL_SHADER_STORAGE_BUFFER) return &g_indexedSSBOBindings[index]; + if (glTarget == GL_TRANSFORM_FEEDBACK_BUFFER) return &g_indexedXFBBindings[index]; return nullptr; } @@ -1403,6 +1417,9 @@ namespace MobileGL::MG_Backend::DirectGLES { for (auto& binding : g_indexedSSBOBindings) { if (binding.id == id) binding = {}; } + for (auto& binding : g_indexedXFBBindings) { + if (binding.id == id) binding = {}; + } if (g_boundPixelPackBufferKnown && g_boundPixelPackBufferId == id) { g_boundPixelPackBufferId = 0; } @@ -1419,9 +1436,21 @@ namespace MobileGL::MG_Backend::DirectGLES { for (auto& binding : g_indexedSSBOBindings) { if (binding.id == id) binding.known = false; } + for (auto& binding : g_indexedXFBBindings) { + if (binding.id == id) binding.known = false; + } } } // namespace + // The capture points belong to the bound transform feedback object, so a bind (or a + // delete, which reverts to the default object) replaces all of them at once with + // state this shadow has never seen. Distrust rather than scrub: the driver's bindings + // are whatever the newly bound object holds, which is NOT necessarily base(0), and + // scrubbing would let a later bind of 0 be false-skipped. + void InvalidateTransformFeedbackBindingShadows() { + for (auto& binding : g_indexedXFBBindings) binding.known = false; + } + void BindBufferBaseCached(GLenum glTarget, Uint index, Uint id) { auto* s = IndexedBindingShadow(glTarget, index); if (s && s->known && s->isBase && s->id == id) return; @@ -1439,6 +1468,7 @@ namespace MobileGL::MG_Backend::DirectGLES { void InvalidateIndexedBufferBindingCache() { for (auto& b : g_indexedUBOBindings) b = {}; for (auto& b : g_indexedSSBOBindings) b = {}; + for (auto& b : g_indexedXFBBindings) b = {}; } void TrimBufferPool() { @@ -5753,6 +5783,10 @@ namespace MobileGL::MG_Backend::DirectGLES { Uint g_fragColorBroadcastCount = 1; Uint32 g_unormFallbackClampOutputMask = 0; Uint g_lastUsedBackendProgramId = 0; + // Every error-queue drain in the program build path is bounded by this: a lost + // context never answers GL_NO_ERROR, and the build runs on the thread that would + // then spin forever. + constexpr Int kMaxDrainedProgramErrors = 32; StateBackendObjectRegistry g_backendProgramObjects; BackendProgramObjectImpl::BackendProgramObjectImpl() { @@ -7523,6 +7557,7 @@ namespace MobileGL::MG_Backend::DirectGLES { // program before it links. SPIRV-Cross keeps user output names verbatim in // the transpiled ESSL (`out vec4 result_0;` stays `result_0`), so the // frontend's requested names carry over unchanged. + SizeT declaredXfbVaryingCount = 0; if (stateProgramObject->GetTransformFeedbackVaryingCount() > 0 && g_GLESFuncs.glTransformFeedbackVaryings != nullptr) { const auto& xfbVaryings = stateProgramObject->GetTransformFeedbackVaryings(); @@ -7548,9 +7583,31 @@ namespace MobileGL::MG_Backend::DirectGLES { } MGLOG_D("Declaring %zu transform feedback varyings on program %u", xfbNames.size(), m_backendProgramId); + // Bounded: a lost context never answers GL_NO_ERROR, and this runs on the + // thread that would then spin forever. + for (Int i = 0; i < kMaxDrainedProgramErrors && g_GLESFuncs.glGetError() != GL_NO_ERROR; ++i) { + } g_GLESFuncs.glTransformFeedbackVaryings(m_backendProgramId, static_cast(xfbNames.size()), xfbNames.data(), stateProgramObject->GetTransformFeedbackBufferMode()); + // Unchecked before. A rejected capture set leaves the program linking happily + // with NO capture set at all, and then every draw of every span records + // nothing while the application reads its buffer's pre-draw bytes and + // GL_NO_ERROR - the signature four conformance families were stuck on. + if (const GLenum xfbError = g_GLESFuncs.glGetError(); xfbError != GL_NO_ERROR) { + String declared; + for (const auto& xfbName : rewrittenXfbNames) { + if (!declared.empty()) declared += ", "; + declared += xfbName; + } + MGLOG_E("The ES driver REJECTED the transform feedback capture set for backend program %u with " + "%s (mode %s): [%s]. Every capture made with GL program %u will record nothing.", + m_backendProgramId, MG_Util::ConvertGLEnumToString(xfbError).c_str(), + MG_Util::ConvertGLEnumToString( + stateProgramObject->GetTransformFeedbackBufferMode()).c_str(), + declared.c_str(), stateProgramObject->GetExternalIndex()); + } + declaredXfbVaryingCount = xfbNames.size(); } // Link program @@ -7593,6 +7650,41 @@ namespace MobileGL::MG_Backend::DirectGLES { } } else { MGLOG_D("Program linked successfully. ID: %u", m_backendProgramId); + // A link that SUCCEEDS can still have dropped the capture set: ESSL rejects a + // requested name the transpiled shader does not actually declare by simply not + // capturing it, and a program whose last vertex-processing stage was rewritten + // by a SPIR-V pass (viewport-index lowering, gl_PerVertex handling, the + // synthesized pass-through tessellation control stage) can end up spelling its + // outputs differently from the frontend's request. Asking the driver what it + // ACTUALLY linked is the only way to tell that apart from a driver that just + // captures nothing - which is the whole ambiguity the empty-capture failures + // across geometry_shader / tessellation_shader / gpu_shader5 / DSA sat on. + if (declaredXfbVaryingCount > 0) { + GLint linkedXfbVaryings = 0; + GLint linkedXfbBufferMode = 0; + g_GLESFuncs.glGetProgramiv(m_backendProgramId, GL_TRANSFORM_FEEDBACK_VARYINGS, + &linkedXfbVaryings); + g_GLESFuncs.glGetProgramiv(m_backendProgramId, GL_TRANSFORM_FEEDBACK_BUFFER_MODE, + &linkedXfbBufferMode); + for (Int i = 0; i < kMaxDrainedProgramErrors && g_GLESFuncs.glGetError() != GL_NO_ERROR; ++i) { + } + const GLenum requestedMode = stateProgramObject->GetTransformFeedbackBufferMode(); + if (static_cast(std::max(linkedXfbVaryings, 0)) != declaredXfbVaryingCount || + static_cast(linkedXfbBufferMode) != requestedMode) { + MGLOG_E("Backend program %u (GL program %u) linked with a capture set the driver does not " + "agree with: asked for %zu varying(s) in mode %s, the driver reports %d varying(s) " + "in mode %s. Captures made with it will be empty or wrongly laid out.", + m_backendProgramId, stateProgramObject->GetExternalIndex(), declaredXfbVaryingCount, + MG_Util::ConvertGLEnumToString(requestedMode).c_str(), linkedXfbVaryings, + MG_Util::ConvertGLEnumToString( + static_cast(linkedXfbBufferMode)).c_str()); + } else { + MGLOG_D("Backend program %u capture set confirmed by the driver: %d varying(s), mode %s", + m_backendProgramId, linkedXfbVaryings, + MG_Util::ConvertGLEnumToString( + static_cast(linkedXfbBufferMode)).c_str()); + } + } } // The driver program was relinked IN PLACE, so its GL name no longer identifies // the executable behind it - and that name is exactly what Use()'s diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 82ed1c7d..36d24936 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -544,12 +544,21 @@ namespace MobileGL::MG_Backend::DirectGLES { // BackendVertexArrayObject::SyncToBackend. extern Uint64 g_bufferBackendIdGeneration; // Redundant-bind cache for INDEXED buffer bindings (glBindBufferBase/Range on - // GL_UNIFORM_BUFFER / GL_SHADER_STORAGE_BUFFER): skips the GL call when the - // (id, range) already at that index matches, like the array-buffer/texture/ - // sampler caches already do. Invalidated on MakeCurrent (context may reset). + // GL_UNIFORM_BUFFER / GL_SHADER_STORAGE_BUFFER / GL_TRANSFORM_FEEDBACK_BUFFER): + // skips the GL call when the (id, range) already at that index matches, like the + // array-buffer/texture/sampler caches already do. Invalidated on MakeCurrent + // (context may reset). + // Binds the transform feedback capture points [0, bufferCount) from the frontend + // state, and touches nothing else - in particular it never binds a zero the + // application did not ask for. See the definition for why that matters on Mali. + void SyncTransformFeedbackBindingPoints(SizeT bufferCount); void BindBufferBaseCached(GLenum glTarget, Uint index, Uint id); void BindBufferRangeCached(GLenum glTarget, Uint index, Uint id, GLintptr offset, GLsizeiptr size); void InvalidateIndexedBufferBindingCache(); + // The transform feedback capture points are per-transform-feedback-OBJECT state, so + // every glBindTransformFeedback swaps all of them under the shadow above. XfbImpl + // calls this on each bind/delete. + void InvalidateTransformFeedbackBindingShadows(); // Re-issues the GL_ATOMIC_COUNTER_BUFFER binding points a program's shaders declare as // GL_SHADER_STORAGE_BUFFER bindings at the reserved slots the transpiled ESSL was built // against (BackendProgramObjectImpl::GetAtomicCounterBindings / diff --git a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp index 16acd253..d76e8019 100644 --- a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp +++ b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp @@ -146,6 +146,20 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_TRIANGLES: return static_cast(count / 3); case GL_TRIANGLE_STRIP: case GL_TRIANGLE_FAN: return count >= 3 ? static_cast(count - 2) : 0; + // Adjacency primitives (GL 4.6 core table 10.1). Only a geometry stage can consume + // them, and it is the ADJACENT-free primitive count that reaches it: 4 vertices per + // line, 6 per triangle, one per step for the strips. Answering 0 here - which is what + // the default arm did - made AccountTransformFeedbackPrimitives bail before it had + // recorded anything, so an adjacency capture advanced neither the captured-vertex + // counter the scattered-capture path is bounded by nor the geometry-capture-draw flag + // that routes the transform feedback queries to the driver's own counter. + case GL_LINES_ADJACENCY: return static_cast(count / 4); + case GL_LINE_STRIP_ADJACENCY: return count >= 4 ? static_cast(count - 3) : 0; + case GL_TRIANGLES_ADJACENCY: return static_cast(count / 6); + case GL_TRIANGLE_STRIP_ADJACENCY: return count >= 6 ? static_cast((count - 4) / 2) : 0; + // GL_PATCHES is deliberately absent: the tessellator's amplification is not knowable + // on the CPU, and answering 0 is what defers GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN + // to the driver's own counter, which is the only correct source for a patch capture. default: return 0; } } @@ -172,11 +186,17 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_LINES: case GL_LINE_STRIP: case GL_LINE_LOOP: + // An adjacency primitive delivers the same line/triangle to the geometry stage; the + // adjacent vertices are context, not part of the primitive. + case GL_LINES_ADJACENCY: + case GL_LINE_STRIP_ADJACENCY: verticesPerPrimitive = 2; break; case GL_TRIANGLES: case GL_TRIANGLE_STRIP: case GL_TRIANGLE_FAN: + case GL_TRIANGLES_ADJACENCY: + case GL_TRIANGLE_STRIP_ADJACENCY: verticesPerPrimitive = 3; break; default: @@ -381,11 +401,21 @@ namespace MobileGL::MG_Impl::GLImpl { case GL_POINTS: compatible = mode == GL_POINTS; break; + // The adjacency modes belong here too (GL 4.6 core table 13.1, ES 3.2 table 12.1). + // This arm is only reached when the program has NO geometry or tessellation + // evaluation stage, and without a geometry stage the adjacent vertices are simply + // ignored (GL 4.6 core 10.1) - the primitive assembled IS a plain line or triangle, + // so the combination is legal and must capture. Omitting them raised a spurious + // GL_INVALID_OPERATION and dropped the draw entirely, leaving the capture buffer + // with its pre-draw bytes. The geometry-stage input table above already carries the + // same four arms; this is the second table catching up with it. case GL_LINES: - compatible = mode == GL_LINES || mode == GL_LINE_STRIP || mode == GL_LINE_LOOP; + compatible = mode == GL_LINES || mode == GL_LINE_STRIP || mode == GL_LINE_LOOP || + mode == GL_LINES_ADJACENCY || mode == GL_LINE_STRIP_ADJACENCY; break; case GL_TRIANGLES: - compatible = mode == GL_TRIANGLES || mode == GL_TRIANGLE_STRIP || mode == GL_TRIANGLE_FAN; + compatible = mode == GL_TRIANGLES || mode == GL_TRIANGLE_STRIP || mode == GL_TRIANGLE_FAN || + mode == GL_TRIANGLES_ADJACENCY || mode == GL_TRIANGLE_STRIP_ADJACENCY; break; default: break; diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 1075657e..655e434d 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -97,6 +97,7 @@ add_executable(MobileGLIntegrationTest Scenarios/VertexAttribBindingScenario.cpp Scenarios/XfbCaptureBufferReuseScenario.cpp Scenarios/XfbPrimitiveQueryScenario.cpp + Scenarios/XfbRepeatedCaptureScenario.cpp Scenarios/VertexArrayEnableDisableScenario.cpp Scenarios/CopyImageLevelRangeScenario.cpp Scenarios/CopyImageLayeredScenario.cpp diff --git a/MobileGL/MG_IntegrationTest/Scenarios/XfbRepeatedCaptureScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/XfbRepeatedCaptureScenario.cpp new file mode 100644 index 00000000..1aea71f4 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/XfbRepeatedCaptureScenario.cpp @@ -0,0 +1,657 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/XfbRepeatedCaptureScenario.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// Scenario - A CAPTURE MUST STILL RECORD WHEN IT IS NOT THE FIRST ONE IN THE PROCESS, +// AND THE CAPTURE STAGE MAY BE ANY OF THE FOUR THAT CAN BE THE LAST ONE. +// +// The conformance suite exposed a whole family of transform feedback failures that no +// existing scenario could reproduce, because every one of them ran ONE capture, from a +// VERTEX stage, in a freshly initialised process. What the suite actually does is +// different in three ways at once, and each of them turned out to matter: +// +// * it runs case after case in ONE GL context, resetting state between them - and the +// reset is not a fresh context. Its transform feedback part +// (framework/opengl/gluStateReset.cpp resetStateGLCore) unbinds the generic +// GL_TRANSFORM_FEEDBACK_BUFFER and then clears every indexed capture point from 0 to +// GL_MAX_TRANSFORM_FEEDBACK_SEPARATE_ATTRIBS, which permanently raises MobileGL's +// touched-binding-point high-water mark. Every later capture that uses fewer points +// than that - i.e. every INTERLEAVED_ATTRIBS capture - then had the unused tail +// re-cleared on the driver immediately before glBeginTransformFeedback. +// ReplayDeqpStateReset below is that reset, reduced to the calls that touch capture +// state, so a defect that only appears from the second capture onwards is reachable +// here instead of only on a device. +// +// * the capture stage is frequently a GEOMETRY or a TESSELLATION EVALUATION shader, +// never a plain vertex shader. The tree had zero coverage for either: none of the +// Xfb* scenarios mentioned tessellation and neither TessellationDrawModeScenario nor +// GeometryDrawModeScenario mentioned transform feedback. +// +// * the capture program frequently has NO FRAGMENT STAGE at all, because it draws +// under GL_RASTERIZER_DISCARD and never rasterises anything. That is legal in +// desktop GL and the shape most "use transform feedback as a readback channel" +// tests are built on. +// +// Every case here asserts the captured BYTES, never just the absence of a GL error: the +// failure this guards against writes nothing and raises nothing, so a buffer that kept +// its poison is the only thing that distinguishes it from success. + +#include +#include +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + // Nothing a capture can legitimately produce, so a component that still reads it + // names the failure ("the capture never reached these bytes") instead of looking + // like an ordinary numeric mismatch. + constexpr int kPoison = -987654; + + const char* const kPassthroughVertexSource = R"(#version 420 core +layout(location = 0) in int vs_in_value; +flat out int vs_out_value; +void main() +{ + vs_out_value = vs_in_value; + gl_Position = vec4(0.0, 0.0, 0.0, 1.0); +} +)"; + + // The primitive_counter shape: one flat int per emitted vertex, several vertices + // per input primitive, so the capture is geometry-AMPLIFIED and the CPU-side + // primitive model cannot predict its length. + const char* const kPointAmplifyingGeometrySource = R"(#version 420 core +layout(points) in; +layout(points, max_vertices = 2) out; +flat in int vs_out_value[]; +flat out int gs_out_value; +void main() +{ + for (int i = 0; i < 2; ++i) + { + gs_out_value = vs_out_value[0]; + gl_Position = gl_in[0].gl_Position; + EmitVertex(); + EndPrimitive(); + } +} +)"; + + // Adjacency input. Only a geometry stage can consume it, and CountPrimitivesForDraw + // used to answer 0 for every adjacency mode, which silently excluded the whole draw + // from the capture accounting. + const char* const kAdjacencyGeometrySource = R"(#version 420 core +layout(lines_adjacency) in; +layout(points, max_vertices = 1) out; +flat in int vs_out_value[]; +flat out int gs_out_value; +void main() +{ + gs_out_value = vs_out_value[1]; + gl_Position = gl_in[1].gl_Position; + EmitVertex(); + EndPrimitive(); +} +)"; + + const char* const kTessControlSource = R"(#version 420 core +layout(vertices = 1) out; +flat in int vs_out_value[]; +patch out int tcs_out_value; +void main() +{ + tcs_out_value = vs_out_value[0]; + gl_TessLevelOuter[0] = 1.0; + gl_TessLevelOuter[1] = 1.0; + gl_TessLevelOuter[2] = 1.0; + gl_TessLevelInner[0] = 1.0; + gl_out[gl_InvocationID].gl_Position = gl_in[0].gl_Position; +} +)"; + + const char* const kTessEvalSource = R"(#version 420 core +layout(triangles, equal_spacing, cw) in; +patch in int tcs_out_value; +flat out int tes_out_value; +void main() +{ + tes_out_value = tcs_out_value; + gl_Position = gl_in[0].gl_Position; +} +)"; + + const char* const kFragmentSource = R"(#version 420 core +flat in int gs_out_value; +out vec4 fragColor; +void main() +{ + fragColor = vec4(float(gs_out_value), 0.0, 0.0, 1.0); +} +)"; + + class XfbRepeatedCaptureScenario : public ScenarioTest { + protected: + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + glGenVertexArrays(1, &m_vao); + glBindVertexArray(m_vao); + glGenBuffers(1, &m_vbo); + glBindBuffer(GL_ARRAY_BUFFER, m_vbo); + const int values[kInputVertices] = {10, 11, 12, 13}; + glBufferData(GL_ARRAY_BUFFER, sizeof(values), values, GL_STATIC_DRAW); + glVertexAttribIPointer(0, 1, GL_INT, 0, nullptr); + glEnableVertexAttribArray(0); + glBindBuffer(GL_ARRAY_BUFFER, 0); + DrainErrors(); + } + + void TearDown() override { + if (!Ready()) return; + glUseProgram(0); + for (const GLuint program : m_programs) { + glDeleteProgram(program); + } + m_programs.clear(); + glBindVertexArray(0); + if (m_vbo != 0) glDeleteBuffers(1, &m_vbo); + if (m_vao != 0) glDeleteVertexArrays(1, &m_vao); + m_vbo = 0; + m_vao = 0; + ScenarioTest::TearDown(); + } + + static constexpr int kInputVertices = 4; + + static void DrainErrors() { + for (int i = 0; i < 16 && glGetError() != GL_NO_ERROR; ++i) { + } + } + + static bool BackendHostsGeometry() { + GLint maxGeometryOutputVertices = 0; + glGetIntegerv(GL_MAX_GEOMETRY_OUTPUT_VERTICES, &maxGeometryOutputVertices); + DrainErrors(); + return maxGeometryOutputVertices >= 2; + } + + static bool BackendHostsTessellation() { + GLint maxTessGenLevel = 0; + glGetIntegerv(GL_MAX_TESS_GEN_LEVEL, &maxTessGenLevel); + DrainErrors(); + return maxTessGenLevel >= 1; + } + + // The transform-feedback-relevant half of deqp's resetStateGLCore, in its order. + // It runs between EVERY pair of conformance cases, and running one capture + // through it is the difference between "the first capture in the process" and + // every other one. + static void ReplayDeqpStateReset() { + glBindVertexArray(0); + glBindBuffer(GL_ARRAY_BUFFER, 0); + glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0); + glDisable(GL_RASTERIZER_DISCARD); + glUseProgram(0); + GLint maxSeparateAttribs = 0; + glGetIntegerv(GL_MAX_TRANSFORM_FEEDBACK_SEPARATE_ATTRIBS, &maxSeparateAttribs); + glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, 0); + for (GLint index = 0; index < maxSeparateAttribs; ++index) { + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, static_cast(index), 0); + } + DrainErrors(); + } + + static std::string InfoLog(GLuint object, bool isShader) { + GLint length = 0; + if (isShader) { + glGetShaderiv(object, GL_INFO_LOG_LENGTH, &length); + } else { + glGetProgramiv(object, GL_INFO_LOG_LENGTH, &length); + } + std::vector buffer(static_cast(length) + 1, '\0'); + if (isShader) { + glGetShaderInfoLog(object, length + 1, nullptr, buffer.data()); + } else { + glGetProgramInfoLog(object, length + 1, nullptr, buffer.data()); + } + return buffer.data(); + } + + GLuint BuildCaptureProgram(const std::vector>& stages, + const char* varying) { + return BuildCaptureProgram(stages, std::vector{varying}); + } + + // Builds a capture program out of `stages` capturing `varyings` interleaved. + // Returns 0 and fills m_buildLog on failure. + GLuint BuildCaptureProgram(const std::vector>& stages, + const std::vector& varyings) { + m_buildLog.clear(); + std::vector shaders; + bool ok = true; + for (const auto& [stage, source] : stages) { + const GLuint shader = glCreateShader(stage); + glShaderSource(shader, 1, &source, nullptr); + glCompileShader(shader); + GLint compiled = 0; + glGetShaderiv(shader, GL_COMPILE_STATUS, &compiled); + shaders.push_back(shader); + if (compiled == GL_FALSE) { + m_buildLog = InfoLog(shader, true); + ok = false; + break; + } + } + GLuint program = 0; + if (ok) { + program = glCreateProgram(); + for (const GLuint shader : shaders) { + glAttachShader(program, shader); + } + glTransformFeedbackVaryings(program, static_cast(varyings.size()), varyings.data(), + GL_INTERLEAVED_ATTRIBS); + glLinkProgram(program); + GLint linked = GL_FALSE; + glGetProgramiv(program, GL_LINK_STATUS, &linked); + if (linked == GL_FALSE) { + m_buildLog = InfoLog(program, false); + glDeleteProgram(program); + program = 0; + } + } + for (const GLuint shader : shaders) { + glDeleteShader(shader); + } + if (program != 0) m_programs.push_back(program); + return program; + } + + // One capture span. `captureMode` is the transform feedback primitive mode, + // `drawMode`/`count` the draw. Returns the capture buffer's contents. + std::vector RunCaptureSpan(GLuint program, GLenum captureMode, GLenum drawMode, GLsizei count, + std::size_t capturedInts) { + std::vector poison(capturedInts, kPoison); + GLuint xfbBuffer = 0; + glGenBuffers(1, &xfbBuffer); + glBindBuffer(GL_ARRAY_BUFFER, xfbBuffer); + glBufferData(GL_ARRAY_BUFFER, static_cast(capturedInts * sizeof(int)), poison.data(), + GL_STATIC_COPY); + glBindBuffer(GL_ARRAY_BUFFER, 0); + // The capture point is the ONLY thing bound; the generic + // GL_TRANSFORM_FEEDBACK_BUFFER binding comes along for the ride, exactly as + // the conformance tests rely on (GL 4.6 core 6.1.1). + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, xfbBuffer); + + glBindVertexArray(m_vao); + glUseProgram(program); + glEnable(GL_RASTERIZER_DISCARD); + glBeginTransformFeedback(captureMode); + glDrawArrays(drawMode, 0, count); + glEndTransformFeedback(); + glDisable(GL_RASTERIZER_DISCARD); + + std::vector readback(capturedInts, kPoison); + glGetBufferSubData(GL_TRANSFORM_FEEDBACK_BUFFER, 0, + static_cast(capturedInts * sizeof(int)), readback.data()); + glUseProgram(0); + glDeleteBuffers(1, &xfbBuffer); + return readback; + } + + static ::testing::AssertionResult CapturedNothing(const std::vector& data) { + for (std::size_t i = 0; i < data.size(); ++i) { + if (data[i] != kPoison) { + return ::testing::AssertionFailure() << "component " << i << " is " << data[i]; + } + } + return ::testing::AssertionSuccess(); + } + + static ::testing::AssertionResult CapturedIs(const std::vector& data, + const std::vector& expected) { + if (data.size() != expected.size()) { + return ::testing::AssertionFailure() + << "captured " << data.size() << " value(s), expected " << expected.size(); + } + for (std::size_t i = 0; i < data.size(); ++i) { + if (data[i] != expected[i]) { + ::testing::AssertionResult failure = ::testing::AssertionFailure(); + failure << "component " << i << " is " << data[i] << ", expected " << expected[i]; + if (data[i] == kPoison) { + failure << " (the capture never reached these bytes)"; + } + return failure; + } + } + return ::testing::AssertionSuccess(); + } + + std::vector m_programs; + std::string m_buildLog; + GLuint m_vao = 0; + GLuint m_vbo = 0; + }; + + // THE REGRESSION GUARD FOR THE WHOLE FAMILY. Two geometry-stage captures in one + // process with the conformance suite's own state reset between them; the assertion + // that matters is on the SECOND one, which is the one every device run failed while + // whichever body happened to land first in its process passed. + TEST_F(XfbRepeatedCaptureScenario, ASecondGeometryCaptureAfterADeqpStateResetStillRecords) { + if (!Ready()) GTEST_SKIP(); + if (!BackendHostsGeometry()) { + GTEST_SKIP() << "no geometry stage on " << Gl().BackendName() << " (" << Gl().RendererString() << ")"; + } + + // Two vertices emitted per input point, so the capture is amplified beyond what + // the CPU primitive model can predict from the draw alone. + const std::vector expected = {10, 10, 11, 11, 12, 12, 13, 13}; + + for (int capture = 0; capture < 3; ++capture) { + // A fresh program per capture, because that is what a fresh conformance case + // builds - and it is what makes the driver recycle program and buffer names. + const GLuint program = BuildCaptureProgram({{GL_VERTEX_SHADER, kPassthroughVertexSource}, + {GL_GEOMETRY_SHADER, kPointAmplifyingGeometrySource}, + {GL_FRAGMENT_SHADER, kFragmentSource}}, + "gs_out_value"); + ASSERT_NE(program, 0u) << "capture " << capture << " program failed to build: " << m_buildLog; + + const std::vector captured = + RunCaptureSpan(program, GL_POINTS, GL_POINTS, kInputVertices, expected.size()); + EXPECT_TRUE(CapturedIs(captured, expected)) + << "capture " << capture << " of 3 in this process" + << (capture == 0 ? "" : " (every earlier one was followed by a deqp-shaped state reset)"); + EXPECT_EQ(glGetError(), GL_NO_ERROR) << "capture " << capture; + + glDeleteProgram(program); + m_programs.pop_back(); + ReplayDeqpStateReset(); + glBindVertexArray(m_vao); + } + } + + // The tessellation half, which had no coverage anywhere in the tree: a capture taken + // from a GL_PATCHES draw, whose last vertex-processing stage is the evaluation shader + // and whose record count only the tessellator knows. + TEST_F(XfbRepeatedCaptureScenario, ACaptureFromAPatchesDrawRecords) { + if (!Ready()) GTEST_SKIP(); + if (!BackendHostsTessellation()) { + GTEST_SKIP() << "no tessellation stages on " << Gl().BackendName() << " (" << Gl().RendererString() + << ")"; + } + + // One input patch of one vertex, all levels at 1: the tessellator emits exactly + // one triangle, so three captured vertices all carrying the first input value. + glPatchParameteri(GL_PATCH_VERTICES, 1); + DrainErrors(); + + const GLuint program = BuildCaptureProgram({{GL_VERTEX_SHADER, kPassthroughVertexSource}, + {GL_TESS_CONTROL_SHADER, kTessControlSource}, + {GL_TESS_EVALUATION_SHADER, kTessEvalSource}}, + "tes_out_value"); + ASSERT_NE(program, 0u) << "patch capture program failed to build: " << m_buildLog; + + const std::vector expected = {10, 10, 10}; + const std::vector captured = RunCaptureSpan(program, GL_TRIANGLES, GL_PATCHES, 1, expected.size()); + EXPECT_TRUE(CapturedIs(captured, expected)); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + } + + // A capture program with NO FRAGMENT STAGE, drawn under GL_RASTERIZER_DISCARD. Legal + // in desktop GL, and the shape most transform-feedback-as-readback tests use; the + // program above only differs from it by the fragment shader, so a failure here is + // specifically about the missing stage. + TEST_F(XfbRepeatedCaptureScenario, ACaptureFromAFragmentlessProgramRecords) { + if (!Ready()) GTEST_SKIP(); + if (!BackendHostsGeometry()) { + GTEST_SKIP() << "no geometry stage on " << Gl().BackendName() << " (" << Gl().RendererString() << ")"; + } + + const GLuint program = BuildCaptureProgram({{GL_VERTEX_SHADER, kPassthroughVertexSource}, + {GL_GEOMETRY_SHADER, kPointAmplifyingGeometrySource}}, + "gs_out_value"); + ASSERT_NE(program, 0u) << "fragmentless capture program failed to build: " << m_buildLog; + + const std::vector expected = {10, 10, 11, 11, 12, 12, 13, 13}; + const std::vector captured = + RunCaptureSpan(program, GL_POINTS, GL_POINTS, kInputVertices, expected.size()); + EXPECT_TRUE(CapturedIs(captured, expected)); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + } + + // An ADJACENCY draw feeding the capture. CountPrimitivesForDraw answered 0 for all + // four adjacency modes, which made the transform feedback accounting skip the draw + // entirely - so neither the captured-vertex counter nor the geometry-capture-draw + // flag moved, and anything downstream of either was working from "nothing happened". + TEST_F(XfbRepeatedCaptureScenario, ACaptureFromAnAdjacencyDrawRecords) { + if (!Ready()) GTEST_SKIP(); + if (!BackendHostsGeometry()) { + GTEST_SKIP() << "no geometry stage on " << Gl().BackendName() << " (" << Gl().RendererString() << ")"; + } + + const GLuint program = BuildCaptureProgram({{GL_VERTEX_SHADER, kPassthroughVertexSource}, + {GL_GEOMETRY_SHADER, kAdjacencyGeometrySource}, + {GL_FRAGMENT_SHADER, kFragmentSource}}, + "gs_out_value"); + ASSERT_NE(program, 0u) << "adjacency capture program failed to build: " << m_buildLog; + + // Four vertices of GL_LINES_ADJACENCY are one line primitive; the shader emits + // the second vertex of the four, which is the line's first real endpoint. + const std::vector expected = {11}; + const std::vector captured = + RunCaptureSpan(program, GL_POINTS, GL_LINES_ADJACENCY, kInputVertices, expected.size()); + EXPECT_TRUE(CapturedIs(captured, expected)); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + } + + // An adjacency draw with NO geometry stage. GL 4.6 core table 13.1 admits + // GL_LINES_ADJACENCY and GL_LINE_STRIP_ADJACENCY under capture mode GL_LINES (and the + // triangle pair under GL_TRIANGLES): without a geometry shader the adjacent vertices + // are ignored and the primitive assembled is a plain line, so the combination is legal + // and must capture. MobileGL's active-capture primitive-mode table listed only the + // non-adjacency modes, so this raised GL_INVALID_OPERATION and dropped the draw + // entirely - the buffer kept its pre-draw bytes and the application saw an error the + // spec does not allow. Distinct from ACaptureFromAnAdjacencyDrawRecords above, which + // HAS a geometry stage and therefore bypasses that table completely. + TEST_F(XfbRepeatedCaptureScenario, AVertexOnlyAdjacencyCaptureRecords) { + if (!Ready()) GTEST_SKIP(); + + const GLuint program = + BuildCaptureProgram({{GL_VERTEX_SHADER, kPassthroughVertexSource}}, "vs_out_value"); + ASSERT_NE(program, 0u) << "vertex-only capture program failed to build: " << m_buildLog; + + // Four vertices of GL_LINES_ADJACENCY are one line whose real endpoints are the + // middle pair, so the capture is those two vertices in order. + const std::vector expected = {11, 12}; + const std::vector captured = + RunCaptureSpan(program, GL_LINES, GL_LINES_ADJACENCY, kInputVertices, expected.size()); + + // THE GUARD FOR THE DEFECT ITSELF, and it is backend-independent: the frontend + // validator must not reject the combination. It used to record + // GL_INVALID_OPERATION and return before the draw was ever issued. + EXPECT_EQ(glGetError(), GL_NO_ERROR) + << "a capture-mode/draw-mode pair GL 4.6 core table 13.1 admits must raise no error"; + + // Whether the capture then RECORDS is a backend question, and the two answer it + // differently. ES 3.2 (10.1) supports the adjacency primitive types only for a + // pipeline with a geometry shader, so DirectGLES has nothing to forward this draw + // to; desktop GL and Vulkan both assemble the plain line and capture it. Asserting + // the data unconditionally would be asserting that DirectGLES emulates a whole ES + // restriction away, which is a separate piece of work and not what this guards. + if (Gl().BackendName() == "DirectGLES") { + GTEST_SKIP() << "DirectGLES cannot forward a geometry-shader-less adjacency draw: ES 3.2 10.1 " + "supports the adjacency primitive types only with a geometry stage. The frontend " + "no longer rejects the draw (checked above), which is the defect this covers."; + } + EXPECT_TRUE(CapturedIs(captured, expected)); + } + + // A CAPTURE MUST NEVER LAND IN A BUFFER THE APPLICATION DID NOT BIND FOR IT. + // + // A capture list may legally begin with gl_NextBuffer, which leaves capture buffer 0 + // with stride 0 and nothing to capture - so glBeginTransformFeedback does not require a + // buffer at point 0 and the application binds only point 1. The driver-side program is + // a single-buffer interleaved capture (the pseudo-varyings are consumed at link time), + // so it writes capture point 0, and MobileGL redirects that into scratch storage and + // scatters the records afterwards. + // + // Two ways that went wrong, both fixed here: the scratch was sized by reading each + // target's stride at its POSITION in a list that skips unbound buffers, which for this + // layout read stride 0 for everything and produced a zero capacity; and when the + // scratch then failed to bind, the span opened anyway onto whatever capture point 0 + // still held from an earlier capture in the process - silently overwriting an unrelated + // application buffer. The first span below exists purely to leave such a binding behind. + TEST_F(XfbRepeatedCaptureScenario, ACaptureListBeginningWithGlNextBufferSparesTheEarlierBuffer) { + if (!Ready()) GTEST_SKIP(); + + const std::size_t capturedInts = 4; + const GLsizeiptr captureBytes = static_cast(capturedInts * sizeof(int)); + + // Span A: an ordinary capture, so capture point 0 is left holding bufferA. + const GLuint programA = + BuildCaptureProgram({{GL_VERTEX_SHADER, kPassthroughVertexSource}}, "vs_out_value"); + ASSERT_NE(programA, 0u) << "plain capture program failed to build: " << m_buildLog; + + std::vector poison(capturedInts, kPoison); + GLuint bufferA = 0; + glGenBuffers(1, &bufferA); + glBindBuffer(GL_ARRAY_BUFFER, bufferA); + glBufferData(GL_ARRAY_BUFFER, captureBytes, poison.data(), GL_STATIC_COPY); + glBindBuffer(GL_ARRAY_BUFFER, 0); + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, bufferA); + + glBindVertexArray(m_vao); + glUseProgram(programA); + glEnable(GL_RASTERIZER_DISCARD); + glBeginTransformFeedback(GL_POINTS); + glDrawArrays(GL_POINTS, 0, kInputVertices); + glEndTransformFeedback(); + glDisable(GL_RASTERIZER_DISCARD); + glUseProgram(0); + + std::vector afterA(capturedInts, kPoison); + glGetBufferSubData(GL_TRANSFORM_FEEDBACK_BUFFER, 0, captureBytes, afterA.data()); + const std::vector spanAExpected = {10, 11, 12, 13}; + ASSERT_TRUE(CapturedIs(afterA, spanAExpected)) << "the setup span itself did not capture"; + + // Span B: gl_NextBuffer first, so buffer 0 captures nothing and only point 1 is bound. + const GLuint programB = BuildCaptureProgram({{GL_VERTEX_SHADER, kPassthroughVertexSource}}, + {"gl_NextBuffer", "vs_out_value"}); + if (programB == 0) { + GTEST_SKIP() << "gl_NextBuffer capture lists are not linkable on " << Gl().BackendName() << " (" + << Gl().RendererString() << "): " << m_buildLog; + } + + GLuint bufferB = 0; + glGenBuffers(1, &bufferB); + glBindBuffer(GL_ARRAY_BUFFER, bufferB); + glBufferData(GL_ARRAY_BUFFER, captureBytes, poison.data(), GL_STATIC_COPY); + glBindBuffer(GL_ARRAY_BUFFER, 0); + // Point 0 released, point 1 is the only destination this capture asks for. + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, 0); + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 1, bufferB); + + glUseProgram(programB); + glEnable(GL_RASTERIZER_DISCARD); + glBeginTransformFeedback(GL_POINTS); + glDrawArrays(GL_POINTS, 0, kInputVertices); + glEndTransformFeedback(); + glDisable(GL_RASTERIZER_DISCARD); + glUseProgram(0); + + // THE ASSERTION THAT MATTERS: bufferA was not a destination of this capture, so it + // must still read exactly what span A left in it. A failure here is the corruption. + std::vector bufferAAfterB(capturedInts, 0); + glBindBuffer(GL_ARRAY_BUFFER, bufferA); + glGetBufferSubData(GL_ARRAY_BUFFER, 0, captureBytes, bufferAAfterB.data()); + glBindBuffer(GL_ARRAY_BUFFER, 0); + EXPECT_TRUE(CapturedIs(bufferAAfterB, spanAExpected)) + << "the gl_NextBuffer capture wrote into the buffer the PREVIOUS span had bound"; + + EXPECT_EQ(glGetError(), GL_NO_ERROR); + + // ...and, where the backend places this layout at all, the buffer it WAS asked to + // write gets the records. That placement is the DirectGLES scatter path, whose + // scratch sizing used to read each target's stride at its POSITION in a list that + // skips unbound capture buffers - which for a leading gl_NextBuffer read stride 0 + // for every target and sized the scratch at zero. DirectVulkan does not implement a + // leading-gl_NextBuffer layout at all (it captures nothing into bufferB); that is a + // pre-existing gap of its own, and the assertion above - that it corrupts nothing + // while declining - is what matters for it. + const bool backendPlacesLeadingNextBuffer = Gl().BackendName() != "DirectVulkan"; + if (backendPlacesLeadingNextBuffer) { + std::vector bufferBAfter(capturedInts, kPoison); + glGetBufferSubData(GL_TRANSFORM_FEEDBACK_BUFFER, 0, captureBytes, bufferBAfter.data()); + EXPECT_TRUE(CapturedIs(bufferBAfter, spanAExpected)); + } + + // Unbound and deleted BEFORE any skip: a capture point left pointing at a buffer + // this test deleted would follow the process into the next scenario. + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 1, 0); + glDeleteBuffers(1, &bufferA); + glDeleteBuffers(1, &bufferB); + + if (!backendPlacesLeadingNextBuffer) { + GTEST_SKIP() << "DirectVulkan does not place a capture list beginning with gl_NextBuffer; it " + "captures nothing, which the no-corruption assertion above has already covered."; + } + } + + // The control for all of the above: a span that never draws must leave the capture + // buffer alone. Without it "the buffer kept its poison" could be read as the correct + // outcome of some path rather than as the bug, and the tightened early returns in + // StartPendingTransformFeedback have to keep this legal case legal. + TEST_F(XfbRepeatedCaptureScenario, ASpanThatNeverDrawsLeavesTheCaptureBufferAlone) { + if (!Ready()) GTEST_SKIP(); + if (!BackendHostsGeometry()) { + GTEST_SKIP() << "no geometry stage on " << Gl().BackendName() << " (" << Gl().RendererString() << ")"; + } + + const GLuint program = BuildCaptureProgram({{GL_VERTEX_SHADER, kPassthroughVertexSource}, + {GL_GEOMETRY_SHADER, kPointAmplifyingGeometrySource}, + {GL_FRAGMENT_SHADER, kFragmentSource}}, + "gs_out_value"); + ASSERT_NE(program, 0u) << "capture program failed to build: " << m_buildLog; + + const std::size_t capturedInts = 8; + std::vector poison(capturedInts, kPoison); + GLuint xfbBuffer = 0; + glGenBuffers(1, &xfbBuffer); + glBindBuffer(GL_ARRAY_BUFFER, xfbBuffer); + glBufferData(GL_ARRAY_BUFFER, static_cast(capturedInts * sizeof(int)), poison.data(), + GL_STATIC_COPY); + glBindBuffer(GL_ARRAY_BUFFER, 0); + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, xfbBuffer); + + glUseProgram(program); + glBeginTransformFeedback(GL_POINTS); + glEndTransformFeedback(); + glUseProgram(0); + + std::vector readback(capturedInts, 0); + glGetBufferSubData(GL_TRANSFORM_FEEDBACK_BUFFER, 0, + static_cast(capturedInts * sizeof(int)), readback.data()); + EXPECT_TRUE(CapturedNothing(readback)); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + + glDeleteBuffers(1, &xfbBuffer); + } + + } // namespace +} // namespace MGITest