diff --git a/MobileGL/Init.cpp b/MobileGL/Init.cpp index 905be751..59554a1f 100644 --- a/MobileGL/Init.cpp +++ b/MobileGL/Init.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -51,6 +52,11 @@ namespace MobileGL { // before a re-initialized library could pair them with the wrong // backend's DeleteSync). MG_Impl::GLImpl::DestroyAllSyncObjects(); + // Queries die with their contexts for the same reason, and their registry + // is the same shape of process-global map: drain it here too, while the + // function table can still pair each backend handle with the backend that + // minted it. + MG_Impl::GLImpl::DestroyAllQueryObjects(); MG_Backend::pActiveBackendObject.reset(); MG_State::pGLContext.reset(); MG_State::pEGLContext.reset(); diff --git a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp index 94470bfa..a917113c 100644 --- a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp @@ -69,6 +69,12 @@ namespace MobileGL::MG_Backend::DirectVulkan { // slot's ownership unambiguous. Uint64 programLifetimeId = 0; Uint32 backendStateVersion = 0; + // glShaderStorageBlockBinding deliberately does NOT bump the backend state + // version, and the pipeline composite is unnamed so the in-place patch in + // DirectVulkan::ShaderStorageBlockBinding can never reach its slot - the + // mirror replay bumps only the program's block-binding version. Without this + // key the composite's slot kept serving the pre-rebind block.binding. + Uint32 blockBindingVersion = 0; Vector storageBlocks; Vector bufferVariables; GLint computeWorkGroupSize[3] = {1, 1, 1}; @@ -156,18 +162,33 @@ namespace MobileGL::MG_Backend::DirectVulkan { auto& cache = g_programResourceCaches[program.GetExternalIndex()]; const Uint64 programLifetimeId = program.GetLifetimeId(); const Uint32 backendStateVersion = program.GetBackendStateVersion(); + const Uint32 blockBindingVersion = program.GetBlockBindingVersion(); // The lifetime id must match too: a new program that reuses a deleted // program's name and happens to land on the same backendStateVersion (both // count from zero) would otherwise be served the dead program's reflection. if (cache.programLifetimeId == programLifetimeId && cache.backendStateVersion == backendStateVersion && (!cache.storageBlocks.empty() || !cache.bufferVariables.empty())) { + if (cache.blockBindingVersion != blockBindingVersion) { + // Only the block bindings moved (glShaderStorageBlockBinding, or the + // pipeline composite's mirror replay - neither touches the backend + // state version): the reflection itself is unchanged, so re-apply the + // overrides by name instead of re-running spirv-reflect. Overrides + // only ever accumulate, so a block without one still holds its + // declared binding. + for (auto& block : cache.storageBlocks) { + const Int rebound = program.GetShaderStorageBlockBindingOverride(block.name); + if (rebound >= 0) block.binding = static_cast(rebound); + } + cache.blockBindingVersion = blockBindingVersion; + } return cache; } cache = {}; cache.programLifetimeId = programLifetimeId; cache.backendStateVersion = backendStateVersion; + cache.blockBindingVersion = blockBindingVersion; Vector modules; Vector validModules; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 28356446..f6f24e6f 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -3323,6 +3323,11 @@ void main() { indexView.indexByteSize > bufferSize - indexView.indexByteOffset) { return false; } + // Recorded-but-unexecuted GPU writes (XFB capture, SSBO, storage texel + // buffer) land in the coherent mapping this scan is about to read; + // submit-and-wait first, exactly like the restart-index rewrite does. + // A no-op unless the gpu-write flag is set. + indexBufferShared->SyncGpuWrites(); indexBufferShared->SyncPersistentMappedRange(); indexBytes = indexBufferShared->MappedData() + indexView.indexByteOffset; } else { @@ -3560,6 +3565,16 @@ void main() { const Uint8* sourceData, SizeT sourceStride, SizeT elementSize, SizeT elementCount, BufferSlice& outSlice) -> Bool { + // A resolved stride of 0 is the binding model's "never advance" (see the + // factory's layout notes): exactly one element is converted and every vertex + // reads it. That single element is read at offset 0, so the stride is never + // actually used - but both converters reject 0 as a degenerate input, which + // made the documented single-element conversion unreachable and silently + // dropped every draw using such a binding. Substitute the element's own + // size; the caller's cache key still carries the distinct stride 0. + if (sourceStride == 0 && elementCount == 1) { + sourceStride = elementSize; + } const void* uploadData = nullptr; VkDeviceSize uploadSize = 0; switch (conversion) { @@ -3693,6 +3708,12 @@ void main() { return false; } + // A GPU-written source (XFB capture, SSBO, storage texel buffer) has its + // bytes produced by commands that are merely RECORDED at this point, and + // MappedData() aliases the coherent GPU memory they will write into - + // converting now would read pre-write garbage. Submit-and-wait first, + // mirroring the restart-index rewrite; a flag-test no-op otherwise. + sourceBufferShared->SyncGpuWrites(); sourceBufferShared->SyncPersistentMappedRange(); const SizeT availableElementCount = sourceStride == 0 ? 1 : 1 + (sourceSize - baseOffset - elementSize) / sourceStride; diff --git a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp index 61199e89..29090d8c 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp @@ -1057,21 +1057,20 @@ namespace MobileGL::MG_Impl::GLImpl { return; } - static Bool allowVSOnlyPrograms; - static Bool initialized = false; - if (!initialized) { - const auto& activeBackendObject = MG_Backend::pActiveBackendObject; - if (!activeBackendObject) { - MGLOG_E_ONCE("activeBackendObject is not initialized!"); - return; - } - const auto& rendererInfo = activeBackendObject->GetRendererInfo(); - allowVSOnlyPrograms = (Int)rendererInfo.StaticBackendCapability.AllowVSOnlyPrograms; - } + // Read fresh every link, never latched in a static: the capability is + // per-backend, and a latch would freeze it across a backend teardown + + // re-initialization (the previous function-static memo here never even set + // its own initialized flag, so it re-read every call anyway - this makes + // the always-fresh behavior the stated one). A struct-field read per + // glLinkProgram costs nothing. const auto& activeBackendObject = MG_Backend::pActiveBackendObject; - if (activeBackendObject) { - programObject->SetMaxFragmentOutputColorNumber(activeBackendObject->GetDynamicParameters().MaxDrawBuffers); + if (!activeBackendObject) { + MGLOG_E_ONCE("activeBackendObject is not initialized!"); + return; } + const Bool allowVSOnlyPrograms = + activeBackendObject->GetRendererInfo().StaticBackendCapability.AllowVSOnlyPrograms; + programObject->SetMaxFragmentOutputColorNumber(activeBackendObject->GetDynamicParameters().MaxDrawBuffers); programObject->Link(!allowVSOnlyPrograms); } diff --git a/MobileGL/MG_Impl/GLImpl/Query/GL_Query.cpp b/MobileGL/MG_Impl/GLImpl/Query/GL_Query.cpp index 15fc7f92..5a7d84f1 100644 --- a/MobileGL/MG_Impl/GLImpl/Query/GL_Query.cpp +++ b/MobileGL/MG_Impl/GLImpl/Query/GL_Query.cpp @@ -648,4 +648,39 @@ namespace MobileGL::MG_Impl::GLImpl { if (!ValidateQueryStreamIndex(__FUNCTION__, target, index)) return; GetQueryiv(target, pname, params); } + + void DestroyAllQueryObjects() { + // Detach the registry under the lock, release outside it - same discipline + // (and the same accepted teardown race) as DestroyAllSyncObjects. Without + // this drain, every query the app left undeleted survived full library + // teardown in the process-global registry: the objects and their backend + // wrappers leaked across Destroy/Initialize cycles, stale ids kept + // answering IsQuery == GL_TRUE in the re-initialized library, and a later + // glDeleteQueries could hand the OLD backend's handle to a DIFFERENT + // backend's DeleteBackendQuery, which casts it to the wrong wrapper type. + UnorderedMap orphans; + { + const std::lock_guard lock(g_queryObjectsMutex); + orphans.swap(g_liveQueryObjects); + g_activeTimeElapsedQueryId = 0; + g_activePrimitivesWrittenQueryId = 0; + g_activePrimitivesGeneratedQueryId = 0; + g_activeSamplesPassedQueryId = 0; + } + if (orphans.empty()) { + return; + } + // Backend handles must be released by the backend that created them, so + // this runs while the function table is still populated. Both backends' + // DeleteBackendQuery are generation-guarded, so a handle whose renderer + // or ES context is already gone frees only the wrapper. + const auto deleteBackendQuery = MG_Backend::gBackendFunctionsTable.GL.DeleteBackendQuery; + for (const auto& [_, queryObject] : orphans) { + if (deleteBackendQuery && queryObject->backendHandle) { + deleteBackendQuery(queryObject->backendHandle); + } + delete queryObject; + } + MGLOG_D("DestroyAllQueryObjects: reclaimed %zu query object(s) the app left undeleted", orphans.size()); + } } // namespace MobileGL::MG_Impl::GLImpl diff --git a/MobileGL/MG_Impl/GLImpl/Query/GL_Query.h b/MobileGL/MG_Impl/GLImpl/Query/GL_Query.h index b85dfb5e..5921e7d2 100644 --- a/MobileGL/MG_Impl/GLImpl/Query/GL_Query.h +++ b/MobileGL/MG_Impl/GLImpl/Query/GL_Query.h @@ -29,4 +29,13 @@ namespace MobileGL::MG_Impl::GLImpl { void GetQueryBufferObjecti64v(GLuint id, GLuint buffer, GLenum pname, GLintptr offset); void GetQueryBufferObjectui64v(GLuint id, GLuint buffer, GLenum pname, GLintptr offset); void QueryCounter(GLuint id, GLenum target); + // Destroys every still-registered query object exactly as DeleteQueries would. + // GL requires queries to die with their context; called only from full library + // teardown (DestroyImpl), where no context survives on any thread, so the + // process-global registry can be drained wholesale. Must run while the backend + // function table is still populated: each backend handle has to be released by + // the backend that created it, never by a later re-initialized one (whose + // DeleteBackendQuery would cast the wrapper to the wrong backend's type). + // Same contract as DestroyAllSyncObjects. + void DestroyAllQueryObjects(); } // namespace MobileGL::MG_Impl::GLImpl diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h index 14535afe..a6cec261 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h @@ -806,6 +806,13 @@ namespace MobileGL::MG_State::GLState { // order - and the name is the only coordinate all three agree on. Absent from the map // means "never rebound", and the shader's declared binding still stands. void SetShaderStorageBlockBinding(const String& blockName, Uint binding) { + // Equality bail-out like SetUniformBlockBinding's: the pipeline composite + // mirror replays every override each draw, and without this every replay + // would churn m_blockBindingVersion and rebuild whatever keys on it. + const auto it = Artifacts().shaderStorageBlockBinding.find(blockName); + if (it != Artifacts().shaderStorageBlockBinding.end() && it->second == static_cast(binding)) { + return; + } Artifacts().shaderStorageBlockBinding[blockName] = static_cast(binding); // Deliberately NOT m_backendStateVersion: Espryt's entry point never forces a // program build off this, and bumping that version would start doing so. The diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/DemoteFloat64Pass.cpp b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/DemoteFloat64Pass.cpp index a2b5292b..7191fd69 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/DemoteFloat64Pass.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/DemoteFloat64Pass.cpp @@ -91,9 +91,15 @@ namespace MobileGL { BlockRelayout(IRContext* irContext, Bool std140) : m_irContext(irContext), m_std140(std140) {} - // Size and alignment of `typeId`, applying every stride decoration it implies - // on the way down. Zero size means "not a type this layout knows how to - // describe"; the caller then leaves the block alone rather than guessing. + // Size and alignment of `typeId`, QUEUING every offset/stride decoration it + // implies on the way down. Zero size means "not a type this layout knows how + // to describe"; the caller then leaves the block alone rather than guessing. + // The queue is what makes that fallback honest: measurement must be + // side-effect-free until it is known to succeed, or a mid-struct failure + // would leave the block half-relaid-out - members before the failing one at + // compacted 32-bit offsets, members after it at the original 64-bit ones, a + // layout matching neither convention. Commit() flushes the queue and is + // called only on a successful Measure of the whole block. struct Extent { Uint32 size = 0; Uint32 alignment = 0; @@ -108,7 +114,29 @@ namespace MobileGL { return extent; } + // Flushes the decoration writes a successful Measure queued. Call exactly + // once, only when Measure returned a non-zero size; a failed measurement's + // queue dies with this per-block instance, leaving the module untouched. + void Commit() { + for (const PendingDecoration& pending : m_pendingWrites) { + if (pending.member) { + ApplyMemberDecoration(pending.targetId, pending.memberIndex, pending.decoration, + pending.value); + } else { + ApplyTypeDecoration(pending.targetId, pending.decoration, pending.value); + } + } + m_pendingWrites.clear(); + } + private: + struct PendingDecoration { + Bool member = false; + Uint32 targetId = 0; + Uint32 memberIndex = 0; + spv::Decoration decoration = spv::Decoration::Offset; + Uint32 value = 0; + }; Extent MeasureUncached(Uint32 typeId) { const Instruction* type = m_irContext->get_def_use_mgr()->GetDef(typeId); if (type == nullptr) return {}; @@ -204,7 +232,17 @@ namespace MobileGL { return length->GetSingleWordInOperand(0); } + // Queue-only during measurement; the module is mutated in Commit(). void SetTypeDecoration(Uint32 targetId, spv::Decoration decoration, Uint32 value) { + m_pendingWrites.push_back({false, targetId, 0, decoration, value}); + } + + void SetMemberDecoration(Uint32 structId, Uint32 member, spv::Decoration decoration, + Uint32 value) { + m_pendingWrites.push_back({true, structId, member, decoration, value}); + } + + void ApplyTypeDecoration(Uint32 targetId, spv::Decoration decoration, Uint32 value) { for (Instruction& annotation : m_irContext->annotations()) { if (annotation.opcode() != spv::Op::OpDecorate) continue; if (annotation.GetSingleWordInOperand(0) != targetId) continue; @@ -216,8 +254,8 @@ namespace MobileGL { } } - void SetMemberDecoration(Uint32 structId, Uint32 member, spv::Decoration decoration, - Uint32 value) { + void ApplyMemberDecoration(Uint32 structId, Uint32 member, spv::Decoration decoration, + Uint32 value) { for (Instruction& annotation : m_irContext->annotations()) { if (annotation.opcode() != spv::Op::OpMemberDecorate) continue; if (annotation.GetSingleWordInOperand(0) != structId) continue; @@ -233,6 +271,7 @@ namespace MobileGL { IRContext* m_irContext = nullptr; Bool m_std140 = true; std::unordered_map m_extents; + std::vector m_pendingWrites; }; } // namespace @@ -529,9 +568,12 @@ namespace MobileGL { // A member shape the layout rules here do not describe. Leaving the block // at its 64-bit offsets keeps the module valid for Vulkan; SPIRV-Cross will // decline it for ESSL, which is the same outcome as before the demotion. + // Nothing was written: Measure only queues, and the queue dies here. MGLOG_D("DemoteFloat64Pass: block %%%u contains a member this pass cannot lay " "out; its 64-bit offsets are left in place", blockType->result_id()); + } else { + relayout.Commit(); } }