From 3012d52e2b76be13fb19d4ca8b166297c1d538d8 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 07:53:05 -0400 Subject: [PATCH 1/6] [MG_Backend] (Disaggregated): P5e fb - the attachment surface, the two by-handle twins and the image unit read records MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - SyncAttachmentSurface loses its frontend attachment parameter (CONTRACT-P5E §5.4). The storage syncs have by-handle forms now, so the object was only there to be cross-checked - and a second answer only a monolith build can produce is not a check. Review N-6's one-directional hole ("record empty, frontend still holding") becomes unrepresentable: the caller's detach and the attach read one test. - The renderbuffer cross-check at the old Managers.cpp:9711 is DELETED rather than transport-gated (§4.4). It was the one HandleOf in this family that was not gated while its texture sibling was; it ran a client-allocator probe on the apply thread under split, legal only because the scope named it and the client was parked. - BackendFramebufferObject::SyncToBackendByHandle and BackendRenderbufferObject::SyncToBackendByHandle land in place of c0e's aborting seams, beside the object forms they overload. The record's eleven surfaces are the point set; the per-point frontend version memo is gone and the record hash is the only gate, with g_attachmentBackendIdGeneration and the re-entry kept because no client-side value can answer what they answer. - SyncReadBufferToBackend's decision moves into ApplyReadBufferFromRecord, which takes no frontend object; both overloads funnel through it. - The reverse index texture -> framebuffers, maintained at the by-handle sync, for the detach walk (tx2 needs it) instead of a walk over every twin's frontend attachments. - Images: ResolveShaderImageRecord and its identity probe are gone - under a transport there is no second answer to arbitrate. SyncImageTextureBinding gains a record overload; Access is DECODED through MGPipeValueTypes.h (ruling 16 / ID-94) and the stale "the encoding does not exist" comment is deleted. The bind's format rules are factored into one helper both arms call rather than written twice. - The image sweep gate is re-keyed onto (ShaderImagesSerial, TextureShutterSerial, ContextSerial, g_backendContextGeneration) under a transport (ruling 7 / ID-86), never a backend re-mint counter; the window/high-water union is kept and the walk reads BoundShaderImages[]. - MarkWritableImageBufferTexturesGpuWritten returns early under a transport - the client owns the GPU-write set; paired with sb's deletion of the backend marks. - BoundImageUnitFormat answers from MGPImageView::InternalFormat under a transport. --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 587 +++++++++++----- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 648 ++++++++++++++---- MobileGL/MG_Backend/DirectGLES/Managers.h | 36 + 3 files changed, 950 insertions(+), 321 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index ed19e4c3..ba047d51 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -239,6 +239,29 @@ namespace MobileGL::MG_Backend::DirectGLES { const auto& st = MG_Pipe::MGPipeApplier(); return target == FramebufferTarget::Draw ? st.DrawFramebuffer() : st.ReadFramebuffer(); } + +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (fb, CONTRACT-P5E.md §5.4 and §0's rule F). THE DECLINE BECOMES THE DETECTOR. + // + // Every framebuffer decline in this file used to fall back to the pre-handle arm, which + // reads GetFramebufferBindingSlot - a BARRIER_PULLED row (FieldOwnership.def). Under an + // active transport that fallback is not a safe default any more: the value it would read + // belongs to a client that is no longer parked behind this apply, so the honest answer is + // the same one the accessor itself would give one line later, raised HERE, where the + // reason is still in scope. Naming the verb is what makes the strict lane's marker table + // ("@") point at the site instead of at the accessor. + // + // ONLY UNDER A TRANSPORT AND ONLY WITH THE FAMILY BIT SET (ruling 1 / §5.8): the + // push-monolith build keeps its frontend arms token for token, and a mask that never + // armed framebuffers has no record to decline from. + static Bool FramebufferRecordArmIsMandatory() { + return MG_Config::Transport != MG_Config::TransportMode::Monolith && FramebufferSubsystemEnabled(); + } + [[noreturn]] static void RefuseFramebufferBindingSlotRead() { + MG_Pipe::MGPipeInputPoisonFatalForVerb(MG_Pipe::MGPipeInputField::GetFramebufferBindingSlot, + MGB_CTX->CurrentVerb()); + } +#endif // MOBILEGL_BUILD_DISAGGREGATED #endif // MOBILEGL_PIPE_PUSH static Bool IsDualSourceBlendFactor(BlendFactor v) { @@ -2145,6 +2168,31 @@ namespace MobileGL::MG_Backend::DirectGLES { static Uint64 g_unitTextureSyncListEpoch = 0; static Uint64 g_unitTextureSyncListSamplingGeneration = 0; +#if MOBILEGL_PIPE_PUSH + // P5e (fb, CONTRACT-P5E.md §5.4): THE TWO ATTACHMENT LISTS GET AN ENTRY OF THEIR OWN, + // and it borrows nothing. + // + // The unit list's entry borrows the binding slot's SharedPtr and re-checks the pairing + // before every replay, because its key (the unit-bindings epoch) is derived state that + // a silent slot swap could get past. An attachment list under the record arm has + // neither problem and cannot afford the borrow: the key IS the record - {Fbo, + // ContentHash} - and every attachment edit moves the hash by construction, so the + // pairing has nothing left to catch; and the borrowed pointer was into a frontend + // FramebufferObject, which is exactly the thing this package stops holding. So an entry + // is the texture HANDLE the record named plus the twin it resolved to, PairingsIntact + // becomes `entry.Res == surface.Res` folded into the rebuild, and the list owns no + // reference to anything the client can free. + // + // The twin pointer is still a BORROW, on the unit list's own terms: the registry keeps + // a twin alive until its object dies, an attached texture cannot die while the record + // that names it stands, and the very next thing done with the entry is a by-handle sync + // that would re-resolve it anyway. + struct FboAttachmentSyncEntry { + MG_Pipe::MGPipeHandle Res = MG_Pipe::kMGPipeNullHandle; + BackendTextureObject* backend = nullptr; + }; +#endif + // Sibling memo for the draw FBO's texture attachments (see the use site in // SyncNeccessaryTextures for the key derivation and the borrow rules, which are the // unit list's). A null FBO pointer means "not stamped". @@ -2169,6 +2217,58 @@ namespace MobileGL::MG_Backend::DirectGLES { // a live one. static Uint64 g_fboTextureSyncListContentHash = 0; static Bool g_fboTextureSyncListRecordKeyed = false; + // P5e (fb): the record-keyed list itself, held apart from the pre-handle one for the + // reason the two key shapes are held apart - one field carrying two shapes is how a + // stale half gets compared against a live one. Its key is + // {Fbo, ContentHash, ContextSerial, g_backendContextGeneration}: the applier's + // ContextSerial replaces GetTextureContextId (a frontend read the record arm may not + // take), and the two version halves are gone because the hash covers them. + static Vector g_fboAttachmentSyncList; + static MG_Pipe::MGPipeHandle g_fboAttachmentSyncListFbo = MG_Pipe::kMGPipeNullHandle; + static Uint64 g_fboAttachmentSyncListContentHash = 0; + static Uint64 g_fboAttachmentSyncListContextSerial = 0; + static Uint g_fboAttachmentSyncListContextGeneration = 0; + static Bool g_fboAttachmentSyncListValid = false; + + // The read framebuffer's, same shape and same reasons. Its own list because it is + // keyed on a different framebuffer and deduped against the draw one. + static Vector g_readFboAttachmentSyncList; + static MG_Pipe::MGPipeHandle g_readFboAttachmentSyncListFbo = MG_Pipe::kMGPipeNullHandle; + static Uint64 g_readFboAttachmentSyncListContentHash = 0; + static Uint64 g_readFboAttachmentSyncListContextSerial = 0; + static Uint g_readFboAttachmentSyncListContextGeneration = 0; + static Bool g_readFboAttachmentSyncListValid = false; + + // Rebuild-or-replay, shared by both lists. The record's texture surfaces ARE the + // membership (§5.4); a renderbuffer-only framebuffer - the common Minecraft frame - + // reduces to the key compare and an empty loop. + // + // ONE CALL PER ENTRY, and it is tx2's by-handle seam rather than the three the + // pre-handle arm makes: SyncTextureToBackendByHandle is the by-handle twin of + // SyncTextureObjectToBackend, which is what the REBUILD has always called here, so the + // params / builtin-sampler / mipmap work and its own clean gate are inside it. Calling + // the three separately would need by-handle forms of the first two that nothing + // declares, and would put this file's copy of the clean gate beside tx2's. + static void SyncRecordAttachmentTextures(const MG_Pipe::MGPFramebufferState& record, + Vector& list, Bool listValid) { + if (listValid) { + for (const auto& entry : list) { + TextureImpl::SyncTextureToBackendByHandle(entry.Res, /*imageBindableStorageRequired=*/false); + } + return; + } + list.clear(); + const auto add = [&](const MG_Pipe::MGPSurface& surface) { + if (surface.Kind != MG_Pipe::kMGPipeSurfaceKindTexture) return; + if (MG_Pipe::MGPipeHandleIsNull(surface.Res)) return; + auto& twin = TextureImpl::SyncTextureToBackendByHandle(surface.Res, + /*imageBindableStorageRequired=*/false); + list.push_back({surface.Res, twin.get()}); + }; + for (Uint i = 0; i < MG_Pipe::kMGPipeMaxColorAttachments; ++i) add(record.Color[i]); + add(record.Depth); + add(record.Stencil); + } #endif // The frontend texture-state keys the per-draw texture stages @@ -2241,6 +2341,74 @@ namespace MobileGL::MG_Backend::DirectGLES { static Uint64 g_readFboTextureSyncListContentHash = 0; static Bool g_readFboTextureSyncListRecordKeyed = false; +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (fb, CONTRACT-P5E.md §5.4): THE RECORD ARM OF BOTH ATTACHMENT LISTS, and it is + // the package's core - this is the one unconditional per-draw live read the framebuffer + // family had, taken on every draw whether the memo hit or missed. + // + // Three frontend reads go in one move: the binding slot (twice), the FramebufferObject + // the entries borrowed their attachment slots from, and GetTextureContextId. The key is + // {Fbo, ContentHash, ContextSerial, g_backendContextGeneration} - the record's own hash + // in place of the slot/object version pair (it covers every surface, and Fbo is inside + // it, so a recycled framebuffer cannot be suppressed against its predecessor), the + // APPLIER's ContextSerial in place of the frontend context id, and the backend context + // generation unchanged because it answers a question about driver names that no + // client-side value can answer. + // + // The two lists are still two, and the dedupe is still "one object on both bindings" - + // asked of the two BOUND HANDLES rather than of two pointers, which is the same + // question with a spelling a recycled slot cannot fool. + static void SyncFramebufferAttachmentTexturesByRecord() { + const auto& st = MG_Pipe::MGPipeApplier(); + const MG_Pipe::MGPipeHandle drawHandle = + st.BoundFramebuffer[SizeT(MG_Pipe::MGPipeFramebufferTarget::Draw)]; + const MG_Pipe::MGPipeHandle readHandle = + st.BoundFramebuffer[SizeT(MG_Pipe::MGPipeFramebufferTarget::Read)]; + const MG_Pipe::MGPFramebufferState* const drawRecord = + BoundFramebufferRecord(FramebufferTarget::Draw); + const MG_Pipe::MGPFramebufferState* const readRecord = + BoundFramebufferRecord(FramebufferTarget::Read); + const Uint64 contextSerial = st.ContextSerial; + + if (drawRecord == nullptr) { + // Not a decline this function can refuse on: SyncCurrentFBO runs before every + // caller of this one and is where the missing record is named and refused + // (RefuseFramebufferBindingSlotRead). Here it only means "nothing to walk". + g_fboAttachmentSyncListValid = false; + g_fboAttachmentSyncList.clear(); + } else { + const Bool listValid = g_fboAttachmentSyncListValid && + g_fboAttachmentSyncListFbo == drawRecord->Fbo && + g_fboAttachmentSyncListContentHash == drawRecord->ContentHash && + g_fboAttachmentSyncListContextSerial == contextSerial && + g_fboAttachmentSyncListContextGeneration == g_backendContextGeneration; + SyncRecordAttachmentTextures(*drawRecord, g_fboAttachmentSyncList, listValid); + g_fboAttachmentSyncListFbo = drawRecord->Fbo; + g_fboAttachmentSyncListContentHash = drawRecord->ContentHash; + g_fboAttachmentSyncListContextSerial = contextSerial; + g_fboAttachmentSyncListContextGeneration = g_backendContextGeneration; + g_fboAttachmentSyncListValid = true; + } + + if (readRecord == nullptr || readHandle == drawHandle) { + g_readFboAttachmentSyncListValid = false; + g_readFboAttachmentSyncList.clear(); + return; + } + const Bool readListValid = g_readFboAttachmentSyncListValid && + g_readFboAttachmentSyncListFbo == readRecord->Fbo && + g_readFboAttachmentSyncListContentHash == readRecord->ContentHash && + g_readFboAttachmentSyncListContextSerial == contextSerial && + g_readFboAttachmentSyncListContextGeneration == g_backendContextGeneration; + SyncRecordAttachmentTextures(*readRecord, g_readFboAttachmentSyncList, readListValid); + g_readFboAttachmentSyncListFbo = readRecord->Fbo; + g_readFboAttachmentSyncListContentHash = readRecord->ContentHash; + g_readFboAttachmentSyncListContextSerial = contextSerial; + g_readFboAttachmentSyncListContextGeneration = g_backendContextGeneration; + g_readFboAttachmentSyncListValid = true; + } +#endif // MOBILEGL_BUILD_DISAGGREGATED + static void SyncReadFramebufferTextureAttachments(const DrawTextureSyncKeys& keys, const MG_State::GLState::FramebufferObject* drawFbo) { const auto& readSlot = GetFramebufferBindingSlotChecked(FramebufferTarget::Read); @@ -2402,6 +2570,15 @@ namespace MobileGL::MG_Backend::DirectGLES { // registry keeps a backend object alive until its frontend texture expires, which an // attached texture cannot. A renderbuffer-only FBO - the common Minecraft frame - // reduces to the key compare and an empty loop. +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (fb): under a transport with the framebuffer bit set, BOTH attachment lists + // are answered from the two records and neither binding slot is touched. The + // pre-handle pair below stays compiled and stays the monolith path (ruling 1). + if (FramebufferRecordArmIsMandatory()) { + SyncFramebufferAttachmentTexturesByRecord(); + return; + } +#endif const auto& drawSlot = GetFramebufferBindingSlotChecked(FramebufferTarget::Draw); const auto& currentFBO = drawSlot.GetBoundObject(); if (currentFBO) { @@ -2545,152 +2722,51 @@ namespace MobileGL::MG_Backend::DirectGLES { // Almost every program (every Minecraft draw) leaves it at zero, which is what keeps the // draw-path staleness check below at one integer test. static Uint g_imageUnitHighWaterMark = 0; - -#if MOBILEGL_PIPE_PUSH - // P4a e3 (D-G3). The pushed image-unit record for this unit, or null when there is - // none to read. + // ---- P5e (fb, CONTRACT-P5E.md §5.4 / ruling 16 / ID-94): the image unit's bind ------ // - // THE IDENTITY CHECK IS THE POINT. set_shader_images is emitted at the validate point - // for a draw, while SyncImageTextureBinding is also the EAGER funnel that - // glBindImageTexture itself runs - so at that moment the newest record describes the - // previous draw. Driving the driver from a record that names a different texture than - // the unit holds would bind one texture with another's level, layer and format, which - // is the class of bug the frontend's own ImageTextureBinding exists to make - // impossible. So a record is used only when it names EXACTLY the texture the unit - // holds, and otherwise this returns null and the frontend binding answers - which is - // also what a build whose client half has not landed always gets. + // P4a e3's ResolveShaderImageRecord IS GONE, identity test, I5 seam log and all. It + // existed because there were TWO answers for one unit - the record and the frontend + // binding - and it had to decide which of them the driver was driven from; its identity + // test (`view.Res != HandleOf(boundTexture)`) was itself a client-allocator probe on the + // apply thread. Under a transport there is no second answer left: the record IS the + // unit, so the arm below reads it and nothing corroborates it. // - // Count is the "has this set ever arrived" test, never the serial: MGPipeApplierReset - // advances the working-state serials whether or not anything was emitted. + // AND ACCESS IS DECODED. The comment this replaced said MGPImageView::Access carried + // "an ENCODING whose definition belongs to the client emitter and does not exist at the + // contract commit". That was STALE at the time and is what ruling 16 / ID-94 corrects: + // MGPipeEncodeImageAccess has folded the three GL names into 0/1/2 since P4a, and c0e + // moved the numbers into MG_Pipe/MGPipeValueTypes.h so one table serves both roles. + // Reading the frontend's GLenum "because it is the same value by construction" was the + // last field of this record the server declined to believe. + + // The bind itself, factored out of both arms, because the format rules below are the + // part that is easy to get subtly different in two copies - and a divergence there is + // an out-of-bounds image read on nineteen of twenty-six formats, not a wrong pixel. + // Everything it takes is a VALUE: the arm above it decides where each one came from. // - // WHY THE SEAM CHECKS ARE ONLY LOUD AT THE VALIDATE POINT (review MAJOR-3). The - // framebuffer seam logs unconditionally because SyncCurrentFBO only ever runs where the - // records are current. This one does not have that property: SyncImageTextureBinding is - // ALSO the eager funnel glBindImageTexture itself runs, and at that moment the newest - // record legitimately describes the PREVIOUS draw, so an unconditional log here would - // fire on every ordinary frame and the grep section 8 mandates would be worthless. The - // latch below is set only around the draw/dispatch sweep, where the set for THIS draw - // has been applied and a mismatch is therefore a mis-keyed emission - the same finding - // against B/C the framebuffer line reports, in the same words, so one grep covers all - // three seams. - // Set around BOTH sweeps in SyncImageTextureBindings - the record-driven one and the - // full pre-handle one - because I2's flip fires precisely when the record arm is NOT - // taken (ShaderImageCount == 0 sends the sweep down the wide path), so a latch that - // covered only the record arm would make that flip permanently unreachable. - static Bool g_imageRecordSeamIsAuthoritative = false; - - static const MG_Pipe::MGPImageView* ResolveShaderImageRecord( - Uint unit, const MG_State::GLState::ITextureObject* boundTexture) { -#if MOBILEGL_BUILD_DISAGGREGATED - // P5c (G6, CONTRACT-P5C §5.4): the image seam's identity check below resolves the - // bound texture's handle by frontend identity - frontend-keyed twin resolution, - // named debt inside the scope - P3b/P4b rekeys the registry onto handles. - const MG_Pipe::MGPipeFrontendKeyedRegistryScope frontendKeyedRegistry; -#endif - // P4a decline-site I1: M - the mask says this family is not switched on, and it - // IS D's SamplerSubsystemEnabled() now. Silent, confirmed at the verification - // round - and it is what keeps I2 below quiet on a mask that never armed images. - if (!SamplerSubsystemEnabled()) return nullptr; - const auto& st = MG_Pipe::MGPipeApplier(); - // P4a decline-site I2: S - FLIPPED AT THE VERIFICATION ROUND to loud-once, under - // the narrowest condition this site can actually test, which is STRONGER than - // the review's "when the program declares images": the validate-point latch is - // set (so this is the draw/dispatch sweep and not the eager glBindImageTexture - // funnel, where an empty set legitimately precedes the first emission) AND the - // caller has already established that this unit HOLDS an image texture - // (SyncImageTextureBinding returns before this on a null one). A unit carrying an - // image texture at a draw with no set_shader_images ever applied is exactly the - // seam defect the review names, and a program that declares no images never - // reaches here at all. - if (st.ShaderImageCount == 0) { - if (g_imageRecordSeamIsAuthoritative) { - MGLOG_E_ONCE("An image record does not describe the binding it names: image unit " - "%u holds a texture at a draw and no set_shader_images has ever been " - "applied; running the pre-handle image bind.", - static_cast(unit)); - } - return nullptr; - } - // P4a decline-site I3: M - a unit outside the received window has no pushed - // answer, so the funnel uses the frontend binding. SILENT, CONFIRMED AT THE - // VERIFICATION ROUND and deliberately not folded into I2's flip: nothing in the - // contract pins the window's membership (A8), so a unit outside it is not - // evidence of a defect the way an EMPTY set is. If A8's measurement ever lets C - // document the membership, this is the site that becomes an assertion. (The - // SWEEP's membership is a different question and is not this window - see - // SyncImageTextureBindings and review MAJOR-1.) - if (unit < st.ShaderImageStart || unit - st.ShaderImageStart >= st.ShaderImageCount) return nullptr; - // P4a decline-site I4: unreachable by PipeApply.h:461-464 (Start + Count above the - // bound is Fatal{ProtocolCorruption} in the applier) - kept as defence, no flip - // taken at the verification round. Unlike F5/F7 it does NOT become an assertion: - // those two are guarded by a latch this file sets and can reason about, while - // this bound belongs to the applier, and an index test that survives into a - // release build is the cheaper half of that division of labour. - if (unit >= st.BoundShaderImages.size()) return nullptr; - const MG_Pipe::MGPImageView& view = st.BoundShaderImages[unit]; - // P4a decline-site I5: S - THE IMAGE SEAM, loud at the validate point (MAJOR-3) - // and silent at the eager funnel for the reason above. No further flip taken at - // the verification round; it shares the latch with I2. - if (view.Res != g_backendTextureObjects.HandleOf(boundTexture)) { - if (g_imageRecordSeamIsAuthoritative) { - MGLOG_E_ONCE("An image record does not describe the binding it names " - "(image unit %u); running the pre-handle image bind.", - static_cast(unit)); - } - return nullptr; - } - return &view; - } -#endif - - void SyncImageTextureBinding(Uint unit) { -#ifdef TRACY_ENABLE - ZoneScopedC(TRACY_ZONECOLOR_BACKEND); -#endif - auto& imageBinding = MGB_CTX->GetImageTextureBinding(static_cast(unit)); - TrackWritableImageBufferUnit(unit, IsWritableImageBufferTexture(imageBinding)); - if (imageBinding.Texture && unit + 1 > g_imageUnitHighWaterMark) { - g_imageUnitHighWaterMark = unit + 1; - } - if (!imageBinding.Texture) { - g_GLESFuncs.glBindImageTexture(unit, 0, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); - return; - } - - auto& backendTexture = SyncTextureObjectToBackend(imageBinding.Texture, true); -#if MOBILEGL_PIPE_PUSH - // P4a e3: the four well-defined fields of the pushed record are the authority for - // this unit once the record names the same texture the unit holds (see - // ResolveShaderImageRecord). ACCESS IS DELIBERATELY NOT AMONG THEM: - // MGPImageView::Access is a Uint8 and a GL access token is not, so the record - // carries an ENCODING whose definition belongs to the client emitter and does not - // exist at the contract commit. Reading it here before that encoding is written - // down would be inventing it. The frontend value answers until then, and it is the - // same value by construction - the client copies the unit's own binding. - const MG_Pipe::MGPImageView* const record = - ResolveShaderImageRecord(unit, imageBinding.Texture.get()); -#define MGB_IMAGE_LAYERED (record ? static_cast(record->Layered) : imageBinding.Layered) -#define MGB_IMAGE_LAYER (record ? static_cast(record->Layer) : imageBinding.Layer) -#define MGB_IMAGE_LEVEL (record ? static_cast(record->Level) : imageBinding.Level) -#define MGB_IMAGE_FORMAT (record ? static_cast(record->InternalFormat) : imageBinding.Format) -#else -#define MGB_IMAGE_LAYERED imageBinding.Layered -#define MGB_IMAGE_LAYER imageBinding.Layer -#define MGB_IMAGE_LEVEL imageBinding.Level -#define MGB_IMAGE_FORMAT imageBinding.Format -#endif - const Bool layerable = SupportsLayeredImageBinding(imageBinding.Texture->GetTarget()); - const GLboolean layered = layerable ? MGB_IMAGE_LAYERED : GL_FALSE; - const GLint layer = layerable ? MGB_IMAGE_LAYER : 0; - // The bind half of the image-format widening. SyncTextureObjectToBackend has just - // allocated this texture's storage in the core carrier of its format (the call above - // is the one that marks it image-bindable), and glBindImageTexture's `format` has to - // name the storage the texture really has: a GL_RG32F bind is GL_INVALID_VALUE on - // Adreno for nineteen of the twenty-six non-core formats and on both Malis for - // twenty-five, and every driver that DOES accept a narrow texture through a wide - // image accepts it silently, reading and writing out of bounds. The frontend's own - // ImageTextureBinding keeps the application's format untouched, so - // GL_IMAGE_BINDING_FORMAT still answers what was passed in. + // COMPILED IN THE PULL BUILD TOO, and that is a deliberate exception to the D-P + // discipline the renderbuffer macros keep (each arm's expression at its original site). + // A macro cannot carry a hundred lines of format reasoning, and the alternative was the + // same hundred lines twice; the function is `static` with ONE caller in the pull build, + // so it has no symbol of its own there and the code it generates is the code that was + // written inline before. Named here rather than discovered by a G1 diff. + // NOLINTNEXTLINE(readability-function-cognitive-complexity) + static void IssueImageTextureBind(Uint unit, BackendTextureObject& backendTexture, + TextureTarget textureTarget, TextureInternalFormat textureFormat, + GLenum appFormat, GLint level, Bool layeredRequested, + GLint layerRequested, GLenum access) { + const Bool layerable = SupportsLayeredImageBinding(textureTarget); + const GLboolean layered = layerable ? static_cast(layeredRequested) : GL_FALSE; + const GLint layer = layerable ? layerRequested : 0; + // The bind half of the image-format widening. The storage sync has just allocated + // this texture's storage in the core carrier of its format (the call above is the + // one that marks it image-bindable), and glBindImageTexture's `format` has to name + // the storage the texture really has: a GL_RG32F bind is GL_INVALID_VALUE on Adreno + // for nineteen of the twenty-six non-core formats and on both Malis for twenty-five, + // and every driver that DOES accept a narrow texture through a wide image accepts it + // silently, reading and writing out of bounds. The frontend's own ImageTextureBinding + // keeps the application's format untouched, so GL_IMAGE_BINDING_FORMAT still answers + // what was passed in. // // Widened from the format the APPLICATION named rather than from the texture's own, // because GL lets the two differ inside one format class and the shader was widened @@ -2718,34 +2794,132 @@ namespace MobileGL::MG_Backend::DirectGLES { // format it asked for so that a samplerBuffer reading the same buffer texture - which // is NOT subscript-rewritten - still sees whole texels. See // BackendTextureObject::m_bufferImageSplitViewId. - GLenum bindFormat = MGB_IMAGE_FORMAT; - GLuint bindTextureId = backendTexture->GetBackendTextureId(); - if (imageBinding.Texture->GetTarget() == TextureTarget::TextureBuffer) { - if (TextureImpl::GetImageBindableBufferSplitFormat(imageBinding.Texture->GetFormat()) != - GL_UNKNOWN_MGL) { + GLenum bindFormat = appFormat; + GLuint bindTextureId = backendTexture.GetBackendTextureId(); + if (textureTarget == TextureTarget::TextureBuffer) { + if (TextureImpl::GetImageBindableBufferSplitFormat(textureFormat) != GL_UNKNOWN_MGL) { if (const GLenum boundFormatSplit = TextureImpl::GetImageBindableBufferSplitFormat( - MG_Util::ConvertGLEnumToTextureInternalFormat(MGB_IMAGE_FORMAT)); + MG_Util::ConvertGLEnumToTextureInternalFormat(appFormat)); boundFormatSplit != GL_UNKNOWN_MGL) { bindFormat = boundFormatSplit; - if (const Uint splitViewId = backendTexture->GetBufferImageSplitViewId(); + if (const Uint splitViewId = backendTexture.GetBufferImageSplitViewId(); splitViewId != 0) { bindTextureId = splitViewId; } } } - } else if (TextureImpl::GetImageBindableStorageWidening(imageBinding.Texture->GetFormat())) { + } else if (TextureImpl::GetImageBindableStorageWidening(textureFormat)) { const auto boundFormatWidening = TextureImpl::GetImageBindableStorageWidening( - MG_Util::ConvertGLEnumToTextureInternalFormat(MGB_IMAGE_FORMAT)); + MG_Util::ConvertGLEnumToTextureInternalFormat(appFormat)); if (boundFormatWidening) { bindFormat = boundFormatWidening.InternalFormat; } } - g_GLESFuncs.glBindImageTexture(unit, bindTextureId, MGB_IMAGE_LEVEL, - layered, layer, imageBinding.Access, bindFormat); -#undef MGB_IMAGE_LAYERED -#undef MGB_IMAGE_LAYER -#undef MGB_IMAGE_LEVEL -#undef MGB_IMAGE_FORMAT + g_GLESFuncs.glBindImageTexture(unit, bindTextureId, level, layered, layer, access, bindFormat); + } + +#if MOBILEGL_PIPE_PUSH + // MGPImageView::Access -> the GL token glBindImageTexture takes. The three numbers are + // MGPipeValueTypes.h's, so this is a spelling change and not a second table; anything + // else on the wire is Fatal{ProtocolCorruption, "ImageView.Access"} AT THE READER, + // which is what the value header's comment asks for (it may not log, so it answers the + // question and the caller owns the refusal). + static GLenum GLAccessForImageView(const MG_Pipe::MGPImageView& view) { + if (!MGPipeImageAccessIsValid(view.Access)) { + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"ImageView.Access\"} An image record " + "carries Access=%u at unit %u, and only 0/1/2 are defined.", + static_cast(view.Access), static_cast(view.Unit)); + std::abort(); + } + switch (MGPipeDecodeImageAccess(view.Access)) { + case MGPipeImageAccess::WriteOnly: return GL_WRITE_ONLY; + case MGPipeImageAccess::ReadWrite: return GL_READ_WRITE; + case MGPipeImageAccess::ReadOnly: + case MGPipeImageAccess::Count: break; + } + return GL_READ_ONLY; + } + + // A writable buffer image, said by the record: the access byte and the resource + // descriptor's storage kind, neither of which is a frontend read. Same question + // IsWritableImageBufferTexture asks of the frontend binding. + static Bool IsWritableImageBufferView(const MG_Pipe::MGPImageView& view) { + if (MG_Pipe::MGPipeHandleIsNull(view.Res)) return false; + if (!MGPipeImageAccessIsValid(view.Access)) return false; + if (!MGPipeImageAccessWrites(MGPipeDecodeImageAccess(view.Access))) return false; + const auto* record = PipeTextureRecordForHandle(view.Res); + return record != nullptr && + static_cast(record->Desc.StorageKind) == TextureStorageType::Buffer; + } + + // The record form. `Res` is the texture, the four well-defined fields are the record's, + // Access is decoded, and the texture's own TARGET and STORAGE FORMAT - the two + // properties the bind rules need and MGPImageView has no room for (24 bytes, no pad) - + // come off the RESOURCE DESCRIPTOR, which is where the server already keeps them. + void SyncImageTextureBinding(const MG_Pipe::MGPImageView& view) { +#ifdef TRACY_ENABLE + ZoneScopedC(TRACY_ZONECOLOR_BACKEND); +#endif + const Uint unit = static_cast(view.Unit); + if (unit >= g_writableImageBufferUnits.size()) return; + TrackWritableImageBufferUnit(unit, IsWritableImageBufferView(view)); + if (MG_Pipe::MGPipeHandleIsNull(view.Res)) { + g_GLESFuncs.glBindImageTexture(unit, 0, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); + return; + } + if (unit + 1 > g_imageUnitHighWaterMark) { + g_imageUnitHighWaterMark = unit + 1; + } + const auto* resource = PipeTextureRecordForHandle(view.Res); + if (resource == nullptr) { + // A record naming a texture the applier has no descriptor for is a seam defect, + // not a binding: there is nothing to allocate image-bindable storage from and + // nothing to widen against. Unbind rather than leave the unit on whatever the + // previous draw put there. + MGLOG_E_ONCE("MGPipe: image unit %u names texture {%u, %u}, which has no applier " + "resource record; unbinding the unit", + static_cast(unit), view.Res.Slot, view.Res.Gen); + g_GLESFuncs.glBindImageTexture(unit, 0, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); + return; + } + const auto textureFormat = static_cast(resource->Desc.InternalFormat); + const TextureTarget textureTarget = PipeTextureTargetForHandle(view.Res); + auto& backendTexture = SyncTextureToBackendByHandle(view.Res, /*imageBindableStorageRequired=*/true); + if (!backendTexture) return; + IssueImageTextureBind(unit, *backendTexture, textureTarget, textureFormat, + static_cast(view.InternalFormat), static_cast(view.Level), + view.Layered != 0, static_cast(view.Layer), + GLAccessForImageView(view)); + } +#endif // MOBILEGL_PIPE_PUSH + + // The pre-handle form: the MONOLITH one, and now purely frontend. P4a e3 read the four + // well-defined fields off the record HERE whenever ResolveShaderImageRecord could + // corroborate the identity; P5e takes that whole apparatus out (§5.4) and gives the + // record its OWN overload above, which is the arm a transport selects (ruling 1). What + // is left is what the pull build compiles and what a push-monolith build runs, and the + // values it binds are the same ones either way - the client copies the unit's own + // binding, which is the argument P4a's own comment made for reading Access here. + void SyncImageTextureBinding(Uint unit) { +#ifdef TRACY_ENABLE + ZoneScopedC(TRACY_ZONECOLOR_BACKEND); +#endif + auto& imageBinding = MGB_CTX->GetImageTextureBinding(static_cast(unit)); + TrackWritableImageBufferUnit(unit, IsWritableImageBufferTexture(imageBinding)); + if (imageBinding.Texture && unit + 1 > g_imageUnitHighWaterMark) { + g_imageUnitHighWaterMark = unit + 1; + } + if (!imageBinding.Texture) { + g_GLESFuncs.glBindImageTexture(unit, 0, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8); + return; + } + + auto& backendTexture = SyncTextureObjectToBackend(imageBinding.Texture, true); + if (!backendTexture) return; + IssueImageTextureBind(unit, *backendTexture, imageBinding.Texture->GetTarget(), + imageBinding.Texture->GetFormat(), imageBinding.Format, + static_cast(imageBinding.Level), imageBinding.Layered != 0, + static_cast(imageBinding.Layer), imageBinding.Access); } // A buffer texture bound to a WRITABLE image unit is a buffer the shader is about to @@ -2760,7 +2934,22 @@ namespace MobileGL::MG_Backend::DirectGLES { // glBindImageTexture performs: that one runs before any shader has touched the buffer, // and flagging there would pull the driver's copy over a shadow the application may // still be writing into. + // + // P5e (fb, CONTRACT-P5E.md §5.4): DELETED UNDER A TRANSPORT, and it is one edit paired + // with sb's deletion of the backend's storage-block GPU-write marks. The two were one + // mechanism: the backend told the frontend's buffer shadow "a shader wrote through + // this" because only the backend knew which bindings a dispatch could reach. Under a + // split the CLIENT owns that set and keeps it itself (MG_Impl/Pipe/GpuWritePending.h), + // so the mark has an owner on the side that reads it - and this body cannot run there + // anyway: every line of it is a frontend read (the image binding, the downcast to + // TextureObjectBuffer, the buffer binding slot) plus a HandleOfBuffer probe behind + // MarkBufferGpuWritten. The gate is at the top rather than at the two call sites so + // there is one statement of it, and the unit-tracking bookkeeping below stays out of + // reach of an apply thread entirely. void MarkWritableImageBufferTexturesGpuWritten() { +#if MOBILEGL_BUILD_DISAGGREGATED + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) return; +#endif if (g_writableImageBufferUnitCount == 0) return; for (Uint unit = 0; unit < g_writableImageBufferUnits.size(); ++unit) { if (!g_writableImageBufferUnits[unit]) continue; @@ -2837,33 +3026,45 @@ namespace MobileGL::MG_Backend::DirectGLES { if (SamplerSubsystemEnabled() && st.ShaderImageCount != 0) { // P4a decline-site I7: the window/mark UNION (MAJOR-1, fixed here); no flip // remains at the verification round, only the A8 measurement above. + // + // P5e (fb) KEEPS THE UNION and says so again, because ruling 7 / ID-86 make + // it contract text now (§5.4: "never narrowed to the window"). Under a + // transport the walk additionally READS the applier's array rather than the + // frontend binding at each unit - a unit inside the mark but outside the + // window keeps whatever MGPImageView the last set that covered it left + // there (D-J2: entries outside the window are not cleared), which is the + // record of the very binding that raised the mark. const Uint32 end = std::min( std::max(st.ShaderImageStart + st.ShaderImageCount, static_cast(g_imageUnitHighWaterMark)), static_cast(unitCount)); - // This IS the validate point - the only two callers are the draw gate and - // PrepareForCompute - so the image seam is authoritative for the length of - // the walk and only for that length. - g_imageRecordSeamIsAuthoritative = true; +#if MOBILEGL_BUILD_DISAGGREGATED + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + for (Uint32 unit = 0; unit < end; ++unit) { + if (unit >= st.BoundShaderImages.size()) break; + SyncImageTextureBinding(st.BoundShaderImages[unit]); + } + return; + } +#endif for (Uint32 unit = 0; unit < end; ++unit) { SyncImageTextureBinding(unit); } - g_imageRecordSeamIsAuthoritative = false; return; } } #endif -#if MOBILEGL_PIPE_PUSH - // The validate point either way: the only two callers are the draw gate and - // PrepareForCompute. See I2. - g_imageRecordSeamIsAuthoritative = true; +#if MOBILEGL_BUILD_DISAGGREGATED + // The wide pre-handle sweep reads the frontend binding of every unit, which an + // apply thread may not do. Under a transport there is nothing for it to do anyway: + // g_imageUnitHighWaterMark is raised only by the funnel above, so if no set has + // ever arrived no unit has ever been given an image, and re-binding 0 on units that + // never held one is the no-op the comment above already proved. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) return; #endif for (Uint unit = 0; unit < unitCount; ++unit) { SyncImageTextureBinding(unit); } -#if MOBILEGL_PIPE_PUSH - g_imageRecordSeamIsAuthoritative = false; -#endif } // What the draw path last swept the image units against. A draw never swept them at all: @@ -2878,6 +3079,15 @@ namespace MobileGL::MG_Backend::DirectGLES { static Uint64 g_imageSweepSamplingGeneration = 0; static Uint g_imageSweepBackendContextGeneration = 0; static Bool g_imageSweepValid = false; +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (fb, ruling 7 / ID-86): the record arm's three applier serials. Held in their own + // fields beside the frontend pair rather than reusing them, for the reason the two + // framebuffer list keys are held apart - one field carrying two key shapes is how a + // stale half gets compared against a live one. + static Uint64 g_imageSweepShaderImagesSerial = 0; + static Uint64 g_imageSweepTextureShutterSerial = 0; + static Uint64 g_imageSweepContextSerial = 0; +#endif // The sweep is a glBindImageTexture per unit, so it must not run per draw: the gate is the // frontend's sampling-resolution generation, which TextureObjectBase::BumpShapeVersion @@ -2899,8 +3109,45 @@ namespace MobileGL::MG_Backend::DirectGLES { // client's own NewShaderImages shutter mixes the same three frontend counters // (texture content, texture params, the program's image-unit version), so the // property is preserved on both sides by construction rather than by agreement. + // + // P5e (fb, ruling 7 / ID-86): PROPERTY 1 IS UNTOUCHED. PROPERTY 2 IS RE-KEYED, and the + // argument behind it is kept rather than dropped - it is what decides WHICH four values + // the new key may be made of. + // + // The frontend sampling-resolution generation cannot be read on an apply thread, so the + // key becomes (ShaderImagesSerial, TextureShutterSerial, ContextSerial, + // g_backendContextGeneration). Three of those are APPLIER-DERIVED - they move when the + // client's own shutters send a new set of images, new texture content or new texture + // parameters, i.e. at exactly the moments the frontend counter moved - and the fourth + // is the backend's ES-context generation, which the old key already carried. + // + // NONE OF THEM IS A BACKEND RE-MINT COUNTER, and that is the whole of the ruling: the + // hazard the old comment names is a texture bound ONLY to an image unit being re-minted + // INSIDE this sweep, which would bump a backend-side epoch after the gate had already + // declined to run. An applier serial moves when the CLIENT said something, strictly + // before the sweep the saying provoked, so the property survives the re-key intact. + // g_backendContextGeneration is not a counter of that kind either - it moves when the + // ES context is rebuilt, which no sweep does. void SyncImageTextureBindingsForDraw(const DrawTextureSyncKeys& keys) { if (g_imageUnitHighWaterMark == 0) return; +#if MOBILEGL_BUILD_DISAGGREGATED + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + const auto& st = MG_Pipe::MGPipeApplier(); + if (g_imageSweepValid && g_imageSweepShaderImagesSerial == st.ShaderImagesSerial && + g_imageSweepTextureShutterSerial == st.TextureShutterSerial && + g_imageSweepContextSerial == st.ContextSerial && + g_imageSweepBackendContextGeneration == g_backendContextGeneration) { + return; + } + SyncImageTextureBindings(); + g_imageSweepShaderImagesSerial = st.ShaderImagesSerial; + g_imageSweepTextureShutterSerial = st.TextureShutterSerial; + g_imageSweepContextSerial = st.ContextSerial; + g_imageSweepBackendContextGeneration = g_backendContextGeneration; + g_imageSweepValid = true; + return; + } +#endif if (g_imageSweepValid && g_imageSweepContextId == keys.contextId && g_imageSweepSamplingGeneration == keys.samplingGeneration && g_imageSweepBackendContextGeneration == g_backendContextGeneration) { diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 9e7b021b..875be79a 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -4376,6 +4376,26 @@ namespace MobileGL::MG_Backend::DirectGLES { return PipeRecordAt(MG_Pipe::MGPipeApplier().RenderbufferResources, res); } + MobileGL::TextureTarget PipeTextureTargetForHandle(MG_Pipe::MGPipeHandle res) { + const auto* record = PipeTextureRecordForHandle(res); + if (record == nullptr) return MobileGL::TextureTarget::Unknown; + // THE INVERSE OF MG_Pipe::MGPipeResourceTargetForTextureTarget, COMPUTED FROM IT rather + // than restated as a second table. MGPipeTypes.h already static_asserts that the + // forward map covers the frontend enum and that it separates the one pair anybody has + // ever folded together (2D vs rectangle), so a linear search over a dozen enumerators + // is total, cannot drift, and runs at most once per image unit per sweep. A second + // switch here is exactly how the two spellings end up disagreeing about a target that + // was added to only one of them. + for (SizeT i = 0; i < static_cast(MobileGL::TextureTarget::TextureTargetCount); ++i) { + const auto target = static_cast(i); + if (MG_Pipe::MGPipeResourceTargetForTextureTarget(target) == + static_cast(record->Desc.Target)) { + return target; + } + } + return MobileGL::TextureTarget::Unknown; + } + const MG_Pipe::MGPipeSamplerCsoRecord* PipeSamplerCsoRecordForHandle(MG_Pipe::MGPipeHandle cso) { return PipeRecordAt(MG_Pipe::MGPipeApplier().SamplerCsos, cso); } @@ -9655,38 +9675,29 @@ namespace MobileGL::MG_Backend::DirectGLES { // is adopted from MGPSurface::Res through the slot table instead of being looked up by // the frontend object's ADDRESS, and the attach SHAPE - layered, level, layer, the // upload target and the texture target - is read off the surface instead of off the - // frontend attachment object. What does NOT move is the storage sync itself: the twins' - // SyncMipmapsToBackend / SyncToBackend still take the frontend object, because that is - // the monolith glue this phase keeps (they read the level shadow for the texels, which - // no record carries), and because those two functions have their OWN handle arms that - // drive the storage from the descriptor. + // frontend attachment object. // - // The frontend object is therefore still handed over, and it is also CROSS-CHECKED: if - // the record's Res does not name the same twin the frontend attachment does, that is a - // seam defect - two sides that disagree about which texture is attached - and it is - // refused loudly rather than resolved in favour of either. + // P5e (fb, CONTRACT-P5E.md §5.4): THE FRONTEND ATTACHMENT PARAMETER IS GONE. It was + // here for two things and P5e retires both. The storage syncs now have by-handle forms + // (SyncMipmapsToBackendByHandle / SyncToBackendByHandle), so the object is no longer + // needed to reach the texels; and the record is now the ONLY statement of what is + // attached, so the cross-checks had nothing left to corroborate against - a second + // answer that only a monolith build could produce is not a check, it is a second + // writer. With the object gone, review N-6's hole ("record empty, frontend still + // holding") is UNREPRESENTABLE rather than refused: the caller's detach and this + // function's attach now read the SAME test, `Kind == None || Res null`. + // + // The two P5c cross-checks that ran HandleOf on the apply thread go with it. The + // renderbuffer one (the one that was NOT transport-gated, unlike its texture sibling) + // is deleted by the rekey rather than gated, which is what CONTRACT-P5E §4.4 asks for: + // gating it would have left a client-allocator probe compiled into a path a run-ahead + // apply reaches, and there is no longer anything for it to say. static Bool SyncAttachmentSurface(GLenum glFBOTarget, const MG_Pipe::MGPSurface& surface, - const MG_State::GLState::FramebufferAttachmentObject& attachmentObject, GLenum glBackendAttachment) { if (surface.Kind == MG_Pipe::kMGPipeSurfaceKindNone || MG_Pipe::MGPipeHandleIsNull(surface.Res)) { - // An empty point. The caller has already detached it where the two sides AGREE - // that the point is empty; attaching nothing is what the legacy arm does here too. - // - // Review N-6: the detach above is decided by the FRONTEND - // (attachmentObject.IsEmpty()) and the attach here by the RECORD, so the one - // direction in which neither fires is "record empty, frontend still holding an - // attachment" - no detach, no attach, and the caller then stamps this point's - // synced version, marking it done with the PREVIOUS owner's image still on the - // driver. Every other record-versus-frontend disagreement in this function is - // loud; this one was the only silent hole and it is closed here rather than - // left one direction wide. - if (!attachmentObject.IsEmpty()) { - MGLOG_E_ONCE("MGPipe: attachment record describes an EMPTY point but the frontend " - "attachment still holds an object - refusing rather than leaving the " - "previous image attached and calling the point synced"); - return false; - } + // An empty point: the caller detached it on exactly this test, so attaching + // nothing here is the whole of the agreement. (The legacy arm does the same.) return true; } if (surface.Kind == MG_Pipe::kMGPipeSurfaceKindTexture) { @@ -9697,31 +9708,6 @@ namespace MobileGL::MG_Backend::DirectGLES { if (!PipeTwinHandleIsAdoptable(TextureImpl::g_backendTextureObjects, surface.Res, "texture")) { return false; } - const auto& textureObject = attachmentObject.GetTexture(); - if (!textureObject) { - MGLOG_E_ONCE("MGPipe: attachment record names texture {%u, %u} but the frontend " - "attachment holds no texture - refusing to attach", - surface.Res.Slot, surface.Res.Gen); - return false; - } -#if MOBILEGL_BUILD_DISAGGREGATED - // P5c (hd): the corroborating cross-check against the frontend object's handle - // is a client-allocator probe (T2). Under an active transport the record is the - // ONLY statement of identity (rule E) and the check does not run; monolith - // keeps it verbatim. The record's own handle was already validated above (N-3). - if (MG_Config::Transport == MG_Config::TransportMode::Monolith && - !(TextureImpl::g_backendTextureObjects.HandleOf(textureObject.get()) == surface.Res)) { -#else - if (!(TextureImpl::g_backendTextureObjects.HandleOf(textureObject.get()) == surface.Res)) { -#endif - MGLOG_E_ONCE("MGPipe: attachment record names texture {%u, %u} but the frontend " - "attachment's texture %u is handle {%u, %u} - refusing rather than " - "attaching one of them", - surface.Res.Slot, surface.Res.Gen, textureObject->GetExternalIndex(), - TextureImpl::g_backendTextureObjects.HandleOf(textureObject.get()).Slot, - TextureImpl::g_backendTextureObjects.HandleOf(textureObject.get()).Gen); - return false; - } auto* twinSlot = AdoptTwinByHandle(TextureImpl::g_backendTextureObjects, surface.Res, "texture"); if (twinSlot == nullptr) { return false; // AdoptTwinByHandle named the refusal @@ -9737,7 +9723,10 @@ namespace MobileGL::MG_Backend::DirectGLES { MGLOG_E_ONCE("%s: No backend texture found for FBO attachment, cannot bind texture.", __func__); return false; } - backendTextureObject->SyncMipmapsToBackend(textureObject); + // P5e (fb): the storage sync BY HANDLE (tx2's seam). The level shadow it reads + // is the applier's staged texture store, not a frontend object, which is what + // let the object parameter go. + backendTextureObject->SyncMipmapsToBackendByHandle(surface.Res); const auto uploadTarget = static_cast(surface.UploadTarget); if (surface.Layered != 0) { g_GLESFuncs.glFramebufferTexture(glFBOTarget, glBackendAttachment, @@ -9778,27 +9767,6 @@ namespace MobileGL::MG_Backend::DirectGLES { "renderbuffer")) { return false; } - const auto& renderbufferObject = attachmentObject.GetRenderbuffer(); - if (!renderbufferObject) { - MGLOG_E_ONCE("MGPipe: attachment record names renderbuffer {%u, %u} but the frontend " - "attachment holds no renderbuffer - refusing to attach", - surface.Res.Slot, surface.Res.Gen); - return false; - } -#if MOBILEGL_BUILD_DISAGGREGATED - // P5c (G6, CONTRACT-P5C §5.4): the corroborating cross-check below resolves the - // frontend renderbuffer's handle by frontend identity - named debt inside the - // scope - P3b/P4b rekeys the registry onto handles. - const MG_Pipe::MGPipeFrontendKeyedRegistryScope frontendKeyedRegistry; -#endif - if (!(RenderbufferImpl::g_backendRenderbufferObjects.HandleOf(renderbufferObject.get()) == - surface.Res)) { - MGLOG_E_ONCE("MGPipe: attachment record names renderbuffer {%u, %u} but the frontend " - "attachment's renderbuffer %u is a different handle - refusing rather " - "than attaching one of them", - surface.Res.Slot, surface.Res.Gen, renderbufferObject->GetExternalIndex()); - return false; - } auto* twinSlot = AdoptTwinByHandle(RenderbufferImpl::g_backendRenderbufferObjects, surface.Res, "renderbuffer"); if (twinSlot == nullptr) { @@ -9812,7 +9780,10 @@ namespace MobileGL::MG_Backend::DirectGLES { MGLOG_E_ONCE("%s: No backend renderbuffer found for FBO attachment.", __func__); return false; } - backendRenderbufferObject->SyncToBackend(renderbufferObject); + // P5e (fb): the four allocation values were already record-supplied (D-D2); what + // moves here is the LOOKUP - the handle the surface carries, never HandleOf on + // an object the apply thread was handed. + backendRenderbufferObject->SyncToBackendByHandle(surface.Res); backendRenderbufferObject->Bind(); g_GLESFuncs.glFramebufferRenderbuffer(glFBOTarget, glBackendAttachment, GL_RENDERBUFFER, backendRenderbufferObject->GetBackendRenderbufferId()); @@ -9914,6 +9885,89 @@ namespace MobileGL::MG_Backend::DirectGLES { } #endif +#if MOBILEGL_PIPE_PUSH + // P5e (fb, §5.4): the record half of SyncReadBufferToBackend, lifted out of it so the + // by-handle sync can run it WITHOUT a frontend object. Nothing inside reads one: the + // frontend index was only ever a diagnostic, and it is passed as a number now + // (GlNameForDiag is 0 on the handle arm, which reads as "the record did not say"). + void BackendFramebufferObject::ApplyReadBufferFromRecord(const MG_Pipe::MGPFramebufferState& record, + Uint glNameForDiag) { + // D-C1 makes the NULL Res the spelling of "no attachment" and c0c's + // kMGPipeSurfaceKindNone makes Kind agree with it on a zero-initialised record. + // Res stays the test - it is the one D-C1 fixes - and Kind is CROSS-CHECKED + // rather than consulted: the two are one statement the emitter has to make + // twice, and a record where they disagree is a seam defect, not an empty point. + // Picking a winner silently is what this refuses to do. + const Bool readSurfaceEmpty = MG_Pipe::MGPipeHandleIsNull(record.ReadSurface.Res); + if (readSurfaceEmpty != (record.ReadSurface.Kind == MG_Pipe::kMGPipeSurfaceKindNone)) { + MGLOG_E_ONCE("MGPipe: framebuffer %u's resolved read surface says Res={%u, %u} and " + "Kind=%u, which disagree about whether it names anything - refusing " + "rather than choosing one of them", + glNameForDiag, record.ReadSurface.Res.Slot, record.ReadSurface.Res.Gen, + record.ReadSurface.Kind); + return; + } + FramebufferAttachmentType pushedReadBuf = FramebufferAttachmentType::None; + if (!readSurfaceEmpty) { + // EVERY FIELD THAT IDENTIFIES THE IMAGE, not the three v1 compared (m-4). + // The same image legally sits at two colour points - a layered attachment of + // one array and a single slice of it are different surfaces of one Res - so a + // partial comparison resolves to the lower index and the refusal branch below + // ("refusing to guess") would then be doing part of the guessing itself. + // InternalFormat and TextureTarget are properties of the RESOURCE rather than + // of the point, so they add nothing to the identity and are left out. + Bool matched = false; + for (Uint i = 0; i < MG_Pipe::kMGPipeMaxColorAttachments; ++i) { + const auto& color = record.Color[i]; + if (color.Res == record.ReadSurface.Res && color.Kind == record.ReadSurface.Kind && + color.Layered == record.ReadSurface.Layered && + color.Level == record.ReadSurface.Level && color.Layer == record.ReadSurface.Layer && + color.UploadTarget == record.ReadSurface.UploadTarget) { + pushedReadBuf = static_cast( + static_cast(FramebufferAttachmentType::Color0) + static_cast(i)); + matched = true; + break; + } + } + if (!matched) { + MGLOG_E_ONCE("MGPipe: framebuffer %u's resolved read surface {%u, %u} matches no " + "colour attachment of its own record - refusing to guess a read " + "buffer", + glNameForDiag, record.ReadSurface.Res.Slot, record.ReadSurface.Res.Gen); + return; + } + } + if (pushedReadBuf == m_frontendReadBuffer) { + return; + } + m_frontendReadBuffer = pushedReadBuf; + + const GLenum glPushedReadBuffer = GetBackendAttachmentType(pushedReadBuf); + if (m_backendReadBuffer != glPushedReadBuffer) { + m_backendReadBuffer = glPushedReadBuffer; + // Still bound as READ first, for the reason the legacy arm gives below: + // glReadBuffer targets whatever FBO is bound to GL_READ_FRAMEBUFFER. + Bind(FramebufferTarget::Read); + g_GLESFuncs.glReadBuffer(glPushedReadBuffer); + } + } + + // P5e (fb): the by-handle entry, for SyncCurrentFBOByRecord's "one object on both + // bindings" skip - the one path that applies a read buffer without doing the rest of + // the sync. A framebuffer with no record is the caller's seam defect, not a silent + // no-op, so it says so. + void BackendFramebufferObject::SyncReadBufferToBackendByHandle(MG_Pipe::MGPipeHandle fbo) { + const auto* record = PushedFramebufferRecord(fbo); + if (record == nullptr) { + MGLOG_E_ONCE("MGPipe: framebuffer handle {%u, %u} has no applier record, so its read " + "buffer cannot be pushed", + fbo.Slot, fbo.Gen); + return; + } + ApplyReadBufferFromRecord(*record, 0); + } +#endif // MOBILEGL_PIPE_PUSH + void BackendFramebufferObject::SyncReadBufferToBackend( const SharedPtr& stateFBOObject) { if (!stateFBOObject) { @@ -9933,6 +9987,11 @@ namespace MobileGL::MG_Backend::DirectGLES { // spelling of "no attachment" independently of Kind's numbering) is // FramebufferAttachmentType::None, i.e. GL_NONE. A surface that matches no colour // point cannot be a legal read buffer and is refused rather than guessed. + // + // P5e (fb): THAT DECISION now lives in ApplyReadBufferFromRecord, which takes no + // frontend object; what is left in this overload is the monolith glue that finds + // the handle by identity. Under an active transport nothing reaches it - + // SyncToBackendByHandle runs the record form directly. if (FramebufferSubsystemEnabled()) { // P5c (hd): with an active transport the handle is the caller's // (m_pushedSyncHandle), never the client allocator's - see SyncToBackend. @@ -9955,66 +10014,7 @@ namespace MobileGL::MG_Backend::DirectGLES { stateFBOObject->GetExternalIndex(), fbo.Slot, fbo.Gen); return; } - // D-C1 makes the NULL Res the spelling of "no attachment" and c0c's - // kMGPipeSurfaceKindNone makes Kind agree with it on a zero-initialised record. - // Res stays the test - it is the one D-C1 fixes - and Kind is CROSS-CHECKED - // rather than consulted: the two are one statement the emitter has to make - // twice, and a record where they disagree is a seam defect, not an empty point. - // Picking a winner silently is what this refuses to do. - const Bool readSurfaceEmpty = MG_Pipe::MGPipeHandleIsNull(record->ReadSurface.Res); - if (readSurfaceEmpty != (record->ReadSurface.Kind == MG_Pipe::kMGPipeSurfaceKindNone)) { - MGLOG_E_ONCE("MGPipe: framebuffer %u's resolved read surface says Res={%u, %u} and " - "Kind=%u, which disagree about whether it names anything - refusing " - "rather than choosing one of them", - stateFBOObject->GetExternalIndex(), record->ReadSurface.Res.Slot, - record->ReadSurface.Res.Gen, record->ReadSurface.Kind); - return; - } - FramebufferAttachmentType pushedReadBuf = FramebufferAttachmentType::None; - if (!readSurfaceEmpty) { - // EVERY FIELD THAT IDENTIFIES THE IMAGE, not the three v1 compared (m-4). - // The same image legally sits at two colour points - a layered attachment of - // one array and a single slice of it are different surfaces of one Res - so a - // partial comparison resolves to the lower index and the refusal branch below - // ("refusing to guess") would then be doing part of the guessing itself. - // InternalFormat and TextureTarget are properties of the RESOURCE rather than - // of the point, so they add nothing to the identity and are left out. - Bool matched = false; - for (Uint i = 0; i < MG_Pipe::kMGPipeMaxColorAttachments; ++i) { - const auto& color = record->Color[i]; - if (color.Res == record->ReadSurface.Res && color.Kind == record->ReadSurface.Kind && - color.Layered == record->ReadSurface.Layered && - color.Level == record->ReadSurface.Level && - color.Layer == record->ReadSurface.Layer && - color.UploadTarget == record->ReadSurface.UploadTarget) { - pushedReadBuf = static_cast( - static_cast(FramebufferAttachmentType::Color0) + static_cast(i)); - matched = true; - break; - } - } - if (!matched) { - MGLOG_E_ONCE("MGPipe: framebuffer %u's resolved read surface {%u, %u} matches no " - "colour attachment of its own record - refusing to guess a read " - "buffer", - stateFBOObject->GetExternalIndex(), record->ReadSurface.Res.Slot, - record->ReadSurface.Res.Gen); - return; - } - } - if (pushedReadBuf == m_frontendReadBuffer) { - return; - } - m_frontendReadBuffer = pushedReadBuf; - - const GLenum glPushedReadBuffer = GetBackendAttachmentType(pushedReadBuf); - if (m_backendReadBuffer != glPushedReadBuffer) { - m_backendReadBuffer = glPushedReadBuffer; - // Still bound as READ first, for the reason the legacy arm gives below: - // glReadBuffer targets whatever FBO is bound to GL_READ_FRAMEBUFFER. - Bind(FramebufferTarget::Read); - g_GLESFuncs.glReadBuffer(glPushedReadBuffer); - } + ApplyReadBufferFromRecord(*record, stateFBOObject->GetExternalIndex()); return; } #endif @@ -10390,7 +10390,24 @@ namespace MobileGL::MG_Backend::DirectGLES { frontendType <= FramebufferAttachmentType::Color31 && (static_cast(frontendType) - static_cast(FramebufferAttachmentType::Color0)) < g_GLESCapabilities.MaxColorAttachments; - if (isColorPoint && attachmentObject.IsEmpty() && glBackendAttachment != GL_NONE) { +#if MOBILEGL_PIPE_PUSH + // P5e (fb, §5.4): ON THE RECORD ARM THE EMPTY POINT IS THE RECORD'S, and it + // is the SAME test SyncAttachmentSurface attaches on - `Kind == None || + // Res null`. Asking the frontend here and the record there is what left + // review N-6's one-directional hole; one test closes it by construction. + const Bool pointIsEmpty = + pushedRecord != nullptr + ? [&] { + const MG_Pipe::MGPSurface* s = + PushedSurfaceForAttachment(*pushedRecord, frontendType); + return s == nullptr || s->Kind == MG_Pipe::kMGPipeSurfaceKindNone || + MG_Pipe::MGPipeHandleIsNull(s->Res); + }() + : attachmentObject.IsEmpty(); +#else + const Bool pointIsEmpty = attachmentObject.IsEmpty(); +#endif + if (isColorPoint && pointIsEmpty && glBackendAttachment != GL_NONE) { g_GLESFuncs.glFramebufferRenderbuffer(glFBOTarget, glBackendAttachment, GL_RENDERBUFFER, 0); } #if MOBILEGL_PIPE_PUSH @@ -10418,8 +10435,8 @@ namespace MobileGL::MG_Backend::DirectGLES { attachmentSynced = false; } } else { - attachmentSynced = SyncAttachmentSurface(glFBOTarget, *pushedSurface, attachmentObject, - glBackendAttachment); + attachmentSynced = + SyncAttachmentSurface(glFBOTarget, *pushedSurface, glBackendAttachment); } } else { attachmentSynced = SyncAttachmentObject(glFBOTarget, attachmentObject, glBackendAttachment); @@ -10511,6 +10528,253 @@ namespace MobileGL::MG_Backend::DirectGLES { } } +#if MOBILEGL_PIPE_PUSH + // ---- P5e (fb, CONTRACT-P5E.md §5.4): the reverse index, texture -> framebuffers ------ + // + // WHY IT EXISTS. ScopedDetachedTextureFramebufferAttachments used to answer "which + // framebuffers currently have this texture attached" by walking every live twin and + // reading each one's FRONTEND attachment array. Under P5e that walk is the last place + // the server holds a frontend framebuffer across records - id re-typed ForEachLive and + // left the StateForHandle read greppable, naming this package as the one that replaces + // it. The records already carry the answer: every applied set_framebuffer_state names + // its eleven surfaces, so the relation is maintained where the record is consumed. + // + // KEYED ON THE TEXTURE SLOT rather than on {slot, gen}. The question a detach asks is + // "does the driver have this texture id hanging off some framebuffer", the points are + // re-derived from the record on every hit, and a stale entry left by a recycled slot + // therefore costs one record walk that matches nothing. The FRAMEBUFFER handles are + // stored whole, so a recycled framebuffer slot never answers for its predecessor + // (FindByHandle refuses a moved Gen). + // + // MAINTAINED AT THE SYNC, NOT AT THE APPLY, and that is the same set rather than a + // smaller one: a framebuffer whose record was never synced has no driver attachment to + // detach, and every path that can have one runs this sync first. + namespace { + UnorderedMap> g_framebuffersByAttachedTextureSlot; + UnorderedMap> g_attachedTextureSlotsByFramebufferSlot; + + void ForgetFramebufferTextureAttachmentsLocked(Uint32 fboSlot) { + auto it = g_attachedTextureSlotsByFramebufferSlot.find(fboSlot); + if (it == g_attachedTextureSlotsByFramebufferSlot.end()) return; + for (const Uint32 textureSlot : it->second) { + auto listIt = g_framebuffersByAttachedTextureSlot.find(textureSlot); + if (listIt == g_framebuffersByAttachedTextureSlot.end()) continue; + auto& list = listIt->second; + list.erase(std::remove_if(list.begin(), list.end(), + [&](MG_Pipe::MGPipeHandle h) { return h.Slot == fboSlot; }), + list.end()); + if (list.empty()) g_framebuffersByAttachedTextureSlot.erase(listIt); + } + g_attachedTextureSlotsByFramebufferSlot.erase(it); + } + } // namespace + + void NoteFramebufferTextureAttachments(MG_Pipe::MGPipeHandle fbo, + const MG_Pipe::MGPFramebufferState& record) { + if (MG_Pipe::MGPipeHandleIsNull(fbo)) return; + ForgetFramebufferTextureAttachmentsLocked(fbo.Slot); + Vector textureSlots; + const auto note = [&](const MG_Pipe::MGPSurface& surface) { + if (surface.Kind != MG_Pipe::kMGPipeSurfaceKindTexture) return; + if (MG_Pipe::MGPipeHandleIsNull(surface.Res)) return; + if (std::find(textureSlots.begin(), textureSlots.end(), surface.Res.Slot) != textureSlots.end()) { + return; // one texture at two points is one entry + } + textureSlots.push_back(surface.Res.Slot); + g_framebuffersByAttachedTextureSlot[surface.Res.Slot].push_back(fbo); + }; + for (Uint i = 0; i < MG_Pipe::kMGPipeMaxColorAttachments; ++i) note(record.Color[i]); + note(record.Depth); + note(record.Stencil); + if (textureSlots.empty()) return; // the common Minecraft frame: renderbuffers only + g_attachedTextureSlotsByFramebufferSlot[fbo.Slot] = std::move(textureSlots); + } + + // BY VALUE, and deliberately: the caller re-binds framebuffers while it walks the + // answer, and anything that syncs one re-enters NoteFramebufferTextureAttachments and + // can rehash the map underneath an iterator. + // + // NOTHING PRUNES A DEAD FRAMEBUFFER'S ENTRY and nothing needs to. A stale handle is + // refused by FindByHandle (its Gen has moved), so it contributes nothing; and the + // moment the slot is reused, the successor's first sync clears its predecessor's rows + // here. The index is therefore bounded by the live framebuffer slot count, which is + // the same bound the twin table itself carries. + Vector FramebuffersAttachingTexture(MG_Pipe::MGPipeHandle texture) { + if (MG_Pipe::MGPipeHandleIsNull(texture)) return {}; + auto it = g_framebuffersByAttachedTextureSlot.find(texture.Slot); + if (it == g_framebuffersByAttachedTextureSlot.end()) return {}; + return it->second; + } + + // ---- P5e (fb, §5.4): BackendFramebufferObject::SyncToBackend BY HANDLE --------------- + // + // The record's ELEVEN SURFACES ARE THE POINT SET. The object form walks the frontend's + // 41 attachment points to learn which exist; this one does not have to, because the + // emitter REFUSES a framebuffer holding a point at or above the wire width + // (FramebufferEmit.h's two refusals) and the FRONT/BACK tokens belong to the default + // framebuffer, which has no twin. So Color[0..7] + Depth + Stencil is the whole of it, + // and a framebuffer that could not be described that way never produced a record at all. + // + // THE PER-POINT VERSION MEMO IS GONE and is not replaced. On the object form it is a + // second, narrower gate under the record hash, hedging against "a frontend attachment + // moved without the record moving" - which is unrepresentable once the record is the + // only statement of what is attached (the hash covers every surface, and Fbo is inside + // it, so a recycled handle cannot be suppressed against its predecessor). Keeping it + // would have meant keeping the frontend array that carries it, which is the whole point + // of this arm. The two SERVER-owned gates stay, because no client-side value can answer + // what they answer: g_attachmentBackendIdGeneration ("did I re-mint a driver id") and + // the re-entry it drives. + void BackendFramebufferObject::SyncToBackendByHandle(MG_Pipe::MGPipeHandle fbo, + FramebufferTarget asTarget) { +#ifdef TRACY_ENABLE + ZoneScopedC(TRACY_ZONECOLOR_BACKEND); +#endif + const MG_Pipe::MGPFramebufferState* const recordPtr = PushedFramebufferRecord(fbo); + if (recordPtr == nullptr) { + MGLOG_E_ONCE("MGPipe: framebuffer handle {%u, %u} has no applier record, so it is not " + "configured here; nothing is bound by this refusal, but the caller's own " + "bind still targets it", + fbo.Slot, fbo.Gen); + return; + } + const MG_Pipe::MGPFramebufferState& record = *recordPtr; + if (record.IsDefault != 0) { + // The default framebuffer has no twin (pDefaultFramebufferInfo->defaultFBO is the + // frontend object and MGPipeHandles.h reserves {0,1} for it), so reaching this + // body with one is a caller that skipped its own IsDefault branch. + MGLOG_E_ONCE("MGPipe: framebuffer handle {%u, %u} describes the DEFAULT framebuffer, " + "which has no twin to configure", + fbo.Slot, fbo.Gen); + return; + } +#if MOBILEGL_BUILD_DISAGGREGATED + // Kept in step for the monolith-glue overloads that still resolve their record + // through it (SyncReadBufferToBackend's push arm). + m_pushedSyncHandle = fbo; +#endif + const GLenum glFBOTarget = MG_Util::ConvertFramebufferTargetToGLEnum(asTarget); + Bind(asTarget); + + FramebufferObject::FramebufferAttachmentArray stateDrawBuffers{}; + DecodePushedDrawBuffers(record, stateDrawBuffers.data()); + + // 1. Remap draw buffers. Same rule as the object form: glDrawBuffers writes the + // DRAW-bound framebuffer's state, so a READ-only sync must neither issue it nor + // stamp the memo (the Minecraft 26.x OIT bug the object form names). + Bool attachmentPointsMoved = false; + const Bool drawBufferClean = + memcmp(m_frontendDrawBuffers, stateDrawBuffers.data(), + FramebufferObject::MAX_DRAW_BUFFERS * sizeof(FramebufferAttachmentType)) == 0; + if (!drawBufferClean && asTarget == FramebufferTarget::Draw) { + memcpy(m_frontendDrawBuffers, stateDrawBuffers.data(), + FramebufferObject::MAX_DRAW_BUFFERS * sizeof(FramebufferAttachmentType)); + std::fill(m_backendDrawBuffers, m_backendDrawBuffers + FramebufferObject::MAX_DRAW_BUFFERS, GL_NONE); + int nEffectiveBuffers = 0; + for (GLint i = 0; i < FramebufferObject::MAX_DRAW_BUFFERS; ++i) { + const auto frontendBuf = stateDrawBuffers[i]; + if (frontendBuf == FramebufferAttachmentType::None) { + m_backendDrawBuffers[i] = GL_NONE; + continue; + } + // DecodePushedDrawBuffers cannot produce a FRONT/BACK token - those are the + // default framebuffer's own and it has no twin - so the object form's + // "shouldn't remap" branch has no counterpart here. + m_backendDrawBuffers[i] = GL_COLOR_ATTACHMENT0 + i; + nEffectiveBuffers = i + 1; + } + g_GLESFuncs.glDrawBuffers(nEffectiveBuffers, m_backendDrawBuffers); + if (RecomputeBackendColorSlots(stateDrawBuffers)) { + m_frontendReadBuffer = FramebufferAttachmentType::Unknown; + attachmentPointsMoved = true; + } + } + + // The four cross-object masks, off the record's own surfaces (ID-12 DV-5). + if (asTarget == FramebufferTarget::Draw) { + Uint32 snormClampOutputMask = 0; + Uint32 unormClampOutputMask = 0; + Uint32 alphaWidenedMask = 0; + Uint32 integerColorMask = 0; + for (Uint i = 0; i < FramebufferObject::MAX_DRAW_BUFFERS && i < 32; ++i) { + const auto frontendBuf = stateDrawBuffers[i]; + if (frontendBuf < FramebufferAttachmentType::Color0 || + frontendBuf > FramebufferAttachmentType::Color31) { + continue; + } + const MG_Pipe::MGPSurface* surface = PushedSurfaceForAttachment(record, frontendBuf); + if (surface == nullptr) continue; // unrepresentable: DecodePushedDrawBuffers' range + if (IsSnormFallbackSurface(*surface)) { + snormClampOutputMask |= (1u << i); + } else if (IsUnormFallbackSurface(*surface)) { + unormClampOutputMask |= (1u << i); + } + if (IsAlphaWidenedColorSurface(*surface)) alphaWidenedMask |= (1u << i); + if (IsIntegerColorSurface(*surface)) integerColorMask |= (1u << i); + } + PrgramImpl::g_snormFallbackClampOutputMask = snormClampOutputMask; + PrgramImpl::g_unormFallbackClampOutputMask = unormClampOutputMask; + g_alphaWidenedDrawBufferMask = alphaWidenedMask; + g_integerColorDrawBufferMask = integerColorMask; + } + + // 2. Remap read buffer, READ-target only, for the reason the object form gives. + if (asTarget == FramebufferTarget::Read) { + ApplyReadBufferFromRecord(record, 0); + } + + // 3. The attachments. Two gates, both server-owned. + const Bool idGenerationMoved = m_syncedBackendIdGeneration != g_attachmentBackendIdGeneration; + if (idGenerationMoved) { + m_syncedBackendIdGeneration = g_attachmentBackendIdGeneration; + } + if (idGenerationMoved || attachmentPointsMoved || + m_syncedRecordHashes[SizeT(asTarget)] != record.ContentHash) { + const auto applyPoint = [&](FramebufferAttachmentType point, + const MG_Pipe::MGPSurface& surface) { + const Bool isColorPoint = point >= FramebufferAttachmentType::Color0 && + point <= FramebufferAttachmentType::Color31; + const GLenum glBackendAttachment = + isColorPoint ? GetBackendAttachmentType(point) + : MG_Util::ConvertFramebufferAttachmentTypeToGLEnum(point); + if (glBackendAttachment == GL_NONE || glBackendAttachment == GL_UNKNOWN_MGL) return; + const Bool empty = surface.Kind == MG_Pipe::kMGPipeSurfaceKindNone || + MG_Pipe::MGPipeHandleIsNull(surface.Res); + // The detach that keeps m_backendColorSlots a permutation of the PHYSICAL + // layout: a colour point whose record says empty must not keep the previous + // owner's image, or glReadBuffer returns it. Bounded by the driver's cap, + // because GL_COLOR_ATTACHMENTn above it is INVALID_ENUM. + if (isColorPoint && empty) { + const Int index = + static_cast(point) - static_cast(FramebufferAttachmentType::Color0); + if (index < g_GLESCapabilities.MaxColorAttachments) { + g_GLESFuncs.glFramebufferRenderbuffer(glFBOTarget, glBackendAttachment, + GL_RENDERBUFFER, 0); + } + return; + } + SyncAttachmentSurface(glFBOTarget, surface, glBackendAttachment); + }; + for (Uint i = 0; i < MG_Pipe::kMGPipeMaxColorAttachments; ++i) { + applyPoint(static_cast( + static_cast(FramebufferAttachmentType::Color0) + static_cast(i)), + record.Color[i]); + } + applyPoint(FramebufferAttachmentType::Depth, record.Depth); + applyPoint(FramebufferAttachmentType::Stencil, record.Stencil); + m_syncedRecordHashes[SizeT(asTarget)] = record.ContentHash; + NoteFramebufferTextureAttachments(fbo, record); + } + + // The walk itself can re-mint a driver id (the storage sync behind a texture + // surface), invalidating points this walk already attached. Re-enter until the + // generation is quiescent, exactly as the object form does - every pass syncs each + // dirty texture clean, so each repeat finds strictly fewer re-mints. + if (m_syncedBackendIdGeneration != g_attachmentBackendIdGeneration) { + SyncToBackendByHandle(fbo, asTarget); + } + } +#endif // MOBILEGL_PIPE_PUSH + GLenum BackendFramebufferObject::GetBackendAttachmentType(FramebufferAttachmentType frontendAtt) const { // Only colour attachments are ever relocated; depth/stencil, the default framebuffer's // FRONT/BACK names and None map straight through. @@ -10950,6 +11214,18 @@ namespace MobileGL::MG_Backend::DirectGLES { // outside the frontend's array, which cannot be addressed at all. Uint BoundImageUnitFormat(Int unit) { if (unit < 0 || unit >= MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS) return 0; +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (fb, §5.4): the format-less image bake asks what format the APPLICATION + // named at this unit, and under a transport the record is the only statement of + // it - MGPImageView::InternalFormat is that same GLenum, copied by the client + // out of the unit's own binding. A unit the applier has never been told about + // answers 0, which is what an unbound unit answers on the frontend arm too. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + const auto& st = MG_Pipe::MGPipeApplier(); + if (static_cast(unit) >= st.BoundShaderImages.size()) return 0; + return static_cast(st.BoundShaderImages[static_cast(unit)].InternalFormat); + } +#endif return static_cast(MGB_CTX->GetImageTextureBinding(unit).Format); } @@ -13798,6 +14074,85 @@ namespace MobileGL::MG_Backend::DirectGLES { #undef MGB_RBO_HEIGHT #undef MGB_RBO_SAMPLES +#if MOBILEGL_PIPE_PUSH + // P5e (fb, CONTRACT-P5E.md §5.4): the same allocation, keyed on the renderbuffer HANDLE. + // + // THE FOUR VALUES WERE ALREADY THE RECORD'S at P4a (D-D2) - what the object form still + // needed the frontend for was the LOOKUP, `HandleOf(stateRBOObject.get())`, a + // client-allocator probe on the apply thread. On this arm the handle arrives from the + // attachment surface that named it, so nothing is probed and nothing is dereferenced. + // An OVERLOAD rather than a changed signature (the monolith glue keeps its tokens). + void BackendRenderbufferObject::SyncToBackendByHandle(MG_Pipe::MGPipeHandle renderbuffer) { +#ifdef TRACY_ENABLE + ZoneScopedC(TRACY_ZONECOLOR_BACKEND); +#endif + const MG_Pipe::MGPipeResourceRecord* const pushedRecord = + PipeRenderbufferRecordForHandle(renderbuffer); + if (pushedRecord == nullptr) { + MGLOG_E_ONCE("MGPipe: renderbuffer handle {%u, %u} has no applier record, so its " + "storage cannot be allocated from the pushed descriptor", + renderbuffer.Slot, renderbuffer.Gen); + return; + } + if (m_isInitialized && m_syncedResourceSerial != 0 && + m_syncedResourceSerial == pushedRecord->Serial) { + return; + } + + // A RE-STORAGE IS A REDEFINITION ON THE SAME DRIVER ID (P4a fable seam F-3): the + // allocation below re-defines the storage behind the name the framebuffer twins + // already attached, and no record of THEIRS moves for it, so the attachment memo + // has to be re-armed the same way a texture re-mint arms it. The first allocation + // is not a redefinition. + if (m_isInitialized) { + ++FramebufferImpl::g_attachmentBackendIdGeneration; + } + + Bind(); + + const auto internalFormat = static_cast(pushedRecord->Desc.InternalFormat); + const Int width = static_cast(pushedRecord->Desc.Width); + const Int height = static_cast(pushedRecord->Desc.Height); + const Int samples = static_cast(pushedRecord->Desc.Samples); + GLenum glInternalFormat, glType, glFormat; + TextureImpl::GenerateRenderbufferFormatInfo(internalFormat, &glInternalFormat, &glFormat, &glType); + + // Drain first, for the object form's reason: a driver that refuses the allocation + // (a multi-gigabyte renderbuffer is refused routinely) used to leave the twin + // claiming storage it does not have, and the attachment then rendered nowhere. + DebugImpl::ErrorLopper::Clear(); + if (samples > 0) { + const auto backendSamples = static_cast(ClampSamplesToBackendSupport( + GetRenderbufferFormatCapabilityTargetIndex(), internalFormat, glFormat, samples)); + g_GLESFuncs.glRenderbufferStorageMultisample(GL_RENDERBUFFER, backendSamples, glInternalFormat, + static_cast(width), + static_cast(height)); + } else { + g_GLESFuncs.glRenderbufferStorage(GL_RENDERBUFFER, glInternalFormat, static_cast(width), + static_cast(height)); + } + if (g_GLESFuncs.glGetError() == GL_OUT_OF_MEMORY) { + MGLOG_E_ONCE("Renderbuffer {%u, %u} storage allocation ran out of memory: %dx%d, " + "samples=%d, format=%s", + renderbuffer.Slot, renderbuffer.Gen, width, height, samples, + MG_Util::ConvertGLEnumToString(glInternalFormat).c_str()); + // NO RecordError HERE. The object form reports the OOM to the application + // through the live GLContext; on this arm there is no application on this side + // of the wire to report it to, and MGB_CTX->RecordError is itself one of the + // BARRIER-PULLED rows (FieldOwnership.def) that rule F forbids an unbarriered + // apply to touch. The client raises its own errors from its own validator. + } + DebugImpl::ErrorLopper::Clear(); + + m_cacheInternalFormat = internalFormat; + m_cacheWidth = width; + m_cacheHeight = height; + m_cacheSamples = samples; + m_syncedResourceSerial = pushedRecord->Serial; + m_isInitialized = true; + } +#endif // MOBILEGL_PIPE_PUSH + TwinRegistry g_backendRenderbufferObjects; } // namespace RenderbufferImpl @@ -13850,13 +14205,10 @@ namespace MobileGL::MG_Backend::DirectGLES { } } // namespace TextureImpl - namespace FramebufferImpl { - void BackendFramebufferObject::SyncToBackendByHandle(MG_Pipe::MGPipeHandle fbo, - FramebufferTarget asTarget) { - (void)asTarget; - MGPipeP5eSeamNotLanded("BackendFramebufferObject::SyncToBackendByHandle", "fb", fbo); - } - } // namespace FramebufferImpl + // FramebufferImpl::BackendFramebufferObject::SyncToBackendByHandle and + // RenderbufferImpl::BackendRenderbufferObject::SyncToBackendByHandle LANDED with package fb + // (P5e); their bodies are beside the object forms they overload, where the attachment walk + // and the storage allocation they share can be read side by side. namespace PrgramImpl { void BackendProgramObjectImpl::SyncToBackendByHandle(MG_Pipe::MGPipeHandle cso) { @@ -13864,11 +14216,5 @@ namespace MobileGL::MG_Backend::DirectGLES { } } // namespace PrgramImpl - namespace RenderbufferImpl { - void BackendRenderbufferObject::SyncToBackendByHandle(MG_Pipe::MGPipeHandle renderbuffer) { - MGPipeP5eSeamNotLanded("BackendRenderbufferObject::SyncToBackendByHandle", "fb", - renderbuffer); - } - } // namespace RenderbufferImpl #endif // MOBILEGL_PIPE_PUSH } // namespace MobileGL::MG_Backend::DirectGLES diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index f0f426d8..256f5f0e 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -724,6 +724,13 @@ 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); + // 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 + // to the wire. TextureTarget::Unknown when the handle names no live texture record, which + // every caller treats as "decline", never as a default. + MobileGL::TextureTarget PipeTextureTargetForHandle(MG_Pipe::MGPipeHandle res); + // The pending-upload entry the applier accumulated for this (uploadTarget, level) of this // texture record, or null (D-D5). SERVER-SIDE STATE, and that is the whole point: the // client clears its own dirty flags at EMISSION for the levels the applier accepted, while @@ -1934,6 +1941,13 @@ namespace MobileGL::MG_Backend::DirectGLES { // still run when SyncCurrentFBO skips the READ-target sync because the same GL FBO is // bound as both draw and read (otherwise glReadBuffer changes would be silently dropped). void SyncReadBufferToBackend(const SharedPtr& stateFBOObject); +#if MOBILEGL_PIPE_PUSH + // P5e (fb, §5.4): the same read-buffer push keyed on the handle, for the one path + // that applies a read buffer without doing the rest of the sync - SyncCurrentFBO's + // "one object is bound to BOTH bindings" skip, where the DRAW pass already did the + // attachment work and only glReadBuffer is READ-target-specific. + void SyncReadBufferToBackendByHandle(MG_Pipe::MGPipeHandle fbo); +#endif #if MOBILEGL_BUILD_DISAGGREGATED // P5c (hd, CONTRACT-P5C §3.2): the framebuffer handle the CURRENT sync is keyed on. // A caller applying a record sets it before SyncToBackend / SyncReadBufferToBackend, @@ -2023,6 +2037,11 @@ namespace MobileGL::MG_Backend::DirectGLES { // did. 0 is never a live hash (a computed 0 is remapped to 1 by the client's // suppressor), so a zeroed memo is a guaranteed miss. Array m_syncedRecordHashes = {0}; + // P5e (fb): the read-buffer decision, taken from the record alone. Both + // SyncReadBufferToBackend overloads funnel through it, so the rule lives in one + // place and the object form is visibly the half that only finds the handle. + // glNameForDiag is 0 on the handle arm, which reads as "the record did not say". + void ApplyReadBufferFromRecord(const MG_Pipe::MGPFramebufferState& record, Uint glNameForDiag); #endif }; @@ -2054,6 +2073,23 @@ namespace MobileGL::MG_Backend::DirectGLES { // describes an object without claiming any binding for it, so asking the record would // give the wrong answer by construction. Bool PushedFramebufferIsBoundTo(FramebufferTarget target, MG_Pipe::MGPipeHandle fbo); + + // ---- P5e (fb, CONTRACT-P5E.md §5.4): the reverse index, texture -> framebuffers ------ + // + // "Which framebuffers currently have this texture attached." The detach walk + // (ScopedDetachedTextureFramebufferAttachments) used to answer it by iterating every + // live twin and reading each one's FRONTEND attachment array, which is the last place + // the server held a frontend framebuffer across records - id re-typed ForEachLive and + // named this package as the one that replaces the read. The relation is maintained + // where the record is consumed (SyncToBackendByHandle's attachment walk), because the + // record's eleven surfaces already say it. + // + // The index answers about a TEXTURE SLOT and returns whole FRAMEBUFFER handles, so a + // recycled framebuffer slot never answers for its predecessor. See the definition for + // why nothing prunes a dead row. + void NoteFramebufferTextureAttachments(MG_Pipe::MGPipeHandle fbo, + const MG_Pipe::MGPFramebufferState& record); + Vector FramebuffersAttachingTexture(MG_Pipe::MGPipeHandle texture); #endif // True when the read buffer names a fixed-point (norm/snorm) attachment that the // backend actually stores in a floating-point format. GL clamps a read from a From 6c37c20f5938839a57c8e906ca3dde6bfe700dbc Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 08:02:24 -0400 Subject: [PATCH 2/6] [MG_Backend] (Disaggregated): P5e fb - the FBO doors, both attachment lists, the binds, the blit and the detach walk read records MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The doors: RefuseFramebufferBindingSlotRead raises Fatal{UnmigratedPipeInput, "GetFramebufferBindingSlot@"} where a decline used to fall through to the pre-handle arm. Under a transport with the framebuffer bit set the fallback is not a safe default - it would read a row belonging to a client this apply is no longer synchronous with - so SyncCurrentFBO's and BindCurrentFBO's declines become the detector CONTRACT-P5E §5.4 asks for. - The two attachment texture lists are the package's core: the one unconditional per-draw live read of this family, taken whether the memo hit or missed. They are now keyed {Fbo, ContentHash, ContextSerial, g_backendContextGeneration} with entries {Res, twin} that borrow nothing, and the dedupe asks the two BOUND HANDLES rather than two pointers. Three frontend reads go in one move: both binding slots, the FramebufferObject the entries borrowed their attachment slots from, and GetTextureContextId. - SyncCurrentFBOByRecord drives the twin through SyncToBackendByHandle / SyncReadBufferToBackendByHandle; NoteStateForHandle is gone, which was the server's last cross-record hold on a frontend framebuffer. - FramebufferRecordMatchesBinding is monolith-only: the record was resolved from the applier's own bound handle, so there is no second answer left to corroborate. - ForceBindCurrentFBO reads the binding slot INSIDE the else instead of on both sides of its own handle arm (S3 finding #7); the three legacy stamps go with it, which is safe for the reason the serial memo is invalidated rather than stamped there. - SyncAndBindFramebufferByHandle configures from the record, so a DSA framebuffer that was never bound is configured correctly instead of being named and bound unconfigured. - Both blit arms take two const MGPFramebufferState&: draw buffers from DrawBuffers[], the read buffer by matching ReadSurface against Color[], kind/level/layer/layered/ format from the surface, Samples from the record, the textures by handle. - The detach walk uses the reverse index and the records instead of ForEachLive + StateForHandle + the frontend attachment array. The frontend-keyed scope stays at this site: every caller reaches it from a barriered row (§4.4). - SyncRenderbufferObjectToBackend refuses by name under a transport - its only caller is the copy-image endpoint, whose renderbuffer form both the emitter and the sink already refuse - and the scope at that site is deleted. - Clear's debug-only draw-FBO diagnostic reads the record under a transport. --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 587 ++++++++++++++---- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 4 +- MobileGL/MG_Backend/DirectGLES/Managers.h | 11 + 3 files changed, 487 insertions(+), 115 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index ba047d51..2168cf7b 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -2570,7 +2570,7 @@ namespace MobileGL::MG_Backend::DirectGLES { // registry keeps a backend object alive until its frontend texture expires, which an // attached texture cannot. A renderbuffer-only FBO - the common Minecraft frame - // reduces to the key compare and an empty loop. -#if MOBILEGL_BUILD_DISAGGREGATED +#if MOBILEGL_PIPE_PUSH && MOBILEGL_BUILD_DISAGGREGATED // P5e (fb): under a transport with the framebuffer bit set, BOTH attachment lists // are answered from the two records and neither binding slot is touched. The // pre-handle pair below stays compiled and stays the monolith path (ruling 1). @@ -3261,8 +3261,19 @@ namespace MobileGL::MG_Backend::DirectGLES { // retires, kept here ONLY as a consistency check and only until package D's twin API // takes the record instead of the object - at which point there is no second answer // left to disagree with. + // + // P5e (fb, CONTRACT-P5E.md §5.4): MONOLITH-ONLY. The sentence above ends "...and only + // until package D's twin API takes the record instead of the object - at which point + // there is no second answer left to disagree with". That point is this package: the + // twin's sync takes the handle, the record was resolved from the applier's OWN bound + // handle, and the only thing this could still consult is the binding slot - a + // BARRIER_PULLED row an apply thread may not read. So under a transport the check does + // not run at all, rather than running on a value it is not allowed to have. static Bool FramebufferRecordMatchesBinding(FramebufferTarget target, const MG_Pipe::MGPFramebufferState& record) { +#if MOBILEGL_BUILD_DISAGGREGATED + if (FramebufferRecordArmIsMandatory()) return true; +#endif const auto& bound = GetFramebufferBindingSlotChecked(target).GetBoundObject(); const Bool boundIsDefault = !bound || bound == MG_Impl::GLImpl::FramebufferImpl::pDefaultFramebufferInfo->defaultFBO; @@ -3388,11 +3399,10 @@ namespace MobileGL::MG_Backend::DirectGLES { continue; } - // MONOLITH GLUE, and named as such: the twin's SyncToBackend still reads the - // frontend object (package D re-keys its BODY onto the record; this package - // owns the call site and the decisions around it). The IDENTITY is already the - // handle - FindByHandle(record.Fbo) - so a recycled FBO cannot be mistaken for - // its predecessor here. + // MONOLITH GLUE, and named as such: on the monolith arm the twin's SyncToBackend + // still reads the frontend object, and the bound object is still this call's + // ARGUMENT. The IDENTITY is already the handle - GetOrCreateByHandle(record.Fbo) - + // so a recycled FBO cannot be mistaken for its predecessor here. // // A1, TAKEN AT THE VERIFICATION ROUND AGAINST WHAT D ACTUALLY BUILT. The twin // is resolved BY HANDLE ONLY: `BackendPtr* GetOrCreateByHandle(MGPipeHandle)` @@ -3400,47 +3410,19 @@ namespace MobileGL::MG_Backend::DirectGLES { // GetOrCreate(StatePtr) returns, deliberately, because it has three ways to // decline - the legacy arm, a slot past the table's sanity bound, and a // generation BEHIND the live entry's - and every one of them has to be visible - // here rather than answered with a parked twin. So the FindByHandle-then- - // GetOrCreate(currentFBO) pair is gone and with it the last place on this arm - // where a twin could be minted against the bound frontend ADDRESS. + // here rather than answered with a parked twin. // - // WHAT DID NOT GO, against the review's A1 sketch, and why: - // * `slot.GetBoundObject()` STAYS. A2's `#else` half never fired - D left - // SyncToBackend(SharedPtr, target) and - // SyncReadBufferToBackend(SharedPtr) textually - // unchanged - so the frontend object is still this call's ARGUMENT. Only - // the twin LOOKUP moved to the handle. - // * FramebufferRecordMatchesBinding (F3) STAYS, and the §2 table's "keep - // verbatim; it is the wording §8.2 greps" is the ruling that governs: with - // the twin now resolved from the record and configured FROM THE BOUND - // OBJECT, the identity check is the only thing standing between a - // mis-keyed record and one framebuffer's attachments written into - // another's twin. Removing it would delete the check that makes this - // rewrite safe. + // P5e (fb, CONTRACT-P5E.md §5.4): AND ON THE RECORD ARM THE OBJECT IS GONE TOO. + // `SyncToBackendByHandle(record.Fbo, target)` takes the record's own handle, so + // there is no binding slot to read, no object to hand over, and no state note + // to leave behind - NoteStateForHandle was the server's last cross-record hold + // on a frontend framebuffer, and the named blit that needed it now reads the + // record too. What is left frontend under a transport is nothing at all. // // THE POINTER-INVALIDATION CONTRACT (Managers.h ~381-385): a handle-arm result // is a stable array element that only a table-GROWING GetOrCreate can move. // GetOrCreateByHandle is exactly such a call, so its result is used and dropped // inside this iteration and never held across another registry call. - auto& slot = GetFramebufferBindingSlotChecked(target); - const auto& currentFBO = slot.GetBoundObject(); - // P4a decline-site F4: S - FLIPPED AT THE VERIFICATION ROUND to the FULL - // fallback the review specified. Still unreachable in practice - // (FramebufferRecordMatchesBinding maps "nothing bound" to boundIsDefault and - // a non-default record is already rejected above), but the `continue` it used - // to take would have left the other target half-run against section 1's - // invariant that a decline is a whole-arm decline. - if (!currentFBO) { - MGLOG_E_ONCE("A framebuffer record does not describe the binding it names: the " - "%s record names {slot %u, gen %u} but no FBO is bound to that " - "binding; running the pre-handle framebuffer sync.", - target == FramebufferTarget::Read ? "READ" : "DRAW", - static_cast(record.Fbo.Slot), - static_cast(record.Fbo.Gen)); - g_fboRecordsTrusted = false; - return false; - } - auto* const twinSlot = g_backendFramebufferObjects.GetOrCreateByHandle(record.Fbo); // A1's third decline. Bit 9 implies bit 10 implies bit 7, so the slot tables are // armed whenever this function runs and the legacy-arm cause cannot fire here; @@ -3465,28 +3447,14 @@ namespace MobileGL::MG_Backend::DirectGLES { if (!backendObj) { backendObj = MakeShared(); } -#if MOBILEGL_BUILD_DISAGGREGATED - // P5c (hd): with an active transport the twin's sync keys its applier-record - // lookup on the record's own handle (m_pushedSyncHandle), never on the client - // allocator (T2), and the table remembers which frontend object the twin is - // synced from so a later handle-only resolution (the named blit) can reach it. - if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { - backendObj->m_pushedSyncHandle = record.Fbo; - g_backendFramebufferObjects.NoteStateForHandle(record.Fbo, currentFBO); - } -#endif - // THE "SAME FBO AS DRAW" SKIP IS NOW A FIELD, not a pointer comparison against - // the object the previous iteration happened to sync. Target = Both is the - // client saying one object is bound to both bindings, so the attachment and + // THE "SAME FBO AS DRAW" SKIP IS A FIELD, not a pointer comparison against the + // object the previous iteration happened to sync. Target = Both is the client + // saying one object is bound to both bindings, so the attachment and // draw-buffer work the DRAW pass already did is not repeated - and the read // buffer, which is READ-target-specific and is what that skip used to drop, is // applied unconditionally on this path. // - // MINOR-1, CORRECTED: the read buffer is applied by SyncReadBufferToBackend - // FROM THE FRONTEND OBJECT, not from MGPFramebufferState::ReadSurface - this - // package reads Fbo, IsDefault, DrawBuffers[] and ContentHash and nothing else. - // // ID-27 (wire review v2, MAJOR-1): THE QUESTION IS ASKED OF THE BOUND HANDLES, // NOT OF THE RECORD'S STORED TARGET, and it is no longer possible to ask it any // other way in this file. `MGPFramebufferState::Target` is the target of the @@ -3499,15 +3467,53 @@ namespace MobileGL::MG_Backend::DirectGLES { // are non-null here (F2 rejected a null on either), and a handle compares by // {slot, gen}, so a recycled slot is not its predecessor. // - // Reading the stored Target degraded to a redundant per-frame read-buffer sync - // rather than to a wrong picture, which is exactly why it needed replacing here - // instead of being caught by a lane: `Target != Both` takes the full - // SyncToBackend path below, which is correct and merely repeats the draw pass's - // attachment work. The wire review found it by reading the emitter. + // MINOR-1's correction is RETIRED by P5e: the read buffer is no longer applied + // "FROM THE FRONTEND OBJECT" on this arm - ApplyReadBufferFromRecord resolves it + // out of MGPFramebufferState::ReadSurface, which was already the record's + // answer, and the by-handle entry is what reaches it here. const auto& boundHandles = st.BoundFramebuffer; - if (target == FramebufferTarget::Read && + const Bool sameObjectOnBothBindings = + target == FramebufferTarget::Read && boundHandles[SizeT(MG_Pipe::MGPipeFramebufferTarget::Draw)] == - boundHandles[SizeT(MG_Pipe::MGPipeFramebufferTarget::Read)]) { + boundHandles[SizeT(MG_Pipe::MGPipeFramebufferTarget::Read)]; + +#if MOBILEGL_BUILD_DISAGGREGATED + if (FramebufferRecordArmIsMandatory()) { + if (sameObjectOnBothBindings) { + backendObj->SyncReadBufferToBackendByHandle(record.Fbo); + } else { + backendObj->SyncToBackendByHandle(record.Fbo, target); + } + StampSyncedFramebufferSerial(target, st.FramebufferSerial); + continue; + } +#endif + auto& slot = GetFramebufferBindingSlotChecked(target); + const auto& currentFBO = slot.GetBoundObject(); + // P4a decline-site F4: S - FLIPPED AT THE VERIFICATION ROUND to the FULL + // fallback the review specified. Still unreachable in practice + // (FramebufferRecordMatchesBinding maps "nothing bound" to boundIsDefault and + // a non-default record is already rejected above), but the `continue` it used + // to take would have left the other target half-run against section 1's + // invariant that a decline is a whole-arm decline. + if (!currentFBO) { + MGLOG_E_ONCE("A framebuffer record does not describe the binding it names: the " + "%s record names {slot %u, gen %u} but no FBO is bound to that " + "binding; running the pre-handle framebuffer sync.", + target == FramebufferTarget::Read ? "READ" : "DRAW", + static_cast(record.Fbo.Slot), + static_cast(record.Fbo.Gen)); + g_fboRecordsTrusted = false; + return false; + } +#if MOBILEGL_BUILD_DISAGGREGATED + // P5c (hd): the push-monolith twin keys its applier-record lookup on the + // record's own handle (m_pushedSyncHandle) rather than on the client allocator. + // The state note is GONE with P5e: nothing resolves a framebuffer by handle and + // then asks the table for an object any more. + backendObj->m_pushedSyncHandle = record.Fbo; +#endif + if (sameObjectOnBothBindings) { backendObj->SyncReadBufferToBackend(currentFBO); StampSyncedFramebufferSerial(target, st.FramebufferSerial); continue; @@ -3536,6 +3542,20 @@ namespace MobileGL::MG_Backend::DirectGLES { // IS D's FramebufferSubsystemEnabled() now. Silent, confirmed at the // verification round. if (FramebufferSubsystemEnabled() && SyncCurrentFBOByRecord()) return; +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (fb, CONTRACT-P5E.md §5.4): AND UNDER A TRANSPORT THE DECLINE IS THE END OF + // IT. The fallback below reads GetFramebufferBindingSlot, a BARRIER_PULLED row; the + // value it would read belongs to a client this apply is no longer synchronous with, + // so "run the pre-handle sync" stops being a safe default and becomes a wrong + // picture taken from torn state. Every cause of the decline is already named by the + // MGLOG_E_ONCE that produced it (F2's half-described / neither-described applier, + // F4's unbound binding, A1's refused handle); this turns that log into the abort the + // contract asks for, with the verb in the message so the strict lane's marker table + // points at the site. + if (FramebufferRecordArmIsMandatory()) { + RefuseFramebufferBindingSlotRead(); + } +#endif #endif const FramebufferTarget fboTargets[] = {FramebufferTarget::Draw, FramebufferTarget::Read}; @@ -4810,6 +4830,15 @@ namespace MobileGL::MG_Backend::DirectGLES { return; } } +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (fb, §5.4): the pre-handle arm below reads the binding slot, so under a transport + // reaching it is the same refusal SyncCurrentFBO takes. Reaching it means either the + // sync's trust latch is clear - and the sync aborted before it could be - or this bind + // ran with no sync in front of it, which the latch exists to catch. + if (FramebufferRecordArmIsMandatory()) { + RefuseFramebufferBindingSlotRead(); + } +#endif #endif auto& slot = GetFramebufferBindingSlotChecked(target); @@ -4944,18 +4973,17 @@ namespace MobileGL::MG_Backend::DirectGLES { backendObj = MakeShared(); } backendObj->m_pushedSyncHandle = fbo; - const auto stateObj = registry.StateForHandle(fbo); - if (stateObj) { - if (forceSync) { - backendObj->InvalidateSyncedState(); - } - backendObj->SyncToBackend(stateObj, target); - } else { - MGLOG_E_ONCE("MGPipe: framebuffer handle {%u, %u} has no frontend object noted on " - "this side - it was never synced through a binding, so its twin is " - "bound unconfigured", - fbo.Slot, fbo.Gen); + // P5e (fb, CONTRACT-P5E.md §5.4): THE STATE NOTE IS GONE. The paragraph above described + // a handle arm that still had to find "the frontend object the sync body walks", and + // its failure mode - a framebuffer named by a DSA entry point that was never bound, so + // no note exists and the twin is bound unconfigured - was the last consequence of that + // detour. SyncToBackendByHandle configures the twin from the RECORD, which every named + // framebuffer has by construction (a Named record precedes every DSA site, ID-19), so + // the never-bound case is now configured correctly rather than named and refused. + if (forceSync) { + backendObj->InvalidateSyncedState(); } + backendObj->SyncToBackendByHandle(fbo, target); backendObj->Bind(target); } #endif // MOBILEGL_BUILD_DISAGGREGATED @@ -4964,12 +4992,20 @@ namespace MobileGL::MG_Backend::DirectGLES { #ifdef TRACY_ENABLE ZoneScopedC(TRACY_ZONECOLOR_BACKEND); #endif - auto& slot = GetFramebufferBindingSlotChecked(target); - const auto& fbo = slot.GetBoundObject(); -#if MOBILEGL_BUILD_DISAGGREGATED +#if MOBILEGL_PIPE_PUSH && MOBILEGL_BUILD_DISAGGREGATED // P5c (hd): with an active transport the bound framebuffer's handle is the applier's // own working state; the object form's registry probe never runs (T2). - if (MG_Config::Transport != MG_Config::TransportMode::Monolith && FramebufferSubsystemEnabled()) { + // + // P5e (fb, CONTRACT-P5E.md §5.4): AND THE BINDING SLOT IS READ INSIDE THE ELSE, NOT + // BEFORE THE IF. The read and the three stamps below used to straddle this arm - S3's + // finding #7 - so the handle path still touched a BARRIER_PULLED row on both sides of + // the one branch that existed to avoid it. The stamps are the LEGACY memo's + // (g_fboSynced*), and skipping them here is safe for exactly the reason the serial memo + // below is invalidated rather than stamped: nothing on this path consulted the frontend + // trio, so the honest thing is to leave it invalid and pay one extra sync. Under a + // transport the legacy arm is never taken anyway (SyncCurrentFBO refuses first), so the + // memo it feeds has no reader. + if (FramebufferRecordArmIsMandatory()) { SyncAndBindFramebufferByHandle( MG_Pipe::MGPipeApplier().BoundFramebuffer[static_cast( target == FramebufferTarget::Read ? MG_Pipe::MGPipeFramebufferTarget::Read @@ -4977,12 +5013,16 @@ namespace MobileGL::MG_Backend::DirectGLES { target); } else #endif - SyncAndBindFramebufferObject(fbo, target); - FramebufferImpl::g_fboSyncedSlotVersions[(SizeT)target] = slot.GetVersion(); - FramebufferImpl::g_fboSyncedObjectVersions[(SizeT)target] = fbo ? fbo->GetObjectVersion() : 0; - FramebufferImpl::g_fboSyncedObjects[(SizeT)target] = fbo.get(); - FramebufferImpl::g_fboSyncedBackendIdGenerations[(SizeT)target] = - FramebufferImpl::g_attachmentBackendIdGeneration; + { + auto& slot = GetFramebufferBindingSlotChecked(target); + const auto& fbo = slot.GetBoundObject(); + SyncAndBindFramebufferObject(fbo, target); + FramebufferImpl::g_fboSyncedSlotVersions[(SizeT)target] = slot.GetVersion(); + FramebufferImpl::g_fboSyncedObjectVersions[(SizeT)target] = fbo ? fbo->GetObjectVersion() : 0; + FramebufferImpl::g_fboSyncedObjects[(SizeT)target] = fbo.get(); + FramebufferImpl::g_fboSyncedBackendIdGenerations[(SizeT)target] = + FramebufferImpl::g_attachmentBackendIdGeneration; + } #if MOBILEGL_PIPE_PUSH // P4a decline-site F9 / MINOR-3: THE HANDLE ARM'S MEMO IS INVALIDATED HERE, NOT // STAMPED, and it is done per target for the reason the memo is kept per target at @@ -6527,15 +6567,32 @@ namespace MobileGL::MG_Backend::DirectGLES { GLfloat rb[4] = {0}; g_GLESFuncs.glReadPixels(100, 100, 1, 1, GL_RGBA, GL_FLOAT, rb); const GLenum rbErr = g_GLESFuncs.glGetError(); - const auto& feFbo = - MGB_CTX->GetFramebufferBindingSlot(FramebufferTarget::Draw).GetBoundObject(); int feDb0 = -1, feDb1 = -1; Uint feIdx = 0, feVer = 0; - if (feFbo) { - feIdx = feFbo->GetExternalIndex(); - feVer = feFbo->GetObjectVersion(); - feDb0 = (int)feFbo->GetDrawBuffers()[0]; - feDb1 = (int)feFbo->GetDrawBuffers()[1]; +#if MOBILEGL_PIPE_PUSH && MOBILEGL_BUILD_DISAGGREGATED + // P5e (fb): DEBUG-ONLY, and still a BARRIER_PULLED read. A diagnostic + // is not a reason to touch the binding slot on an apply thread, so + // under a transport the same four numbers come off the draw record - + // the external index has no record counterpart and stays 0, which reads + // as "the record did not say". + if (FramebufferRecordArmIsMandatory()) { + if (const auto* rec = BoundFramebufferRecord( + FramebufferTarget::Draw)) { + feVer = static_cast(rec->ContentHash); + feDb0 = (int)rec->DrawBuffers[0]; + feDb1 = (int)rec->DrawBuffers[1]; + } + } else +#endif + { + const auto& feFbo = + MGB_CTX->GetFramebufferBindingSlot(FramebufferTarget::Draw).GetBoundObject(); + if (feFbo) { + feIdx = feFbo->GetExternalIndex(); + feVer = feFbo->GetObjectVersion(); + feDb0 = (int)feFbo->GetDrawBuffers()[0]; + feDb1 = (int)feFbo->GetDrawBuffers()[1]; + } } MGLOG_D("CLEARV fbo=%d clrErr=0x%x rbErr=0x%x cc.x=%g cleared=%d firstDb=0x%x prevReadBuf=0x%x " "feFbo=%u feVer=%u feDb=[%d,%d] stored=(%g,%g,%g,%g)", @@ -8566,6 +8623,212 @@ namespace MobileGL::MG_Backend::DirectGLES { return handled & mask; } +#if MOBILEGL_PIPE_PUSH + // ---- P5e (fb, CONTRACT-P5E.md §5.4): the same aspect plan, from the two RECORDS --------- + // + // The blit was the last entry point in this family that still reached into two frontend + // FramebufferObjects on every call - GetDrawBuffers, GetReadBuffer, GetAttachment and then + // six texture properties per aspect - and it did it on BOTH arms, the named one reaching + // the objects through the twin table's state note. Every one of those questions is a field + // of MGPFramebufferState: + // + // GetDrawBuffers() -> DrawBuffers[8] (attachment index, -1 = None) + // GetReadBuffer() -> ReadSurface matched against Color[] (the same match + // SyncReadBufferToBackend makes; ReadSurface is resolved + // from THIS framebuffer's own read buffer under every + // Target, Named included) + // GetAttachment(point) -> PushedSurfaceForAttachment + // IsTexture / IsLayered -> Kind / Layered + // GetTextureLevel / Layer -> Level / Layer + // GetFormat -> InternalFormat (the attachment's, which is the texture's) + // GetSamples -> record.Samples + // + // SAMPLES COMES OFF THE RECORD RATHER THAN OFF EACH TEXTURE, and that is a deliberate + // narrowing: the frontend form asked each endpoint texture, the record states it per + // FRAMEBUFFER. A framebuffer whose attachments disagree about sample count is incomplete, + // so the two cannot differ where this substitution is allowed to fire; and the test the + // answer feeds ("either end is multisample -> leave it to the driver") is conservative in + // the direction that hands the call back. + static FramebufferAttachmentType PushedReadBufferPoint(const MG_Pipe::MGPFramebufferState& record) { + if (MG_Pipe::MGPipeHandleIsNull(record.ReadSurface.Res)) return FramebufferAttachmentType::None; + for (Uint i = 0; i < MG_Pipe::kMGPipeMaxColorAttachments; ++i) { + const auto& color = record.Color[i]; + if (color.Res == record.ReadSurface.Res && color.Kind == record.ReadSurface.Kind && + color.Layered == record.ReadSurface.Layered && color.Level == record.ReadSurface.Level && + color.Layer == record.ReadSurface.Layer && + color.UploadTarget == record.ReadSurface.UploadTarget) { + return static_cast( + static_cast(FramebufferAttachmentType::Color0) + static_cast(i)); + } + } + return FramebufferAttachmentType::None; + } + + static GLbitfield BlitLayeredDestinationAspects(const MG_Pipe::MGPFramebufferState& readRecord, + const MG_Pipe::MGPFramebufferState& drawRecord, GLint srcX0, + GLint srcY0, GLint srcX1, GLint srcY1, GLint dstX0, + GLint dstY0, GLint dstX1, GLint dstY1, GLbitfield mask) { + if (mask == 0) return 0; + if (!g_GLESFuncs.glCopyImageSubData) return 0; + if (!MG_Util::SelfTest::BlitIgnoresDestinationArrayLayer(g_GLESFuncs)) return 0; + // The default framebuffer has no layers to get wrong. + if (readRecord.IsDefault != 0 || drawRecord.IsDefault != 0) return 0; + + const Int width = srcX1 - srcX0; + const Int height = srcY1 - srcY0; + const Bool oneToOne = width > 0 && height > 0 && (dstX1 - dstX0) == width && (dstY1 - dstY0) == height; + // The scissor clips a blit and does not clip a copy, so an enabled scissor makes the two + // different operations no matter how the rectangles line up. + const Bool scissorEnabled = + (RenderStateImpl::g_syncedRenderStateParameters.ScissorTestEnabledMask & 1u) != 0; + + using MobileGL::FramebufferAttachmentType; + struct AspectPlan { + GLbitfield bit; + FramebufferAttachmentType source; + FramebufferAttachmentType destination; + }; + // The buffer is FOUND rather than assumed to be slot 0, for the frontend form's reason: + // a blit writes every ENABLED draw buffer, and glDrawBuffers(NONE, NONE, NONE, + // COLOR_ATTACHMENT0) leaves slot 0 empty while still naming exactly one destination. + // -1 is the record's spelling of GL_NONE and is not "no such attachment". + Int enabledDrawBuffers = 0; + FramebufferAttachmentType colorDestination = FramebufferAttachmentType::None; + for (Uint i = 0; i < MG_Pipe::kMGPipeMaxColorAttachments; ++i) { + const Int8 index = drawRecord.DrawBuffers[i]; + if (index < 0) continue; + ++enabledDrawBuffers; + if (enabledDrawBuffers == 1) { + colorDestination = static_cast( + static_cast(FramebufferAttachmentType::Color0) + static_cast(index)); + } + } + const AspectPlan plans[] = { + {GL_COLOR_BUFFER_BIT, PushedReadBufferPoint(readRecord), colorDestination}, + {GL_DEPTH_BUFFER_BIT, FramebufferAttachmentType::Depth, FramebufferAttachmentType::Depth}, + {GL_STENCIL_BUFFER_BIT, FramebufferAttachmentType::Stencil, FramebufferAttachmentType::Stencil}, + }; + + GLbitfield handled = 0; + for (const AspectPlan& plan : plans) { + if ((mask & plan.bit) == 0) continue; + if (plan.source == FramebufferAttachmentType::Unknown || + plan.destination == FramebufferAttachmentType::Unknown || + plan.source == FramebufferAttachmentType::None || + plan.destination == FramebufferAttachmentType::None) { + continue; + } + const MG_Pipe::MGPSurface* const sourceSurface = + FramebufferImpl::PushedSurfaceForAttachment(readRecord, plan.source); + const MG_Pipe::MGPSurface* const destinationSurface = + FramebufferImpl::PushedSurfaceForAttachment(drawRecord, plan.destination); + if (sourceSurface == nullptr || destinationSurface == nullptr) continue; + // Renderbuffers have no layers, so a destination that is one cannot be hitting this. + if (sourceSurface->Kind != MG_Pipe::kMGPipeSurfaceKindTexture || + destinationSurface->Kind != MG_Pipe::kMGPipeSurfaceKindTexture) { + continue; + } + if (MG_Pipe::MGPipeHandleIsNull(sourceSurface->Res) || + MG_Pipe::MGPipeHandleIsNull(destinationSurface->Res)) { + continue; + } + // Layer 0 is the case the driver gets right, and a LAYERED attachment + // (glFramebufferTexture with no layer) blits its layer 0 by spec - neither is this + // defect. + if (destinationSurface->Layer == 0) continue; + if (destinationSurface->Layered != 0 || sourceSurface->Layered != 0) continue; + + // glCopyImageSubData moves texel blocks: same format both ends, or it is a different + // operation. Multisample endpoints would additionally have to agree on sample count, + // which is a resolve the driver still owns. + if (sourceSurface->InternalFormat != destinationSurface->InternalFormat) continue; + if (readRecord.Samples > 0 || drawRecord.Samples > 0) continue; + // Copying an image region onto itself is undefined for glCopyImageSubData, and a blit + // whose source and destination overlap is undefined for GL too - so this is not a + // shape to substitute FOR, it is one to leave exactly as the application wrote it. + if (sourceSurface->Res == destinationSurface->Res && + sourceSurface->Level == destinationSurface->Level && + sourceSurface->Layer == destinationSurface->Layer) { + continue; + } + + // A combined depth-stencil texture is ONE image to glCopyImageSubData: it carries both + // aspects across whether or not the mask asked for both. Taking only GL_DEPTH_BUFFER_BIT + // on a DEPTH24_STENCIL8 destination would overwrite a stencil the application asked to + // keep, so the copy is only allowed when the mask covers everything the format holds. + const auto format = static_cast(destinationSurface->InternalFormat); + const Bool hasDepth = MG_Util::IsDepthFormatInternalFormat(format); + const Bool hasStencil = MG_Util::IsStencilFormatInternalFormat(format); + if (hasDepth && (mask & GL_DEPTH_BUFFER_BIT) == 0) continue; + if (hasStencil && (mask & GL_STENCIL_BUFFER_BIT) == 0) continue; + // ... and having carried both, it must be credited with both, or the caller hands the + // stencil half to the driver and it lands on layer 0 after all. + const GLbitfield aspectBits = + hasDepth || hasStencil + ? static_cast((hasDepth ? GL_DEPTH_BUFFER_BIT : 0) | + (hasStencil ? GL_STENCIL_BUFFER_BIT : 0)) + : static_cast(GL_COLOR_BUFFER_BIT); + if ((handled & aspectBits) == aspectBits) continue; + + if (!oneToOne || scissorEnabled || (plan.bit == GL_COLOR_BUFFER_BIT && enabledDrawBuffers != 1)) { + MGLOG_E_ONCE("BlitFramebuffer: this driver ignores a non-zero destination array layer and this " + "blit cannot be expressed as a copy (%s), so it will land on layer 0", + !oneToOne ? "it scales or flips" + : scissorEnabled ? "the scissor test is enabled" + : "the destination has more than one draw buffer"); + continue; + } + + auto& backendSource = + TextureImpl::SyncTextureToBackendByHandle(sourceSurface->Res, + /*imageBindableStorageRequired=*/false); + if (!backendSource) continue; + const GLuint sourceName = backendSource->GetBackendTextureId(); + // BY VALUE past this point, for the reason the copy-image endpoint builder gives: + // the second sync can grow the registry and relocate the first result. + auto& backendDestinationSlot = + TextureImpl::SyncTextureToBackendByHandle(destinationSurface->Res, + /*imageBindableStorageRequired=*/false); + if (!backendDestinationSlot) continue; + const GLuint destinationName = backendDestinationSlot->GetBackendTextureId(); + if (sourceName == 0 || destinationName == 0) continue; + const GLenum sourceTarget = TextureImpl::ConvertTextureTargetToBackendGLEnum( + static_cast(sourceSurface->TextureTarget)); + const GLenum destinationTarget = TextureImpl::ConvertTextureTargetToBackendGLEnum( + static_cast(destinationSurface->TextureTarget)); + + ClearGLErrors(); + g_GLESFuncs.glCopyImageSubData(sourceName, sourceTarget, static_cast(sourceSurface->Level), + srcX0, srcY0, static_cast(sourceSurface->Layer), + destinationName, destinationTarget, + static_cast(destinationSurface->Level), dstX0, dstY0, + static_cast(destinationSurface->Layer), width, height, 1); + if (const GLenum error = g_GLESFuncs.glGetError(); error != GL_NO_ERROR) { + // The driver blit still runs for this aspect - onto the wrong layer, but the + // substitute has to leave the call no worse off than it found it. + MGLOG_E_ONCE("BlitFramebuffer: the layered-destination copy substitute failed with %s; the " + "driver blit will run instead and land on layer 0", + MG_Util::ConvertGLEnumToString(error).c_str()); + continue; + } + handled |= aspectBits; + } + return handled & mask; + } + + // The two endpoint records of a blit, or nulls. `fbo` null-or-default means "the record of + // whatever is bound to that target", which is what the bound arm asks; a named blit passes + // the handles its record carried. + static const MG_Pipe::MGPFramebufferState* BlitEndpointRecord(MG_Pipe::MGPipeHandle fbo, + FramebufferTarget boundTarget) { + const auto& st = MG_Pipe::MGPipeApplier(); + if (MG_Pipe::MGPipeHandleIsNull(fbo)) { + return boundTarget == FramebufferTarget::Read ? st.ReadFramebuffer() : st.DrawFramebuffer(); + } + return st.FramebufferRecordFor(fbo); + } +#endif // MOBILEGL_PIPE_PUSH + void BlitFramebuffer(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1, GLbitfield mask, GLenum filter) { #if MOBILEGL_LOG_ACTIVE_LEVEL <= MOBILEGL_LOG_LEVEL_DEBUG && MOBILEGL_ENABLE_SCOPE_MARKER @@ -8595,23 +8858,18 @@ namespace MobileGL::MG_Backend::DirectGLES { readFbo.Slot, readFbo.Gen, drawFbo.Slot, drawFbo.Gen, srcX0, srcY0, srcX1, srcY1, dstX0, dstY0, dstX1, dstY1, mask, MG_Util::ConvertGLEnumToString(filter).c_str()); - // See the bound arm below: only the probed defect makes this do anything. It - // reads the two frontend objects' attachments (the object-class channel P3b/P4b - // retires), reached through the twins' state notes - and for a default endpoint - // the default framebuffer object itself, exactly as the bound arm's binding-slot - // read hands it. A missing object only skips the workaround. - const auto objectFor = [](MG_Pipe::MGPipeHandle handle) { - if (handle == MG_Pipe::kMGPipeDefaultFramebuffer) { - return SharedPtr( - MG_Impl::GLImpl::FramebufferImpl::pDefaultFramebufferInfo->defaultFBO); - } - return FramebufferImpl::g_backendFramebufferObjects.StateForHandle(handle); - }; - const auto readObj = objectFor(readFbo); - const auto drawObj = objectFor(drawFbo); - if (readObj && drawObj) { - mask &= ~BlitLayeredDestinationAspects(readObj, drawObj, srcX0, srcY0, srcX1, srcY1, - dstX0, dstY0, dstX1, dstY1, mask); + // P5e (fb, §5.4): only the probed defect makes this do anything, and it now + // asks the two RECORDS. The state-note detour this used to take - the twin + // table handing back the frontend object it was synced from, plus + // pDefaultFramebufferInfo->defaultFBO for a default endpoint - is gone with the + // note itself; a named framebuffer always has a record (a Named record precedes + // every DSA site, ID-19), so the "missing object skips the workaround" case is + // now a missing record and means the same thing. + const auto* readRecord = BlitEndpointRecord(readFbo, FramebufferTarget::Read); + const auto* drawRecord = BlitEndpointRecord(drawFbo, FramebufferTarget::Draw); + if (readRecord != nullptr && drawRecord != nullptr) { + mask &= ~BlitLayeredDestinationAspects(*readRecord, *drawRecord, srcX0, srcY0, srcX1, + srcY1, dstX0, dstY0, dstX1, dstY1, mask); } if (mask != 0) { IssueBlitWithResolveFallback(srcX0, srcY0, srcX1, srcY1, dstX0, dstY0, dstX1, dstY1, @@ -8651,6 +8909,24 @@ namespace MobileGL::MG_Backend::DirectGLES { dstX1, dstY1, mask, MG_Util::ConvertGLEnumToString(filter).c_str()); // A no-op on every driver that honours a non-zero destination array layer, which is all // of them but the probed one. Whatever it performs itself is taken out of the mask. +#if MOBILEGL_PIPE_PUSH && MOBILEGL_BUILD_DISAGGREGATED + // P5e (fb, §5.4): the bound arm's two binding-slot reads were the last GetBoundObject + // pair on this path; under a transport the two bound RECORDS answer instead + // (FramebufferRecordFor(st.BoundFramebuffer[t]), through BlitEndpointRecord's null + // case). SyncCurrentFBO ran a few lines up and refuses if either is missing, so a null + // here means the blit is against the default framebuffer's record, which the overload + // declines on IsDefault. + if (FramebufferRecordArmIsMandatory()) { + const auto* readRecord = + BlitEndpointRecord(MG_Pipe::kMGPipeNullHandle, FramebufferTarget::Read); + const auto* drawRecord = + BlitEndpointRecord(MG_Pipe::kMGPipeNullHandle, FramebufferTarget::Draw); + if (readRecord != nullptr && drawRecord != nullptr) { + mask &= ~BlitLayeredDestinationAspects(*readRecord, *drawRecord, srcX0, srcY0, srcX1, + srcY1, dstX0, dstY0, dstX1, dstY1, mask); + } + } else +#endif mask &= ~BlitLayeredDestinationAspects( MGB_CTX->GetFramebufferBindingSlot(FramebufferTarget::Read).GetBoundObject(), MGB_CTX->GetFramebufferBindingSlot(FramebufferTarget::Draw).GetBoundObject(), srcX0, srcY0, @@ -9152,6 +9428,81 @@ namespace MobileGL::MG_Backend::DirectGLES { } }; +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (fb, CONTRACT-P5E.md §5.4): THE REVERSE INDEX REPLACES THE WALK. id's note + // above names this package as the one that does it, and the reason is the line it + // points at: StateForHandle handed the server a frontend FramebufferObject per live + // twin, which is a SharedPtr to client memory held across records - the last one in + // this family. The record already says which points hold which texture, so the + // question "which framebuffers have this one attached" is answered by an index + // maintained where the record is applied, and the points are re-read from the + // record rather than from the frontend attachment array. + // + // THE SITE ITSELF STAYS INSIDE THE FRONTEND-KEYED SCOPE (§4.4 keeps the class + // exactly here, at DirectGLES.cpp:8837 in the contract's list): every caller of + // this scope reaches it from a BARRIERED row - GenerateMipmap and the two CopyTex + // endpoints - so HandleOf on the texture the caller was handed is legal, and it is + // P8/P9 that retires it. What P5e removes is the per-FRAMEBUFFER frontend hold, + // which is not the caller's own object and has no such excuse. + if (FramebufferRecordArmIsMandatory()) { + const MG_Pipe::MGPipeHandle textureHandle = + TextureImpl::g_backendTextureObjects.HandleOf(texture.get()); + if (MG_Pipe::MGPipeHandleIsNull(textureHandle)) return; + const auto detachPoint = [&](FramebufferImpl::BackendFramebufferObject& backendFBO, + FramebufferAttachmentType frontendType, + const MG_Pipe::MGPSurface& surface) { + if (surface.Kind != MG_Pipe::kMGPipeSurfaceKindTexture) return; + if (!(surface.Res == textureHandle)) return; + + GLenum backendAttachment = GL_NONE; + if (frontendType >= FramebufferAttachmentType::Color0 && + frontendType <= FramebufferAttachmentType::Color31) { + backendAttachment = backendFBO.GetBackendAttachmentType(frontendType); + } else { + backendAttachment = MG_Util::ConvertFramebufferAttachmentTypeToGLEnum(frontendType); + } + if (backendAttachment == GL_NONE || backendAttachment == GL_UNKNOWN_MGL) return; + + GLenum textureTarget = TextureImpl::ConvertTextureUploadTargetToBackendGLEnum( + static_cast(surface.UploadTarget)); + if (textureTarget == GL_UNKNOWN_MGL) { + textureTarget = TextureImpl::ConvertTextureTargetToBackendGLEnum( + static_cast(surface.TextureTarget)); + } + + const GLuint backendFBOId = backendFBO.GetBackendFramebufferId(); + FramebufferImpl::BindFramebufferId(GL_DRAW_FRAMEBUFFER, backendFBOId); + if (surface.Layered != 0) { + g_GLESFuncs.glFramebufferTexture(GL_DRAW_FRAMEBUFFER, backendAttachment, 0, 0); + } else { + g_GLESFuncs.glFramebufferTexture2D(GL_DRAW_FRAMEBUFFER, backendAttachment, + textureTarget, 0, 0); + } + ClearGLErrors(); + m_detachedAttachments.push_back({backendFBOId, backendAttachment, textureTarget, + backendTextureId, static_cast(surface.Level), + surface.Layered != 0}); + }; + for (const MG_Pipe::MGPipeHandle fbo : + FramebufferImpl::FramebuffersAttachingTexture(textureHandle)) { + auto* twinSlot = FramebufferImpl::g_backendFramebufferObjects.FindByHandle(fbo); + if (twinSlot == nullptr || !*twinSlot) continue; // a recycled slot answers null + const auto* record = FramebufferImpl::PushedFramebufferRecord(fbo); + if (record == nullptr || record->IsDefault != 0) continue; + auto& backendFBO = **twinSlot; + for (Uint i = 0; i < MG_Pipe::kMGPipeMaxColorAttachments; ++i) { + detachPoint(backendFBO, + static_cast( + static_cast(FramebufferAttachmentType::Color0) + + static_cast(i)), + record->Color[i]); + } + detachPoint(backendFBO, FramebufferAttachmentType::Depth, record->Depth); + detachPoint(backendFBO, FramebufferAttachmentType::Stencil, record->Stencil); + } + return; + } +#endif // Already inside #if MOBILEGL_PIPE_PUSH, so no second guard here: the arm choice // below is the RUNTIME one. if (EsprytSlotTablesEnabled()) { @@ -9946,9 +10297,19 @@ namespace MobileGL::MG_Backend::DirectGLES { const SharedPtr& renderbufferObject) { if (!renderbufferObject) return nullptr; #if MOBILEGL_BUILD_DISAGGREGATED - // P5c (G6, CONTRACT-P5C §5.4): frontend-keyed twin resolution (and the mint on a first - // sync), named debt inside the scope - P3b/P4b rekeys the registry onto handles. - const MG_Pipe::MGPipeFrontendKeyedRegistryScope frontendKeyedRegistry; + // P5e (fb, CONTRACT-P5E.md §4.4): the scope at this site is DELETED, and with it the + // arm that needed it. The only caller is the glCopyImageSubData endpoint builder, and a + // RENDERBUFFER endpoint is refused on the wire before it is emitted and again at the + // sink (PipeApplier.cpp's ServerUnmigratedVerbFatal("CopyImageSubData+RENDERBUFFER")), + // so under a transport nothing can reach here with one. Saying so by name is better + // than a find-or-MINT against the frontend address that the refusal already proved + // unreachable - if it ever becomes reachable, the name is where to start. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + MGLOG_F("MGPipe: Fatal{UnmigratedVerb, \"CopyImageSubData+RENDERBUFFER\"} a renderbuffer " + "endpoint reached the backend under an active transport, where both the client " + "emitter and the sink refuse one"); + std::abort(); + } #endif SharedPtr backendRenderbufferObject; if (auto* slot = RenderbufferImpl::g_backendRenderbufferObjects.Find(renderbufferObject.get())) { diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 875be79a..46cacf4e 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -9659,8 +9659,8 @@ namespace MobileGL::MG_Backend::DirectGLES { // (D-C1), which is every point a framebuffer can hold on the handle arm: D-C3 refuses // bit 9 outright on a driver reporting more than 8 colour attachments, and the // FRONT/BACK points belong to the DEFAULT framebuffer, which has no twin at all. - static const MG_Pipe::MGPSurface* PushedSurfaceForAttachment(const MG_Pipe::MGPFramebufferState& record, - FramebufferAttachmentType point) { + const MG_Pipe::MGPSurface* PushedSurfaceForAttachment(const MG_Pipe::MGPFramebufferState& record, + FramebufferAttachmentType point) { if (point == FramebufferAttachmentType::Depth) return &record.Depth; if (point == FramebufferAttachmentType::Stencil) return &record.Stencil; if (point < FramebufferAttachmentType::Color0 || point > FramebufferAttachmentType::Color31) { diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 256f5f0e..c9fac796 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -2090,6 +2090,17 @@ namespace MobileGL::MG_Backend::DirectGLES { void NoteFramebufferTextureAttachments(MG_Pipe::MGPipeHandle fbo, const MG_Pipe::MGPFramebufferState& record); Vector FramebuffersAttachingTexture(MG_Pipe::MGPipeHandle texture); + + // The record's surface for an attachment POINT, or null when this record does not + // describe that point at all (Color8..Color31, and the default framebuffer's FRONT/BACK + // tokens). MGPFramebufferState carries Color[8] + Depth + Stencil, which is every point + // a framebuffer can hold on the handle arm - D-C3 refuses bit 9 outright on a driver + // reporting more than 8 colour attachments. P5e (fb) exports it: the twin's attachment + // walk, the named blit's aspect plan and the detach walk all ask the same question of + // the same record, and a second copy of the Color/Depth/Stencil dispatch beside each of + // them is how one of them ends up describing a point differently from the others. + const MG_Pipe::MGPSurface* PushedSurfaceForAttachment(const MG_Pipe::MGPFramebufferState& record, + FramebufferAttachmentType point); #endif // True when the read buffer names a fixed-point (norm/snorm) attachment that the // backend actually stores in a floating-point format. GL clamps a read from a From f4675d1ac05a7c9be5996051db92894c1386abf7 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 08:06:59 -0400 Subject: [PATCH 3/6] [MG_Backend] (Disaggregated): P5e fb - SCAFFOLD, REVERTED BEFORE HEAD: tx2's two by-handle seams, so fb can measure its own lane - fb calls TextureImpl::SyncTextureToBackendByHandle and BackendTextureObject::SyncMipmapsToBackendByHandle from four places. tx2 owns those bodies and is being written in parallel, so with c0e's aborting stubs every integration-split entry that touches a texture dies on the seam and fb's own isplit and strict numbers cannot be read at all. - This commit bodies the two temporarily, in the one way that does not pre-empt tx2's design: the frontend texture is resolved from the record's own GL name through the BARRIER-PULLED sticky forward GetTextureObject, exactly as the copy-image sink resolves its endpoints, and the existing frontend overloads do the work. Correct under lockstep, and deliberately NOT the shape tx2 will write - tx2 drives the storage from the descriptor and holds no frontend object. - IT IS REVERTED BY THE NEXT-BUT-ONE COMMIT. It exists in history so the integrator can see exactly what fb assumed about the seam and what fb's measured numbers rest on. --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 41 +++++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 46cacf4e..cb29827d 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -14193,15 +14193,48 @@ namespace MobileGL::MG_Backend::DirectGLES { } // namespace namespace TextureImpl { + // ================================================================================ + // P5e SCAFFOLD (fb) - THIS COMMIT IS REVERTED BEFORE fb's HEAD. DO NOT LAND IT. + // ================================================================================ + // tx2 owns these two bodies and is being written in parallel. fb CALLS them from the + // attachment surface sync, the two attachment texture lists, the image sweep and the + // named blit, so with the c0e stubs in place every integration-split entry that touches + // a texture aborts on the seam rather than on anything fb wrote - which leaves fb's own + // isplit and strict numbers unmeasurable. + // + // So these two are bodied HERE, temporarily, in the ONLY way that does not pre-empt + // tx2's design: the frontend texture is resolved from the record's own GL name through + // the BARRIER-PULLED sticky forward GetTextureObject - exactly as the copy-image sink + // resolves its endpoints (PipeApplier.cpp) - and the existing frontend overloads then + // do the work. That is correct under lockstep (the client is parked, which is what + // makes a sticky forward legal) and it is deliberately NOT what tx2 will write: tx2 + // drives the storage from the descriptor and has no frontend object at all. + // + // What it costs, and what to read past when reading fb's numbers: each call adds a + // GetTextureObject pull, so the strict lane sees GetTextureObject@ markers that + // belong to this scaffold and not to fb. SharedPtr& SyncTextureToBackendByHandle(MG_Pipe::MGPipeHandle texture, Bool imageBindableStorageRequired) { - (void)imageBindableStorageRequired; - MGPipeP5eSeamNotLanded("TextureImpl::SyncTextureToBackendByHandle", "tx2", texture); + static SharedPtr s_scaffoldNone; + const auto* record = PipeTextureRecordForHandle(texture); + if (record == nullptr) { + s_scaffoldNone = nullptr; + return s_scaffoldNone; + } + const auto& stateObj = MGB_CTX->GetTextureObject(static_cast(record->Desc.GlNameForDiag)); + if (!stateObj) { + s_scaffoldNone = nullptr; + return s_scaffoldNone; + } + return SyncTextureObjectToBackend(stateObj, imageBindableStorageRequired); } void BackendTextureObject::SyncMipmapsToBackendByHandle(MG_Pipe::MGPipeHandle texture) { - MGPipeP5eSeamNotLanded("BackendTextureObject::SyncMipmapsToBackendByHandle", "tx2", - texture); + const auto* record = PipeTextureRecordForHandle(texture); + if (record == nullptr) return; + const auto& stateObj = MGB_CTX->GetTextureObject(static_cast(record->Desc.GlNameForDiag)); + if (!stateObj) return; + SyncMipmapsToBackend(stateObj); } } // namespace TextureImpl From fcd7c5c87af0fc094cd44c6331f029300ef1a97c Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 08:09:43 -0400 Subject: [PATCH 4/6] [MG_Backend] (Disaggregated): P5e fb - the eager image-bind funnel stops reading the frontend binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The strict lane named GetImageTextureBinding@BindImageTexture on thirteen entries: bind_shader_image is an unbarriered row (CONTRACT-P5E §2.2) whose backend entry point receives GL arguments and a GL NAME - MGPImageBind::Res stops at the sink - so this funnel had no way to resolve the twin and read the frontend binding instead. - Under a transport it now records only the high-water mark and returns. Nothing is lost: the sweep at the next validate point re-binds every unit up to the mark from the applier's own MGPImageView array, and the sink bumps the texture shutter serial immediately after this call, which is one of the four values the sweep's gate is keyed on - so the sweep that follows a bind cannot be suppressed. An image unit has no reader but a shader, and no shader runs in between. - The mark itself must still be raised here, because it is the "no draw in this context can be reading an image" early-out. --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 2168cf7b..12332409 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -2722,6 +2722,32 @@ namespace MobileGL::MG_Backend::DirectGLES { // Almost every program (every Minecraft draw) leaves it at zero, which is what keeps the // draw-path staleness check below at one integer test. static Uint g_imageUnitHighWaterMark = 0; + +#if MOBILEGL_PIPE_PUSH && MOBILEGL_BUILD_DISAGGREGATED + // P5e (fb): the eager glBindImageTexture funnel's HANDLE ARM, and it binds nothing. + // + // `bind_shader_image` is an unbarriered row (§2.2) whose backend entry point takes GL + // arguments and a GL NAME - the record's own MGPipeHandle (MGPImageBind::Res) stops at + // the sink - so there is no way to resolve the texture's twin from what arrives here, + // and the frontend binding this used to read is a BARRIER_PULLED row. + // + // Nothing is lost by deferring: SyncImageTextureBindings at the next validate point + // re-binds every unit up to the mark from the applier's own MGPImageView array, and the + // sink bumps the texture shutter serial immediately after this call + // (MGPipeApplierNoteTextureStateMoved), which is one of the four values the sweep's gate + // is keyed on - so the sweep that follows this bind cannot be suppressed. An image unit + // has no reader but a shader, and no shader runs between here and that validate point. + // + // WHAT MUST STILL HAPPEN HERE is the high-water mark: it is the "no draw in this context + // can be reading an image" early-out, so a unit that is given a texture without raising + // it would be skipped by every sweep afterwards. + static void NoteImageUnitBoundWithoutReadingTheFrontend(Uint unit, Bool holdsTexture) { + if (!holdsTexture) return; + if (unit + 1 > g_imageUnitHighWaterMark) { + g_imageUnitHighWaterMark = unit + 1; + } + } +#endif // ---- P5e (fb, CONTRACT-P5E.md §5.4 / ruling 16 / ID-94): the image unit's bind ------ // // P4a e3's ResolveShaderImageRecord IS GONE, identity test, I5 seam log and all. It @@ -10590,12 +10616,23 @@ namespace MobileGL::MG_Backend::DirectGLES { void BindImageTexture(GLuint unit, GLuint texture, GLint level, GLboolean layered, GLint layer, GLenum access, GLenum format) { - (void)texture; (void)level; (void)layered; (void)layer; (void)access; (void)format; +#if MOBILEGL_PIPE_PUSH && MOBILEGL_BUILD_DISAGGREGATED + // P5e (fb): see NoteImageUnitBoundWithoutReadingTheFrontend. Under a transport this + // entry point records the high-water mark and lets the next validate point's sweep do + // the bind from the record; reading the frontend image binding here is the + // GetImageTextureBinding row that rule F forbids, and `bind_shader_image` is not a + // barriered row. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + TextureImpl::NoteImageUnitBoundWithoutReadingTheFrontend(unit, texture != 0); + return; + } +#endif + (void)texture; TextureImpl::SyncImageTextureBinding(unit); } From aa38f0a05bb94b2a91553c6a7dcf4b4d11742d7e Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 08:14:46 -0400 Subject: [PATCH 5/6] [MG_Test] (Disaggregated): P5e fb - the three constants on both sides, the server's point set, and the resize that must re-emit - ImageEmit.TheThreeAccessConstantsArePinnedOnBothSidesOfTheWire: ruling 16 / ID-94 is that the encode and the decode are ONE table, so the case reaches both halves - the numbers, MGPipeEncodeImageAccess, MGPipeDecodeImageAccess, the validity predicate and the two read/write predicates - and then round-trips all three GL tokens through a real glBindImageTexture so what is pinned is the byte that actually crosses. The drift it catches (1 <-> 2) is a `writeonly` image the shader can read back, which no pixel test on a lockstep tree could see while the frontend value was answering. - FramebufferEmit.TheRecordsElevenSurfacesAreTheWholePointSetTheServerReads: the server twin of AnAttachmentPointAboveTheWireWidthIsRefusedNotTruncated. P5e spends that refusal as a licence - the by-handle attachment walk iterates Color[0..7] + Depth + Stencil instead of the frontend's 41 points - so both halves are pinned: a fully-populated framebuffer round-trips every point into the applier's record, and a point above the width leaves the record the server holds completely unmoved (hash and FramebufferSerial), rather than merely counting a refusal. - FramebufferEmit.ADefaultFramebufferResizeReEmitsTheRecordWithItsNewExtent: ruling 15, which S3 flagged as unsettled and said was load-bearing because under run-ahead there is no other extent source. The case drives the surface-changed consumer's own call - AllocateStorage on the attachment at the new extent, no rebind, no re-attach - which is exactly the shape the framebuffer shutter would have suppressed. - All three get their pull-build skip twin through the suites' X-macro lists (G2/G14). --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 2 +- MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp | 128 +++++++++++++++++- MobileGL/MG_Test/Pipe/ImageEmitTest.cpp | 84 +++++++++++- 3 files changed, 211 insertions(+), 3 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 12332409..87b4734e 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -259,7 +259,7 @@ namespace MobileGL::MG_Backend::DirectGLES { } [[noreturn]] static void RefuseFramebufferBindingSlotRead() { MG_Pipe::MGPipeInputPoisonFatalForVerb(MG_Pipe::MGPipeInputField::GetFramebufferBindingSlot, - MGB_CTX->CurrentVerb()); + MG_Pipe::gPipeInputs.CurrentVerb()); } #endif // MOBILEGL_BUILD_DISAGGREGATED #endif // MOBILEGL_PIPE_PUSH diff --git a/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp b/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp index 7860fff4..1988b6a5 100644 --- a/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp @@ -242,7 +242,9 @@ TEST(FramebufferEmit, TheEmitterIsOneNeverDestroyedProcessSingleton) { X(FramebufferEmit, ADrawBufferTokenAboveTheWireWidthIsRefusedNotTruncated) \ X(FramebufferEmit, ALayeredCubeAttachmentDoesNotAssertAFaceItCannotKnow) \ X(FramebufferEmit, EveryNonTexturePointCarriesTheUnknownSentinelsRatherThanZero) \ - X(FramebufferEmit, ADeadFramebuffersNamedRecordLatchIsRetired) + X(FramebufferEmit, ADeadFramebuffersNamedRecordLatchIsRetired) \ + X(FramebufferEmit, TheRecordsElevenSurfacesAreTheWholePointSetTheServerReads) \ + X(FramebufferEmit, ADefaultFramebufferResizeReEmitsTheRecordWithItsNewExtent) #define MGL_DECLARE_PULL_SKIP(Suite, Name) \ TEST(Suite, Name) { GTEST_SKIP() << "compiled only under MOBILEGL_PIPE_PUSH"; } @@ -579,6 +581,130 @@ TEST(FramebufferEmit, AnAttachmentPointAboveTheWireWidthIsRefusedNotTruncated) { EXPECT_GE(Framebuffers().RefusedCount(), 1u) << "the refusal was not counted"; } +// ============================ P5e (fb), CONTRACT-P5E.md §5.4 ============================ +// +// THE SERVER TWIN of AnAttachmentPointAboveTheWireWidthIsRefusedNotTruncated above. +// +// That case pins the CLIENT half: a framebuffer holding a point at or above the wire width is +// refused rather than truncated. P5e turns that refusal into a licence the server SPENDS - the +// by-handle attachment walk stopped iterating the frontend's 41 points and now walks +// Color[0..7] + Depth + Stencil and nothing else, on the argument that the record's eleven +// surfaces ARE the point set. So the statement the server relies on needs a case of its own, +// and it has two halves: +// +// 1. a framebuffer that fills every point the wire can describe round-trips ALL of them into +// the applier's record - so "eleven" is not eleven minus whatever the emitter dropped; +// 2. when a point above the width appears, the record the server is holding does not change +// at all. The refusal is not merely counted: the server keeps describing the framebuffer +// as it last legally was, rather than a truncated version of what it now is. +// +// Half 2 is the one that would go quiet on its own. A truncating emitter would still refuse +// nothing, publish a record, and leave the server's walk perfectly self-consistent over eight +// colour points while one attachment silently stopped existing. +TEST(FramebufferEmit, TheRecordsElevenSurfacesAreTheWholePointSetTheServerReads) { + FramebufferScope scope; + const auto fbo = MakeShared(21); + Vector> colors; + for (Uint i = 0; i < kMGPipeMaxColorAttachments; ++i) { + colors.push_back(MakeColorTexture(70 + i, 32)); + fbo->AttachTexture(static_cast( + static_cast(FramebufferAttachmentType::Color0) + static_cast(i)), + colors.back(), TextureUploadTarget::Texture2D); + } + const auto depth = MakeShared(21); + depth->SetInternalFormat(TextureInternalFormat::Depth24Stencil8); + depth->AllocateStorage(IntVec2{32, 32}); + fbo->AttachRenderbuffer(FramebufferAttachmentType::Depth, depth); + const auto stencil = MakeShared(22); + stencil->SetInternalFormat(TextureInternalFormat::Depth24Stencil8); + stencil->AllocateStorage(IntVec2{32, 32}); + fbo->AttachRenderbuffer(FramebufferAttachmentType::Stencil, stencil); + BindDrawAndRead(fbo, fbo); + Framebuffers().EmitFramebufferState(Ctx()); + ASSERT_EQ(Framebuffers().RefusedCount(), 0u) << "a full but legal framebuffer was refused"; + + // The wire's colour width and the server's walk width are ONE number. + static_assert(std::size(MGPFramebufferState{}.Color) == kMGPipeMaxColorAttachments, + "the server walks Color[0..kMGPipeMaxColorAttachments); the array must be that wide"); + + const MGPFramebufferState* const applied = MGPipeApplier().DrawFramebuffer(); + ASSERT_NE(applied, nullptr); + for (Uint i = 0; i < kMGPipeMaxColorAttachments; ++i) { + const MGPipeHandle expected = + MGPipeSlots().FindByLifetimeId(MGPipeKind::Texture, colors[i]->GetLifetimeId()); + EXPECT_FALSE(MGPipeHandleIsNull(expected)) << "colour point " << i << " minted no handle"; + EXPECT_TRUE(applied->Color[i].Res == expected) + << "colour point " << i << " did not reach the server's record"; + EXPECT_EQ(applied->Color[i].Kind, kMGPipeSurfaceKindTexture) << "at colour point " << i; + } + EXPECT_EQ(applied->Depth.Kind, kMGPipeSurfaceKindRenderbuffer) << "the Depth point"; + EXPECT_EQ(applied->Stencil.Kind, kMGPipeSurfaceKindRenderbuffer) << "the Stencil point"; + + // HALF 2. A point the wire cannot describe appears; the record the server holds must not + // move, because a moved one would be a description of a framebuffer that does not exist. + const Uint64 hashBefore = applied->ContentHash; + const Uint64 serialBefore = MGPipeApplier().FramebufferSerial; + const auto beyond = MakeColorTexture(90, 32); + fbo->AttachTexture(static_cast( + static_cast(FramebufferAttachmentType::Color0) + + static_cast(kMGPipeMaxColorAttachments)), + beyond, TextureUploadTarget::Texture2D); + Framebuffers().EmitFramebufferState(Ctx()); + EXPECT_GE(Framebuffers().RefusedCount(), 1u) << "the over-wide framebuffer was not refused"; + const MGPFramebufferState* const after = MGPipeApplier().DrawFramebuffer(); + ASSERT_NE(after, nullptr); + EXPECT_EQ(after->ContentHash, hashBefore) + << "a refused framebuffer still moved the record the server walks - the eleven surfaces " + "are no longer the whole point set"; + EXPECT_EQ(MGPipeApplier().FramebufferSerial, serialBefore) + << "a refused framebuffer advanced FramebufferSerial, which is the server's clean gate"; +} + +// ============================ P5e (fb), ruling 15 / ID-98's §5.4 note ==================== +// +// AN EGL SURFACE RESIZE RE-EMITS THE DEFAULT FRAMEBUFFER'S RECORD, extent and all. +// +// S3 could not settle this and flagged it: FramebufferEmit's dirty rule is +// m_anyAttachmentGeneration plus the object/slot version, and a resize that moved NEITHER would +// be suppressed - leaving the server with the old extent and no other source for it, because +// under run-ahead the frontend's is unreachable. The brief's ruling 15 verified the path +// (ClientSession's surface-changed consumer -> TextureObjectBase::PipePublishLevelDescriptor -> +// MGP_NOTE_AGGREGATE(FramebufferAttachment) -> the framebuffer shutter) and told fb to pin it. +// +// THE CASE DRIVES THE CONSUMER'S OWN CALL, not a synthetic bump: AllocateStorage on the colour +// attachment at the new extent is literally what ClientSession does when the swapchain +// publishes a new size. No rebind, no re-attach, no draw-buffer edit - which is exactly the +// shape that would have been suppressed. +TEST(FramebufferEmit, ADefaultFramebufferResizeReEmitsTheRecordWithItsNewExtent) { + FramebufferScope scope; + const auto color = MakeColorTexture(95, 64); + const auto fbo = MakeShared(22); + fbo->AttachTexture(FramebufferAttachmentType::Color0, color, TextureUploadTarget::Texture2D); + BindDrawAndRead(fbo, fbo); + Framebuffers().EmitFramebufferState(Ctx()); + + const MGPFramebufferState* applied = MGPipeApplier().DrawFramebuffer(); + ASSERT_NE(applied, nullptr); + ASSERT_EQ(applied->Width, 64u); + ASSERT_EQ(applied->Height, 64u); + const Uint64 hashBefore = applied->ContentHash; + const Uint64 emissionsBefore = Framebuffers().EmissionCount(); + + // THE RESIZE, exactly as the surface-changed consumer performs it. + color->AllocateStorage(TextureUploadTarget::Texture2D, 0, + MipmapInput{IntVec3{128, 96, 1}, static_cast(128 * 96 * 4)}); + Framebuffers().EmitFramebufferState(Ctx()); + + EXPECT_GT(Framebuffers().EmissionCount(), emissionsBefore) + << "a resize of an attached surface published no framebuffer record - under run-ahead the " + "server would keep the old extent and has no other source for it"; + applied = MGPipeApplier().DrawFramebuffer(); + ASSERT_NE(applied, nullptr); + EXPECT_NE(applied->ContentHash, hashBefore) << "the resize did not move the record's content hash"; + EXPECT_EQ(applied->Width, 128u); + EXPECT_EQ(applied->Height, 96u); +} + // ============================ D-D2 ============================ // // RED BEFORE THIS PACKAGE LANDED. RenderbufferObject's three storage setters bump no version diff --git a/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp b/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp index 290dd289..71bfa31e 100644 --- a/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp @@ -357,7 +357,8 @@ TEST(ImageEmit, AMakeCurrentClearsTheImageSetAndAdvancesItsSerial) { X(ImageEmit, AnInternalFormatChangeAloneStillEmitsTheSet) \ X(ImageEmit, TheApplicationsFormatAndAccessTravelUnrecast) \ X(ImageEmit, AnImageBoundTextureIsMarkedShaderImageBoundAtTheBind) \ - X(ImageEmit, TheBindFeedsTheImageUnitHighWaterMark) + X(ImageEmit, TheBindFeedsTheImageUnitHighWaterMark) \ + X(ImageEmit, TheThreeAccessConstantsArePinnedOnBothSidesOfTheWire) #define MGL_DECLARE_PULL_SKIP(Suite, Name) \ TEST(Suite, Name) { GTEST_SKIP() << "compiled only under MOBILEGL_PIPE_PUSH"; } @@ -487,6 +488,87 @@ void main() { imageStore(img, ivec2(0), vec4(1.0)); } GL::UseProgram(0); } + // ============================ P5e, ruling 16 / ID-94 ============================ + // + // THE THREE CONSTANTS, PINNED ON BOTH SIDES OF THE WIRE, in one case, because the whole of + // ruling 16 is that they are ONE table and not two that happen to agree. + // + // Before P5e the client folded the three GL names into 0/1/2 in ImageEmit.h and the server + // read MGPImageBinding::Access off the FRONTEND instead of decoding the byte, under a + // comment saying the encoding "does not exist at the contract commit" - which was stale + // when it was written. c0e moved the numbers into MG_Pipe/MGPipeValueTypes.h and fb made + // the backend decode through them, so the failure this case exists to catch is the two + // halves drifting: an encode that writes 1 for GL_WRITE_ONLY against a decode that reads 1 + // as GL_READ_WRITE is a `writeonly` image the shader can read back, and no pixel test on a + // lockstep tree would see it because the frontend value was answering. + // + // 0 IS READ-ONLY AND THAT IS ASSERTED RATHER THAN ASSUMED: a zeroed MGPImageView must + // decode to the most restrictive access, so a record that forgot to set the field can only + // lose a write, never invent one. + TEST(ImageEmit, TheThreeAccessConstantsArePinnedOnBothSidesOfTheWire) { + // --- the numbers themselves ------------------------------------------------------- + EXPECT_EQ(kMGPipeImageAccessReadOnly, 0u); + EXPECT_EQ(kMGPipeImageAccessWriteOnly, 1u); + EXPECT_EQ(kMGPipeImageAccessReadWrite, 2u); + EXPECT_EQ(static_cast(MGPipeImageAccess::Count), 3u); + + // --- the CLIENT half: the encode names those enumerators, not literals ------------- + EXPECT_EQ(MGPipeEncodeImageAccess(GL_READ_ONLY), kMGPipeImageAccessReadOnly); + EXPECT_EQ(MGPipeEncodeImageAccess(GL_WRITE_ONLY), kMGPipeImageAccessWriteOnly); + EXPECT_EQ(MGPipeEncodeImageAccess(GL_READ_WRITE), kMGPipeImageAccessReadWrite); + + // --- the SERVER half: the decode is the inverse, and only those three are valid ----- + EXPECT_TRUE(MGPipeImageAccessIsValid(kMGPipeImageAccessReadOnly)); + EXPECT_TRUE(MGPipeImageAccessIsValid(kMGPipeImageAccessWriteOnly)); + EXPECT_TRUE(MGPipeImageAccessIsValid(kMGPipeImageAccessReadWrite)); + EXPECT_FALSE(MGPipeImageAccessIsValid(3)) + << "a fourth value must be Fatal{ProtocolCorruption, \"ImageView.Access\"} at the reader"; + EXPECT_EQ(MGPipeDecodeImageAccess(kMGPipeImageAccessReadOnly), MGPipeImageAccess::ReadOnly); + EXPECT_EQ(MGPipeDecodeImageAccess(kMGPipeImageAccessWriteOnly), MGPipeImageAccess::WriteOnly); + EXPECT_EQ(MGPipeDecodeImageAccess(kMGPipeImageAccessReadWrite), MGPipeImageAccess::ReadWrite); + + // The two predicates the backend's writable-image set and the barrier plan ask with. + EXPECT_TRUE(MGPipeImageAccessReads(MGPipeImageAccess::ReadOnly)); + EXPECT_FALSE(MGPipeImageAccessWrites(MGPipeImageAccess::ReadOnly)); + EXPECT_FALSE(MGPipeImageAccessReads(MGPipeImageAccess::WriteOnly)); + EXPECT_TRUE(MGPipeImageAccessWrites(MGPipeImageAccess::WriteOnly)); + EXPECT_TRUE(MGPipeImageAccessReads(MGPipeImageAccess::ReadWrite)); + EXPECT_TRUE(MGPipeImageAccessWrites(MGPipeImageAccess::ReadWrite)); + + // --- and the byte that actually crosses, for each of the three ------------------- + // + // ROUND TRIP THROUGH A REAL BIND, not through the encode alone: what the case is about + // is the value the SERVER will decode, and that is MGPImageView::Access as the emitter + // wrote it. + const struct { + GLenum gl; + Uint8 encoded; + MGPipeImageAccess decoded; + } kCases[] = { + {GL_READ_ONLY, kMGPipeImageAccessReadOnly, MGPipeImageAccess::ReadOnly}, + {GL_WRITE_ONLY, kMGPipeImageAccessWriteOnly, MGPipeImageAccess::WriteOnly}, + {GL_READ_WRITE, kMGPipeImageAccessReadWrite, MGPipeImageAccess::ReadWrite}, + }; + for (const auto& c : kCases) { + EmitterScope scope; + const GLuint program = MakeComputeProgram(kImageCompute); + GL::UseProgram(program); + const GLuint texture = MakeImageTexture(); + GL::BindImageTexture(1, texture, 0, GL_FALSE, 0, c.gl, GL_RGBA8); + Emitter().EmitShaderImages(Ctx()); + ASSERT_GE(Emitter().ImageSetCount(), 1u); + ASSERT_GE(Emitter().LastShaderImages().Count, 2u); + const MGPImageView& view = Emitter().LastImageViews()[1]; + EXPECT_EQ(view.Access, c.encoded) + << "glBindImageTexture(access=0x" << std::hex << static_cast(c.gl) << std::dec + << ") did not travel as the shared constant"; + ASSERT_TRUE(MGPipeImageAccessIsValid(view.Access)); + EXPECT_EQ(MGPipeDecodeImageAccess(view.Access), c.decoded) + << "the server's decode does not invert the client's encode"; + GL::UseProgram(0); + } + } + TEST(ImageEmit, AnAccessModeChangeAloneStillEmitsTheSet) { EmitterScope scope; const GLuint program = MakeComputeProgram(kImageCompute); From ca1beb6958730894b30fceef22f80f2510a7c204 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 08:30:18 -0400 Subject: [PATCH 6/6] [MG_Backend] (Disaggregated): P5e fb - revert the tx2 scaffold; the seam goes back to c0e's stub - Reverts f4675d1a. tx2 owns TextureImpl::SyncTextureToBackendByHandle and BackendTextureObject::SyncMipmapsToBackendByHandle; fb bodied them temporarily only so that fb's own integration-split and strict lanes could be measured at all, and this puts the c0e stubs back so the two packages cannot collide at the merge. - WHAT THIS COSTS, AND IT IS SAID HERE RATHER THAN DISCOVERED: with the stubs back, every integration-split entry whose framebuffer has a TEXTURE attachment - or whose draw binds an image - aborts on Fatal{UnmigratedVerb, "TextureImpl::SyncTextureToBackendByHandle"} rather than on anything fb wrote. fb's lane is green the moment tx2 lands beside it; the numbers measured with the scaffold are in the package report, with the scaffold's own cost (a GetTextureObject pull per call) named. --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 41 ++------------------- 1 file changed, 4 insertions(+), 37 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index cb29827d..46cacf4e 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -14193,48 +14193,15 @@ namespace MobileGL::MG_Backend::DirectGLES { } // namespace namespace TextureImpl { - // ================================================================================ - // P5e SCAFFOLD (fb) - THIS COMMIT IS REVERTED BEFORE fb's HEAD. DO NOT LAND IT. - // ================================================================================ - // tx2 owns these two bodies and is being written in parallel. fb CALLS them from the - // attachment surface sync, the two attachment texture lists, the image sweep and the - // named blit, so with the c0e stubs in place every integration-split entry that touches - // a texture aborts on the seam rather than on anything fb wrote - which leaves fb's own - // isplit and strict numbers unmeasurable. - // - // So these two are bodied HERE, temporarily, in the ONLY way that does not pre-empt - // tx2's design: the frontend texture is resolved from the record's own GL name through - // the BARRIER-PULLED sticky forward GetTextureObject - exactly as the copy-image sink - // resolves its endpoints (PipeApplier.cpp) - and the existing frontend overloads then - // do the work. That is correct under lockstep (the client is parked, which is what - // makes a sticky forward legal) and it is deliberately NOT what tx2 will write: tx2 - // drives the storage from the descriptor and has no frontend object at all. - // - // What it costs, and what to read past when reading fb's numbers: each call adds a - // GetTextureObject pull, so the strict lane sees GetTextureObject@ markers that - // belong to this scaffold and not to fb. SharedPtr& SyncTextureToBackendByHandle(MG_Pipe::MGPipeHandle texture, Bool imageBindableStorageRequired) { - static SharedPtr s_scaffoldNone; - const auto* record = PipeTextureRecordForHandle(texture); - if (record == nullptr) { - s_scaffoldNone = nullptr; - return s_scaffoldNone; - } - const auto& stateObj = MGB_CTX->GetTextureObject(static_cast(record->Desc.GlNameForDiag)); - if (!stateObj) { - s_scaffoldNone = nullptr; - return s_scaffoldNone; - } - return SyncTextureObjectToBackend(stateObj, imageBindableStorageRequired); + (void)imageBindableStorageRequired; + MGPipeP5eSeamNotLanded("TextureImpl::SyncTextureToBackendByHandle", "tx2", texture); } void BackendTextureObject::SyncMipmapsToBackendByHandle(MG_Pipe::MGPipeHandle texture) { - const auto* record = PipeTextureRecordForHandle(texture); - if (record == nullptr) return; - const auto& stateObj = MGB_CTX->GetTextureObject(static_cast(record->Desc.GlNameForDiag)); - if (!stateObj) return; - SyncMipmapsToBackend(stateObj); + MGPipeP5eSeamNotLanded("BackendTextureObject::SyncMipmapsToBackendByHandle", "tx2", + texture); } } // namespace TextureImpl