diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index f94ee1e5..1cc58a8c 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, + MG_Pipe::gPipeInputs.CurrentVerb()); + } +#endif // MOBILEGL_BUILD_DISAGGREGATED #endif // MOBILEGL_PIPE_PUSH static Bool IsDualSourceBlendFactor(BlendFactor v) { @@ -2839,6 +2862,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". @@ -2863,6 +2911,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 @@ -2935,6 +3035,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); @@ -3158,6 +3326,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_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). + if (FramebufferRecordArmIsMandatory()) { + SyncFramebufferAttachmentTexturesByRecord(); + return; + } +#endif const auto& drawSlot = GetFramebufferBindingSlotChecked(FramebufferTarget::Draw); const auto& currentFBO = drawSlot.GetBoundObject(); if (currentFBO) { @@ -3302,151 +3479,76 @@ namespace MobileGL::MG_Backend::DirectGLES { // 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. +#if MOBILEGL_PIPE_PUSH && MOBILEGL_BUILD_DISAGGREGATED + // P5e (fb): the eager glBindImageTexture funnel's HANDLE ARM, and it binds nothing. // - // 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. + // `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. // - // Count is the "has this set ever arrived" test, never the serial: MGPipeApplierReset - // advances the working-state serials whether or not anything was emitted. + // 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. // - // 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) { + // 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; } - 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. + // ---- 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 + // 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. + // + // 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. + // + // 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 @@ -3474,34 +3576,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 @@ -3516,7 +3716,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; @@ -3593,33 +3808,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: @@ -3634,6 +3861,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 @@ -3655,8 +3891,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) { @@ -3770,8 +4043,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; @@ -3897,11 +4181,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)` @@ -3909,47 +4192,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; @@ -3974,28 +4229,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 @@ -4008,15 +4249,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; @@ -4045,6 +4324,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}; @@ -5456,6 +5749,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); @@ -5590,18 +5892,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 @@ -5610,12 +5911,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 @@ -5623,12 +5932,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 @@ -7708,15 +8021,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)", @@ -9752,6 +10082,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 @@ -9781,23 +10317,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, @@ -9837,6 +10368,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, @@ -10444,6 +10993,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()) { @@ -11316,9 +11940,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())) { @@ -11599,12 +12233,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); } diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 281ba459..57d2f443 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -4452,6 +4452,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); } @@ -9959,8 +9979,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) { @@ -9975,38 +9995,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) { @@ -10017,31 +10028,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 @@ -10057,7 +10043,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, @@ -10098,27 +10087,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) { @@ -10132,7 +10100,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()); @@ -10234,6 +10205,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) { @@ -10253,6 +10307,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. @@ -10275,66 +10334,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 @@ -10710,7 +10710,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 @@ -10738,8 +10755,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); @@ -10831,6 +10848,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. @@ -11529,6 +11793,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); } @@ -14453,6 +14729,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 @@ -14611,13 +14966,10 @@ namespace MobileGL::MG_Backend::DirectGLES { #endif } // 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. // PrgramImpl's seam is GONE, not stubbed: package pg wrote // BackendProgramObjectImpl::SyncToBackendByHandle's real body beside the frontend overload @@ -14625,11 +14977,5 @@ namespace MobileGL::MG_Backend::DirectGLES { // here would make the linker's answer depend on link order, which is the one failure this // block's own comment was written to prevent. - 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 3e49bb55..8105d415 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -744,6 +744,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 @@ -2150,6 +2157,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, @@ -2239,6 +2253,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 }; @@ -2270,6 +2289,34 @@ 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); + + // 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 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);