From 1ebe6abeb272dd4be474ae24ec7cbfc88f11568a Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 08:08:04 -0400 Subject: [PATCH 1/2] [MG_Pipe, MG_Impl, MG_State, MG_Remote, MG_Backend, MG_Test] (Disaggregated): P5e sb - set_shader_buffers is emitted, routed and applied - MGPShaderBuffers::WritableMask is Uint32[3] and the POD grows 32 -> 40 (ID-104). One Uint32 could describe only the first 32 of an 84-point window, so a storage buffer bound at point 32 or above read as unwritable with nothing able to see it; the ruling widens the field rather than narrowing the window, which would change what an application may bind. MGPipeShaderBufferMaskHas / ...Set are the one spelling of the mask, so the emitter, the applier and the four backend sites cannot disagree about which word a point lives in. - MGPipeApplySetShaderBuffers has its body: per-class window, Start/Count validated against the capacity, the mask copied by word, ONE ShaderBuffersSerial advanced on every applied record and ADVANCED - never zeroed - by both resets, which also clear the three windows. - Route rows for opcode 38 on both arms (Mono_ and Wire_) and the server sink in the wire codec's decode switch, which is where a kCtxState row's sink lives - PipeApplier.cpp's ServerVerbSink has no method for it and the brief's "OnSetShaderBuffers" names no code. The codec also bounds Class and Start+Count where Count becomes a byte length. - New MG_Impl/Pipe/ShaderBufferEmit.h: one record per class, window = the touched high-water mark clamped to 84, zero early-out before any hash, a BASE binding travelling as kMGPipeWholeBuffer so the server re-resolves the extent at use, three suppressor slots (ruling 11), and the writable mask that replaces the backend's GPU-write walk. - BufferState gains a per-target bind-point generation (push-only, G1) and dirty bits 15/16/17 shutter on it instead of the buffer CONTENT aggregate. That aggregate answered the wrong question in both directions: any glBufferSubData fired all three, and glBindBufferBase fired none of them, because it mutates a binding point through a returned reference. Same defect class as P4a's glBindSampler hole. - Init.cpp publishes consumer bit 13 for DirectGLES and not for Magma (ID-106), in the same commit as the wired constant, and PipeFill's liveness gate asks R-8 for bit 13 in particular plus bit 13-requires-bit-7. - `Touch` joins the dirty-surface scanner's mutator prefixes; TouchBufferBindingPoint gets its row and SetNamedTransformFeedbackBinding's is rewritten now that bit 17 reads a generation a binding moves. Negative control 6 is re-pointed: the pair it used to refute is a live answer, and a control that has quietly become positive is what the self-test is for. --- MobileGL/MG_Backend/Init.cpp | 14 +- MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp | 21 + MobileGL/MG_Impl/Pipe/PipeFill.cpp | 91 +++- MobileGL/MG_Impl/Pipe/ShaderBufferEmit.h | 277 ++++++++++ MobileGL/MG_Impl/Pipe/Tracker.h | 54 +- MobileGL/MG_Pipe/DirtySurface.def | 56 +- MobileGL/MG_Pipe/MGPipeTypes.h | 44 +- MobileGL/MG_Pipe/PipeApply.cpp | 108 +++- MobileGL/MG_Pipe/PipeApply.h | 9 +- MobileGL/MG_Pipe/PipeRoute.cpp | 6 + MobileGL/MG_Pipe/PipeRoute.h | 12 + MobileGL/MG_Remote/CapsCodec.cpp | 4 + MobileGL/MG_Remote/Client/WireTables.cpp | 6 + MobileGL/MG_Remote/Wire/PipeWireCodec.cpp | 39 +- .../GLState/BufferState/BufferState.h | 33 ++ MobileGL/MG_State/GLState/Core.cpp | 11 + MobileGL/MG_State/GLState/Core.h | 9 + MobileGL/MG_Test/Pipe/CMakeLists.txt | 11 +- MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp | 75 ++- .../MG_Test/Pipe/ShaderBufferEmitTest.cpp | 508 ++++++++++++++++++ MobileGL/MG_Test/Pipe/TrackerTest.cpp | 75 +++ MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp | 12 +- MobileGL/MG_Test/Wire/RemoteClientTest.cpp | 10 +- scripts/gen_pipe_dirty_surface.py | 38 +- 24 files changed, 1434 insertions(+), 89 deletions(-) create mode 100644 MobileGL/MG_Impl/Pipe/ShaderBufferEmit.h create mode 100644 MobileGL/MG_Test/Pipe/ShaderBufferEmitTest.cpp diff --git a/MobileGL/MG_Backend/Init.cpp b/MobileGL/MG_Backend/Init.cpp index 97168fe4..341e7ece 100644 --- a/MobileGL/MG_Backend/Init.cpp +++ b/MobileGL/MG_Backend/Init.cpp @@ -97,9 +97,21 @@ namespace MobileGL::MG_Backend { // rather than aborting. constexpr Bool kMGPipeP5eRunAheadReady = false; + // P5e (sb, ID-106). DirectGLES GAINS BIT 13 - the indexed buffer binding points - and + // Magma deliberately does not. c0e landed the phase constant and the dirty-bit map with + // this row still reading P4a's mask, because withholding a consumer bit is the safe + // direction while no emitter exists: the client's R-8 gate then keeps the whole family + // on the legacy pull path. The moment ShaderBufferEmit.h's wired constant leaves 0 the + // two have to move together, so they are one commit. + // + // DIRECTVULKAN STAYS ON P4a's MASK. The bit says "this server reads set_shader_buffers + // instead of walking the client's binding-point table", and Magma's UniformManager does + // no such thing - it has no binding-point records at all, and P5e leaves it lockstep + // (CONTRACT-P5E.md §6). Claiming the bit there would make the client emit a family + // nothing consumes, which is ID-39's failure with a different family's name on it. Uint64 ConsumedSubsystemsFor(BackendType type) { switch (type) { - case BackendType::DirectGLES: return MG_Pipe::kMGPipeSubsystemsMigratedAtP4a; + case BackendType::DirectGLES: return MG_Pipe::kMGPipeSubsystemsMigratedAtP5e; case BackendType::DirectVulkan: return MG_Pipe::kMGPipeSubsystemsMigratedAtP4a & ~MG_Pipe::kMGPipeSubsystemResources; default: return 0; diff --git a/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp b/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp index fef2853e..9608a9dc 100644 --- a/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp +++ b/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp @@ -1497,6 +1497,20 @@ namespace MobileGL::MG_Impl::GLImpl { return; } MG_State::pGLContext->TouchBufferBindingPoint(bufferTarget, pointIndex); +#if MOBILEGL_PIPE_PUSH + // P5e (sb, CONTRACT-P5E.md §5.6). THE BIND-POINT GENERATION, and it is bumped HERE - + // before the writes below and after every refusal above - rather than inside + // BindingSlotRange1D, for two reasons. The slot has no back-pointer to the state + // container that owns it, which is the same reason every other aggregate is bumped from + // the entry point; and a bump on a bind that is about to be REJECTED by + // ValidateBufferName is an over-fire the set-hash suppressor absorbs, while a bump + // placed after the writes would be skipped by the `buffer == 0` early return below - + // which is an unbind, the one case a shutter may least afford to miss. + // + // The dirty bits this feeds are 15/16/17 (Tracker.h); before P5e they shuttered on the + // buffer CONTENT aggregate, which no bind has ever moved. + MG_State::pGLContext->NoteBufferBindPointChanged(bufferTarget); +#endif auto& point = MG_State::pGLContext->GetBufferBindingPoint(bufferTarget, pointIndex); SharedPtr bufferObject; @@ -1620,6 +1634,13 @@ namespace MobileGL::MG_Impl::GLImpl { return; } MG_State::pGLContext->TouchBufferBindingPoint(bufferTarget, index); +#if MOBILEGL_PIPE_PUSH + // P5e (sb): the other half of BindBufferBase_State's bump - same generation, same + // placement, same reason. A glBindBufferRange moves the point's EXTENT as well as what + // is bound to it, and the record carries the resolved Offset/Size, so a range change + // with no object change still has to reach the emitter. + MG_State::pGLContext->NoteBufferBindPointChanged(bufferTarget); +#endif auto& point = MG_State::pGLContext->GetBufferBindingPoint(bufferTarget, index); SharedPtr bufferObject; diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index fe0999ef..14d6109c 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -1319,11 +1320,41 @@ namespace MobileGL::MG_Pipe { // the commit that gives the emitter its body - so a client path that lands before its // emitter does is inert by construction rather than by everyone remembering to check; the // third is P4aFamilyHasItsConsumer above and the fourth is P4aFamilyDependenciesAreSet. + // P5e (sb, ID-106 and CONTRACT-P5E.md §1). THE BINDING-POINT FAMILY's TWO EXTRA + // CONJUNCTS, kept beside P4a's rather than folded into them, because it asks a + // DIFFERENT consumer question. P4a's four families all ride the resource family's one + // signal (P4aFamilyHasItsConsumer says why); bit 13's consumer question is bit 13's + // own - MG_Backend/Init.cpp publishes it in the same commit that sets + // ShaderBufferEmit.h's wired constant, and a server that does not publish it is a + // server whose four binding-point walks still read the frontend, so a record sent to it + // would be stored and never looked at while the client latched its suppressor. + // + // AND BIT 13 REQUIRES BIT 7, for bit 11's reason one family over: every + // MGPBufferRange::Res names a Buffer handle and only bit 7 puts one in the resource + // slot table, so without it every EnsureBufferResourceForHandle on the server would + // mint a twin with no record behind it. Withholding the whole family is the safe + // direction - the legacy frontend walk runs untouched on both sides. + Bool P5eFamilyIsLive(Uint64 subsystem, Uint64 pushMask) { + if ((subsystem & kMGPipeSubsystemBufferBindings) == 0) return true; + if ((pushMask & kMGPipeSubsystemResources) == 0) return false; +#if MOBILEGL_BUILD_DISAGGREGATED + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + return MG_Remote::Client::CapsMirrorInstance().ServerConsumes( + kMGPipeSubsystemBufferBindings); + } +#endif + // Under monolith the "consumer" is the backend that registered the resource op + // table, exactly as it is for P4a's four: there is no caps snapshot to ask and the + // binding-point walks live in the same DirectGLES that registers it. + return MGPipeGetResourceOps() != nullptr; + } + Bool FamilyIsLive(Uint64 subsystem, Uint64 wired) { const Uint64 pushMask = MG_Config::Features.PipePush; return (pushMask & subsystem) != 0 && (wired & subsystem) != 0 && P4aFamilyHasItsConsumer(subsystem) && - P4aFamilyDependenciesAreSet(subsystem, pushMask); + P4aFamilyDependenciesAreSet(subsystem, pushMask) && + P5eFamilyIsLive(subsystem, pushMask); } // ---- THE FAMILY SEAM ---- @@ -2248,7 +2279,14 @@ namespace MobileGL::MG_Pipe { kMGPipeWiredFramebufferSubsystem | kMGPipeWiredTextureSubsystem | kMGPipeWiredSamplerSubsystem | - kMGPipeWiredProgramSubsystem; + kMGPipeWiredProgramSubsystem | + // P5e (sb): the indexed buffer binding + // points, by the same rule and from the same + // kind of header. ID-106 pins the other half + // of the switch: MG_Backend/Init.cpp's + // consumer mask gains bit 13 in the same + // commit, or R-8 withholds the whole family. + kMGPipeWiredBufferBindingSubsystem; // Each family constant is either 0 or its own subsystem bit and nothing else. Without // this a header that set the wrong constant - the sampler bit in the program header, // say - would switch the wrong family on and every gate would still pass. @@ -2264,6 +2302,9 @@ namespace MobileGL::MG_Pipe { static_assert(kMGPipeWiredProgramSubsystem == 0 || kMGPipeWiredProgramSubsystem == kMGPipeSubsystemPrograms, "ProgramEmit.h's wired constant must be 0 or the program bit"); + static_assert(kMGPipeWiredBufferBindingSubsystem == 0 || + kMGPipeWiredBufferBindingSubsystem == kMGPipeSubsystemBufferBindings, + "ShaderBufferEmit.h's wired constant must be 0 or the binding-point bit"); // A field an emitted call supplies COMPLETELY, so the residual fill may stop pulling // it. Two rows of Coverage.def's emitted list do not qualify and each has its reason @@ -2985,6 +3026,22 @@ namespace MobileGL::MG_Pipe { Uint64 DrainTextureSubData(GLContext& ctx) { return MGPipeTextureEmitterInstance().DrainTextureSubData(ctx); } + + // ---- P5e's two, and they are TWO adapters over THREE records (sb, §5.6) ---- + // + // The split is the DIRTY BITS' and not the classes': bit 15 is the uniform binding + // points and bit 16 is the two writable classes, which share a shutter because a + // storage bind and a counter bind are the same event to every reader of the record. + // Bit 17's family (set_stream_output_targets) has no adapter at all - XFB stays + // lockstep for the whole of P5e (§5.7) - and that absence is the catalogue's split, + // not an omission. + Uint64 EmitConstBuffers(GLContext& ctx) { + return MGPipeShaderBufferEmitterInstance().EmitConstBuffers(ctx); + } + + Uint64 EmitShaderBuffers(GLContext& ctx) { + return MGPipeShaderBufferEmitterInstance().EmitShaderBuffers(ctx); + } } // namespace Uint64 MGPipeVertexAttribDefaultRepairCount() { return g_attribDefaultRepairs; } @@ -3105,12 +3162,17 @@ namespace MobileGL::MG_Pipe { #else constexpr Bool contextValuesWireLive = false; #endif + // AND THE SEVENTH IS P5eFamilyIsLive (ID-106), which is the same sentence for the + // binding-point family and asks bit 13's OWN consumer bit rather than the resource + // family's. It answers true for every subsystem but bit 13, so nothing that emitted + // before P5e changes. const auto wants = [&](MGPipeDirty bit) { const Uint64 subsystem = MGPipeSubsystemForDirty(bit); return subsystem != 0 && (pushMask & subsystem) != 0 && (kMGPipeWiredSubsystems & subsystem) != 0 && P4aFamilyHasItsConsumer(subsystem) && P4aFamilyDependenciesAreSet(subsystem, pushMask) && + P5eFamilyIsLive(subsystem, pushMask) && (dirty & MGPipeDirtyBit(bit)) != 0; }; Uint64 payloadBytes = 0; @@ -3182,6 +3244,13 @@ namespace MobileGL::MG_Pipe { MGPipeSamplerEmitterInstance().Reset(); MGPipeImageEmitterInstance().Reset(); MGPipeProgramEmitterInstance().Reset(); + // P5e (sb): and the binding-point emitter's mirrors. MGPipeApplierReset clears all + // three of the applier's windows and ADVANCES ShaderBuffersSerial, so the emitter's + // latch has to reset with it - the three suppressor slots are already cleared by + // InvalidateAll() above, and without that the first emission after a make-current + // would be suppressed as unchanged and the server would draw against a window it + // had just been told to empty. + MGPipeShaderBufferEmitterInstance().Reset(); g_residualDue = true; } @@ -3231,6 +3300,24 @@ namespace MobileGL::MG_Pipe { payloadBytes += EmitGlobalConstants(*ctx); } + // P5e's segment (sb, §5.6): the indexed buffer binding points, AFTER the program + // segment for the same "code organisation, not a contract" reason ARCHITECTURE.md 5.4 + // gives for P4a's order - all of a verb's set_*/bind_* complete before the verb, and + // the server resolves a point against its own descriptor at its sync point. It reads + // better here because the uniform window is what the program's block bindings INDEX + // (DirectGLES.cpp's UBO loop), so the two records that describe a draw's uniform + // buffers stand together. + // + // NOTHING FOR BIT 17: set_stream_output_targets stays unemitted for the whole of P5e + // (§5.7). The bit is still computed, counted and mapped onto this subsystem, so an + // operator clearing bit 13 gets the whole family's frontend walk back. + if (wants(MGPipeDirty::NewConstBuffers)) { + payloadBytes += EmitConstBuffers(*ctx); + } + if (wants(MGPipeDirty::NewShaderBuffers)) { + payloadBytes += EmitShaderBuffers(*ctx); + } + if (wants(MGPipeDirty::NewPipelineState) || wants(MGPipeDirty::NewRenderState)) { payloadBytes += EmitRenderState(*ctx, dirty, tracker.FreshlyPrimed()); } diff --git a/MobileGL/MG_Impl/Pipe/ShaderBufferEmit.h b/MobileGL/MG_Impl/Pipe/ShaderBufferEmit.h new file mode 100644 index 00000000..530fc6c4 --- /dev/null +++ b/MobileGL/MG_Impl/Pipe/ShaderBufferEmit.h @@ -0,0 +1,277 @@ +// MobileGL - MobileGL/MG_Impl/Pipe/ShaderBufferEmit.h +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +#pragma once +#include + +// The CLIENT side of set_shader_buffers - the INDEXED BUFFER BINDING POINTS (P5e package sb, +// MG_Remote/CONTRACT-P5E.md §5.6, rulings 10 and 11). The call has been in the catalogue since +// P4a with no emitter, no route and no applier; this file is the emitter half. +// +// ONE RECORD PER CLASS, THREE CLASSES. MGPShaderBuffers::Class names which binding-point array +// the record describes - Uniform=0, ShaderStorage=1, AtomicCounter=2 - so the three are three +// emissions through one route and one applier, not one record with three tails. That is also +// why the suppressor has THREE SLOTS (ruling 11): a single slot would let a uniform set cancel +// the previous storage set, and the shader-storage bindings would be suppressed as "unchanged" +// by a record that described something else entirely. +// +// XFB IS NOT ONE OF THEM (§5.7). The transform-feedback capture points are span-scoped state +// latched at glBeginTransformFeedback, and set_stream_output_targets carries a Generation this +// payload has no field for - so the catalogue's split between the two rows stays, XFB keeps its +// lockstep escalation, and dirty bit 17 fires for a family that emits nothing this phase. +// +// TWO INVARIANTS THAT MUST SURVIVE INTO THE BODY: +// 1. THE HIGH-WATER-ZERO EARLY-OUT. A class whose touched high-water mark is 0 emits nothing, +// BEFORE any hash and before any walk. Minecraft's touched SSBO and atomic-counter counts +// are both 0, so the whole family costs one integer read per class per validate point on +// the workload this phase exists for. +// 2. A BASE BINDING TRAVELS AS kMGPipeWholeBuffer AND NEVER AS A RESOLVED EXTENT (§5.6, +// table 0). glBindBufferBase does not freeze anything: GL resolves the range against the +// object's size at every USE, and BindingSlotRange1D::GetRange() reproduces that by asking +// the bound object. Resolving it HERE would freeze the size as of the emission, and under +// run-ahead a glBufferData issued between the emission and the apply would then bind the +// OLD extent - silently, with the right handle. So Offset/Size are copied only for an +// EXPLICIT range (glBindBufferRange), and a base binding says "whole buffer" and lets the +// server re-resolve against its own descriptor. +// +// THE WINDOW IS THE TOUCHED HIGH-WATER MARK (ruling 10), the same value the backend's own walk +// has used since P2 and the same value MGPContextValues already carries. A program-derived +// window would be narrower for the uniform class - Minecraft reaches UBOs through the program's +// block bindings, so a program binding block 0 at GL point 83 pays a 84-entry record - but that +// is a NARROWING and this package is a correctness landing; BRIEF-P5E §5 lists it as trailing. +// +// HEADER-ONLY, and the wired constant below is what switches the family on, for the ownership +// reason PipeFill.cpp states in full: that file belongs to the contract package for the whole +// phase, so the bit that turns an emitter on is a constant in the emitter's OWN header. +#if MOBILEGL_PIPE_PUSH +#include +#include +#include +#include // MGPipeMixShutter, the one shutter-mixing function +#include +#include +#include +#include +#include +#include + +#include + +namespace MobileGL::MG_Pipe { + + // WHICH SUBSYSTEM BIT THIS BUILD ACTUALLY EMITS FOR. PipeFill.cpp ORs the per-family + // constants into kMGPipeWiredSubsystems, so the bit is added by the commit that gives this + // emitter its body and no file is touched twice. + // + // IT MUST MOVE WITH MG_Backend/Init.cpp's CONSUMER BIT (ID-106). c0e deliberately withheld + // bit 13 from ConsumedSubsystemsFor because no emitter existed and withholding is the safe + // direction; the client's R-8 liveness gate (PipeFill.cpp's P5eFamilyIsLive) asks the server + // for THIS bit in particular, so a build that set the constant here and left the server's + // mask alone would emit nothing at all and look exactly like a build that had not landed. + // + // TURNING IT ON DOES NOT RETIRE A PULL. GetBufferBindingPoint is the family's Coverage.def + // emitted row and PipeFill.cpp's EmittedCallSuppliesTheWholeField answers FALSE for it: the + // field is four raw bases into the frontend's binding-point table and the record carries + // resolved {handle, offset, size} triples, so the residual fill keeps writing the mirror and + // the verify comparator keeps proving it. What retires the pull is the four Espryt consumers + // reading the applier's window instead, which is the other half of this package. + inline constexpr Uint64 kMGPipeWiredBufferBindingSubsystem = kMGPipeSubsystemBufferBindings; + + // The BufferTarget each wire class names. One table, because the class is a wire value and + // the target is a frontend enum, and a switch written at each of the four call sites is four + // chances to pair Uniform with ShaderStorage. + inline constexpr BufferTarget MGPipeBufferTargetForShaderBufferClass(Uint32 cls) { + switch (cls) { + case kMGPipeShaderBufferClassShaderStorage: return BufferTarget::ShaderStorage; + case kMGPipeShaderBufferClassAtomicCounter: return BufferTarget::AtomicCounter; + default: return BufferTarget::Uniform; + } + } + + // And the suppressor slot, for ruling 11's reason - one per class. + inline constexpr MGPipeSuppressorSlot MGPipeSuppressorSlotForShaderBufferClass(Uint32 cls) { + switch (cls) { + case kMGPipeShaderBufferClassShaderStorage: + return MGPipeSuppressorSlot::SetShaderBuffersShaderStorage; + case kMGPipeShaderBufferClassAtomicCounter: + return MGPipeSuppressorSlot::SetShaderBuffersAtomicCounter; + default: return MGPipeSuppressorSlot::SetShaderBuffersUniform; + } + } + + // D-G3's shape, the same one the three unit sets use: XXH64 over the tail with the header's + // own discriminators mixed in. Class is mixed BECAUSE the three classes share nothing but + // this function - two classes whose windows happen to hold the same ranges must not hash the + // same, or the per-class A/B tallies would be reading each other's traffic - and the mask + // words are mixed because they are a FIELD of the record that the entries alone do not + // determine (a uniform range and a storage range with the same handle differ only there). + inline Uint64 MGPipeShaderBufferSetContentHash(const MGPBufferRange* entries, Uint32 cls, + Uint32 start, Uint32 count, + const Uint32* writableMask) { + Uint64 hash = XXH64(entries, static_cast(count) * sizeof(MGPBufferRange), 0); + hash = MGPipeMixShutter(hash, cls); + hash = MGPipeMixShutter(hash, start); + hash = MGPipeMixShutter(hash, count); + for (Uint32 w = 0; w < kMGPipeShaderBufferWritableMaskWords; ++w) { + hash = MGPipeMixShutter(hash, writableMask[w]); + } + return hash; + } + + class MGPipeShaderBufferEmitter { + public: + using GLContext = MG_State::GLState::GLContext; + + // Dirty bit 15 (NEW_CONST_BUFFERS) -> the uniform binding points. + Uint64 EmitConstBuffers(GLContext& ctx) { + return EmitClass(ctx, kMGPipeShaderBufferClassUniform); + } + + // Dirty bit 16 (NEW_SHADER_BUFFERS) -> the two WRITABLE classes, which share a bit + // because they share a shutter: a storage bind and a counter bind are both "the + // shader's writable binding points moved", and a workload that touches one without the + // other pays one suppressed record rather than a second dirty bit. + Uint64 EmitShaderBuffers(GLContext& ctx) { + return EmitClass(ctx, kMGPipeShaderBufferClassShaderStorage) + + EmitClass(ctx, kMGPipeShaderBufferClassAtomicCounter); + } + + // The validate point's FreshlyPrimed arm. A fresh context is a fresh set of binding + // points AND a fresh applier window (MGPipeApplierReset clears all three), so the + // emitter's mirrors go with them. The suppressor slots are invalidated beside this call + // by InvalidateAll(); without that the first emission after a make-current would be + // suppressed as unchanged and the server would draw against a cleared window. + void Reset() { + for (Uint32 cls = 0; cls < kMGPipeShaderBufferClassCount; ++cls) { + m_last[cls] = MGPShaderBuffers{}; + } + } + + void ResetCounters() { + for (Uint32 cls = 0; cls < kMGPipeShaderBufferClassCount; ++cls) m_emissions[cls] = 0; + } + + // What a unit case reads. The emitter hands m_entries straight to the route, so "what + // was emitted" costs no copy at all - VertexInputEmit.h's G6/G7 property. + const MGPShaderBuffers& LastHeader(Uint32 cls) const { + return m_last[cls < kMGPipeShaderBufferClassCount ? cls : 0]; + } + const Array& LastRanges() const { + return m_entries; + } + Uint64 EmissionCount(Uint32 cls) const { + return m_emissions[cls < kMGPipeShaderBufferClassCount ? cls : 0]; + } + + private: + Uint64 EmitClass(GLContext& ctx, Uint32 cls) { + const BufferTarget target = MGPipeBufferTargetForShaderBufferClass(cls); + // INVARIANT 1, and it is one integer read on every draw of every application that + // never binds an indexed buffer of this class. + const SizeT touched = ctx.GetTouchedBufferBindingPointCount(target); + if (touched == 0) return 0; + const Uint32 count = touched < kMGPipeMaxBufferBindingPoints + ? static_cast(touched) + : kMGPipeMaxBufferBindingPoints; + // NO CLIENT-SIDE CLAMP TO THE DEVICE's LIMIT (ruling 10). MobileGL advertises the + // GL 4.5 minimum of 84 uniform binding points while ES 3.2's is 72, and which of + // them the driver can actually hold is a SERVER question: the backend clamps to + // GL_MAX_UNIFORM_BUFFER_BINDINGS exactly as it does today, because a client reading + // a device capability would be answering it from the wrong side of the wire. + + // The writable mask is rebuilt from scratch for every emission: it is a property of + // the window as it stands now, not an accumulation, and a sticky one would keep + // claiming a point that has since been unbound. + Uint32 writableMask[kMGPipeShaderBufferWritableMaskWords] = {}; + const Bool classIsWritable = cls != kMGPipeShaderBufferClassUniform; + + for (Uint32 i = 0; i < count; ++i) { + const auto& point = ctx.GetBufferBindingPoint(target, i); + const auto& object = point.GetBoundObject(); + MGPBufferRange& entry = m_entries[i]; + entry = MGPBufferRange{}; + if (!object) { + // A null handle is "nothing bound at that point", which is what the server + // binds 0 for. Spelled by clearing the entry rather than by shortening the + // window: the window is the touched high-water mark and a hole in the + // middle of it is ordinary. + entry.Res = kMGPipeNullHandle; + continue; + } + entry.Res = MGPipeSlots().Acquire(MGPipeKind::Buffer, object->GetLifetimeId()); + // D-A3's sticky bind mask, ORed HERE for the reason VertexInputEmit.h ORs it at + // every draw: a buffer given its storage through glNamedBufferData and only ever + // bound with glBindBufferBase is bound to NOTHING at its resource emission, so + // the mask sampled there would never carry its UNIFORM / SHADER_STORAGE / + // ATOMIC_COUNTER bit. This is the resolution site, and it is sticky, so one + // validate point is enough for the rest of the buffer's life. + MGPipeResourceTrackerInstance().NoteBoundAs(entry.Res, target); + // INVARIANT 2. HasExplicitRange() is exactly "this point was bound with + // glBindBufferRange": BindingSlotRange1D keeps the flag precisely so GetRange() + // can re-resolve a BASE binding against the object every time it is asked, and + // that re-resolution is what must happen on the SERVER rather than here. + if (point.HasExplicitRange()) { + const auto range = point.GetRange(); + entry.Offset = static_cast(range.start); + entry.Size = static_cast(range.end - range.start); + } else { + entry.Offset = 0; + entry.Size = kMGPipeWholeBuffer; + } + // WHAT REPLACES THE BACKEND's GPU-WRITE WALK. Every bound storage-buffer and + // atomic-counter point is marked writable, which is exactly the set + // MarkShaderStorageBuffersGpuWritten and SyncAtomicCounterBuffers used to walk + // the frontend for - conservative in the same direction and for the same reason + // (GpuWritePending.h: an over-approximate set costs a readback, an under- + // approximate one reads a stale shadow and says nothing). The uniform class + // never sets a bit: a UBO is read-only to the shader by definition. + if (classIsWritable) MGPipeShaderBufferMaskSet(writableMask, i); + } + + const Uint64 hash = + MGPipeShaderBufferSetContentHash(m_entries.data(), cls, 0, count, writableMask); + if (!MGPipeSetHashSuppressorInstance().ShouldEmit( + MGPipeSuppressorSlotForShaderBufferClass(cls), hash)) { + return 0; + } + MGPShaderBuffers& header = m_last[cls]; + header = MGPShaderBuffers{}; + header.Class = cls; + header.Start = 0; + header.Count = count; + for (Uint32 w = 0; w < kMGPipeShaderBufferWritableMaskWords; ++w) { + header.WritableMask[w] = writableMask[w]; + } + // HostSpanCount IS 0 ALWAYS on Espryt, and that is a ruling rather than an omission: + // the second var-tail exists for kCapNeedsHostUboBytes (Magma's named-UBO ring) and + // that bit is 0 for the whole of P5 (CONTRACT-P5.md table 0). The codec's host-span + // honesty pass is the guard that says so out loud if a backend ever publishes it. + header.HostSpanCount = 0; + header.ContentHash = hash; + MGPipeRouteSetShaderBuffers(header, m_entries.data()); + ++m_emissions[cls]; + return sizeof(MGPShaderBuffers) + static_cast(count) * sizeof(MGPBufferRange); + } + + // ONE entry buffer for all three classes, not three. A class is built and routed before + // the next one is built (EmitShaderBuffers above calls EmitClass twice in sequence and + // the route copies or stages the tail synchronously in both arms), so three would be + // three times 84 * 24 bytes of resident staging for no reader. + Array m_entries{}; + MGPShaderBuffers m_last[kMGPipeShaderBufferClassCount]{}; + Uint64 m_emissions[kMGPipeShaderBufferClassCount] = {0, 0, 0}; + }; + + inline MGPipeShaderBufferEmitter& MGPipeShaderBufferEmitterInstance() { + // NEVER DESTROYED, for MGPipeTrackerInstance()' reason; heap-constructed and + // intentionally leaked at exit, like every other MGPipe process singleton. + static MGPipeShaderBufferEmitter* emitter = new MGPipeShaderBufferEmitter(); + return *emitter; + } +} // namespace MobileGL::MG_Pipe +#endif // MOBILEGL_PIPE_PUSH diff --git a/MobileGL/MG_Impl/Pipe/Tracker.h b/MobileGL/MG_Impl/Pipe/Tracker.h index 01cee868..8ed7316d 100644 --- a/MobileGL/MG_Impl/Pipe/Tracker.h +++ b/MobileGL/MG_Impl/Pipe/Tracker.h @@ -551,7 +551,11 @@ namespace MobileGL::MG_Pipe { // ---- the object-class bits 9..17 ---- const Uint64 textureContent = ctx.GetAnyTextureContentGeneration(); const Uint64 textureParams = ctx.GetAnyTextureParamsGeneration(); - const Uint64 buffers = ctx.GetAnyBufferChangeGeneration(); + // P5e (sb): the buffer CONTENT aggregate is no longer read here at all. It was + // bits 15/16/17's whole shutter and it answered the wrong question for every one of + // them (see those bits below); the aggregate itself stays, because + // MGPipeAggregate::BufferChange is still one of the six the walk reports and a + // counter with no reader is a different removal from a shutter with a better input. // Bit 9. The VAO attribute aggregate mixed with the bound VAO's identity is // already exact for the SET - it is bumped by all three Bump*Version functions, @@ -662,10 +666,50 @@ namespace MobileGL::MG_Pipe { now[Index(MGPipeDirty::NewShaderImages)] = MGPipeMixShutter( MGPipeMixShutter(MGPipeMixShutter(textureContent, textureParams), programImages), ctx.GetTextureBindGeneration()); - now[Index(MGPipeDirty::NewConstBuffers)] = buffers; - now[Index(MGPipeDirty::NewShaderBuffers)] = buffers; - now[Index(MGPipeDirty::NewSoTargets)] = - MGPipeMixShutter(buffers, ctx.GetTransformFeedbackGeneration()); + // ---- bits 15/16/17, REWRITTEN AT P5e (sb, MG_Remote/CONTRACT-P5E.md §5.6) ---- + // + // ALL THREE USED TO READ `buffers` - the buffer CONTENT aggregate - AND THAT WAS + // WRONG IN BOTH DIRECTIONS AT ONCE. Over: any glBufferSubData anywhere fired all + // three, which is the same width bit 10 was narrowed out of at P3a. Under, and this + // is the half that mattered: glBindBufferBase / glBindBufferRange mutate a binding + // point through a returned reference, which moves the slot's own Uint16 version and + // NOTHING the content aggregate reads - so + // `glBindBufferBase(UNIFORM,1,A); draw; glBindBufferBase(UNIFORM,1,B); draw` fired + // no bit at all. Harmless while nothing was emitted for these three; an + // under-fire the moment set_shader_buffers is, and under-firing renders stale, + // which this file's own rule calls the dangerous direction. Same defect class as + // P4a's glBindSampler hole (c0d), closed the same way: the shutter reads the + // generation the mutator actually moves (BufferState::NoteBindPointChanged). + // + // THE CONTENT AGGREGATE LEAVES ALL THREE and is not replaced by anything: whether + // the BYTES behind a bound buffer moved is the resource family's question, answered + // server-side by the resource record's own Serial, and the record these bits emit + // carries {handle, offset, size} - none of which a glBufferSubData changes. A base + // binding's extent is the one thing that could, and it does not travel resolved: + // Size is kMGPipeWholeBuffer and the server re-resolves it at use (§5.6). + // + // BIT 15 MIXES THE PROGRAM IDENTITY IN, for bit 12's and bit 14's reason one family + // over (fable seams F-1/F-2): the uniform window is resolved FOR THE PROGRAM IN USE + // - the UBO loop indexes it by the program's own block bindings - so a glUseProgram + // alone must re-open it. `shader` is that identity in both arms (lifetime id x link + // version, or stageLinks under a pipeline). + // + // BIT 16 IS TWO TARGETS, one bit: a storage-buffer bind and an atomic-counter bind + // are both "the shader's writable binding points moved", they are emitted together + // at the same validate point, and splitting them would buy one suppressed record on + // a workload that binds one without the other. + // + // BIT 17 KEEPS THE TRANSFORM-FEEDBACK GENERATION beside the new bind-point one: + // the capture points are span-scoped state latched at Begin, so when a span opens + // matters as much as what is bound. Nothing is emitted for it this phase (§5.7). + now[Index(MGPipeDirty::NewConstBuffers)] = MGPipeMixShutter( + ctx.GetBufferBindPointGeneration(BufferTarget::Uniform), shader); + now[Index(MGPipeDirty::NewShaderBuffers)] = MGPipeMixShutter( + ctx.GetBufferBindPointGeneration(BufferTarget::ShaderStorage), + ctx.GetBufferBindPointGeneration(BufferTarget::AtomicCounter)); + now[Index(MGPipeDirty::NewSoTargets)] = MGPipeMixShutter( + ctx.GetBufferBindPointGeneration(BufferTarget::TransformFeedback), + ctx.GetTransformFeedbackGeneration()); Uint32 dirty = 0; for (SizeT i = 0; i < kMGPipeDirtyCount; ++i) { diff --git a/MobileGL/MG_Pipe/DirtySurface.def b/MobileGL/MG_Pipe/DirtySurface.def index 641ad53e..a803041d 100644 --- a/MobileGL/MG_Pipe/DirtySurface.def +++ b/MobileGL/MG_Pipe/DirtySurface.def @@ -284,16 +284,37 @@ /* which is the window no shutter and no pull can cover, and the reason those sites */ \ /* had to come inside this gate rather than stay a recorded blind spot. */ \ X(NoteUnitTouched, kPulledPartialShutter|NEW_SAMPLER_VIEWS) \ - /* NOT NEW_SO_TARGETS, and this one was false on EVERY path: GLContext::SetNamed */ \ - /* TransformFeedbackBinding either binds a BufferState binding point (index == the */ \ - /* bound XFB object) or writes a saved-bindings entry, and NEW_SO_TARGETS mixes the */ \ - /* buffer-CONTENT aggregate with the transform-feedback generation - the first moves */ \ - /* only at BufferObject.cpp's content sites, the second only in BeginTransformFeedback. */ \ - /* A binding moves neither. It reaches the backend the same way every other buffer */ \ - /* binding point does, through GetBufferBindingPoint in the verb class's may-read mask, */ \ - /* so the honest answer is the pull. Narrowing it is P3b's, when it takes the subsystem */ \ - /* over and the binding points get a generation of their own. */ \ - X(SetNamedTransformFeedbackBinding, kPulledEveryVerb) \ + /* P5e (sb) REWRITES THIS ROW, AND THE OLD ONE ENDED WITH ITS OWN CONDITION: "narrowing */ \ + /* it is P3b's, when it takes the subsystem over and the binding points get a */ \ + /* generation of their own". They have one now (BufferState::NoteBindPointChanged), bit */ \ + /* 17's shutter reads it, and this function bumps it - so the paragraph that said */ \ + /* NEW_SO_TARGETS was "false on EVERY path" is withdrawn, because what made it false */ \ + /* was that the bit mixed the buffer-CONTENT aggregate, which no binding has ever moved.*/ \ + /* */ \ + /* kPulledPartialShutter AND NOT A BARE BIT, because the function has two arms: with */ \ + /* `index == m_boundTransformFeedback` it binds a real BufferState binding point and */ \ + /* bumps, and otherwise it writes the NAMED object's saved-bindings entry, which is not */ \ + /* a binding point at all and deliberately bumps nothing. So the bit fires on some of */ \ + /* the paths that mutate and not all, which is exactly what this answer is for; what */ \ + /* holds on every mutating path is still the pull through GetBufferBindingPoint. */ \ + X(SetNamedTransformFeedbackBinding, kPulledPartialShutter|NEW_SO_TARGETS) \ + /* P5e (sb), THE ONE NAME THE `Touch` PREFIX SURFACES, on two call sites - GL_Buffer.cpp's*/ \ + /* BindBufferBase_State and BindBufferRange_State. It moves the per-target high-water */ \ + /* mark that bounds every indexed-binding-point walk there is: the backend's, the */ \ + /* client's GPU-write sweep's, and set_shader_buffers' emitted window. */ \ + /* */ \ + /* kPulledEveryVerb, in GetMaxTouchedTextureUnit's exact words one family over: the */ \ + /* field it writes (GetTouchedBufferBindingPointCount) has no shutter of its own and */ \ + /* needs none - it is in kDraw's, kDispatch's and kXfbSpan's may-read masks, so the */ \ + /* residual fill copies it at every verb of those classes, and since P5c rv */ \ + /* set_context_values carries it as a value whose gate is the whole-record hash. */ \ + /* */ \ + /* IT NAMES NO BIT, AND THAT IS NOT THE SAME AS THE MUTATION GOING UNPUBLISHED. Both */ \ + /* call sites bump the bind-point generation on the line below this call, so bits 15/16/ */ \ + /* 17 fire for the bind - but they fire for the BINDING, through a different member, and */ \ + /* a row here that named them would claim this mutator moves a shutter it does not touch */ \ + /* (the derivation refutes exactly that, which is how the claim was tested). */ \ + X(TouchBufferBindingPoint, kPulledEveryVerb) \ /* ---- P4a, THE FOUR THE WIDENED PREFIX SET SURFACES. Every one of them moves a field */ \ /* P3a or P4a pushes and none of them was visible to the scan before, because none */ \ /* begins with one of the ten words the pattern matched. */ \ @@ -415,10 +436,23 @@ // // The ten mutators that reach a tainted body (--check prints the count) all carry a prose // answer, which no derivation checks; these two mutators are the first that carry a bit answer. +// +// P5e (sb) ADDS A THIRD, for the same property of the scanner and a third unplaceable write: +// +// SetNamedTransformFeedbackBinding +// -> MGPipeSlots().Acquire() calls GetLifetimeId() through its reference +// parameter `buffer`, which the analysis cannot root in a member +// +// The claim it marks is plain: the bound arm of this function bumps the transform-feedback +// target's bind-point generation (BufferState::NoteBindPointChanged), which is exactly what +// bit 17's shutter reads since P5e. The mark is on the BIT the kPulledPartialShutter row +// names, not on the pull, so what this list suspends is the narrowing claim and never the +// answer that holds on every path. #define MGP_DIRTY_SURFACE_UNDECIDED_LIST(X) \ X(UseProgram, NEW_SHADER) \ X(UseProgram, NEW_SAMPLER_VIEWS) \ X(UseProgram, NEW_SHADER_IMAGES) \ - X(BindVertexArray, NEW_VERTEX_ELEMENTS) + X(BindVertexArray, NEW_VERTEX_ELEMENTS) \ + X(SetNamedTransformFeedbackBinding, NEW_SO_TARGETS) // clang-format on diff --git a/MobileGL/MG_Pipe/MGPipeTypes.h b/MobileGL/MG_Pipe/MGPipeTypes.h index 01adf005..bfc8fd15 100644 --- a/MobileGL/MG_Pipe/MGPipeTypes.h +++ b/MobileGL/MG_Pipe/MGPipeTypes.h @@ -978,17 +978,57 @@ namespace MobileGL::MG_Pipe { inline constexpr Uint32 kMGPipeShaderBufferClassAtomicCounter = 2; inline constexpr Uint32 kMGPipeShaderBufferClassCount = 3; + // P5e, ID-104. HOW MANY Uint32 WORDS THE WRITABLE MASK NEEDS TO COVER THE WINDOW IT IS + // ATTACHED TO, and it is a derived number rather than a literal because the two used to + // disagree: WritableMask was ONE Uint32 while the window has been 84 points since P4a, so + // the mask could only ever describe the first 32 of them and a shader-storage buffer bound + // at point 32 or above was silently unwritable as far as the record was concerned. + // + // THE RULING WIDENS THE FIELD RATHER THAN NARROWING THE WINDOW (ID-104): narrowing to 32 + // would change what an application can bind, which is an observable, while the POD growing + // 32 -> 40 bytes is paid by a record that fires at most once per class per change and is + // confined to the push build. THREE Uint32 WORDS AND NOT A Uint64+Uint32 PAIR: 96 >= 84 + // either way, but one word type means one shift width, so the set and the test below are a + // single expression each instead of a low half and a high half that have to agree. + inline constexpr Uint32 kMGPipeShaderBufferWritableMaskWords = 3; + struct MGPShaderBuffers { Uint32 Class; // MGPipeTypes.h's kMGPipeShaderBufferClass* - Uniform | ShaderStorage | // AtomicCounter Uint32 Start; Uint32 Count; - Uint32 WritableMask; + // Bit i (LSB-first within word i/32) = "the shader may WRITE through binding point i", + // for classes 1 and 2. Always read and written through the two helpers below, never by + // hand: an open-coded shift is how the emitter and the four consumers come to disagree + // about which word a point lives in. + Uint32 WritableMask[kMGPipeShaderBufferWritableMaskWords]; Uint32 HostSpanCount; // 0, or Count when the kHostSpan tail is present (D-B8) Uint32 Pad0; Uint64 ContentHash; }; - MGP_ASSERT_POD(MGPShaderBuffers, 32); + MGP_ASSERT_POD(MGPShaderBuffers, 40); + + // THE ONE SPELLING OF THE MASK, for the reason kMGPipeShaderBufferClass* is the one + // spelling of the class: the emitter sets these bits on the client and four backend sites + // read them on the server, and a mask indexed by hand in five places is five chances to + // pick the wrong word. Both take the words rather than the record so the APPLIER's copy of + // the mask - which is per class and lives in MGPipeApplierState, not in a payload - goes + // through exactly the same arithmetic. + // + // OUT OF RANGE IS "NOT WRITABLE" AND "WRITE NOTHING", never an out-of-bounds word: the + // capacity is the wire's bound and the codec refuses a record past it, so this is the + // second belt rather than the first. + inline constexpr Bool MGPipeShaderBufferMaskHas(const Uint32* words, Uint32 index) { + if (index >= kMGPipeMaxBufferBindingPoints) return false; + return (words[index >> 5] & (Uint32{1} << (index & 31u))) != 0; + } + inline constexpr void MGPipeShaderBufferMaskSet(Uint32* words, Uint32 index) { + if (index >= kMGPipeMaxBufferBindingPoints) return; + words[index >> 5] |= Uint32{1} << (index & 31u); + } + static_assert(kMGPipeShaderBufferWritableMaskWords * 32u >= kMGPipeMaxBufferBindingPoints, + "the writable mask no longer covers the binding-point window it describes " + "(ID-104: widen the mask, never narrow the window)"); // Var-tail header: MGPBufferRange[Count] then Uint32 offsets[Count]. struct MGPStreamOutputTargets { diff --git a/MobileGL/MG_Pipe/PipeApply.cpp b/MobileGL/MG_Pipe/PipeApply.cpp index 52f91983..4b94e8d2 100644 --- a/MobileGL/MG_Pipe/PipeApply.cpp +++ b/MobileGL/MG_Pipe/PipeApply.cpp @@ -1239,6 +1239,22 @@ namespace MobileGL::MG_Pipe { ++g_applier.RefusedNoConsumer; return true; } + + // P5e (sb, CONTRACT-P5E.md §5.6): the three binding-point windows, cleared in one + // place because BOTH resets clear them and the two used to drift on exactly this kind + // of member (the framebuffer family's four). The SERIAL is deliberately NOT touched + // here - each caller advances it beside its own siblings, where the "advance, never + // zero" rule is written out. + void ClearShaderBufferWindows() { + for (Uint32 cls = 0; cls < kMGPipeShaderBufferClassCount; ++cls) { + g_applier.BoundShaderBuffers[cls] = {}; + g_applier.ShaderBufferStart[cls] = 0; + g_applier.ShaderBufferCount[cls] = 0; + for (Uint32 w = 0; w < kMGPipeShaderBufferWritableMaskWords; ++w) { + g_applier.ShaderBufferWritableMask[cls][w] = 0; + } + } + } } // namespace void MGPipeApplierReset() { @@ -1363,11 +1379,22 @@ namespace MobileGL::MG_Pipe { g_applier.DrawProgram = kMGPipeNullHandle; g_applier.DispatchProgram = kMGPipeNullHandle; g_applier.BoundShaderCso = kMGPipeNullHandle; + // P5e (sb, CONTRACT-P5E.md §5.6): the three binding-point windows are per-context + // WORKING state and go with the rest of it - a returning context has its own + // glBindBufferBase history and may not inherit the one this applier was left holding. + // The SERIAL advances rather than restarting, for the reason written out above: the + // clearing is itself a change every backend memo has to hear about, and a counter that + // restarts walks back through values already stamped into a memo that outlived the + // switch. The EMITTER's latch resets with it (PipeFill.cpp's FreshlyPrimed arm + // invalidates the three suppressor slots), or the first emission after a make-current + // would be suppressed as unchanged and the server would draw with a cleared window. + ClearShaderBufferWindows(); ++g_applier.FramebufferSerial; ++g_applier.SamplerViewsSerial; ++g_applier.SamplerStatesSerial; ++g_applier.ShaderImagesSerial; ++g_applier.ProgramBindingSerial; + ++g_applier.ShaderBuffersSerial; } void MGPipeApplierReleaseObjectRecords() { @@ -1424,11 +1451,18 @@ namespace MobileGL::MG_Pipe { g_applier.BoundShaderImages = {}; g_applier.ShaderImageStart = 0; g_applier.ShaderImageCount = 0; + // P5e (sb): and the three binding-point windows, for the paragraph above's reason in + // its own words - every MGPBufferRange::Res names a resource record this function has + // just dropped, so a window left populated here is a set of handles into an empty + // table, which the next resolve either refuses or answers with somebody else's record + // on a slot the next context re-mints. + ClearShaderBufferWindows(); ++g_applier.FramebufferSerial; ++g_applier.SamplerViewsSerial; ++g_applier.SamplerStatesSerial; ++g_applier.ShaderImagesSerial; ++g_applier.ProgramBindingSerial; + ++g_applier.ShaderBuffersSerial; } void MGPipeApplyCreateRenderState(const MGPRenderStateDesc& desc, const void* chunkBytes) { @@ -2830,25 +2864,69 @@ namespace MobileGL::MG_Pipe { } // --------------------------------------------------------------------------------- - // P5e's two entry points, DECLARED AND REFUSED HERE (MG_Remote/CONTRACT-P5E.md §1) + // P5e's two entry points, DECLARED BY c0e (MG_Remote/CONTRACT-P5E.md §1) // --------------------------------------------------------------------------------- // - // Package sb writes the first body and package pg the second. Until then a record that - // reached either one would be a binding set the server accepted and dropped, so the answer - // is a named abort rather than a quiet return: the picture would be wrong and the lane - // would be green, which is the one failure mode this campaign spends its refusals on. + // set_shader_buffers HAS ITS BODY (package sb); set_program_bindings still refuses by name + // until package pg writes its own. A record that reached an unbodied one would be a binding + // set the server accepted and dropped, so the answer there is a named abort rather than a + // quiet return: the picture would be wrong and the lane would be green, which is the one + // failure mode this campaign spends its refusals on. + + // set_shader_buffers, ONE RECORD PER CLASS (CONTRACT-P5E.md §5.6, rulings 10/11). The whole + // body is "validate the window, copy the tail into that class's array, take the mask, move + // the serial" - the binding points are WORKING STATE and the objects they name are resolved + // by the backend at its own sync point, from the handle each range carries. // - // Neither is reachable today - no route row installs the table slot and the wire decoder's - // arms for both opcodes validate and decline (PipeCatalogueTest pins both slots null) - so - // this is a link-time seam, not a runtime one. + // IT DOES NOT GO THROUGH ApplyUnitWindow, and the reason is worth a line rather than a + // diff: that helper's refusal message names "the merged texture-unit space" and the three + // unit sets share one capacity, while this row has THREE windows keyed by Class and two + // refusals of its own that the contract names by string ("SetShaderBuffers.Class" / + // ".Count"). A shared helper that had to grow a class parameter and a message parameter + // would be the same code with more ways to pass the wrong one. + // + // AND THERE IS NO NoP4aConsumer BELT HERE, deliberately. P4a's belt exists because a client + // clears a per-level dirty flag on ACCEPTANCE, so accepting on a backend that consumes + // nothing loses uploads (ID-39). This family's liveness gate is the client's own + // P5eFamilyIsLive (PipeFill.cpp), which asks R-8 for bit 13 in particular - a server that + // does not publish bit 13 never receives one of these records at all - and storing a window + // no backend reads costs nothing and loses nothing, because the emitter's only latch is the + // set-hash suppressor and the record it latched did cross. void MGPipeApplySetShaderBuffers(const MGPShaderBuffers& hdr, const MGPBufferRange* tail) { - (void)tail; - MGLOG_F("MGPipe: Fatal{UnmigratedVerb, \"set_shader_buffers\"} - the applier entry point " - "is declared by P5e package c0e and bodied by package sb; a record for class %u " - "with %u range(s) reached it, which means a route was installed ahead of its " - "consumer", - hdr.Class, hdr.Count); - std::abort(); + if (hdr.Class >= kMGPipeShaderBufferClassCount) { + MGP_TRIP_WIRE_REPORT("MGPipe: " MGP_TRIP_WIRE_TAG("ProtocolCorruption") + " SetShaderBuffers.Class - class %u names no binding-point " + "array; the three are Uniform=0, ShaderStorage=1, " + "AtomicCounter=2", + hdr.Class); + return; + } + const Uint64 end = Uint64{hdr.Start} + Uint64{hdr.Count}; + if (end > kMGPipeMaxBufferBindingPoints || (hdr.Count != 0 && tail == nullptr)) { + MGP_TRIP_WIRE_REPORT("MGPipe: " MGP_TRIP_WIRE_TAG("ProtocolCorruption") + " SetShaderBuffers.Count {class=%u, start=%u, count=%u}: the " + "window runs past the %u binding points the wire carries, or a " + "non-empty set carries no ranges", + hdr.Class, hdr.Start, hdr.Count, + static_cast(kMGPipeMaxBufferBindingPoints)); + return; + } + // THE WINDOW IS THE BOUND AND ENTRIES OUTSIDE IT ARE NOT CLEARED, exactly as the three + // unit sets and set_vertex_buffers work: the record is "the last set as received", and + // a binding at or above Count means "nothing bound" to every reader (the frontend + // array's default says the same thing, which is what makes the two pictures agree). + auto& destination = g_applier.BoundShaderBuffers[hdr.Class]; + for (Uint32 i = 0; i < hdr.Count; ++i) destination[hdr.Start + i] = tail[i]; + g_applier.ShaderBufferStart[hdr.Class] = hdr.Start; + g_applier.ShaderBufferCount[hdr.Class] = hdr.Count; + for (Uint32 w = 0; w < kMGPipeShaderBufferWritableMaskWords; ++w) { + g_applier.ShaderBufferWritableMask[hdr.Class][w] = hdr.WritableMask[w]; + } + // ONE SERIAL FOR ALL THREE CLASSES, and it moves on EVERY applied record - a backend + // memo that asks "have the binding points moved since I last bound them" must hear + // about a uniform set exactly as it hears about a storage set, and splitting the serial + // per class would make each of the four consumers pick which one to read. + ++g_applier.ShaderBuffersSerial; } void MGPipeApplySetProgramBindings(const MGPProgramBindings& hdr, const Int32* blockBindings, diff --git a/MobileGL/MG_Pipe/PipeApply.h b/MobileGL/MG_Pipe/PipeApply.h index 232ef337..f7468b93 100644 --- a/MobileGL/MG_Pipe/PipeApply.h +++ b/MobileGL/MG_Pipe/PipeApply.h @@ -752,7 +752,14 @@ namespace MobileGL::MG_Pipe { // the backend's own GPU-write marking walk (DirectGLES.cpp's three MarkBufferGpuWritten // sites), which sb deletes under a transport because the client already owns the // GPU-write set and the server's walk is over client memory. - Uint32 ShaderBufferWritableMask[kMGPipeShaderBufferClassCount] = {0, 0, 0}; + // + // THREE WORDS PER CLASS, NOT ONE (P5e, ID-104): the window is 84 points and c0e's + // single Uint32 could describe only the first 32 of them, so a storage buffer bound at + // point 32 or above read as read-only here while the record said nothing was wrong. + // Always through MGPipeShaderBufferMaskHas / ...Set (MGPipeTypes.h), which is the same + // arithmetic the payload's own mask goes through - one table, both sides. + Uint32 ShaderBufferWritableMask[kMGPipeShaderBufferClassCount] + [kMGPipeShaderBufferWritableMaskWords] = {}; // ONE serial for all three classes, ++ on every applied record and ADVANCED (never // zeroed) by MGPipeApplierReset, for VertexBuffersSerial's reason - and the emitter's // latch resets with it, or the first emission after a make-current is suppressed as diff --git a/MobileGL/MG_Pipe/PipeRoute.cpp b/MobileGL/MG_Pipe/PipeRoute.cpp index a9b4e359..e9851079 100644 --- a/MobileGL/MG_Pipe/PipeRoute.cpp +++ b/MobileGL/MG_Pipe/PipeRoute.cpp @@ -225,6 +225,11 @@ namespace MobileGL::MG_Pipe { MGP_MONO_TAIL(SetSamplerViews, MGPSamplerViews, MGPBoundView) MGP_MONO_TAIL(BindSamplerStates, MGPSamplerStates, MGPipeHandle) MGP_MONO_TAIL(SetShaderImages, MGPShaderImages, MGPImageView) + // P5e (sb, CONTRACT-P5E.md §5.6). Catalogued since P4a with no producer and no + // consumer; the adapter lands with the applier's body. The generic tail shape fits + // because the SECOND tail is absent on every Espryt configuration (HostSpanCount is 0 + // while kCapNeedsHostUboBytes is 0, which is the whole of P5). + MGP_MONO_TAIL(SetShaderBuffers, MGPShaderBuffers, MGPBufferRange) MGP_MONO_TAIL(SetVertexAttribDefaults, MGPVertexAttribDefaults, MGPAttribValue) #undef MGP_MONO_PLAIN @@ -342,6 +347,7 @@ namespace MobileGL::MG_Pipe { gMGPipeContext.SetSamplerViews = &Mono_SetSamplerViews; gMGPipeContext.BindSamplerStates = &Mono_BindSamplerStates; gMGPipeContext.SetShaderImages = &Mono_SetShaderImages; + gMGPipeContext.SetShaderBuffers = &Mono_SetShaderBuffers; gMGPipeContext.SetGlobalConstants = &Mono_SetGlobalConstants; gMGPipeContext.SetVertexAttribDefaults = &Mono_SetVertexAttribDefaults; gMGPipeContext.SetPixelPackState = &Mono_SetPixelPackState; diff --git a/MobileGL/MG_Pipe/PipeRoute.h b/MobileGL/MG_Pipe/PipeRoute.h index 7e794bf9..01396a0e 100644 --- a/MobileGL/MG_Pipe/PipeRoute.h +++ b/MobileGL/MG_Pipe/PipeRoute.h @@ -381,6 +381,18 @@ namespace MobileGL::MG_Pipe { MGP_SetShaderImages(&hdr, tail, hdr.Count); } + // ---- indexed buffer binding points (P5e, sb) ---------------------------------------- + // + // ONE ROUTE FOR THREE RECORDS: the class is a FIELD, not an opcode, so Uniform, + // ShaderStorage and AtomicCounter all come through here and the applier keys on hdr.Class. + // The second var-tail (MGHostSpan[HostSpanCount], D-B8) is never present on Espryt - + // kCapNeedsHostUboBytes is 0 for the whole of P5 - so this wrapper states one tail, and the + // codec's honesty pass is the guard that says so out loud if a backend ever asks for the + // other one. + inline void MGPipeRouteSetShaderBuffers(const MGPShaderBuffers& hdr, const MGPBufferRange* tail) { + MGP_SetShaderBuffers(&hdr, tail, hdr.Count); + } + // ---- programs ---------------------------------------------------------------------- inline void MGPipeRouteCreateShaderState(const MGPProgramDesc& desc, const MG_State::GLState::LinkArtifacts* link, diff --git a/MobileGL/MG_Remote/CapsCodec.cpp b/MobileGL/MG_Remote/CapsCodec.cpp index b6d841e7..12ef320e 100644 --- a/MobileGL/MG_Remote/CapsCodec.cpp +++ b/MobileGL/MG_Remote/CapsCodec.cpp @@ -60,6 +60,10 @@ namespace MobileGL::MG_Remote { // assertion that turns "room to P8" from a comment into a build break. static_assert(MG_Pipe::kMGPipeSubsystemsMigratedAtP4a <= 0xFFFFull, "the subsystem mask no longer fits CallMask's sixteen consumer bits"); + // P5e (sb): and the phase constant that is actually published now, so the assertion tracks + // the highest allocated bit rather than the highest bit at the time it was written. + static_assert(MG_Pipe::kMGPipeSubsystemsMigratedAtP5e <= 0xFFFFull, + "the subsystem mask no longer fits CallMask's sixteen consumer bits"); namespace { diff --git a/MobileGL/MG_Remote/Client/WireTables.cpp b/MobileGL/MG_Remote/Client/WireTables.cpp index 947fb771..14667922 100644 --- a/MobileGL/MG_Remote/Client/WireTables.cpp +++ b/MobileGL/MG_Remote/Client/WireTables.cpp @@ -222,6 +222,11 @@ namespace MobileGL::MG_Remote::Client { MGP_WIRE_TAIL(SetSamplerViews, MGPSamplerViews, MGPBoundView) MGP_WIRE_TAIL(BindSamplerStates, MGPSamplerStates, MGPipeHandle) MGP_WIRE_TAIL(SetShaderImages, MGPShaderImages, MGPImageView) + // P5e (sb, CONTRACT-P5E.md §5.6): the indexed buffer binding points, one record per + // class. The generic tail wrapper carries the FIRST tail only, which is the whole of + // the record on Espryt - the optional MGHostSpan tail exists for kCapNeedsHostUboBytes + // and that bit is 0 for the whole of P5. + MGP_WIRE_TAIL(SetShaderBuffers, MGPShaderBuffers, MGPBufferRange) MGP_WIRE_TAIL(SetVertexAttribDefaults, MGPVertexAttribDefaults, MGPAttribValue) #undef MGP_WIRE_PLAIN @@ -649,6 +654,7 @@ namespace MobileGL::MG_Remote::Client { gMGPipeContext.SetSamplerViews = &Wire_SetSamplerViews; gMGPipeContext.BindSamplerStates = &Wire_BindSamplerStates; gMGPipeContext.SetShaderImages = &Wire_SetShaderImages; + gMGPipeContext.SetShaderBuffers = &Wire_SetShaderBuffers; gMGPipeContext.SetGlobalConstants = &Wire_SetGlobalConstants; gMGPipeContext.SetVertexAttribDefaults = &Wire_SetVertexAttribDefaults; gMGPipeContext.SetPixelPackState = &Wire_SetPixelPackState; diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 342b3e6e..cd9e4403 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -652,6 +652,17 @@ namespace MobileGL::MG_Remote::Wire { // (MGPipeTypes.h:820-823), because the two arrays stay index-aligned; a third // value would let a record describe spans for ranges it does not have. const auto& p = *static_cast(payload); + // P5e (sb, CONTRACT-P5E.md §1): the class and the window, bounded HERE as well as + // in the applier and for the reason SetShaderImages' Count is bounded here - this + // is the arm that turns Count into a byte length, so a forged count has to be + // refused before it is multiplied rather than after. + if (p.Class >= kMGPipeShaderBufferClassCount) { + WireProtocolFatalAt("SetShaderBuffers.Class", p.Class, kMGPipeShaderBufferClassCount); + } + if (static_cast(p.Start) + p.Count > kMGPipeMaxBufferBindingPoints) { + WireProtocolFatalAt("SetShaderBuffers.Count", static_cast(p.Start) + p.Count, + kMGPipeMaxBufferBindingPoints); + } if (p.HostSpanCount != 0 && p.HostSpanCount != p.Count) { WireProtocolFatalAt("SetShaderBuffers.HostSpanCount", p.HostSpanCount, p.Count); } @@ -1830,16 +1841,17 @@ namespace MobileGL::MG_Remote::Wire { return true; case MGPWireOp::SetShaderBuffers: { - // NO APPLIER ENTRY POINT EXISTS, and P5 does not invent one: the call is on - // BRIEF §4's exclusion list, so a consumer written here would be a semantics - // nobody can test this phase. What the record needs and cannot get later is its - // TAIL ARITHMETIC, which MGPipeWireRecordLayout above has already run - including - // the HostSpanCount-is-0-or-Count rule, the thing that keeps the two arrays - // index-aligned. + // P5e (sb, MG_Remote/CONTRACT-P5E.md §5.6): THE APPLIER ENTRY POINT EXISTS NOW and + // this is the row's sink. The paragraph that stood here said "no applier entry + // point exists, and P5 does not invent one: the call is on BRIEF §4's exclusion + // list" - true for the whole of P5 through P5d, and withdrawn by the phase whose + // entire point is that the server stops walking the client's binding-point table. // - // kCapNeedsHostUboBytes is 0 for the whole of P5 (table 0), so the second tail is - // always absent here; the honesty pass below is what says so out loud if it ever - // is not. + // THE HOST-SPAN PASS STILL RUNS FIRST, and it runs whether or not the record is + // applied: kCapNeedsHostUboBytes is 0 for the whole of P5 (table 0), so the second + // tail is always absent here, and this pass is what says so out loud if it ever is + // not. A span that was going to be refused must be refused BEFORE the applier has + // stored the window it rides with. if (layout.TailCount == 2 && layout.TailBytes[1] != 0) { const auto* spans = reinterpret_cast(tailAt(1)); const Uint64 count = layout.TailBytes[1] / sizeof(MGHostSpan); @@ -1853,11 +1865,16 @@ namespace MobileGL::MG_Remote::Wire { CheckHostSpanIsHonest(span, *m_segments); } } - return false; + MGPipeApplySetShaderBuffers(*static_cast(payload), + reinterpret_cast(tailAt(0))); + return true; } case MGPWireOp::SetStreamOutputTargets: - // Same as above: no applier, off the reduced path, both tails validated. + // No applier entry point, off the reduced path, both tails validated. UNLIKE + // set_shader_buffers above this row stays unemitted for the whole of P5e (§5.7): + // XFB is lockstep, its capture points are span-scoped state latched at Begin, and + // the payload carries a Generation that no applier state has a home for. return false; case MGPWireOp::SetProgramBindings: { diff --git a/MobileGL/MG_State/GLState/BufferState/BufferState.h b/MobileGL/MG_State/GLState/BufferState/BufferState.h index 82ab6dc6..7a92c2b4 100644 --- a/MobileGL/MG_State/GLState/BufferState/BufferState.h +++ b/MobileGL/MG_State/GLState/BufferState/BufferState.h @@ -71,11 +71,44 @@ namespace MobileGL::MG_State::GLState { // shutter on in P2 - five bits over one aggregate until P3b splits them. void NoteBufferChanged() { ++m_anyBufferChangeGeneration; } Uint64 GetAnyBufferChangeGeneration() const { return m_anyBufferChangeGeneration; } + + // P5e (sb, MG_Remote/CONTRACT-P5E.md §1, §5.6). "DID ANY INDEXED BINDING POINT OF THIS + // TARGET MOVE" - one counter per BufferBindPointTargets entry, and it is the P4b hole + // closed rather than a new convenience. + // + // WHAT WAS WRONG. Dirty bits 15/16/17 shuttered on GetAnyBufferChangeGeneration() - the + // CONTENT aggregate - while glBindBufferBase / glBindBufferRange mutate a binding point + // through a returned reference (BindingSlotRange1D::Bind / SetRange), which moves the + // slot's own Uint16 version and nothing the tracker reads. So + // `glBindBufferBase(UNIFORM,1,A); draw; glBindBufferBase(UNIFORM,1,B); draw` fired no bit + // at all: harmless while nothing was emitted for those bits, and an UNDER-FIRE the moment + // set_shader_buffers is, because the server would go on binding A. It is the same defect + // class as P4a's glBindSampler hole (c0d) and it is closed the same way - the shutter + // reads the generation the mutator moves. + // + // A COUNTER AND NOT THE SLOT'S OWN VERSION: the slot version is a WRAPPING Uint16 per + // point, and a shutter over 84 of them would have to observe all 84 through the widened + // counter every draw. One monotone Uint64 per target answers "could this target's window + // have moved" in one read, which is what a per-draw shutter can afford. + // + // #if MOBILEGL_PIPE_PUSH so the pull build's BufferState does not resize (G1) - the same + // licence TextureState::NoteImageUnitTouched took at P5d r3. + void NoteBindPointChanged(const BufferTarget target) { + auto it = std::find(BufferBindPointTargets.begin(), BufferBindPointTargets.end(), target); + if (it == BufferBindPointTargets.end()) return; + ++m_bindPointGeneration[std::distance(BufferBindPointTargets.begin(), it)]; + } + Uint64 GetBindPointGeneration(const BufferTarget target) const { + auto it = std::find(BufferBindPointTargets.begin(), BufferBindPointTargets.end(), target); + if (it == BufferBindPointTargets.end()) return 0; + return m_bindPointGeneration[std::distance(BufferBindPointTargets.begin(), it)]; + } #endif private: #if MOBILEGL_PIPE_PUSH Uint64 m_anyBufferChangeGeneration = 0; + Array m_bindPointGeneration{}; #endif UnorderedMap> m_bufferObjects; IndexGenerator m_indexGenerator; diff --git a/MobileGL/MG_State/GLState/Core.cpp b/MobileGL/MG_State/GLState/Core.cpp index 34e98dc4..509fc34c 100644 --- a/MobileGL/MG_State/GLState/Core.cpp +++ b/MobileGL/MG_State/GLState/Core.cpp @@ -1522,6 +1522,17 @@ namespace MobileGL::MG_State { Bool hasExplicitRange) { if (bufferIndex >= MAX_TRANSFORM_FEEDBACK_BUFFERS) return; if (index == m_boundTransformFeedback) { +#if MOBILEGL_PIPE_PUSH + // P5e (sb): the THIRD writer of a transform-feedback binding point, and the one + // a reader of GL_Buffer.cpp alone would miss - glTransformFeedbackBufferBase / + // ...Range on the CURRENTLY BOUND object land here rather than in + // BindBufferBase_State. Dirty bit 17 shutters on this generation, so a bump + // left out here would make the DSA spelling of a capture-point bind invisible + // while the non-DSA one was not. The saved-bindings arm below writes no binding + // POINT at all (it writes the named object's own table) and deliberately bumps + // nothing. + NoteBufferBindPointChanged(BufferTarget::TransformFeedback); +#endif auto& point = m_bufferState.GetBindingPoint(BufferTarget::TransformFeedback, bufferIndex); point.Bind(buffer); if (buffer && hasExplicitRange) { diff --git a/MobileGL/MG_State/GLState/Core.h b/MobileGL/MG_State/GLState/Core.h index 6ed617f1..8c8343fd 100644 --- a/MobileGL/MG_State/GLState/Core.h +++ b/MobileGL/MG_State/GLState/Core.h @@ -251,6 +251,15 @@ namespace MobileGL { Uint64 GetAnyBufferChangeGeneration() const { return m_bufferState.GetAnyBufferChangeGeneration(); } + // P5e (sb): the per-target INDEXED BINDING POINT generation, which is what dirty + // bits 15/16/17 shutter on now. See BufferState::NoteBindPointChanged for why + // the content aggregate above could not answer the question. + void NoteBufferBindPointChanged(BufferTarget target) { + m_bufferState.NoteBindPointChanged(target); + } + Uint64 GetBufferBindPointGeneration(BufferTarget target) const { + return m_bufferState.GetBindPointGeneration(target); + } // The sixth aggregate lives here rather than on a state container because // the values it guards do too (m_currentVertexAttributes). void NoteVertexAttribDefaultChanged() { ++m_anyVertexAttribDefaultGeneration; } diff --git a/MobileGL/MG_Test/Pipe/CMakeLists.txt b/MobileGL/MG_Test/Pipe/CMakeLists.txt index a82e513c..bc595c7f 100644 --- a/MobileGL/MG_Test/Pipe/CMakeLists.txt +++ b/MobileGL/MG_Test/Pipe/CMakeLists.txt @@ -141,7 +141,10 @@ endif() # have to come back to this file to add a case. # # P4a's six are FramebufferEmitTest, TextureEmitTest, SamplerEmitTest, ImageEmitTest, -# ProgramEmitTest and CompositeResolverTest. Each lands from the contract commit with one case +# ProgramEmitTest and CompositeResolverTest; P5e's one is ShaderBufferEmitTest (package sb, the +# indexed buffer binding points), which takes the same shape because it has the same needs - a +# live frontend context, a route that reaches the applier, and refusals that are a log line in +# a shipped push build. Each lands from the contract commit with one case # that pins the shape its later cases depend on - the emitter is one never-destroyed process # singleton, or the composite band has exactly one door - so none of them is an empty file # waiting for a package, and none of them can be registered wrongly without a red test. @@ -153,7 +156,8 @@ endif() # HERE is what keeps the later packages out of this file. foreach(pipeTest ResourceEmitTest VertexInputEmitTest FramebufferEmitTest TextureEmitTest SamplerEmitTest ImageEmitTest - ProgramEmitTest CompositeResolverTest) + ProgramEmitTest CompositeResolverTest + ShaderBufferEmitTest) add_executable(${pipeTest} ${pipeTest}.cpp) target_include_directories(${pipeTest} PRIVATE @@ -212,7 +216,8 @@ gtest_discover_tests(PipeCatalogueTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS un gtest_discover_tests(FieldOwnershipTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) foreach(pipeTest ResourceEmitTest VertexInputEmitTest FramebufferEmitTest TextureEmitTest SamplerEmitTest ImageEmitTest - ProgramEmitTest CompositeResolverTest) + ProgramEmitTest CompositeResolverTest + ShaderBufferEmitTest) gtest_discover_tests(${pipeTest} DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) endforeach() gtest_discover_tests(MagmaPipeIdentityTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) diff --git a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp index 359dd707..c946261a 100644 --- a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp +++ b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp @@ -162,14 +162,16 @@ TEST(PipeCatalogue, GeneratedTablesHoldTheWholeCatalogue) { // A row nobody has migrated is null - which is exactly what "this subsystem has not been // migrated, keep pulling" means (plan B section 4.1). // -// UNTIL P5 R-17 THAT WAS EVERY ROW, and this case said so. It is now EXACTLY THE 42 ROWS WITH -// NO MGPipeApply* ENTRY POINT (80 - the 38 that have one; the number was 34 at P5, 39 after -// P5b's five sink-only verbs, rv's set_context_values grew BOTH sides of the difference, and -// P5e's set_program_bindings grew the null side alone): -// the other 38 have an applier, R-17 installs adapters over them, +// UNTIL P5 R-17 THAT WAS EVERY ROW, and this case said so. It is now EXACTLY THE 41 ROWS WITH +// NO BODIED MGPipeApply* ENTRY POINT (80 - the 39 that have one; the number was 34 at P5, 39 +// after P5b's five sink-only verbs, rv's set_context_values grew BOTH sides of the difference, +// P5e's set_program_bindings grew the null side alone, and P5e's set_shader_buffers moved one +// row from the null side to the other - sb gave it its applier, its adapter and its emitter in +// one commit): +// the other 39 have an applier, R-17 installs adapters over them, // and a null there would no longer mean "keep pulling" - `MG_Impl/Pipe`'s call sites go through // the thunks, so a null would mean "call through a null pointer". The number is asserted rather -// than the emptiness, because "38 installed" and "41 still null" are the two halves of a +// than the emptiness, because "39 installed" and "41 still null" are the two halves of a // partition and a case that checked only one of them would pass an installer that had // overwritten rows it does not own. // THE NAME IS KEPT, AND SO IS THE STATEMENT IT MAKES - only the ROWS it makes it about have @@ -182,13 +184,18 @@ TEST(PipeCatalogue, UninstalledTablesAreAllNull) { const void* const* context = reinterpret_cast(&gMGPipeContext); #if MOBILEGL_PIPE_PUSH MGPipeInstallMonolithTables(); - // The 42 rows with no MGPipeApply* entry point are still null, and null still means "this + // The 41 rows with no MGPipeApply* body are still null, and null still means "this // subsystem has not been migrated, keep pulling". Named rather than counted, because the // count is the other case's job and two cases asserting the same number would both go red // for one change. P5c's two control records are among them by design (CONTRACT-P5C.md §5: // no MGPipeApply* entry point, no monolith producer - under a transport they reach // WireVerbSink instead). - EXPECT_EQ(gMGPipeContext.SetShaderBuffers, nullptr); + // + // P5e (sb): SetShaderBuffers HAS LEFT THIS LIST and the inversion is itself the proof the + // route landed - the row was pinned null "since P4a" by two cases, and its applier, its + // adapter and its emitter all arrive together. SetStreamOutputTargets stays, because XFB + // stays lockstep for the whole of P5e (CONTRACT-P5E.md §5.7). + EXPECT_NE(gMGPipeContext.SetShaderBuffers, nullptr); EXPECT_EQ(gMGPipeContext.SetStreamOutputTargets, nullptr); EXPECT_EQ(gMGPipeContext.DrawVbo, nullptr); EXPECT_EQ(gMGPipeContext.Present, nullptr); @@ -236,20 +243,24 @@ TEST(PipeCatalogue, ExactlyTheRoutedRowsAreInstalledAndTheRestAreStillNull) { EXPECT_EQ(installed + nulls, static_cast(kMGPipeCallCount)); #if MOBILEGL_PIPE_PUSH - // 34 + 4 = 38, and the split is the honest shape of R-17 rather than an implementation - // detail: 38 is the number of MGPipeApply* entry points PipeApply.h declares (37 at P5, and - // P5c rv's set_context_values - CONTRACT-P5C.md §5.3 - is the 38th), 34 of them - // fit a GENERATED row and go in the two tables, and FOUR cannot be expressed by any - // generated signature and go in the hand-written escape table beside them + // 35 + 4 = 39, and the split is the honest shape of R-17 rather than an implementation + // detail: 39 is the number of MGPipeApply* entry points PipeApply.h declares WITH A BODY + // (37 at P5, P5c rv's set_context_values was the 38th, and P5e sb's set_shader_buffers is + // the 39th), 35 of them fit a GENERATED row and go in the two tables, and FOUR cannot be + // expressed by any generated signature and go in the hand-written escape table beside them // (ResourceRespecify's uncarried initialBytes, ResourceFlushRange's likewise, // MapPersistent's size + seedBytes + void* return, CreateShaderState's seven blobrefs and // two typed pointers - each one a CONTRACT-P5 ruling, see MG_Pipe/PipeRoute.h). // + // set_program_bindings (opcode 80) is DECLARED and still aborting by name until package pg + // writes its body, so it is not counted here and its row is pinned null below - which is + // the same statement this pair of numbers has always made about an unimplemented row. + // // BOTH NUMBERS ARE ASSERTED. If the escape table were left out of this case, moving a row // out of the generated tables and forgetting to install its escape would read as a smaller // "installed" count and nothing else - and the call site would take a null. - EXPECT_EQ(installed, 34u) << "the routed rows and the applier's entry points disagree"; - EXPECT_EQ(nulls, static_cast(kMGPipeCallCount) - 34u); + EXPECT_EQ(installed, 35u) << "the routed rows and the applier's entry points disagree"; + EXPECT_EQ(nulls, static_cast(kMGPipeCallCount) - 35u); const void* const* escapes = reinterpret_cast(&gMGPipeRouteEscapes); SizeT escapesInstalled = 0; for (SizeT i = 0; i < sizeof(MGPipeRouteEscapes) / sizeof(void*); ++i) { @@ -257,14 +268,17 @@ TEST(PipeCatalogue, ExactlyTheRoutedRowsAreInstalledAndTheRestAreStillNull) { } EXPECT_EQ(escapesInstalled, 4u) << "an escape row is null; its call site would take a null " "pointer rather than fall back to anything"; - EXPECT_EQ(installed + escapesInstalled, 38u) - << "the two tables plus the escapes must be exactly PipeApply.h's entry points"; + EXPECT_EQ(installed + escapesInstalled, 39u) + << "the two tables plus the escapes must be exactly PipeApply.h's bodied entry points"; // And the rows that MUST still be null, named rather than counted: these are calls with no // applier at all (CONTRACT-P5 table 1 rows 13, 14: "no applier entry point exists"), plus // the two verbs the census measured as having zero MG_Impl call sites. An installer that // filled one of these would be claiming an implementation that does not exist. - EXPECT_EQ(gMGPipeContext.SetShaderBuffers, nullptr); + // + // P5e (sb): row 13 (set_shader_buffers) is no longer one of them - see the sibling case + // above. Row 14 (set_stream_output_targets) still is. + EXPECT_NE(gMGPipeContext.SetShaderBuffers, nullptr); EXPECT_EQ(gMGPipeContext.SetStreamOutputTargets, nullptr); EXPECT_EQ(gMGPipeContext.DrawVbo, nullptr); EXPECT_EQ(gMGPipeContext.Present, nullptr); @@ -1095,7 +1109,15 @@ TEST(PipeCatalogue, HostSpanResolvesTheMonolithPointer) { // ranges - the majority - pay nothing for a payload whose shape is not frozen yet. TEST(PipeCatalogue, BufferRangeCarriesNoInlineHostSpan) { static_assert(sizeof(MGPBufferRange) == 24); - static_assert(sizeof(MGPShaderBuffers) == 32); + // P5e (sb, ID-104): 32 -> 40. WritableMask was ONE Uint32 against an 84-point window, so + // it could describe only the first 32 points and a storage buffer bound at point 32 or + // above read as read-only with nothing able to see it. The ruling widens the FIELD rather + // than narrowing the window - narrowing would change what an application may bind - and the + // eight bytes are paid by a record that fires at most once per class per change, in a push + // build only. The ABI fingerprint already moved this phase for opcode 80. + static_assert(sizeof(MGPShaderBuffers) == 40); + static_assert(sizeof(MGPShaderBuffers::WritableMask) * 8u >= kMGPipeMaxBufferBindingPoints, + "the mask must cover the window the same record declares"); EXPECT_LT(sizeof(MGPBufferRange), sizeof(MGHostSpan)); // The call still declares the span it may carry, so the transport lays the tail out. @@ -1114,6 +1136,21 @@ TEST(PipeCatalogue, BufferRangeCarriesNoInlineHostSpan) { b.HostSpanCount = 4; EXPECT_FALSE(MGPipeVerify(a, b, &field)); EXPECT_STREQ(field, "HostSpanCount"); + + // P5e (sb, ID-104): THE COMPARATOR SEES EVERY WORD OF THE WIDENED MASK, and the HIGH one + // is what the case is for. PipeFields.def names WritableMask once and the array overload + // of MGPipeFieldEqual walks it, so this would have passed before the widening too - on the + // 32 points that existed. Point 83 is the one the old field could not describe at all. + b = a; + ASSERT_TRUE(MGPipeVerify(a, b, &field)); + MGPipeShaderBufferMaskSet(b.WritableMask, kMGPipeMaxBufferBindingPoints - 1); + EXPECT_TRUE(MGPipeShaderBufferMaskHas(b.WritableMask, kMGPipeMaxBufferBindingPoints - 1)); + EXPECT_FALSE(MGPipeShaderBufferMaskHas(a.WritableMask, kMGPipeMaxBufferBindingPoints - 1)); + EXPECT_FALSE(MGPipeVerify(a, b, &field)); + EXPECT_STREQ(field, "WritableMask"); + // And out of range is "not writable" / "write nothing" rather than a word past the end. + MGPipeShaderBufferMaskSet(b.WritableMask, kMGPipeMaxBufferBindingPoints); + EXPECT_FALSE(MGPipeShaderBufferMaskHas(b.WritableMask, kMGPipeMaxBufferBindingPoints)); } // The buffer half of resource_subdata has no level and no box of its own: [offset, size) diff --git a/MobileGL/MG_Test/Pipe/ShaderBufferEmitTest.cpp b/MobileGL/MG_Test/Pipe/ShaderBufferEmitTest.cpp new file mode 100644 index 00000000..afa6132c --- /dev/null +++ b/MobileGL/MG_Test/Pipe/ShaderBufferEmitTest.cpp @@ -0,0 +1,508 @@ +// MobileGL - MobileGL/MG_Test/Pipe/ShaderBufferEmitTest.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 + +// P5e's set_shader_buffers - the INDEXED BUFFER BINDING POINTS - on both sides of the call +// (MG_Remote/CONTRACT-P5E.md §5.6, rulings 10 and 11; package sb). +// +// WHAT THIS SUITE IS FOR. Three properties of this family are invisible in a picture and are +// exactly what an optimisation or a merge deletes: +// 1. the ZERO EARLY-OUT - a class nothing has bound emits nothing, before any hash; +// 2. a BASE binding travels as kMGPipeWholeBuffer and never as a resolved extent, so a +// glBufferData between the emission and the apply binds the NEW extent as GL does; +// 3. THREE SUPPRESSOR SLOTS, one per class, so an emission of one class never cancels +// another's (ruling 11) - a single slot would make the shader-storage set disappear +// whenever a uniform set happened to hash the same way. +// Plus ID-104's widened WritableMask, whose whole point is the points a Uint32 could not +// describe: the case that matters is point 83, not point 3. +// +// THE SUITE IS `ShaderBufferEmit`, not `ShaderBufferEmitTest`: the file is XTest.cpp and the +// suite is X, this directory's convention. +// +// IT HAS ITS OWN main() for VertexInputEmitTest's reason: the applier refuses a malformed +// window, and that verdict is a log line in a shipped push build and std::abort() in a poison +// or verify one. Every case is a visible SKIP in a pull build rather than a vanishing test, so +// `ctest -N` stays name-for-name identical between the pull and the push trees (G2/G14). + +#include + +#include +#include +#include +#include +#include + +#if defined(_WIN32) +#include +#define MGTEST_HAVE_FORK 0 +#else +#include +#include +#include +#define MGTEST_HAVE_FORK 1 +#endif + +#include "Includes.h" +#include +#if MOBILEGL_PIPE_PUSH +#include "Init.h" +#include +// MOBILEGL_PIPE_POISON is DERIVED in the header below (PipeInputs.h:20-26) and nowhere else, +// so a TU that tests it without this include silently reads it as 0 - and then +// ExpectRefusedNaming takes the in-process arm while the applier's trip wire really does +// abort, which takes the whole binary down with it. ImageEmitTest carries the same include for +// the same reason and learned it the same way. +#include +#include +#include +#include +#include +#include +#include +#include +#include +#endif + +using namespace MobileGL; +using namespace MobileGL::MG_Pipe; + +namespace { + String g_logPath; + + int ProcessId() { +#if defined(_WIN32) + return _getpid(); +#else + return static_cast(getpid()); +#endif + } +} // namespace + +#if !MOBILEGL_PIPE_PUSH +namespace { + // G2/G14: the pull build gets the same ctest names, skipping. +#define MGL_SHADER_BUFFER_EMIT_TEST_LIST(X) \ + X(ShaderBufferEmit, TheEmitterIsOneNeverDestroyedProcessSingleton) \ + X(ShaderBufferEmit, AnUntouchedClassEmitsNothingWithoutHashing) \ + X(ShaderBufferEmit, ABaseBindingTravelsAsWholeBufferAndARangeBindingTravelsResolved) \ + X(ShaderBufferEmit, OnlyTheWritableClassesSetAMaskAndItReachesTheLastPoint) \ + X(ShaderBufferEmit, EachClassLatchesItsOwnSuppressorSlot) \ + X(ShaderBufferEmit, TheAppliedWindowIsPerClassAndTheSerialMovesOnEveryRecord) \ + X(ShaderBufferEmit, AWindowPastTheCapacityOrAnUnknownClassIsRefusedRatherThanStored) \ + X(ShaderBufferEmit, AResetApplierCarriesNoBindingPointStateOverAndTheSerialAdvances) + +#define MGL_DECLARE_PULL_SKIP(Suite, Name) \ + TEST(Suite, Name) { GTEST_SKIP() << "compiled only under MOBILEGL_PIPE_PUSH"; } + MGL_SHADER_BUFFER_EMIT_TEST_LIST(MGL_DECLARE_PULL_SKIP) +#undef MGL_DECLARE_PULL_SKIP +} // namespace +#else + +namespace { + namespace GL = MobileGL::MG_Impl::GLImpl; + using GLContext = MG_State::GLState::GLContext; + + // Each case starts from an emitter with no latch, an applier with no window and a + // suppressor that has never seen a hash, because every property below is about what the + // NEXT emission does. + struct EmitterScope { + EmitterScope() { Clear(); } + ~EmitterScope() { Clear(); } + EmitterScope(const EmitterScope&) = delete; + EmitterScope& operator=(const EmitterScope&) = delete; + + static void Clear() { + MGPipeInstallMonolithTables(); // idempotent; the route has to reach the applier + MGPipeShaderBufferEmitterInstance().Reset(); + MGPipeShaderBufferEmitterInstance().ResetCounters(); + MGPipeSetHashSuppressorInstance().InvalidateAll(); + for (Uint32 cls = 0; cls < kMGPipeShaderBufferClassCount; ++cls) { + MGPipeApplier().BoundShaderBuffers[cls] = {}; + MGPipeApplier().ShaderBufferStart[cls] = 0; + MGPipeApplier().ShaderBufferCount[cls] = 0; + for (Uint32 w = 0; w < kMGPipeShaderBufferWritableMaskWords; ++w) { + MGPipeApplier().ShaderBufferWritableMask[cls][w] = 0; + } + } + } + }; + + GLContext& Ctx() { return *MG_State::pGLContext; } + MGPipeShaderBufferEmitter& Emitter() { return MGPipeShaderBufferEmitterInstance(); } + + // ---- driving a refusal, in ImageEmitTest's shape and for its reason ------------------ + // + // The applier's trip wires report through a log line in a shipped push build and + // std::abort() in a poison or verify one - and MOBILEGL_PIPE_POISON is DERIVED from + // MOBILEGL_BUILD_DISAGGREGATED behind PipeInputs.h, so the split build this campaign gates + // on is the aborting arm. A case that drove one in process would therefore take the whole + // binary down, which is why the drive is a forked child there and the parent reads SIGABRT + // plus the line the wire appended to the log. + std::string ReadLog(std::streamoff from = 0) { + std::ifstream in(g_logPath, std::ios::binary); + if (from > 0) in.seekg(from, std::ios::beg); + std::ostringstream ss; + ss << in.rdbuf(); + return ss.str(); + } + + std::streamoff LogEnd() { + std::ifstream in(g_logPath, std::ios::binary | std::ios::ate); + return in ? static_cast(in.tellg()) : std::streamoff{0}; + } + +#if MGTEST_HAVE_FORK + struct ChildResult { + int Status = -1; + std::string Log; + }; + + template + ChildResult RunInChild(Body body) { + ChildResult result; + // The log path is NOT unlinked: main() has already opened it, fork() duplicates the + // handle, and removing it would leave the child writing into a deleted inode while the + // parent read an empty file. Only what the child appended is this drive's evidence. + const std::streamoff before = LogEnd(); + std::fflush(nullptr); + const pid_t pid = ::fork(); + if (pid < 0) return result; + if (pid == 0) { + body(); + ::_exit(0); + } + int status = 0; + if (::waitpid(pid, &status, 0) != pid) return result; + result.Status = status; + result.Log = ReadLog(before); + return result; + } + + Bool DiedOfAbort(const ChildResult& r) { + return WIFSIGNALED(r.Status) && WTERMSIG(r.Status) == SIGABRT; + } + std::string DescribeStatus(const ChildResult& r) { + if (r.Status < 0) return "fork/waitpid failed"; + if (WIFEXITED(r.Status)) return "exited " + std::to_string(WEXITSTATUS(r.Status)); + if (WIFSIGNALED(r.Status)) return "signal " + std::to_string(WTERMSIG(r.Status)); + return "status " + std::to_string(r.Status); + } +#endif // MGTEST_HAVE_FORK + + template + void ExpectRefusedNaming(const char* needle, Body body) { +#if MOBILEGL_PIPE_POISON || MOBILEGL_PIPE_VERIFY +#if MGTEST_HAVE_FORK + const std::string tagged = std::string("Fatal{ProtocolCorruption} ") + needle; + const ChildResult child = RunInChild(body); + EXPECT_TRUE(DiedOfAbort(child)) << DescribeStatus(child) << "; log: " << child.Log; + EXPECT_NE(child.Log.find(tagged), std::string::npos) + << "the gate fired without naming what it refused; wanted \"" << tagged + << "\"; log: " << child.Log; +#else + (void)needle; + (void)body; // no fork on this platform; the verdict here is std::abort() +#endif +#else + const std::string tagged = std::string("ProtocolCorruption ") + needle; + const std::string before = ReadLog(); + body(); + EXPECT_NE(ReadLog().substr(before.size()).find(tagged), std::string::npos) + << "the gate refused without saying what it refused; wanted \"" << tagged << "\""; +#endif + } + + GLuint MakeBuffer(GLenum target, GLsizeiptr size) { + GLuint name = 0; + GL::GenBuffers(1, &name); + GL::BindBuffer(target, name); + GL::BufferData(target, size, nullptr, GL_DYNAMIC_DRAW); + return name; + } + + // The frontend keeps the touched high-water mark per target for the life of the context and + // only ever raises it, so a case that binds point 83 raises it for every case after it in + // the same process. Each case therefore states the window it expects rather than assuming + // it is the number of points it just bound. + Uint32 TouchedWindow(BufferTarget target) { + return static_cast(Ctx().GetTouchedBufferBindingPointCount(target)); + } + + TEST(ShaderBufferEmit, TheEmitterIsOneNeverDestroyedProcessSingleton) { + EXPECT_EQ(&MGPipeShaderBufferEmitterInstance(), &MGPipeShaderBufferEmitterInstance()); + } + + // PROPERTY 1, and it is what makes this whole family cost one integer read per validate + // point on a workload that binds no indexed buffer - which is every Minecraft frame: + // MC's touched SSBO and atomic-counter counts are both 0. + TEST(ShaderBufferEmit, AnUntouchedClassEmitsNothingWithoutHashing) { + EmitterScope scope; + ASSERT_EQ(TouchedWindow(BufferTarget::AtomicCounter), 0u) + << "this case needs a class nothing in this process has ever bound"; + EXPECT_EQ(Emitter().EmitShaderBuffers(Ctx()), 0u); + EXPECT_EQ(Emitter().EmissionCount(kMGPipeShaderBufferClassAtomicCounter), 0u); + EXPECT_EQ(MGPipeApplier().ShaderBufferCount[kMGPipeShaderBufferClassAtomicCounter], 0u); + } + + // PROPERTY 2, the one that is silent under run-ahead: glBindBufferBase does not freeze an + // extent - GL resolves it against the object's size at every USE - so resolving it at emit + // would make a glBufferData issued between the emission and the apply bind the OLD size, + // with the right handle and no diagnostic anywhere. glBindBufferRange DOES pin a window, + // and that one travels resolved. + TEST(ShaderBufferEmit, ABaseBindingTravelsAsWholeBufferAndARangeBindingTravelsResolved) { + EmitterScope scope; + GLint alignment = 0; + GL::GetIntegerv(GL_UNIFORM_BUFFER_OFFSET_ALIGNMENT, &alignment); + if (alignment <= 0) alignment = 256; + const GLuint whole = MakeBuffer(GL_UNIFORM_BUFFER, 1024); + const GLuint ranged = MakeBuffer(GL_UNIFORM_BUFFER, 1024); + GL::BindBufferBase(GL_UNIFORM_BUFFER, 0, whole); + GL::BindBufferRange(GL_UNIFORM_BUFFER, 1, ranged, alignment, 256); + + ASSERT_NE(Emitter().EmitConstBuffers(Ctx()), 0u) << "the uniform window did not emit"; + const MGPShaderBuffers& header = Emitter().LastHeader(kMGPipeShaderBufferClassUniform); + EXPECT_EQ(header.Class, kMGPipeShaderBufferClassUniform); + EXPECT_EQ(header.Start, 0u); + EXPECT_EQ(header.Count, TouchedWindow(BufferTarget::Uniform)); + EXPECT_EQ(header.HostSpanCount, 0u) + << "kCapNeedsHostUboBytes is 0 for the whole of P5, so Espryt ships no second tail"; + + const auto& ranges = Emitter().LastRanges(); + EXPECT_EQ(ranges[0].Offset, 0u); + EXPECT_EQ(ranges[0].Size, kMGPipeWholeBuffer) + << "a base binding must let the SERVER re-resolve the extent at use"; + EXPECT_FALSE(MGPipeHandleIsNull(ranges[0].Res)); + EXPECT_EQ(ranges[1].Offset, static_cast(alignment)); + EXPECT_EQ(ranges[1].Size, 256u); + EXPECT_NE(ranges[0].Res.Slot, ranges[1].Res.Slot) << "two buffers, two handles"; + + // And the record reached the applier through the monolith adapter this route installs. + EXPECT_EQ(MGPipeApplier().ShaderBufferCount[kMGPipeShaderBufferClassUniform], header.Count); + EXPECT_EQ(MGPipeApplier().BoundShaderBuffers[kMGPipeShaderBufferClassUniform][0].Size, + kMGPipeWholeBuffer); + } + + // ID-104's case. The mask is what replaces the backend's own GPU-write walk, and until this + // phase it was a single Uint32 against an 84-point window - so a storage buffer bound at + // point 32 or above was unwritable as far as the record said, with nothing able to see it. + // + // THE EMITTER HALF BINDS THE HIGHEST POINT THE TARGET ALLOWS rather than a literal 83: the + // frontend's array is 84 deep, but glBindBufferBase(GL_SHADER_STORAGE_BUFFER, ...) is + // bounded by GL_MAX_SHADER_STORAGE_BUFFER_BINDINGS, which is a DEVICE number and is exactly + // what ruling 10 says the client must not pre-clamp the WIRE to. The wire-field half - that + // the mask reaches point 83 at all - is driven through the applier below, where the window + // is 84 by construction, and pinned again in PipeCatalogueTest. + TEST(ShaderBufferEmit, OnlyTheWritableClassesSetAMaskAndItReachesTheLastPoint) { + EmitterScope scope; + GLint storageBindings = 0; + GL::GetIntegerv(GL_MAX_SHADER_STORAGE_BUFFER_BINDINGS, &storageBindings); + ASSERT_GT(storageBindings, 0); + const Uint32 top = static_cast(storageBindings) - 1u; + const GLuint storage = MakeBuffer(GL_SHADER_STORAGE_BUFFER, 512); + const GLuint uniform = MakeBuffer(GL_UNIFORM_BUFFER, 512); + GL::BindBufferBase(GL_SHADER_STORAGE_BUFFER, static_cast(top), storage); + GL::BindBufferBase(GL_UNIFORM_BUFFER, 0, uniform); + + ASSERT_NE(Emitter().EmitShaderBuffers(Ctx()), 0u); + const MGPShaderBuffers& storageHeader = + Emitter().LastHeader(kMGPipeShaderBufferClassShaderStorage); + ASSERT_EQ(storageHeader.Count, top + 1u) + << "the window is the touched high-water mark, so binding the top point describes " + "every point below it too"; + EXPECT_TRUE(MGPipeShaderBufferMaskHas(storageHeader.WritableMask, top)) + << "the shader may write through binding point " << top << " and the mask must say so"; + EXPECT_FALSE(MGPipeShaderBufferMaskHas(storageHeader.WritableMask, 0u)) + << "nothing is bound at storage point 0, so nothing is writable there"; + + ASSERT_NE(Emitter().EmitConstBuffers(Ctx()), 0u); + const MGPShaderBuffers& uniformHeader = + Emitter().LastHeader(kMGPipeShaderBufferClassUniform); + for (Uint32 w = 0; w < kMGPipeShaderBufferWritableMaskWords; ++w) { + EXPECT_EQ(uniformHeader.WritableMask[w], 0u) + << "a UBO is read-only to the shader by definition (word " << w << ")"; + } + // And the applier carries the mask by class, not by record. + EXPECT_TRUE(MGPipeShaderBufferMaskHas( + MGPipeApplier().ShaderBufferWritableMask[kMGPipeShaderBufferClassShaderStorage], top)); + EXPECT_FALSE(MGPipeShaderBufferMaskHas( + MGPipeApplier().ShaderBufferWritableMask[kMGPipeShaderBufferClassUniform], top)); + + // THE WIRE FIELD's OWN HALF (ID-104): a mask bit at the LAST point the window may + // describe survives the record and the applier. A Uint32 field could not have carried + // it at all - it would have been silently dropped, and the server would have read the + // point as read-only while the shader wrote to it. + const Uint32 last = kMGPipeMaxBufferBindingPoints - 1; + MGPBufferRange range{}; + range.Res = MGPipeHandle{41, 1}; + range.Size = kMGPipeWholeBuffer; + MGPShaderBuffers forged{}; + forged.Class = kMGPipeShaderBufferClassAtomicCounter; + forged.Start = last; + forged.Count = 1; + MGPipeShaderBufferMaskSet(forged.WritableMask, last); + MGPipeApplySetShaderBuffers(forged, &range); + EXPECT_TRUE(MGPipeShaderBufferMaskHas( + MGPipeApplier().ShaderBufferWritableMask[kMGPipeShaderBufferClassAtomicCounter], last)) + << "the widened mask must describe the whole 84-point window the record declares"; + } + + // RULING 11. One slot for the call would make each class's emission cancel the previous + // class's latch: the storage set would go out, the uniform set would go out behind it, and + // then a storage set that had NOT changed would go out again - or, worse on the other side + // of the coin, an unchanged uniform set would be re-sent and a changed storage set would + // not, because the slot held the other class's hash. + TEST(ShaderBufferEmit, EachClassLatchesItsOwnSuppressorSlot) { + EmitterScope scope; + const GLuint storage = MakeBuffer(GL_SHADER_STORAGE_BUFFER, 512); + const GLuint uniform = MakeBuffer(GL_UNIFORM_BUFFER, 512); + GL::BindBufferBase(GL_SHADER_STORAGE_BUFFER, 2, storage); + GL::BindBufferBase(GL_UNIFORM_BUFFER, 2, uniform); + + ASSERT_NE(Emitter().EmitConstBuffers(Ctx()), 0u); + ASSERT_NE(Emitter().EmitShaderBuffers(Ctx()), 0u); + const Uint64 uniformEmissions = Emitter().EmissionCount(kMGPipeShaderBufferClassUniform); + const Uint64 storageEmissions = + Emitter().EmissionCount(kMGPipeShaderBufferClassShaderStorage); + + // Nothing moved: both classes must be suppressed, and each on its OWN latch. + EXPECT_EQ(Emitter().EmitConstBuffers(Ctx()), 0u); + EXPECT_EQ(Emitter().EmitShaderBuffers(Ctx()), 0u); + EXPECT_EQ(Emitter().EmissionCount(kMGPipeShaderBufferClassUniform), uniformEmissions); + EXPECT_EQ(Emitter().EmissionCount(kMGPipeShaderBufferClassShaderStorage), storageEmissions); + + // ONE class moves. The other must stay suppressed - with a shared slot it would be + // re-sent, and the case would pass for the wrong reason if it only asserted the mover. + GL::BindBufferBase(GL_SHADER_STORAGE_BUFFER, 2, 0); + EXPECT_NE(Emitter().EmitShaderBuffers(Ctx()), 0u); + EXPECT_EQ(Emitter().EmissionCount(kMGPipeShaderBufferClassShaderStorage), + storageEmissions + 1u); + EXPECT_EQ(Emitter().EmitConstBuffers(Ctx()), 0u) + << "the uniform class did not move and must not be re-sent on the storage class's " + "account"; + EXPECT_EQ(Emitter().EmissionCount(kMGPipeShaderBufferClassUniform), uniformEmissions); + } + + // The applier's half, driven directly so the window arithmetic is stated rather than + // inferred from an emission: three classes, three windows, ONE serial. + TEST(ShaderBufferEmit, TheAppliedWindowIsPerClassAndTheSerialMovesOnEveryRecord) { + EmitterScope scope; + MGPBufferRange ranges[2]{}; + ranges[0].Res = MGPipeHandle{11, 1}; + ranges[0].Size = kMGPipeWholeBuffer; + ranges[1].Res = MGPipeHandle{12, 1}; + ranges[1].Offset = 64; + ranges[1].Size = 128; + + const Uint64 before = MGPipeApplier().ShaderBuffersSerial; + MGPShaderBuffers header{}; + header.Class = kMGPipeShaderBufferClassShaderStorage; + header.Start = 0; + header.Count = 2; + MGPipeShaderBufferMaskSet(header.WritableMask, 1); + MGPipeApplySetShaderBuffers(header, ranges); + + EXPECT_EQ(MGPipeApplier().ShaderBufferCount[kMGPipeShaderBufferClassShaderStorage], 2u); + EXPECT_EQ(MGPipeApplier().BoundShaderBuffers[kMGPipeShaderBufferClassShaderStorage][1].Offset, + 64u); + EXPECT_EQ(MGPipeApplier().ShaderBuffersSerial, before + 1u); + // THE OTHER TWO CLASSES ARE UNTOUCHED, which is the whole reason Class is a field: one + // record describes one array and says nothing about the other two. + EXPECT_EQ(MGPipeApplier().ShaderBufferCount[kMGPipeShaderBufferClassUniform], 0u); + EXPECT_EQ(MGPipeApplier().ShaderBufferCount[kMGPipeShaderBufferClassAtomicCounter], 0u); + + header.Class = kMGPipeShaderBufferClassUniform; + header.Count = 1; + MGPipeApplySetShaderBuffers(header, ranges); + EXPECT_EQ(MGPipeApplier().ShaderBufferCount[kMGPipeShaderBufferClassUniform], 1u); + EXPECT_EQ(MGPipeApplier().ShaderBufferCount[kMGPipeShaderBufferClassShaderStorage], 2u) + << "the storage window survived a uniform record"; + EXPECT_EQ(MGPipeApplier().ShaderBuffersSerial, before + 2u) + << "ONE serial for all three classes, and it moves on every applied record"; + } + + // The two refusals the contract names by string (§1), and they are asserted BY NAME: a + // trip wire that fired without saying which field it refused would leave a reader of the + // lane guessing between a class nobody allocated and a window nobody can hold. + TEST(ShaderBufferEmit, AWindowPastTheCapacityOrAnUnknownClassIsRefusedRatherThanStored) { + EmitterScope scope; + ExpectRefusedNaming("SetShaderBuffers.Count", [] { + MGPBufferRange range{}; + range.Res = MGPipeHandle{21, 1}; + MGPShaderBuffers header{}; + header.Class = kMGPipeShaderBufferClassAtomicCounter; + header.Start = kMGPipeMaxBufferBindingPoints - 1; + header.Count = 4; // Start + Count = 87 > 84 + const Uint64 before = MGPipeApplier().ShaderBuffersSerial; + MGPipeApplySetShaderBuffers(header, &range); + // Only reached on the non-aborting arm, and the point of reaching it is that a + // refusal which had stored half a window would be worse than an abort. + EXPECT_EQ(MGPipeApplier().ShaderBufferCount[kMGPipeShaderBufferClassAtomicCounter], 0u); + EXPECT_EQ(MGPipeApplier().ShaderBuffersSerial, before) + << "a refused record must not move the serial a twin memoises on"; + }); + ExpectRefusedNaming("SetShaderBuffers.Class", [] { + MGPBufferRange range{}; + range.Res = MGPipeHandle{22, 1}; + MGPShaderBuffers header{}; + header.Class = kMGPipeShaderBufferClassCount; // one past the last class + header.Start = 0; + header.Count = 1; + const Uint64 before = MGPipeApplier().ShaderBuffersSerial; + MGPipeApplySetShaderBuffers(header, &range); + EXPECT_EQ(MGPipeApplier().ShaderBuffersSerial, before); + }); + } + + // A make-current clears the three windows and ADVANCES the serial, for the reason + // VertexBuffersSerial's own comment gives: a counter that restarts walks back through every + // value it has already stamped into a backend memo that outlived the switch. + TEST(ShaderBufferEmit, AResetApplierCarriesNoBindingPointStateOverAndTheSerialAdvances) { + EmitterScope scope; + MGPipeApplierState& applier = MGPipeApplier(); + applier.ShaderBufferStart[kMGPipeShaderBufferClassUniform] = 0; + applier.ShaderBufferCount[kMGPipeShaderBufferClassUniform] = 7; + applier.BoundShaderBuffers[kMGPipeShaderBufferClassUniform][0].Res = MGPipeHandle{31, 1}; + MGPipeShaderBufferMaskSet( + applier.ShaderBufferWritableMask[kMGPipeShaderBufferClassShaderStorage], + kMGPipeMaxBufferBindingPoints - 1); + applier.ShaderBuffersSerial = 99; + + MGPipeApplierReset(); + + EXPECT_EQ(MGPipeApplier().ShaderBufferCount[kMGPipeShaderBufferClassUniform], 0u); + EXPECT_TRUE(MGPipeHandleIsNull( + MGPipeApplier().BoundShaderBuffers[kMGPipeShaderBufferClassUniform][0].Res)); + EXPECT_FALSE(MGPipeShaderBufferMaskHas( + MGPipeApplier().ShaderBufferWritableMask[kMGPipeShaderBufferClassShaderStorage], + kMGPipeMaxBufferBindingPoints - 1)); + EXPECT_EQ(MGPipeApplier().ShaderBuffersSerial, 100u); + EXPECT_GT(MGPipeApplier().ShaderBuffersSerial, 99u) + << "ADVANCED, never zeroed: no value this counter has handed a twin may come back"; + } +} // namespace +#endif // MOBILEGL_PIPE_PUSH + +int main(int argc, char** argv) { + namespace fs = std::filesystem; + const fs::path path = fs::temp_directory_path() / + ("mobilegl-shaderbufferemit-test-" + std::to_string(ProcessId()) + ".log"); + std::error_code ec; + fs::remove(path, ec); + g_logPath = path.string(); +#if defined(_WIN32) + _putenv_s("MOBILEGL_LOG_FILE_PATH", g_logPath.c_str()); +#else + setenv("MOBILEGL_LOG_FILE_PATH", g_logPath.c_str(), 1); +#endif +#if MOBILEGL_PIPE_PUSH + MobileGL::Initialize(); +#endif + ::testing::InitGoogleTest(&argc, argv); + const int rc = RUN_ALL_TESTS(); + fs::remove(path, ec); + return rc; +} diff --git a/MobileGL/MG_Test/Pipe/TrackerTest.cpp b/MobileGL/MG_Test/Pipe/TrackerTest.cpp index 9711e0cb..3d0184dc 100644 --- a/MobileGL/MG_Test/Pipe/TrackerTest.cpp +++ b/MobileGL/MG_Test/Pipe/TrackerTest.cpp @@ -19,6 +19,9 @@ #if MOBILEGL_PIPE_PUSH #include +// P5e (sb): the binding-point shutter cases drive the REAL glBindBufferBase entry point, +// because the claim they make is about what a frontend mutator publishes. +#include #include #include #include @@ -85,6 +88,8 @@ namespace { X(TrackerWalk, AProgramSwitchAloneFiresTheSamplerViewBit) \ X(TrackerWalk, ATextureParameterAloneFiresTheSamplerViewBit) \ X(TrackerWalk, AProgramSwitchBetweenEqualImageUnitCountersFiresTheShaderImageBit) \ + X(TrackerWalk, ARebindOfOneUniformPointToADifferentBufferFiresTheConstBufferBit) \ + X(TrackerWalk, TheBindingPointBitsDoNotFireOnAnUnrelatedBufferWrite) \ X(TrackerAttribPayload, AFloatWriteCarriesTheFloatBitsAndNamesItsClass) \ X(TrackerAttribPayload, AnIntWriteCarriesTheIntWordsAndNamesItsClass) \ X(TrackerAttribPayload, AUintWriteCarriesTheUintWordsAndNamesItsClass) \ @@ -579,6 +584,76 @@ namespace { // // THIS IS THE ONE CASE THE OLD SHUTTER COULD NOT PASS, which is why it is here rather // than in the narrowing commit's prose. + // ---- P5e (sb): the indexed binding points, bits 15/16/17 ------------------------- + // + // THE RED-ONCE FOR THE SHUTTER REWRITE (CONTRACT-P5E.md §5.6, brief red-once (b)). Until + // P5e all three bits shuttered on the buffer CONTENT aggregate, and glBindBufferBase moves + // a binding point through a returned reference - BindingSlotRange1D::Bind, which advances + // the slot's own Uint16 version - so this sequence fired NOTHING. Harmless while nothing + // was emitted for the bits; an under-fire the moment set_shader_buffers is, because the + // server would go on binding the first buffer. Same defect class as P4a's glBindSampler + // hole, closed the same way: the shutter reads the generation the mutator moves. + // + // The action under test is the BIND ITSELF, through the real GL entry point, because the + // claim is about what a frontend mutator publishes and not about what a generation does + // once bumped (ID-102). + TEST_F(TrackerWalk, ARebindOfOneUniformPointToADifferentBufferFiresTheConstBufferBit) { + GLuint a = 0; + GLuint b = 0; + MG_Impl::GLImpl::GenBuffers(1, &a); + MG_Impl::GLImpl::GenBuffers(1, &b); + + MG_Impl::GLImpl::BindBufferBase(GL_UNIFORM_BUFFER, 1, a); + Walk(); + Walk(); + ASSERT_EQ(m_tracker.LastDirty() & MGPipeDirtyBit(MGPipeDirty::NewConstBuffers), 0u) + << "the steady state must be quiet before the interesting half of this case"; + + // The same point, a different buffer. Nothing about any buffer's CONTENTS moved. + MG_Impl::GLImpl::BindBufferBase(GL_UNIFORM_BUFFER, 1, b); + Walk(); + EXPECT_NE(m_tracker.LastDirty() & MGPipeDirtyBit(MGPipeDirty::NewConstBuffers), 0u) + << "NEW_CONST_BUFFERS did not fire when a uniform binding point moved onto another " + "buffer - the emitted window would still name the first one"; + // And the unbind, which is the case a shutter may least afford to miss: it leaves the + // high-water mark where it was, so nothing but the generation can carry it. + Walk(); + ASSERT_EQ(m_tracker.LastDirty() & MGPipeDirtyBit(MGPipeDirty::NewConstBuffers), 0u); + MG_Impl::GLImpl::BindBufferBase(GL_UNIFORM_BUFFER, 1, 0); + Walk(); + EXPECT_NE(m_tracker.LastDirty() & MGPipeDirtyBit(MGPipeDirty::NewConstBuffers), 0u) + << "NEW_CONST_BUFFERS did not fire for an unbind"; + m_cache.Reset(); + } + + // The other half of the rewrite, and the reason it is a narrowing rather than a swap: the + // content aggregate LEFT all three bits, so a glBufferSubData into an unrelated buffer no + // longer republishes every binding-point set in the context. Whether the BYTES behind a + // bound buffer moved is the resource family's question, answered server-side by the + // resource record's own Serial; what these records carry is {handle, offset, size}. + TEST_F(TrackerWalk, TheBindingPointBitsDoNotFireOnAnUnrelatedBufferWrite) { + GLuint bound = 0; + MG_Impl::GLImpl::GenBuffers(1, &bound); + MG_Impl::GLImpl::BindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, bound); + Walk(); + Walk(); + const Uint32 family = MGPipeDirtyBit(MGPipeDirty::NewConstBuffers) | + MGPipeDirtyBit(MGPipeDirty::NewShaderBuffers) | + MGPipeDirtyBit(MGPipeDirty::NewSoTargets); + ASSERT_EQ(m_tracker.LastDirty() & family, 0u) + << "the steady state must be quiet before the interesting half of this case"; + + const SharedPtr unrelated = Ctx().CreateBufferObject(64); + unrelated->Respecify(4096, nullptr); + Array bytes{}; + unrelated->UploadSubData(DataPtr{bytes.data(), bytes.size()}, 0); + ASSERT_NE(Ctx().GetAnyBufferChangeGeneration(), 0u) << "the buffer aggregate did move"; + Walk(); + EXPECT_EQ(m_tracker.LastDirty() & family, 0u) + << "a write to a buffer bound to no binding point republished the binding-point sets"; + m_cache.Reset(); + } + TEST_F(TrackerWalk, TheIndexBufferBitDoesNotFireOnAnUnrelatedBufferWrite) { const SharedPtr indices = Ctx().CreateBufferObject(1); indices->Respecify(64, nullptr); diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index 47ca3189..6aae7665 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -756,9 +756,11 @@ TEST_F(PipeWireCodecTest, KHostSpanClassIsValidatedEvenThoughP5ProducesNone) { kInvalidSeq); bool applied = false; ASSERT_TRUE(wire.PumpOne(&applied)); - // No applier entry point exists and the call is off the reduced path, so the honest answer - // is "this build does not implement it" - after the tails have been checked. - EXPECT_FALSE(applied); + // P5e (sb): the applier entry point EXISTS now, so the record is applied after the tails + // have been checked. The case's own subject is unchanged and is what its name says - the + // kHostSpan class is validated whether or not P5 ever produces one - and the flip is the + // proof that the honesty pass runs IN FRONT of the apply rather than instead of it. + EXPECT_TRUE(applied); } // ===================================================================================== @@ -877,7 +879,9 @@ TEST_F(PipeWireCodecTest, SetShaderBuffersCarriesBothTailsWhenTheSpanTailIsPrese kInvalidSeq); bool applied = false; ASSERT_TRUE(wire.PumpOne(&applied)); - EXPECT_FALSE(applied); + // P5e (sb): applied, for the reason the sibling case above states. BOTH tails still cross + // and both are still laid out by MGPipeWireRecordLayout, which is what this case is about. + EXPECT_TRUE(applied); } TEST_F(PipeWireCodecTest, ResourceSubDataCarriesABlobAndARegionTailTogether) { diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index fc6699ea..e99cc8ab 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -1124,6 +1124,10 @@ TEST(PipeRouting, TheInstalledClientArmIsWireAndNotMonolithAndEveryRoutedRowMove C1F_MOVED(Context, SetSamplerViews); C1F_MOVED(Context, BindSamplerStates); C1F_MOVED(Context, SetShaderImages); + // P5e (sb, CONTRACT-P5E.md §5.6): set_shader_buffers is the 35th routed row. It rides both + // tables like every other set_* with an applier entry point - the class is a FIELD of the + // payload, so all three binding-point classes go through this one cell. + C1F_MOVED(Context, SetShaderBuffers); C1F_MOVED(Context, SetGlobalConstants); C1F_MOVED(Context, SetVertexAttribDefaults); C1F_MOVED(Context, SetPixelPackState); @@ -1146,9 +1150,9 @@ TEST(PipeRouting, TheInstalledClientArmIsWireAndNotMonolithAndEveryRoutedRowMove C1F_ESCAPE(CreateShaderState); #undef C1F_ESCAPE const SizeT movedContext = CountDifferingCells(gMGPipeContext, MGPipeMonolithContext()); - EXPECT_EQ(movedScreen + movedContext, 34u) - << "exactly the 34 generated routed rows must differ from the monolith adapters " - "(33 at P5, + set_context_values at P5c rv); " + EXPECT_EQ(movedScreen + movedContext, 35u) + << "exactly the 35 generated routed rows must differ from the monolith adapters " + "(33 at P5, + set_context_values at P5c rv, + set_shader_buffers at P5e sb); " << movedScreen + movedContext << " did, so a row was left on the monolith adapter (it would run the applier on the GL " "thread under split) or an unrouted row was overwritten"; diff --git a/scripts/gen_pipe_dirty_surface.py b/scripts/gen_pipe_dirty_surface.py index e1d0d6c9..96598747 100644 --- a/scripts/gen_pipe_dirty_surface.py +++ b/scripts/gen_pipe_dirty_surface.py @@ -84,8 +84,17 @@ SCAN_ROOTS = (os.path.join(REPO_ROOT, "MobileGL", "MG_Impl", "GLImpl"), # which is that they create or destroy objects rather than move a pushed field, and each # object class's creation and destruction is already answered by its own Mark*ForDeletion row # plus the constructor-time resource_create. +# P5e WIDENS IT BY ONE MORE WORD, `Touch`, and the hole it closes is the same coverage hole +# `Use` and `Bind` closed at P4a. `GLContext::TouchBufferBindingPoint` is the high-water mark +# every indexed-binding-point walk is bounded by - the backend's, the client's GPU-write sweep's +# and, since P5e, set_shader_buffers' emitted window - so a call that moves it moves the SIZE of +# a pushed record, and none of the three sites that call it was visible to this scan because +# none begins with one of the twelve words the pattern matched. The complete set the widening +# surfaces was enumerated by grep before the change landed: ONE name on two call sites, both in +# GL_Buffer.cpp's BindBufferBase_State / BindBufferRange_State, which already carry rows for +# their other mutators. MUTATOR_PREFIXES = ("Add", "Set", "Mark", "Bump", "Allocate", "Truncate", "Record", "Notify", - "Begin", "End", "Use", "Bind") + "Begin", "End", "Use", "Bind", "Touch") MUTATOR_RE = re.compile(r"pGLContext->\s*((?:%s)\w*)\s*\(" % "|".join(MUTATOR_PREFIXES)) # The SECOND publish mechanism (MG_Pipe/PipeMutation.h). It carries the FIELD, not a mutator @@ -1604,22 +1613,27 @@ def self_test(scanned, bits, publishers, movers, moved, outside=None, undecided_ tripped(any(p.startswith("MISSING publisher") for p in problems), "5 (a dropped render-state publisher)") - # 6. THE CONTROL FOR THE OBJECT-CLASS HALF, and it is again the shape of a defect that - # was actually in this file: X(SetNamedTransformFeedbackBinding, NEW_SO_TARGETS) - # named a shutter (the buffer-content aggregate mixed with the transform-feedback - # generation) that GLContext::SetNamedTransformFeedbackBinding moves on no path - it - # binds a BufferState binding point or writes a saved-bindings entry, and neither is - # a buffer CONTENT write or a BeginTransformFeedback. The row has to be RED - and - # the analysis reaches (by name, through `Bind(`) a body it cannot fully read, so the - # red it can honestly print is "UNDECIDED and unmarked", never "supported". + # 6. THE CONTROL FOR THE OBJECT-CLASS HALF, on a mutator the analysis CANNOT fully read, + # so the red it can honestly print is "UNDECIDED and unmarked" rather than "supported". + # The pair is dead in the plainest way available: glTransformFeedbackBufferBase writes + # a transform-feedback binding point or a named object's saved-bindings entry, and + # NEW_VERTEX_ELEMENTS' shutter reads the bound VAO's identity and attribute generation. + # + # IT USED TO NAME NEW_SO_TARGETS, because that pairing was a defect this file actually + # carried: bit 17 mixed the buffer-CONTENT aggregate, which no binding has ever moved. + # P5e (sb) gave the binding points a generation of their own and bit 17 now reads it, so + # the old pair is a LIVE answer (kPulledPartialShutter|NEW_SO_TARGETS, marked undecided + # for this same taint) and could no longer be the control - a negative control that has + # quietly become positive is the failure this whole self-test exists to make loud, and + # it was loud: this one stopped tripping in the commit that rewrote the row. with_dead_shutter = dict(real) - with_dead_shutter["SetNamedTransformFeedbackBinding"] = "NEW_SO_TARGETS" + with_dead_shutter["SetNamedTransformFeedbackBinding"] = "NEW_VERTEX_ELEMENTS" problems = check_mapping(with_dead_shutter, real_duplicates, scanned, bits, publishers, movers, moved, outside, real_marks) verdicts = derive_bit_answers(with_dead_shutter, bits, movers, moved, outside) - tripped(any(("NEW_SO_TARGETS for SetNamedTransformFeedbackBinding" in p + tripped(any(("NEW_VERTEX_ELEMENTS for SetNamedTransformFeedbackBinding" in p and p.startswith(("UNDER-FIRING", "UNDECIDED"))) for p in problems) - and verdicts[("SetNamedTransformFeedbackBinding", "NEW_SO_TARGETS")][0] != SUPPORTED, + and verdicts[("SetNamedTransformFeedbackBinding", "NEW_VERTEX_ELEMENTS")][0] != SUPPORTED, "6 (an object-class answer whose shutter the mutator never moves)") # 6b. the same family, on a mutator the analysis reads completely, so the red is the # verdict itself: a texture-bind bump moves nothing NEW_GLOBAL_CONSTANTS compares. From b004e5dcbbbde39f20d5457dde1faf1520e1226a Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 08:22:45 -0400 Subject: [PATCH 2/2] [MG_Backend] (Disaggregated): P5e sb - Espryt's four binding-point walks read the record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - SyncBufferBindingPointsByRecord(class, glTarget), file-local beside the walk it replaces: the window is the applier's ShaderBufferStart/Count, each entry resolves through EnsureBufferResourceForHandle(nullptr, Res), and whole-vs-range is `Offset == 0 && Size == kMGPipeWholeBuffer` - so obj->GetSize() leaves the function and the extent of a base binding is re-resolved HERE, against this server's own storage, which is what makes a glBufferData between the emission and the apply bind the new extent as GL does. The ES clamp stays exactly where it was: the device ceiling is a server question. - The arm is transport + bit 13, with bit 13 requiring bit 7 refused by name rather than half-run - ResolveVertexInputSubsystemArm's shape, and for its reason: without bit 7 every point would log once and `continue` WITHOUT unbinding. - SyncAtomicCounterBuffers reads the applier's window instead of GetBufferBindingPointCount's sticky forward; glBindings stays the backend program's own list. A binding outside the window takes the UNBIND arm rather than being skipped, because skipping would leave whatever the driver last had at that ESSL point. - The UBO loop's POINT half reads BoundShaderBuffers[Uniform][binding] in the same index space, with IsBufferDrawCleanByHandle / FindBufferResourceForHandle in place of the frontend probes. GetUniformBlockBinding above it is untouched - that line is package pg's. - MarkShaderStorageBuffersGpuWritten and SyncAtomicCounterBuffers' MarkBufferGpuWritten are DELETED under a transport. They walked client memory and wrote into a client object, which is the one thing that cannot survive run-ahead, and the client's own MarkGpuWritesForDraw has covered the same set since P5d. Withholding the client's two rows turns the atomic scenarios red (5 vs 13, 100 vs 116), which is what says the duplicate really is gone. - Every frontend arm is kept token for token under Transport=monolith (G1, ruling 1), as the `if (…) else` the indirect buffer already uses. --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 277 +++++++++++++++++- 1 file changed, 274 insertions(+), 3 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index ed19e4c3..ba9710d4 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -488,6 +488,111 @@ namespace MobileGL::MG_Backend::DirectGLES { // TODO: deletion for deleted objects namespace BufferImpl { +#if MOBILEGL_BUILD_DISAGGREGATED + // ---- P5e (sb, MG_Remote/CONTRACT-P5E.md §5.6): THE INDEXED BINDING POINTS BY RECORD -- + // + // WHICH ARM THIS SERVER TAKES, resolved once. Two conjuncts and both are the phase's + // standard shape (§5.8): a live TRANSPORT - the push build under Transport=monolith + // keeps its frontend arms token for token, which is what the verify comparator needs + // and what makes RUN_AHEAD=0 a pure wait-rule A/B on identical server code - AND the + // family bit, so an operator can put the whole binding-point family back on the + // frontend walk with one cleared bit. + // + // BIT 13 REQUIRES BIT 7, refused here rather than half-run, in + // ResolveVertexInputSubsystemArm's exact shape and for its exact reason: every + // MGPBufferRange::Res is resolved through the resource slot table and only bit 7 puts + // twins there, so the pair would produce a walk in which every point logged once and + // `continue`d WITHOUT unbinding - every draw then reading through whatever the driver + // last had at that point. + Bool ResolveBufferBindingSubsystemArm() { + if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return false; + const Bool bitSet = + (MG_Config::Features.PipePush & MG_Pipe::kMGPipeSubsystemBufferBindings) != 0; + const Bool resourcesBitSet = + (MG_Config::Features.PipePush & MG_Pipe::kMGPipeSubsystemResources) != 0; + if (bitSet && !resourcesBitSet) { + MGLOG_E("MGPipe: kMGPipeSubsystemBufferBindings (bit 13) is set but " + "kMGPipeSubsystemResources (bit 7) is clear; every MGPBufferRange::Res " + "resolves through the resource slot table, which only bit 7 populates - " + "REFUSING bit 13 and running the legacy binding-point walk. Set bit 7 as " + "well, or clear both"); + return false; + } + MGLOG_D("MGPipe: Espryt binding-point family runs the %s arm", + bitSet ? "record" : "legacy"); + return bitSet; + } + + // FILE-LOCAL AND INLINE-MEMOISED, for EsprytSlotTablesEnabled's reason: this is + // consulted on the per-draw path (twice in SyncNeccessaryBuffers, once per UBO in the + // program rebind), and out of line it would be a call through the PLT per consult. + Bool BindingPointsComeFromRecords() { + static const Bool enabled = ResolveBufferBindingSubsystemArm(); + return enabled; + } + + // The record arm of SyncBufferBindingPoints. Same shape, same order, same branches; the + // four frontend reads become the applier's window and a handle per entry. + // + // WHAT THE WINDOW IS: ShaderBufferCount[class] starting at ShaderBufferStart[class], + // which is the client's touched high-water mark clamped to the 84 the wire carries + // (ruling 10). It is NOT clamped on the client to the device's ceiling, so the ES clamp + // below stays exactly where it was - a client reading GL_MAX_UNIFORM_BUFFER_BINDINGS + // would be answering a driver question from the wrong side of the wire. + // + // WHOLE-VS-RANGE IS `Offset == 0 && Size == kMGPipeWholeBuffer` and obj->GetSize() + // disappears from this function: a base binding does not freeze an extent, the client + // deliberately does not resolve one, and the re-resolution happens HERE against the + // server's own storage - which is what makes a glBufferData issued between the emission + // and this apply bind the NEW extent, as GL does. + void SyncBufferBindingPointsByRecord(Uint32 shaderBufferClass, GLenum glTarget) { +#ifdef TRACY_ENABLE + ZoneScopedC(TRACY_ZONECOLOR_BACKEND); +#endif + const MG_Pipe::MGPipeApplierState& st = MG_Pipe::MGPipeApplier(); + const Uint32 start = st.ShaderBufferStart[shaderBufferClass]; + SizeT end = static_cast(start) + st.ShaderBufferCount[shaderBufferClass]; + // ...and never past what the ES driver itself can hold, exactly as the frontend + // walk does and for the same reason recorded there. + if (glTarget == GL_UNIFORM_BUFFER && g_GLESCapabilities.MaxUniformBufferBindings > 0) { + end = std::min(end, static_cast(g_GLESCapabilities.MaxUniformBufferBindings)); + } + for (SizeT i = start; i < end; ++i) { + const MG_Pipe::MGPBufferRange& entry = + st.BoundShaderBuffers[shaderBufferClass][i]; + if (MG_Pipe::MGPipeHandleIsNull(entry.Res)) { + BindBufferBaseCached(glTarget, static_cast(i), 0); + continue; + } + auto* backendResource = EnsureBufferResourceForHandle(nullptr, entry.Res); + if (!backendResource || backendResource->id == 0) { + MGLOG_E_ONCE("No backend buffer found for %s binding point %zu (handle {%u, %u}).", + MG_Util::ConvertGLEnumToString(glTarget).c_str(), i, entry.Res.Slot, + entry.Res.Gen); + continue; + } + const auto backendBufferId = backendResource->id; + if (entry.Offset == 0 && entry.Size == MG_Pipe::kMGPipeWholeBuffer) { + BindBufferBaseCached(glTarget, static_cast(i), backendBufferId); + } else { + // CLAMPED AGAINST THE SERVER's OWN STORAGE, which is what the frontend walk + // clamped against obj->GetSize() for: a range that outruns the storage is a + // GL_INVALID_VALUE the driver would raise on a bind the application already + // made legally against a buffer that has since shrunk. + const SizeT storage = backendResource->storageSize; + const SizeT rangeStart = std::min(static_cast(entry.Offset), storage); + const SizeT rangeEnd = + std::min(static_cast(entry.Offset + entry.Size), storage); + BindBufferRangeCached(glTarget, static_cast(i), backendBufferId, + static_cast(rangeStart), + static_cast(rangeEnd - rangeStart)); + } + } + } +#else + inline Bool BindingPointsComeFromRecords() { return false; } +#endif + void SyncBufferBindingPoints(BufferTarget target, GLenum glTarget) { #ifdef TRACY_ENABLE ZoneScopedC(TRACY_ZONECOLOR_BACKEND); @@ -598,6 +703,25 @@ namespace MobileGL::MG_Backend::DirectGLES { // the frontend's CPU shadow. Flagging them makes the next MapBuffer/GetBufferSubData // pull the real contents back (BufferObject::SyncGpuWrites). void MarkShaderStorageBuffersGpuWritten() { +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (sb, CONTRACT-P5E.md §5.6): DELETED UNDER A TRANSPORT, and this is the one + // site of the three that cannot merely be re-expressed by handle. The walk is over + // CLIENT memory - the frontend's binding-point array and the BufferObject it names - + // and the mark it makes reaches into a client object, so under run-ahead it would be + // reading and writing memory the client has already moved on from. Rule F. + // + // NOTHING IS LOST, and it is verified rather than asserted: the CLIENT already owns + // this exact set. MarkShaderStorageBindings (MG_Remote/Client/GpuWritePending.cpp) + // walks the same touched high-water mark over the same binding points and calls + // MarkGpuWritten on the same objects, from MarkGpuWritesForDraw in BeforeDrawVerb - + // i.e. BEFORE the draw crosses, on the GL thread, where the objects live. Under a + // transport this backend walk was a DUPLICATE of it (scout S5 §2), and the + // per-producer tally GpuWritePending.h keeps is what says so out loud. + // + // The applier's ShaderBufferWritableMask survives as the server's record of WHICH + // points a shader may write through, which is what P9's narrowing channel will name. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) return; +#endif const SizeT bindingPointCnt = MGB_CTX->GetTouchedBufferBindingPointCount(BufferTarget::ShaderStorage); for (SizeT i = 0; i < bindingPointCnt; ++i) { @@ -617,6 +741,67 @@ namespace MobileGL::MG_Backend::DirectGLES { void SyncAtomicCounterBuffers(const Vector& glBindings, Int esslBindingTop) { #ifdef TRACY_ENABLE ZoneScopedC(TRACY_ZONECOLOR_BACKEND); +#endif +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (sb, §5.6): THE RECORD ARM. `glBindings` stays the backend program's own list + // - it is server-owned since the link and names which GL counter bindings THIS + // program declares - and only the points themselves move sides. The bound is the + // applier's window instead of GetBufferBindingPointCount's sticky forward, which is + // the row this site was the Espryt reader of. + if (BindingPointsComeFromRecords()) { + const MG_Pipe::MGPipeApplierState& st = MG_Pipe::MGPipeApplier(); + const Uint32 windowStart = st.ShaderBufferStart[MG_Pipe::kMGPipeShaderBufferClassAtomicCounter]; + const SizeT windowEnd = static_cast(windowStart) + + st.ShaderBufferCount[MG_Pipe::kMGPipeShaderBufferClassAtomicCounter]; + for (const Int glBinding : glBindings) { + if (glBinding < 0) continue; + const Int esslBinding = esslBindingTop - glBinding; + // Already diagnosed once when the block was transpiled; nothing was bound to + // it there either, so there is nothing to unbind here. + if (esslBinding < 0) continue; + // OUTSIDE THE WINDOW IS "NOTHING BOUND", which is what the frontend array's + // default says too - so it takes the unbind arm rather than being skipped. + // Skipping would leave whatever the driver last had at that ESSL point, + // which is the hole the frontend walk's `continue` never had because its + // bound was the whole 84-point array. + const MG_Pipe::MGPBufferRange* entry = nullptr; + if (static_cast(glBinding) >= windowStart && + static_cast(glBinding) < windowEnd) { + entry = &st.BoundShaderBuffers[MG_Pipe::kMGPipeShaderBufferClassAtomicCounter] + [static_cast(glBinding)]; + } + if (entry == nullptr || MG_Pipe::MGPipeHandleIsNull(entry->Res)) { + BindBufferBaseCached(GL_SHADER_STORAGE_BUFFER, static_cast(esslBinding), 0); + continue; + } + auto* backendResource = EnsureBufferResourceForHandle(nullptr, entry->Res); + if (!backendResource || backendResource->id == 0) { + MGLOG_E_ONCE("No backend buffer found for atomic counter binding point %d " + "(handle {%u, %u}).", + glBinding, entry->Res.Slot, entry->Res.Gen); + continue; + } + if (entry->Offset == 0 && entry->Size == MG_Pipe::kMGPipeWholeBuffer) { + BindBufferBaseCached(GL_SHADER_STORAGE_BUFFER, static_cast(esslBinding), + backendResource->id); + } else { + const SizeT storage = backendResource->storageSize; + const SizeT rangeStart = std::min(static_cast(entry->Offset), storage); + const SizeT rangeEnd = + std::min(static_cast(entry->Offset + entry->Size), storage); + BindBufferRangeCached(GL_SHADER_STORAGE_BUFFER, static_cast(esslBinding), + backendResource->id, static_cast(rangeStart), + static_cast(rangeEnd - rangeStart)); + } + // AND NO MarkBufferGpuWritten HERE. It was the second of the three backend + // GPU-write sites and it reached into a client object; the client's + // MarkAtomicCounterBindings (GpuWritePending.cpp) already marks the same set + // from MarkGpuWritesForDraw, WIDER on purpose - it marks every touched + // counter point rather than only the ones this program declares, which is + // the safe direction for an over-approximate set. + } + return; + } #endif const SizeT pointCount = MGB_CTX->GetBufferBindingPointCount(BufferTarget::AtomicCounter); for (const Int glBinding : glBindings) { @@ -976,7 +1161,17 @@ namespace MobileGL::MG_Backend::DirectGLES { // SSBOs are different: their block bindings are baked into the ESSL at compile time and // BindCurrentProgramWithResources binds no SSBO points, so this is their sole draw-path // binder (e.g. Flywheel's indirect vertex shaders pull instance data from storage buffers). - SyncBufferBindingPoints(BufferTarget::ShaderStorage, GL_SHADER_STORAGE_BUFFER); +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (sb, §5.6): the record arm, selected by transport + bit 13. Same `if (…) else` + // shape the indirect buffer above already uses. + if (BindingPointsComeFromRecords()) { + SyncBufferBindingPointsByRecord(MG_Pipe::kMGPipeShaderBufferClassShaderStorage, + GL_SHADER_STORAGE_BUFFER); + } else +#endif + { + SyncBufferBindingPoints(BufferTarget::ShaderStorage, GL_SHADER_STORAGE_BUFFER); + } MarkShaderStorageBuffersGpuWritten(); } @@ -985,8 +1180,22 @@ namespace MobileGL::MG_Backend::DirectGLES { ZoneScopedC(TRACY_ZONECOLOR_BACKEND); #endif ProcessDeferredBufferReleases(); - SyncBufferBindingPoints(BufferTarget::Uniform, GL_UNIFORM_BUFFER); - SyncBufferBindingPoints(BufferTarget::ShaderStorage, GL_SHADER_STORAGE_BUFFER); + // THE COMPUTE PATH IS THE ONE THAT NEEDS THE FRONTEND-INDEXED UNIFORM PASS, and it + // is why the uniform class is emitted at all: compute does NOT go through the + // per-program block remap in BindCurrentProgramWithResources, so these are the + // points a compute shader actually reads. +#if MOBILEGL_BUILD_DISAGGREGATED + if (BindingPointsComeFromRecords()) { + SyncBufferBindingPointsByRecord(MG_Pipe::kMGPipeShaderBufferClassUniform, + GL_UNIFORM_BUFFER); + SyncBufferBindingPointsByRecord(MG_Pipe::kMGPipeShaderBufferClassShaderStorage, + GL_SHADER_STORAGE_BUFFER); + } else +#endif + { + SyncBufferBindingPoints(BufferTarget::Uniform, GL_UNIFORM_BUFFER); + SyncBufferBindingPoints(BufferTarget::ShaderStorage, GL_SHADER_STORAGE_BUFFER); + } MarkShaderStorageBuffersGpuWritten(); if (includeDispatchIndirectBuffer) { #if MOBILEGL_BUILD_DISAGGREGATED @@ -5461,6 +5670,68 @@ namespace MobileGL::MG_Backend::DirectGLES { // Connect buffer to backend binding point auto binding = currentProgram->GetUniformBlockBinding(i); +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (sb, §5.6): THE POINT HALF, by record. `binding` above is the + // PROGRAM's block binding and stays exactly where it is (it is package + // pg's row); what moves is the point it indexes. The index space is the + // same one - Start is 0 by contract - so `binding` addresses the + // applier's window directly, and a binding at or above Count means + // "nothing bound", which is what the frontend array's default said too. + if (BufferImpl::BindingPointsComeFromRecords()) { + const MG_Pipe::MGPipeApplierState& st = MG_Pipe::MGPipeApplier(); + const Uint32 windowStart = + st.ShaderBufferStart[MG_Pipe::kMGPipeShaderBufferClassUniform]; + const SizeT windowEnd = + static_cast(windowStart) + + st.ShaderBufferCount[MG_Pipe::kMGPipeShaderBufferClassUniform]; + if (static_cast(binding) < windowStart || + static_cast(binding) >= windowEnd) { + continue; // nothing bound there - the frontend arm's `if (bufferObj)` + } + const MG_Pipe::MGPBufferRange& entry = + st.BoundShaderBuffers[MG_Pipe::kMGPipeShaderBufferClassUniform] + [static_cast(binding)]; + if (MG_Pipe::MGPipeHandleIsNull(entry.Res)) continue; + // The clean probe is the SAME five questions IsBufferDrawClean asks + // the frontend object, asked by handle: the twin's identity, the + // resource record's Serial against the twin's synced one, and the + // pending-work sets - all server-owned (D-A4). The drawCleanEpoch + // short-circuit is unchanged; it was never the point read. + auto* backendResource = BufferImpl::FindBufferResourceForHandle(entry.Res); + if (!backendResource || backendResource->drawCleanEpoch != bufferEpoch) { + if (backendResource && BufferImpl::IsBufferDrawCleanByHandle( + entry.Res, backendResource, nullptr)) { + backendResource->drawCleanEpoch = bufferEpoch; + } else { + backendResource = + BufferImpl::EnsureBufferResourceForHandle(nullptr, entry.Res); + } + } + if (backendResource && backendResource->id != 0) { + // WHOLE-VS-RANGE IS THE RECORD's TEST, not `range.end == 0`: a + // base binding travels as kMGPipeWholeBuffer precisely so the + // extent is re-resolved HERE, against the storage this server + // holds, rather than frozen at the client's emission. + if (entry.Offset == 0 && entry.Size == MG_Pipe::kMGPipeWholeBuffer) { + BufferImpl::BindBufferBaseCached(GL_UNIFORM_BUFFER, lastUBOBinding, + backendResource->id); + } else { + const SizeT storage = backendResource->storageSize; + const SizeT rangeStart = + std::min(static_cast(entry.Offset), storage); + const SizeT rangeEnd = std::min( + static_cast(entry.Offset + entry.Size), storage); + BufferImpl::BindBufferRangeCached( + GL_UNIFORM_BUFFER, lastUBOBinding, backendResource->id, + static_cast(rangeStart), + static_cast(rangeEnd - rangeStart)); + } + } else { + MGLOG_E_ONCE("No backend buffer found for UBO binding, cannot bind UBO."); + } + continue; + } +#endif auto& point = MGB_CTX->GetBufferBindingPoint(BufferTarget::Uniform, binding); auto& bufferObj = point.GetBoundObject(); auto range = point.GetRange();