diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index a7ebf22a..0d7909f2 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -1431,6 +1431,7 @@ namespace MobileGL::MG_Backend::DirectGLES { g_fboSyncedSlotVersions[SizeT(target)] = slotVersion; g_fboSyncedObjectVersions[SizeT(target)] = objectVersion; g_fboSyncedObjects[SizeT(target)] = fbo; + g_fboSyncedBackendIdGenerations[SizeT(target)] = g_attachmentBackendIdGeneration; } void SyncCurrentFBO() { @@ -1461,9 +1462,13 @@ namespace MobileGL::MG_Backend::DirectGLES { const Uint16 slotVersion = slot.GetVersion(); const Uint16 objectVersion = currentFBO ? currentFBO->GetObjectVersion() : 0; auto* currentPtr = currentFBO.get(); + // The backend-id generation joins the triple: a backend texture re-mint + // (RecreateBackendTexture) moves no frontend version, so without it the + // early-out would keep the driver FBO on the deleted texture name. if (slotVersion == g_fboSyncedSlotVersions[SizeT(target)] && objectVersion == g_fboSyncedObjectVersions[SizeT(target)] && - currentPtr == g_fboSyncedObjects[SizeT(target)]) { + currentPtr == g_fboSyncedObjects[SizeT(target)] && + g_fboSyncedBackendIdGenerations[SizeT(target)] == g_attachmentBackendIdGeneration) { lastUpdatedFBO = currentPtr; continue; } @@ -2129,6 +2134,15 @@ namespace MobileGL::MG_Backend::DirectGLES { static Bool g_broadcastMemoValid = false; static Uint g_broadcastMemoCount = 1; + // The identity+version key above is only monotonic WITHIN one GLContext: a + // library teardown + re-init frees every FramebufferObject and restarts the + // draw slot's counter at zero, so a recycled FBO address with coinciding + // fresh versions would false-hit. Cleared at the same boundaries as the + // structurally identical SyncCurrentFBO trio (InvalidateFramebufferBindingCache). + void InvalidateBroadcastMemo() { + g_broadcastMemoValid = false; + } + void SyncCurrentProgram(const SharedPtr& currentProgram) { #ifdef TRACY_ENABLE ZoneScopedC(TRACY_ZONECOLOR_BACKEND); @@ -2312,6 +2326,8 @@ namespace MobileGL::MG_Backend::DirectGLES { 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; } static void BindCurrentProgramWithResources( @@ -8304,6 +8320,9 @@ namespace MobileGL::MG_Backend::DirectGLES { // Conservatively drop the redundant-glUseProgram guard: re-issuing one bind // after a MakeCurrent is cheaper than trusting a possibly-reset context. PrgramImpl::g_lastUsedBackendProgramId = 0; + // The GLContext becoming current may be a fresh one whose slot versions + // restarted at zero; the broadcast memo's key is only monotonic within one. + PrgramImpl::InvalidateBroadcastMemo(); BufferImpl::InvalidateIndexedBufferBindingCache(); BufferImpl::InvalidatePixelBufferBindingCaches(); FramebufferImpl::InvalidateFramebufferBindingCache(); @@ -8832,6 +8851,7 @@ namespace MobileGL::MG_Backend::DirectGLES { FramebufferImpl::InvalidateFramebufferBindingCache(); VertexArrayImpl::InvalidateVAOBindingCache(); PixelStoreImpl::InvalidatePackStateCache(); + PrgramImpl::InvalidateBroadcastMemo(); // Texture ids belong to the dying context; wrappers destroyed later must // not glDeleteTextures a recycled name in a successor context. ++g_backendContextGeneration; diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index d5245e70..45d32f75 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -577,6 +577,9 @@ namespace MobileGL::MG_Backend::DirectGLES { // immutable storage, and any prior mutable store is replaced anyway. if (resource->id != 0) { NoteBufferIdDeleted(resource->id); + // Driver VAOs may have this id baked into attribute/element bindings + // keyed on frontend versions this re-mint does not move. + ++g_bufferBackendIdGeneration; g_GLESFuncs.glDeleteBuffers(1, &resource->id); resource->id = 0; resource->immutableStorage = false; @@ -833,6 +836,10 @@ namespace MobileGL::MG_Backend::DirectGLES { g_bufferMutationEpoch.fetch_add(1, std::memory_order_release); } + // See the declaration: re-mints of a live resource's driver id. Written only on + // the context thread (both re-mint sites run there), read only by the VAO sync. + Uint64 g_bufferBackendIdGeneration = 0; + void RegisterBufferBackendOps() { MG_State::GLState::SetBufferBackendOps(&g_glesBufferBackendOps); // Frontend writes issued while ops were unregistered advanced change @@ -960,6 +967,9 @@ namespace MobileGL::MG_Backend::DirectGLES { // here, on the thread that can, and the id is re-minted below. if (resource->immutableStorage && !resource->persistentMapped && resource->id != 0) { NoteBufferIdDeleted(resource->id); + // Same as the persistent-map re-mint: the dying id may be baked into + // driver VAO bindings whose frontend versions do not move for this. + ++g_bufferBackendIdGeneration; g_GLESFuncs.glDeleteBuffers(1, &resource->id); resource->id = 0; resource->immutableStorage = false; @@ -1640,8 +1650,24 @@ namespace MobileGL::MG_Backend::DirectGLES { // PrepareForDraw's BindCurrentVAO establishes the draw binding regardless. const Uint32 currentConfigVersion = stateVAOObject->GetConfigVersion(); const Uint16 currentIndexBufferVersion = stateVAOObject->GetIndexBufferBindingSlot().GetVersion(); - const Bool attributesDirty = !m_hasSyncedConfigVersion || m_syncedConfigVersion != currentConfigVersion; - const Bool indexBufferDirty = currentIndexBufferVersion != m_syncedIndexBufferVersion; + // A live buffer's driver id was re-minted since this twin's last emit + // (persistent-map adoption / immutable-store retire): every baked binding may + // hold the dead id while every frontend version still matches, so force a + // full re-emit. Read once; each buffer re-mints at most once per walk (its + // first EnsureBufferResource this draw), before its id is baked, so stamping + // the entry value at the end is exact - and a stale stamp only costs one + // extra full emit. + const Uint64 currentBufferIdGeneration = BufferImpl::g_bufferBackendIdGeneration; + const Bool bufferIdsRemitted = m_syncedBufferIdGeneration != currentBufferIdGeneration; + const Bool attributesDirty = + bufferIdsRemitted || !m_hasSyncedConfigVersion || m_syncedConfigVersion != currentConfigVersion; + // Identity joins the version compare: the slot version is a wrapping Uint16, + // so a wrapped-back count with a different buffer bound must still read dirty. + const MG_State::GLState::BufferObject* currentIndexBufferObject = + stateVAOObject->GetIndexBufferBindingSlot().GetBoundObject().get(); + const Bool indexBufferDirty = bufferIdsRemitted || + currentIndexBufferVersion != m_syncedIndexBufferVersion || + currentIndexBufferObject != m_syncedIndexBufferObject; // The baseInstance shift lives in the attribute offsets the driver already holds, so // a change of baseInstance has to re-emit the divisor'd arrays even when the frontend @@ -1675,10 +1701,10 @@ namespace MobileGL::MG_Backend::DirectGLES { } } - Bool needsSyncFormat = allAttributeVersions[attribIndex].FormatVersion != - m_syncedAttributeVersions[attribIndex].FormatVersion; - Bool needsSyncBuffer = allAttributeVersions[attribIndex].BufferVersion != - m_syncedAttributeVersions[attribIndex].BufferVersion; + Bool needsSyncFormat = bufferIdsRemitted || allAttributeVersions[attribIndex].FormatVersion != + m_syncedAttributeVersions[attribIndex].FormatVersion; + Bool needsSyncBuffer = bufferIdsRemitted || allAttributeVersions[attribIndex].BufferVersion != + m_syncedAttributeVersions[attribIndex].BufferVersion; if (!needsSyncFormat && !needsSyncBuffer && !needsSyncBaseInstance) continue; // Defence in depth. The frontend already declines glVertexAttribLFormat on this @@ -1798,6 +1824,7 @@ namespace MobileGL::MG_Backend::DirectGLES { if (indexBufferSynced) { m_syncedIndexBufferVersion = currentIndexBufferVersion; + m_syncedIndexBufferObject = currentIndexBufferObject; } } @@ -1809,6 +1836,7 @@ namespace MobileGL::MG_Backend::DirectGLES { if (emitAttributes) { m_syncedFetchBaseInstance = fetchBaseInstance; } + m_syncedBufferIdGeneration = currentBufferIdGeneration; } void BackendVertexArrayObject::SyncClientSideAttributesForDrawArrays( @@ -1946,6 +1974,11 @@ namespace MobileGL::MG_Backend::DirectGLES { void BackendTextureObject::RecreateBackendTexture() { if (m_backendTextureId != 0) { ScratchFBOImpl::NoteTextureIdDeleted(m_backendTextureId); + // Application FBO twins that attached the dying id memoize on FRONTEND + // attachment versions, which this backend-side re-mint does not move; + // without this bump their driver FBOs would keep the deleted name + // attached forever (see g_attachmentBackendIdGeneration). + ++FramebufferImpl::g_attachmentBackendIdGeneration; if (m_contextGeneration == g_backendContextGeneration) { g_GLESFuncs.glDeleteTextures(1, &m_backendTextureId); } @@ -3517,6 +3550,9 @@ namespace MobileGL::MG_Backend::DirectGLES { m_backendReadBuffer = GL_NONE; std::fill(m_syncedFrontendAttachmentVersions.begin(), m_syncedFrontendAttachmentVersions.end(), static_cast(~0u)); + // Every attachment version is invalidated above, so the next walk re-attaches + // everything regardless; stamp the generation so it does not re-arm twice. + m_syncedBackendIdGeneration = g_attachmentBackendIdGeneration; } static Bool SyncAttachmentObject(GLenum glFBOTarget, @@ -4005,6 +4041,15 @@ namespace MobileGL::MG_Backend::DirectGLES { } // -------------------- Attach texture to backend FBO ----------------------- + // A backend texture id was re-minted since this twin's last walk + // (RecreateBackendTexture): any point here may still hold the dead id while + // its frontend attachment version is unchanged, so the memo below would skip + // exactly the attachment that needs repair. Re-arm every point first. + if (m_syncedBackendIdGeneration != g_attachmentBackendIdGeneration) { + std::fill(m_syncedFrontendAttachmentVersions.begin(), m_syncedFrontendAttachmentVersions.end(), + static_cast(~0u)); + m_syncedBackendIdGeneration = g_attachmentBackendIdGeneration; + } const auto& attachments = stateFBOObject->GetAllAttachmentObjects(); const auto& attachmentVersions = stateFBOObject->GetAllFramebufferAttachmentVersions(); for (SizeT i = 0; i < attachments.size(); ++i) { @@ -4093,6 +4138,18 @@ namespace MobileGL::MG_Backend::DirectGLES { } #endif } + + // The walk itself can re-mint an id (SyncAttachmentObject -> + // SyncMipmapsToBackend -> RecreateBackendTexture), invalidating points this + // walk already attached or version-skipped - e.g. one texture attached at two + // points. Re-enter until the generation is quiescent: every pass syncs each + // dirty texture clean, so each repeat finds strictly fewer re-mints and the + // common case (no re-mint) never takes a second pass. The head's draw/read- + // buffer syncs are memoized against their own shadows, so a repeat re-walks + // only the attachments. + if (m_syncedBackendIdGeneration != g_attachmentBackendIdGeneration) { + SyncToBackend(stateFBOObject, asTarget); + } } GLenum BackendFramebufferObject::GetBackendAttachmentType(FramebufferAttachmentType frontendAtt) const { @@ -4119,6 +4176,8 @@ namespace MobileGL::MG_Backend::DirectGLES { Array g_fboSyncedObjectVersions = {0}; Array g_fboSyncedObjects = {}; + Uint64 g_attachmentBackendIdGeneration = 0; + Array g_fboSyncedBackendIdGenerations = {0}; } // namespace FramebufferImpl namespace ScratchFBOImpl { diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 7cb98587..34eb45e0 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -346,6 +346,14 @@ namespace MobileGL::MG_Backend::DirectGLES { // client-attribute staging buffers): scrub every buffer-binding shadow that // could false-skip when the name is recycled. void NoteBufferIdDeleted(Uint id); + // Bumped whenever a live GLESBufferResource's driver id is retired and re-minted + // while its frontend buffer stays alive (persistent-map adoption, immutable-store + // retire). The VAO twins' baked glVertexAttribPointer / element-array bindings + // key on FRONTEND versions, which a backend-side re-mint does not move - without + // this generation the driver VAO would keep fetching through the deleted id (or + // its retained store) forever. Compared and stamped by + // BackendVertexArrayObject::SyncToBackend. + extern Uint64 g_bufferBackendIdGeneration; // Redundant-bind cache for INDEXED buffer bindings (glBindBufferBase/Range on // GL_UNIFORM_BUFFER / GL_SHADER_STORAGE_BUFFER): skips the GL call when the // (id, range) already at that index matches, like the array-buffer/texture/ @@ -465,6 +473,11 @@ namespace MobileGL::MG_Backend::DirectGLES { Array m_clientAttributeBufferIds; Bool m_isInitialized = false; Uint16 m_syncedIndexBufferVersion = 0; + // Identity of the buffer the version above was stamped against. Raw and never + // dereferenced: the slot version is a wrapping Uint16 (see the ResolvedDrawBuffers + // IBO memo and the packed_pixels postmortem at BindCurrentFBO), so the version + // alone would read a wrapped-back count with a different buffer bound as clean. + const MG_State::GLState::BufferObject* m_syncedIndexBufferObject = nullptr; // Aggregate gate over the per-attribute walk below: the frontend bumps its config // version on every per-attribute version bump (the three Bump*Version functions are // its only writers), so an unchanged config version proves every per-attribute @@ -480,6 +493,11 @@ namespace MobileGL::MG_Backend::DirectGLES { // Kept here because it describes what was last EMITTED, which is what the next sync // has to correct. Uint32 m_syncedFetchBaseInstance = 0; + // BufferImpl::g_bufferBackendIdGeneration as of this twin's last emit. A + // mismatch means some live buffer's driver id was re-minted since; the ids + // baked into the driver VAO's attribute/element bindings may be dead even + // though every frontend version matches, so the next sync re-emits them all. + Uint64 m_syncedBufferIdGeneration = 0; }; extern StateBackendObjectRegistry @@ -799,6 +817,11 @@ namespace MobileGL::MG_Backend::DirectGLES { using FramebufferObject = MG_State::GLState::FramebufferObject; FramebufferObject::FramebufferAttachmentVersionArray m_syncedFrontendAttachmentVersions = {0}; + // g_attachmentBackendIdGeneration as of this twin's last attachment walk. A + // mismatch means some backend texture id was re-minted since, and any of this + // twin's attachment points may still hold the dead id even though the frontend + // attachment versions match - so the walk re-attaches everything first. + Uint64 m_syncedBackendIdGeneration = 0; }; extern StateBackendObjectRegistry @@ -888,6 +911,19 @@ namespace MobileGL::MG_Backend::DirectGLES { extern Array g_fboSyncedObjects; + // Bumped whenever a live backend texture's driver id is re-minted while its + // frontend texture may still be attached to application FBOs + // (BackendTextureObject::RecreateBackendTexture - e.g. a respecify of a texture + // whose backend storage went immutable). The FBO twins' attachment memos key on + // FRONTEND attachment versions, which a backend-side re-mint does not move, so + // the driver FBO would keep the deleted texture name attached forever. The + // SyncCurrentFBO gate compares this generation (below) to re-enter the sync, + // and each twin re-arms its per-attachment memo on a mismatch (SyncToBackend). + extern Uint64 g_attachmentBackendIdGeneration; + // What g_attachmentBackendIdGeneration was when SyncCurrentFBO last stamped each + // target; part of the synced tuple above. + extern Array g_fboSyncedBackendIdGenerations; + // Driver-level READ/DRAW framebuffer-binding shadow. Every backend // glBindFramebuffer routes through BindFramebufferId so scoped helpers can // save/restore the current binding without a glGetIntegerv round-trip (that diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp index 47ddbde9..caf17a7a 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp @@ -287,8 +287,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { if (m_frameBoundaryCounter - it->second->lastUsedFrameBoundary > kRetireAgeBoundaries) { it = m_cache.erase(it); // Invalidate every VAO's state-pointer memo: the erased node's - // address may be reused by a future insert. - ++m_evictionEpoch; + // address may be reused by a future insert. Advance through the + // process-wide source so the value stays unique across factory + // instances (see the member comment). + m_evictionEpoch = ++s_evictionEpochSource; } else { ++it; } diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h index e5642fbd..7e13525e 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h @@ -125,7 +125,17 @@ namespace MobileGL::MG_Backend::DirectVulkan { // construction); a memo is honored only while its recorded epoch // matches, so an evicted entry can never be dereferenced through a // stale memo. - Uint64 m_evictionEpoch = 1; + // + // Drawn from a process-wide source, never a per-instance counter: the VAO + // memos outlive this factory (they live on pGLContext's VAOs, the renderer + // is destroyed and recreated on EGL surface release/re-create), so a fresh + // factory restarting at a dead factory's epoch value would honor its + // dangling entry pointers. The constructor takes a value strictly greater + // than anything a predecessor ever stamped, so a dead factory's memo can + // never compare equal here - the same never-reused idiom as the lifetime ids. + // Single-threaded like the rest of the factory (renderer-thread only). + static inline Uint64 s_evictionEpochSource = 0; + Uint64 m_evictionEpoch = ++s_evictionEpochSource; static inline XXH64_state_t* m_hashState = XXH64_createState(); }; } // namespace MobileGL::MG_Backend::DirectVulkan diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkClearManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkClearManager.cpp index c4c589a4..a49c8c76 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkClearManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkClearManager.cpp @@ -166,7 +166,15 @@ namespace MobileGL::MG_Backend::DirectVulkan { void VkClearManager::MergeClearPayload(ClearAttachmentPayload& dst, const ClearAttachmentPayload& src) { dst.mask |= src.mask; if ((src.mask & GL_COLOR_BUFFER_BIT) != 0) { + // The whole colour story travels together (same rule as + // VkRenderPassManager::QueueRenderbufferClear): a glClearBufferiv/uiv + // payload carries its value in colorInt/colorUint and its branch selector + // in colorEncoding - dropping them here would leave the pending clear + // reading as an all-zero float one. dst.color = src.color; + dst.colorEncoding = src.colorEncoding; + dst.colorInt = src.colorInt; + dst.colorUint = src.colorUint; } if ((src.mask & GL_DEPTH_BUFFER_BIT) != 0) { dst.depth = src.depth; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.cpp index 6203b443..9ef72b17 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.cpp @@ -831,6 +831,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { // recreated since (texture + renderbuffer image epochs), and no pending clear (which alters // load ops). Any of these differing forces the full recompute below. Portable to VK 1.1. if (activeRenderPass != nullptr && m_rpFastValid && m_rpFastFbo == &fbo && + m_rpFastFboLifetimeId == fbo.GetLifetimeId() && m_rpFastFboVersion == fbo.GetObjectVersion() && m_rpFastSwapchainIndex == swapchainImageIndex && m_rpFastTexEpoch == m_textureManager.GetTextureImageEpoch() && m_rpFastRbEpoch == m_renderbufferImageEpoch && @@ -855,6 +856,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { // epochs AFTER ComputeHash: its attachment SyncTexture can create an image (bump the epoch). m_rpFastValid = true; m_rpFastFbo = &fbo; + m_rpFastFboLifetimeId = fbo.GetLifetimeId(); m_rpFastFboVersion = fbo.GetObjectVersion(); m_rpFastSwapchainIndex = swapchainImageIndex; m_rpFastTexEpoch = m_textureManager.GetTextureImageEpoch(); @@ -1507,7 +1509,23 @@ namespace MobileGL::MG_Backend::DirectVulkan { ClearAttachmentPayload clearPayload{}; SharedPtr liveTexture; if (pending.hasInlinePayload) { - clearPayload = pending.inlinePayload; + // The inline payload was snapshotted when the entry was CREATED, but the + // clear VALUE is not part of the entry's hash - a cache hit with a newer + // glClear would replay the creation-time value and drop the new one (the + // texture path below is immune because it re-reads the live payload). + // Same defense as ClearAttachmentsOnActiveRenderPass: prefer the live + // pending clear, fall back to the snapshot only when none is queued. + if (s_renderPassManager != nullptr && + s_renderPassManager->GetPendingRenderbufferClear(pending.renderbuffer, clearPayload)) { + if ((clearPayload.mask & GL_COLOR_BUFFER_BIT) != 0 && pending.renderbuffer != nullptr && + MG_Util::GetBaseInternalFormatComponentCount(pending.renderbuffer->GetInternalFormat()) == + 3) { + // RGB renderbuffers are backed by an RGBA image; the missing alpha reads as 1. + ForceOpaqueClearAlpha(clearPayload); + } + } else { + clearPayload = pending.inlinePayload; + } } else { if (pending.key.texture == nullptr || !s_clearManager->GetPendingClear(pending.key, clearPayload, liveTexture)) { diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.h index 531a46be..0cf11ee4 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.h @@ -289,6 +289,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { // or a pending clear. Portable to Vulkan 1.1 (no dynamic_rendering / imageless FB needed). Bool m_rpFastValid = false; const MG_State::GLState::FramebufferObject* m_rpFastFbo = nullptr; + // The FBO's never-reused lifetime id joins the raw pointer + Uint16 version: + // a deleted FBO reallocated at the same address whose fresh setup performed + // the same number of version bumps would otherwise compare equal (both count + // from 0), serving the dead framebuffer's pass to the new object. + Uint64 m_rpFastFboLifetimeId = 0; Uint16 m_rpFastFboVersion = 0; Uint32 m_rpFastSwapchainIndex = 0; Uint64 m_rpFastTexEpoch = 0; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp index c068f076..233f472d 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp @@ -1993,6 +1993,13 @@ namespace MobileGL::MG_Backend::DirectVulkan { texture.GetExternalIndex(), MG_Util::ConvertTextureUploadTargetToString(uploadTarget).c_str(), static_cast(format), static_cast(imageInfo.usage)); + // The preserved image was written by GPU work that may still be in flight + // (preserve requires layout != UNDEFINED); park it on the deferred ring + // like every other destruction path instead of letting the unique_ptr + // destroy it synchronously under the GPU. + if (preservedResource) { + DeferResourceRelease(Move(*preservedResource)); + } return false; } } @@ -2015,6 +2022,12 @@ namespace MobileGL::MG_Backend::DirectVulkan { static_cast(imageInfo.samples), static_cast(imageInfo.format)); resource.image = VK_NULL_HANDLE; resource.allocation = nullptr; + // Same as the probe failure above: the preserved live image must go through + // the deferred ring, never a synchronous destructor while frames that + // reference it are still in flight. + if (preservedResource) { + DeferResourceRelease(Move(*preservedResource)); + } return false; } ++m_textureImageEpoch; // a new attachment image invalidates cached render passes diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 92b9469a..28356446 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -3986,7 +3986,7 @@ void main() { // Skips the per-draw GetBackendResource chase into a cold resource object. Bool sliceStillValid = false; const Uint64 frameSerial = m_bufferManager.GetFrameSerial(); - if (indexMemo->indexFrameSerial == frameSerial && + if (indexMemo->indexFrameSerial == frameSerial && !indexMemo->indexBufferMapped && indexMemo->indexSliceEpochCounter == m_bufferManager.GetSliceEpochCounter()) { sliceStillValid = true; } @@ -4049,6 +4049,9 @@ void main() { indexMemo->indexVkBuffer = slice.buffer; indexMemo->indexSliceOffset = slice.offset; indexMemo->indexFrameSerial = m_bufferManager.GetFrameSerial(); + // A host-mapped EBO can mutate its shadow with no epoch bump; the hit + // path declines on this flag (mirror of anyBufferMapped). + indexMemo->indexBufferMapped = indexBufferShared->IsMapped(); } } const VkDeviceSize indexBindOffset = @@ -5718,6 +5721,22 @@ void main() { if (program.GetBackendStateVersion() != snap.programVersion) { return false; } + // glBegin/EndTransformFeedback moves no key this fast path otherwise observes + // (the design makes capture a compile-option FLAG precisely because no version + // bumps, VulkanRenderer.h's pipeline-memo note) - but the snapshot bakes that + // flag into resolvedTransformFlags and the pipeline. Recompute the one dynamic + // bit (the full path's exact predicate) and decline on a mismatch, or the first + // captured draw after glBeginTransformFeedback would bind the undecorated + // variant and silently capture nothing while the CPU bookkeeping advances. + const Bool wantsXfbCapture = m_transformFeedbackFeatureEnabled && + MG_State::pGLContext->IsTransformFeedbackActive() && + program.GetTransformFeedbackVaryingCount() > 0; + const Bool snapHasXfbCapture = + static_cast(ProgramFactory::CompileOptionFlags(snap.resolvedTransformFlags) & + ProgramFactory::CompileOptionBit::XfbCapture); + if (wantsXfbCapture != snapHasXfbCapture) { + return false; + } // A changed VAO does NOT decline: the VAO only feeds the pipeline's vertex // input state (re-resolved below through the layout-keyed memo, so N VAOs // sharing one attribute layout share one pipeline) and the vertex/index @@ -5731,6 +5750,7 @@ void main() { const auto& drawFbo = MG_State::pGLContext->GetFramebufferBindingSlot(FramebufferTarget::Draw).GetBoundObject(); if (static_cast(drawFbo.get()) != snap.drawFbo || + drawFbo->GetLifetimeId() != snap.drawFboLifetimeId || drawFbo->GetObjectVersion() != snap.fboVersion) { return false; } @@ -5914,8 +5934,14 @@ void main() { } const Uint64 samplingResolutionGeneration = MG_State::pGLContext->GetSamplingResolutionGeneration(); if (samplingResolutionGeneration != snap.samplingResolutionGeneration) { - snap.samplingResolutionGeneration = samplingResolutionGeneration; - samplerDescriptorsUnchanged = false; + // Decline, not re-arm: snap.resolvedTransformFlags bakes the + // ExplicitLod0Sampling verdict, which reads the effective sampler's + // filters/aniso/LOD range - exactly the state this counter tracks. + // Re-arming the stamp here would rebuild the descriptors but keep the + // stale SPIR-V variant forever (every later draw compares equal again). + // Same shape as the erase-epoch declines above; costs one full-path draw + // per sampler/shape change, and the full path's LOD memo re-probes. + return false; } // Everything the full path would re-resolve is provably unchanged - or, for @@ -6095,11 +6121,18 @@ void main() { const Uint64 lodProgramLifetimeId = program.GetLifetimeId(); const Uint32 lodProgramVersion = program.GetBackendStateVersion(); const Uint64 lodBindGeneration = MG_State::pGLContext->GetTextureBindGeneration(); + // The probe also reads the EFFECTIVE sampler's filters/aniso/LOD range + // (ProgramSamplesOnlySingleLevelTextures), and those setters bump ONLY the + // sampling-resolution generation - not the texture params version the sum + // below covers. Without this key a filter/aniso change would keep serving + // the stale verdict. + const Uint64 lodSamplingGeneration = MG_State::pGLContext->GetSamplingResolutionGeneration(); Bool lodMemoHit = false; if (m_lastLodDecisionValid && m_lastSampledSetValid && m_lastLodProgramLifetimeId == lodProgramLifetimeId && m_lastLodProgramVersion == lodProgramVersion && - m_lastLodBindGeneration == lodBindGeneration && m_lastLodBaseFlags == transformFlags && + m_lastLodBindGeneration == lodBindGeneration && + m_lastLodSamplingGeneration == lodSamplingGeneration && m_lastLodBaseFlags == transformFlags && m_lastSampledSetProgramLifetimeId == lodProgramLifetimeId && m_lastSampledSetProgramVersion == lodProgramVersion && m_lastSampledSetBindGeneration == lodBindGeneration) { @@ -6124,6 +6157,7 @@ void main() { m_lastLodProgramLifetimeId = lodProgramLifetimeId; m_lastLodProgramVersion = lodProgramVersion; m_lastLodBindGeneration = lodBindGeneration; + m_lastLodSamplingGeneration = lodSamplingGeneration; m_lastLodBaseFlags = baseFlags; m_lastLodResultFlags = transformFlags; m_lastLodParamsSum = 0; // filled below once the sampled set is known @@ -6467,6 +6501,7 @@ void main() { snap.vaoLifetimeId = vao.GetLifetimeId(); snap.vaoConfigVersion = vao.GetConfigVersion(); snap.drawFbo = drawFbo.get(); + snap.drawFboLifetimeId = drawFbo->GetLifetimeId(); snap.fboVersion = drawFbo->GetObjectVersion(); snap.drawFboIsDefault = drawFboIsDefault; snap.viewportCount = ResolveDrawViewportCount(programObj.writesViewportIndexBuiltin); @@ -13825,6 +13860,15 @@ void main() { VkPipeline pipeline = VK_NULL_HANDLE; VK_VERIFY(vkCreateComputePipelines(m_device, VK_NULL_HANDLE, 1, &pipelineInfo, nullptr, &pipeline), "GetOrCreateComputePipeline, vkCreateComputePipelines"); + // A failed creation must never be memoized - same contract as + // PipelineFactory::GetOrCreatePipeline: caching the null would serve it back + // for the rest of the process and every dispatch of this program would be + // silently skipped. Retrying costs one failed vkCreateComputePipelines per + // dispatch, which is the correct price. + if (pipeline == VK_NULL_HANDLE) { + MGLOG_E("GetOrCreateComputePipeline: vkCreateComputePipelines failed; not caching the failure"); + return VK_NULL_HANDLE; + } m_computePipelines.emplace(programObj.hash, pipeline); return pipeline; } diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h index d021df94..4df06667 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h @@ -807,6 +807,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { Uint32 m_lastLodProgramVersion = 0; Uint64 m_lastLodBindGeneration = 0; Uint64 m_lastLodParamsSum = 0; + // Sampling-resolution generation at probe time. The probe reads the effective + // sampler's filters/aniso/LOD range, whose setters bump only this counter - + // the params-version sum above never moves for them. + Uint64 m_lastLodSamplingGeneration = 0; ProgramFactory::CompileOptionFlags m_lastLodBaseFlags = {}; ProgramFactory::CompileOptionFlags m_lastLodResultFlags = {}; @@ -844,6 +848,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { Uint64 vaoLifetimeId = 0; Uint32 vaoConfigVersion = 0; const void* drawFbo = nullptr; + // Never-reused lifetime id beside the raw pointer + Uint16 version: a + // deleted FBO recycled at the same address with the same fresh version + // count would otherwise compare equal (same ABA as the render-pass + // manager's fast-path memo). + Uint64 drawFboLifetimeId = 0; Uint16 fboVersion = 0; Bool drawFboIsDefault = false; Uint renderStateVersion = 0; @@ -1067,6 +1076,14 @@ namespace MobileGL::MG_Backend::DirectVulkan { VkBuffer indexVkBuffer = VK_NULL_HANDLE; VkDeviceSize indexSliceOffset = 0; Uint64 indexFrameSerial = 0; + // The EBO carried a host map when the slice was recorded - the mirror of + // anyBufferMapped on the vertex half. A shadow-backed (non-adopted) + // persistent map mutates its shadow with no API call and no epoch bump, so + // the one-compare rescue must decline and re-run the acquire, whose + // SyncPersistentMappedRange is the push-down. A map taken AFTER the record + // is already covered: AcquirePersistentMap bumps the slice epoch for the + // request itself, adopted or declined. + Bool indexBufferMapped = false; // Bound per draw (first bindingCount elements). VkBuffer vkBuffers[kMaxBindings] = {}; diff --git a/MobileGL/MG_State/GLState/Core.cpp b/MobileGL/MG_State/GLState/Core.cpp index 32b9dd2d..87ac75e6 100644 --- a/MobileGL/MG_State/GLState/Core.cpp +++ b/MobileGL/MG_State/GLState/Core.cpp @@ -646,9 +646,17 @@ namespace MobileGL::MG_State { for (SizeT stage = 0; stage < ProgramPipelineObject::kGraphicsStageCount; ++stage) { const auto& stageProgram = pipeline->GetStageProgram(static_cast(stage)); if (!stageProgram) continue; - for (const auto& shader : stageProgram->GetAttachedShaders()) { - if (!shader || static_cast(shader->GetShaderStage()) != stage) continue; - composite->AttachShader(shader); + // The stage program contributes the shaders its LAST LINK consumed, never + // its live attach list: per GL 4.6 7.3/7.4 a pipeline stage executes the + // stage program as last linked - glAttachShader and glCompileShader take + // effect only at the program's next link - and neither of those moves the + // link version this cache keys on, so reading live state here would let a + // post-link attach or recompile leak into the composite while the signature + // still hits. The pinned (source, node) makes the composite's Link() + // consume the very inputs that link consumed. + for (const auto& ref : stageProgram->GetLinkedShaderSnapshot()) { + if (!ref.shader || static_cast(ref.shader->GetShaderStage()) != stage) continue; + composite->AttachShaderWithPinnedLinkInput(ref); anyStage = true; } } diff --git a/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.cpp b/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.cpp index abedbf79..b8e54d19 100644 --- a/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.cpp +++ b/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.cpp @@ -9,7 +9,18 @@ #include "FramebufferObject.h" #include "MG_Util/Types.h" +#include + namespace MobileGL::MG_State::GLState { + // Starts at 1 so a zero-initialized memo slot can never carry a live object's id. + // Atomic for the same reason as the VAO counter: it costs nothing, and a duplicate + // id would resurrect exactly the ABA this id exists to kill. + static std::atomic s_nextFramebufferLifetimeId{1}; + + Uint64 FramebufferObject::AllocateLifetimeId() { + return s_nextFramebufferLifetimeId.fetch_add(1, std::memory_order_relaxed); + } + // FramebufferAttachmentObject FramebufferAttachmentObject::FramebufferAttachmentObject( const SharedPtr& texture, TextureUploadTarget textureUploadTarget, Int level, diff --git a/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.h b/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.h index f526891b..aebe2162 100644 --- a/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.h +++ b/MobileGL/MG_State/GLState/FramebufferState/FramebufferObject.h @@ -148,13 +148,25 @@ namespace MobileGL { Uint16 GetObjectVersion() const { return m_objectVersion; } + // Globally-unique, never-reused id for THIS object's lifetime - the same + // contract as VertexArrayObject::GetLifetimeId(), and needed for the same + // reason: neither the GL name nor the heap address can tell a + // deleted-and-recreated framebuffer from the original, and m_objectVersion + // starts at 0 for every new object, so a backend memo keyed on + // (pointer, version) alone would silently inherit the dead object's entry + // (see VkRenderPassManager's per-draw fast-path memo). + Uint64 GetLifetimeId() const { return m_lifetimeId; } + Uint GetExternalIndex() const; Bool IsDefaultFramebuffer() const { return m_externalIndex == 0; } private: + static Uint64 AllocateLifetimeId(); + void BumpAttachmentVersion(FramebufferAttachmentType type); const Uint m_externalIndex = 0; + const Uint64 m_lifetimeId = AllocateLifetimeId(); FramebufferAttachmentObjectArray m_attachmentObjects; FramebufferAttachmentVersionArray m_attachmentVersions; diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp index fc6c3294..ea61ba33 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp @@ -393,6 +393,14 @@ namespace MobileGL::MG_State::GLState { return true; } + bool ProgramObject::AttachShaderWithPinnedLinkInput(const LinkedShaderRef& ref) { + if (!AttachShader(ref.shader)) { + return false; + } + m_pinnedLinkInputs[ref.shader.get()] = ref; + return true; + } + SizeT ProgramObject::DetachShader(const SharedPtr& shader) { MGLOG_D("DetachShader called for shader %p from ProgramObject %u", shader.get(), m_externalIndex); if (!ShaderIsAttached(shader)) { @@ -475,6 +483,8 @@ namespace MobileGL::MG_State::GLState { AddDefaultFragmentShaderIfMissing(); } if (m_shaders.empty()) { + // This IS the last link now, and it consumed nothing. + m_linkedShaderSnapshot.clear(); m_artifacts.infoLog = "No shader objects are attached to program."; MGLOG_E("ProgramObject %u: Link failed - no shader objects attached.", m_externalIndex); return; @@ -505,8 +515,19 @@ namespace MobileGL::MG_State::GLState { Vector> deps; deps.reserve(m_shaders.size()); task->in.shaders.reserve(m_shaders.size()); + m_linkedShaderSnapshot.clear(); + m_linkedShaderSnapshot.reserve(m_shaders.size()); for (const auto& shader : m_shaders) { - const SharedPtr& node = shader->CompiledNodeForLink(); + // A pipeline composite pins the (source, node) each stage program's LAST link + // consumed (AttachShaderWithPinnedLinkInput); an ordinary program takes the + // shader's current ones. Without the pin a post-link recompile would leak a + // shader the stage program never linked into the composite. + SharedPtr sourcePtr = shader->GetShaderSourcePtr(); + SharedPtr node = shader->CompiledNodeForLink(); + if (const auto pinned = m_pinnedLinkInputs.find(shader.get()); pinned != m_pinnedLinkInputs.end()) { + sourcePtr = pinned->second.source; + node = pinned->second.node; + } if (node) { // This link is now an observer of that node's result, and the ShaderObject is // no longer the only route to it: without the marker, the ordinary @@ -515,7 +536,10 @@ namespace MobileGL::MG_State::GLState { node->MarkLinkReferenced(); if (!node->IsTerminal()) deps.push_back(node); } - task->in.shaders.push_back({shader->GetShaderStage(), shader->GetShaderSourcePtr(), node}); + task->in.shaders.push_back({shader->GetShaderStage(), sourcePtr, node}); + // What "as last linked" will mean for this program from now on - the pipeline + // composite cache rebuilds from exactly this set (GetProgramForDraw). + m_linkedShaderSnapshot.push_back({shader, sourcePtr, node}); } // Phase B of the same link: SPIR-V generation, spirv-opt and the global-UBO routing diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h index 16173bff..14535afe 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h @@ -60,6 +60,26 @@ namespace MobileGL::MG_State::GLState { Vector>& GetAttachedShaders(); const Vector>& GetAttachedShaders() const; + + // One shader exactly as this program's last Link() consumed it: the object, the + // source snapshot, and the compile node taken at that link's enqueue. GL 4.6 7.3/7.4 + // makes this triple - not the live attach list, not the shader's current compile - + // what a program pipeline stage executes ("as last linked"): glAttachShader and + // glCompileShader take effect only at the program's next link, yet neither moves + // m_linkVersion, so anything keyed on the link generation must consume this + // snapshot rather than re-read the live state. + struct LinkedShaderRef { + SharedPtr shader; + SharedPtr source; + SharedPtr node; + }; + // The last link's full input set; empty when this program has never linked (or its + // last link had no shaders attached). GL-thread-owned, rebuilt in Link()'s prologue. + const Vector& GetLinkedShaderSnapshot() const { return m_linkedShaderSnapshot; } + // Pipeline-composite attach: AttachShader plus a pin that makes THIS program's + // Link() consume ref's (source, node) instead of the shader's current ones, so a + // post-link recompile of the stage program's shader cannot leak into the composite. + bool AttachShaderWithPinnedLinkInput(const LinkedShaderRef& ref); const String& GetInfoLog() const { return Artifacts().infoLog; } // glCreateShaderProgramv folds the shader's compile log into the program's log, which // is the only place a caller can read it from once the shader name is gone. @@ -1223,6 +1243,13 @@ namespace MobileGL::MG_State::GLState { // glGetAttachedShaders / GL_ATTACHED_SHADERS / the orphan-shader sweep need no join. Vector> m_shaders; Vector> m_detachedShaders; // Store detached shaders and remove on next link + // See GetLinkedShaderSnapshot. Holding the SharedPtrs here is deliberate: the + // "as last linked" set must survive detach-and-delete of its shaders (the + // glCreateShaderProgramv shape) until the next link replaces it. + Vector m_linkedShaderSnapshot; + // See AttachShaderWithPinnedLinkInput. Populated only on pipeline composites, + // which never detach, so entries need no removal path. GL-thread-owned. + UnorderedMap m_pinnedLinkInputs; // Link INPUTS (all "take effect at the next link" per GL): glBindAttribLocation, // glBindFragDataLocation(Indexed), glTransformFeedbackVaryings, and the draw-buffer diff --git a/MobileGL/MG_State/GLState/ProgramState/ShaderObject.cpp b/MobileGL/MG_State/GLState/ProgramState/ShaderObject.cpp index 2dc96c44..86059076 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ShaderObject.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ShaderObject.cpp @@ -140,6 +140,13 @@ namespace MobileGL::MG_State::GLState { } void ShaderObject::Compile() { + // The compile-environment snapshot is taken HERE, on the GL thread, and handed to + // the job. Everything the pipeline needs to know about the device comes through it, + // never through pActiveBackendObject - that is what makes the body movable. + // Hoisted above the memo check because the memo must be env-disciplined too (below). + const SharedPtr env = + MG_Util::ShaderTranspiler::GetCurrentCompileEnv(); + // P0b layer 1, as a tri-state: the memo is "the node in m_compiled was built from // the string m_source still points at". SetShaderSource only swaps that pointer when // the text actually differs, so this is a pointer compare, and it covers Pending as @@ -152,7 +159,18 @@ namespace MobileGL::MG_State::GLState { // ClaimParsedShader's on-demand re-parse needs - a real recompile would have handed // the next link a fresh parse, the no-op hands it a fresh re-parse of the identical // source instead. Same result, one parse either way. - if (HasMemoizedCompile()) return; + // + // The environment joins the check (ShaderSourceKey.h's memo-hazard rule: a memo + // must never be handed back under an environment other than the one it was + // computed against). Layers 2 and 3 key on the fingerprint, but this memo sits + // ABOVE both, so without this compare a node computed against a dead environment + // - e.g. a compute shader rejected against the pre-capability fallback limits - + // would keep answering forever while a fresh object with byte-identical source + // compiles fine. The fingerprint is a content hash, so a republish of identical + // capabilities still hits. + if (HasMemoizedCompile() && m_compiled->env != nullptr && m_compiled->env->fingerprint == env->fingerprint) { + return; + } // Two reasons to stay on this thread, one rule. Without the async flag the whole // path must be byte-identical to the synchronous implementation, and a cache-less @@ -168,12 +186,6 @@ namespace MobileGL::MG_State::GLState { // glMaxShaderCompilerThreadsKHR(0) and a flag-off build both bypass sharing exactly // as they bypass the pool, and their behaviour stays byte-identical to pre-stage-6. const Bool runOnPool = m_preprocessCache && MG_Util::Async::AsyncShaderCompileActive(); - - // The compile-environment snapshot is taken HERE, on the GL thread, and handed to - // the job. Everything the pipeline needs to know about the device comes through it, - // never through pActiveBackendObject - that is what makes the body movable. - const SharedPtr env = - MG_Util::ShaderTranspiler::GetCurrentCompileEnv(); const Uint64 sourceHash = ShaderPreprocessCache::HashSource(*m_source); // ---- P1 stage 6: adopt an equivalent compile instead of enqueueing a duplicate ----