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.