diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 0cb77673..6b18f809 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -4566,6 +4566,24 @@ namespace MobileGL::MG_Backend::DirectGLES { return PipeRecordAt(MG_Pipe::MGPipeApplier().RenderbufferResources, res); } + // P3b/P4b (wave 2-D, package D3). See the declaration for why a view's storage clauses are + // asked of another record, why null means "ask again" and why the deletion order cannot + // strand this walk. + const MG_Pipe::MGPipeResourceRecord* PipeTextureStorageRecordForRecord( + const MG_Pipe::MGPipeResourceRecord& record) { + // ONE hop is the shape the client emits and the second is already generous; the bound is + // here so a cyclic chain is a null answer rather than a hang, not because a real chain + // is ever this long. + constexpr Uint32 kViewChainBound = 4; + const MG_Pipe::MGPipeResourceRecord* storage = &record; + for (Uint32 hop = 0; hop < kViewChainBound; ++hop) { + if (MG_Pipe::MGPipeHandleIsNull(storage->Desc.ViewOf)) return storage; + storage = PipeTextureRecordForHandle(storage->Desc.ViewOf); + if (storage == nullptr) return nullptr; + } + return nullptr; + } + MobileGL::TextureTarget PipeTextureTargetForHandle(MG_Pipe::MGPipeHandle res) { const auto* record = PipeTextureRecordForHandle(res); if (record == nullptr) return MobileGL::TextureTarget::Unknown; @@ -7517,11 +7535,19 @@ namespace MobileGL::MG_Backend::DirectGLES { MGLOG_D("Storage texture of view %u has no ES name yet.", record.Desc.GlNameForDiag); return; } + // P3b/P4b D3: THE SERIAL THIS TWIN STAMPS IS THE STORAGE'S, not this view's, because + // the gate that reads it back (IsDrawSyncCleanByRecord) now asks the same record. It + // is read AFTER the storage sync above - which has just consumed the owner's pending + // uploads - and taken by VALUE, because the applier's vector may move under any later + // call and a Uint64 cannot dangle. A storage record that has gone answers 0, which + // the gate reads as "never clean": the direction that re-syncs. + const MG_Pipe::MGPipeResourceRecord* storageRecord = PipeTextureStorageRecordForRecord(record); + const Uint64 storageSerial = storageRecord != nullptr ? storageRecord->Serial : 0; if (m_isInitialized && m_viewSourceBackendTextureId == storageBackendTextureId) { // The steady case, and the only one this arm has to be fast at: one record read, // one integer compare and the serial stamp that keeps the aggregate clean gate // (IsDrawSyncCleanByRecord) answering true for the next draw. - m_syncedResourceSerial = record.Serial; + m_syncedResourceSerial = storageSerial; return; } // The view window is a sampler-view CSO, created even when this @@ -7563,7 +7589,8 @@ namespace MobileGL::MG_Backend::DirectGLES { m_prevTextureInfo = {static_cast(record.Desc.InternalFormat), record.Desc.Width, record.Desc.Height, record.Desc.Depth, view.NumLevels, 0, record.Desc.Samples, record.Desc.FixedSampleLocations != 0}; - m_syncedResourceSerial = record.Serial; + // The storage's, for the steady arm's reason above (P3b/P4b D3). + m_syncedResourceSerial = storageSerial; } #endif @@ -15515,9 +15542,20 @@ namespace MobileGL::MG_Backend::DirectGLES { Bool BackendTextureObject::IsDrawSyncCleanByRecord(MG_Pipe::MGPipeHandle res, const MG_Pipe::MGPipeResourceRecord& record) const { (void)res; + // P3b/P4b D3: THE TWO STORAGE CLAUSES ARE ASKED OF THE STORAGE RECORD, which is this + // record for every texture that owns its texels and the record named by Desc.ViewOf + // for a glTextureView. A view's own Serial is moved by nothing an upload does - the + // client keys every resource_subdata on the storage owner, and the applier moves that + // record's Serial and PendingUploads alone - so reading the view's here answered + // "clean" for every owner write after the view's first sample. The three PARAMETER + // clauses below stay this record's: a view has its own texture parameters and its own + // built-in sampler, which is the whole point of the name existing. See + // PipeTextureStorageRecordForRecord and CONTRACT-P5E.md §5.2. + const MG_Pipe::MGPipeResourceRecord* storage = PipeTextureStorageRecordForRecord(record); + if (storage == nullptr) return false; // SyncMipmapsToBackend's own early-out, verbatim. - if (!m_isInitialized || m_syncedResourceSerial == 0 || m_syncedResourceSerial != record.Serial || - !record.PendingUploads.empty()) { + if (!m_isInitialized || m_syncedResourceSerial == 0 || m_syncedResourceSerial != storage->Serial || + !storage->PendingUploads.empty()) { return false; } // SyncTextureParamsToBackend's (ResolvePushedTextureParams' gate). diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 812d9c0d..21e61029 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -770,6 +770,44 @@ namespace MobileGL::MG_Backend::DirectGLES { // composite: this reader hides the split behind one lookup, exactly as the wire does. const MG_Pipe::MGPipeShaderCsoRecord* PipeShaderCsoRecordForHandle(MG_Pipe::MGPipeHandle cso); + // P3b/P4b (wave 2-D, package D3): THE STORAGE RECORD BEHIND A TEXTURE RECORD - the record of + // the texture it views for a glTextureView, and itself for every other texture. + // + // WHY THERE IS A SECOND READER. A view owns no texels, and the client keeps ONE emission + // cursor per storage: an upload through a view's own name is remapped onto its storage owner + // before it is emitted (MG_Impl/Pipe/TextureEmit.h, `ViewOf names the storage owner, and ONE + // HOP always reaches storage`), so every resource_subdata for either name is keyed on the + // OWNER and the applier moves the OWNER's Serial and PendingUploads alone + // (MG_Pipe/PipeApply.cpp, ApplyTextureUpload). A view therefore has a Serial that no upload + // ever moves, and the two STORAGE clauses of its draw-path clean gate have to be asked of + // this record instead of its own - which is what the monolith gate gets for free, because + // GetContentVersion() through a view is forwarded to the owner + // (MG_State/GLState/TextureState/TextureObjectView.cpp:100-102). CONTRACT-P5E.md §5.2 states + // the rule; without it a view that has been sampled once never sees another owner write. + // + // NULL IS "ASK AGAIN", NEVER "CLEAN". Every caller treats a null answer as a reason to RUN + // the sync, which is the direction that re-uploads rather than the direction that shows stale + // texels, and it raises nothing: a missing record is PipeTextureRecordForHandle's own + // null-on-miss answer one level up, not a protocol refusal. + // + // BOUNDED, THOUGH ONE HOP IS ALWAYS ENOUGH. glTextureView composes a view-of-a-view onto the + // ROOT at creation - the spec's additive min-level rule - and TextureObjectView's invariant + // is that its owner "is never a view itself", so Desc.ViewOf already names storage. The walk + // is written transitively anyway and gives up after a fixed number of hops, because THIS side + // may not depend on a client invariant to terminate: a chain the wire could make cyclic ends + // as a null answer rather than as a hang. + // + // DELETION ORDERING IS SAFE BY CONSTRUCTION, AND CHECKED ANYWAY. A view holds a strong + // SharedPtr to its storage owner (TextureObjectView::m_storageOwner) and the client emits + // resource_destroy from the DESTRUCTOR - "the last SharedPtr to this object dropping, not the + // glDelete* that only marks the name and leaves a still-bound object very much alive" + // (MG_State/GLState/TextureState/TextureObject.cpp:63) - so an owner's record cannot die + // while a record that views it is live, however the application orders its glDeleteTextures. + // If one ever did, PipeTextureRecordForHandle tests Live and Gen, so a recycled slot answers + // null rather than a stranger's record. + const MG_Pipe::MGPipeResourceRecord* PipeTextureStorageRecordForRecord( + const MG_Pipe::MGPipeResourceRecord& record); + // P5e (fb): the texture's own TARGET, from its descriptor. The image-unit bind needs it // (glBindImageTexture's layered/format rules are per target) and MGPImageView has no room // for it - 24 bytes, no pad - so it is read off the resource record instead of being added diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 1c741550..a4907006 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -2651,34 +2651,25 @@ if (MOBILEGL_BUILD_DISAGGREGATED) # TextureViewUsable() rather than failing - which is what the Tcp arm did until the fixture # entry below got the same knob. # - # ONE CASE IS EXCLUDED BY NAME, and it is a defect this package FOUND rather than a lane - # problem. TheOwnerUploadDrainRunsAgainAfterADrawBoundary is red on all three transports and - # green under monolith: + # ALL FOUR CASES RUN ON ALL THREE ARMS. TheOwnerUploadDrainRunsAgainAfterADrawBoundary was + # EXCLUDED BY NAME here by package D2, which found the defect but could not fix it inside its + # own footprint: # # after a view has been SAMPLED once, a later glTexSubImage through the OWNER's own name # never becomes visible through the view. # - # The client half is provably correct - the case's own counter assertions pass, so the - # records were emitted and the applier accepted them. The server's per-texture clean gate - # BackendTextureObject::IsDrawSyncCleanByRecord (Managers.cpp:15433) compares the VIEW's - # record.Serial and record.PendingUploads, and an owner-side resource_subdata moves only the - # OWNER's (MG_Pipe/PipeApply.cpp:1175). Nothing anywhere propagates a serial bump from a - # storage owner to the records that view it: MGPResourceDesc::ViewOf is a forward edge with - # no reverse index, and the applier reads it only in the descriptor-equality helper - # (PipeApply.cpp:666). Under monolith the same gate reads GetContentVersion() THROUGH the - # view, which TextureObjectView forwards to the owner (TextureObjectView.cpp:100-102) - the - # forwarding the split gate has no equivalent of. CONTRACT-P5E.md:387-395 states the split - # gate in full and has the same hole, so the implementation is faithful to the contract. - # - # NOT EXCLUDED FROM THE MONOLITH LANES, where it is green and is the regression net for the - # forwarding that works. Listed in notes/p34b/split-scenario-census.md and - # notes/p34b/espryt-d2.md rather than deleted (G14). The fix is a one-line-shaped change in - # MG_Pipe/PipeApply.cpp with an O(#texture records) scan behind it, or a reverse index in - # MGPipeResourceRecord - an owner's decision, not this package's, and it is outside D2's - # declared footprint. - mgl_itest_register_split_arms( - "TextureViewAliasScenario.*-TextureViewAliasScenario.TheOwnerUploadDrainRunsAgainAfterADrawBoundary" - "") + # Package D3 fixed it and this is its red-once (rule J): the case is red on Split, Spawn AND + # Tcp at D2's head a7c06f4f with this exclusion lifted and nothing else changed, and green on + # all three with the fix. The client half was never at fault - the case's own counter + # assertions pass on the red run, so the records were emitted and the applier accepted them. + # The server's per-texture clean gate BackendTextureObject::IsDrawSyncCleanByRecord + # (MG_Backend/DirectGLES/Managers.cpp) compared the VIEW's record.Serial and + # record.PendingUploads, while an owner-side resource_subdata moves only the OWNER's + # (MG_Pipe/PipeApply.cpp ApplyTextureUpload). The gate now RESOLVES THROUGH Desc.ViewOf to the + # storage owner's record for those two clauses, which is the split equivalent of the monolith + # gate's GetContentVersion() forwarding (TextureObjectView.cpp:100-102). CONTRACT-P5E.md §5.2 + # states the rule the code now implements. + mgl_itest_register_split_arms("TextureViewAliasScenario.*" "") # ---- D2: THE UPLOAD-SHAPE GOLD, UNDER ALL THREE TRANSPORTS ------------------------------ # diff --git a/MobileGL/MG_Remote/CONTRACT-P5E.md b/MobileGL/MG_Remote/CONTRACT-P5E.md index cfb8f4a9..7b99f510 100644 --- a/MobileGL/MG_Remote/CONTRACT-P5E.md +++ b/MobileGL/MG_Remote/CONTRACT-P5E.md @@ -388,11 +388,38 @@ vertex-elements CSO stays identity-addressed. Every draw-path entry is `SyncTextureToBackendByHandle(h, imageBindable)`: record first, twin by `GetOrCreateByHandle`, three syncs `(h, *rec)`; the by-value twin copy + second `Find` (`DirectGLES.cpp:1745-1755, 1781-1805`) go with the map arm. Clean = `m_isInitialized && -m_syncedResourceSerial == rec->Serial && rec->PendingUploads.empty() && m_syncedParamsSerial == +m_syncedResourceSerial == storage->Serial && storage->PendingUploads.empty() && m_syncedParamsSerial == rec->ParamsSerial && !rec->Params.ForceResync && !m_forceTextureParamsResync && m_syncedBuiltinSampler == rec->Params.BuiltinSampler && m_syncedBuiltinSamplerSerial == SamplerCso(rec->Params.BuiltinSampler)->Serial && !rec->Params.SamplerResync && !m_forceSamplerResync -&& rec->Desc.StorageKind == Mipmap` — no new state. The four in-body live reads (`GetTarget` at +&& rec->Desc.StorageKind == Mipmap` — no new state. + +**`storage` is the STORAGE RECORD, which is `rec` itself for a texture that owns its texels and +the record `rec->Desc.ViewOf` names for a `glTextureView`** (P3b/P4b wave 2-D package D3; +`Managers.cpp PipeTextureStorageRecordForRecord`). This clause was `rec->Serial` / +`rec->PendingUploads` through P5e and **that was wrong**, which is why it is corrected here rather +than restated: a view owns no texels and the client keeps ONE emission cursor per storage — an +upload through a view's own name is remapped onto its owner before it is emitted +(`MG_Impl/Pipe/TextureEmit.h`) — so every `resource_subdata` for either name is keyed on the OWNER +and `ApplyTextureUpload` moves the OWNER's `Serial` and `PendingUploads` alone. A view's own +`Serial` is moved by nothing an upload does, so a gate that read it answered CLEAN for every owner +write after the view's first sample and the view went on sampling the texels it was minted with. +The monolith gate has this for free: `GetContentVersion()` through a view is forwarded to the owner +(`TextureObjectView.cpp:100-102`). The three PARAMETER clauses stay `rec`'s — a view has its own +texture parameters and its own built-in sampler, which is the whole reason the second name exists — +as does `Desc.StorageKind`. The resolution is transitive and BOUNDED (one hop always reaches +storage, because `glTextureView` composes a view-of-a-view onto the root at creation, but this side +may not depend on a client invariant to terminate), and a storage record that cannot be resolved is +**not clean**: the direction that re-syncs, not a refusal, and it raises nothing. +`SyncTextureViewToBackendByRecord` stamps that same storage serial — or 0, which reads as never +clean — at both its arms, so the stamp and the gate name one quantity. Deletion ordering cannot +strand the walk: a view holds a strong reference to its storage owner and the client emits +`resource_destroy` from the DESTRUCTOR rather than from `glDeleteTextures` +(`TextureObject.cpp:63`), so an owner's record outlives every record that views it whatever order +the application deletes the two names in; `Live`/`Gen` are checked anyway, so a recycled slot +answers null rather than a stranger's record. + +The four in-body live reads (`GetTarget` at `Managers.cpp:8876-8877, 8620-8621`; `IsTextureView` `:7060`; the TexBuffer backing `:8223-8231`) read `Desc.Target / Desc.ViewOf / Desc.BufferForTexBuffer`; `GetExternalIndex` in logs becomes `Desc.GlNameForDiag`. The unit work list is `{Res, backend}` keyed `(ContextSerial,