diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 7837d147..a133d29d 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -3037,17 +3037,6 @@ void main() { vkBuffers.assign(bindingCount, VK_NULL_HANDLE); vkOffsets.assign(bindingCount, 0); - auto findBufferByKey = [&](SizeT bufferKey) -> const SharedPtr* { - const auto& attrs = vao.GetAllAttributes(); - for (Uint32 location = 0; location < MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS; ++location) { - const auto& attr = attrs[location]; - if (attr.Buffer && reinterpret_cast(attr.Buffer.get()) == bufferKey) { - return &attr.Buffer; - } - } - return nullptr; - }; - auto uploadConvertedStream = [&](VertexInputStateFactory::VertexStreamConversion conversion, const MG_State::GLState::VertexAttribute& attribute, const Uint8* sourceData, SizeT sourceStride, @@ -3150,14 +3139,14 @@ void main() { continue; } - const SizeT bufferKey = vertexInputState.bindingBufferKeys[binding]; - // The VAO attribute already holds the buffer's SharedPtr; use it by reference directly - // instead of re-resolving it from the GL context by external index (a map lookup + - // atomic refcount every binding every draw). - const SharedPtr* sourceBufferSharedPtr = findBufferByKey(bufferKey); - MOBILEGL_ASSERT(sourceBufferSharedPtr != nullptr && *sourceBufferSharedPtr != nullptr, + // VertexInputStateFactory fills bindingBufferKeys[b] and bindingAttributeLocations[b] + // from the SAME loop iteration, one binding per enabled attribute with no merging, so + // this attribute's Buffer IS the SharedPtr by construction - no need to search the VAO's + // 32 slots for it. The client-memory branch above has already returned, so the location + // is in range here. + const auto& sourceBufferShared = vao.GetAttribute(bindingLocation).Buffer; + MOBILEGL_ASSERT(sourceBufferShared != nullptr, "UploadAndBindVertexStreams failed to resolve source buffer"); - const auto& sourceBufferShared = *sourceBufferSharedPtr; BufferSlice slice{}; const SizeT sourceSize = sourceBufferShared->GetSize(); const SizeT baseOffset = @@ -3401,8 +3390,12 @@ void main() { substituteRestartIndex = restartIndex; } - const auto* indexBuffer = - pIndexBufferView->forceClientMemory ? nullptr : vao.GetIndexBufferBindingSlot().GetBoundObject().get(); + // Bound by reference so the SharedPtr below is the one already in hand rather than a fresh + // GL-name map lookup plus an atomic refcount pair on every indexed draw - the vertex path + // above documents the same cost. + const SharedPtr& indexBufferShared = + vao.GetIndexBufferBindingSlot().GetBoundObject(); + const auto* indexBuffer = pIndexBufferView->forceClientMemory ? nullptr : indexBufferShared.get(); if (indexBuffer == nullptr) { // No element-array buffer: the view's byte offset is a raw client pointer // (desktop drivers accept client-memory indices and the GL CTS relies on @@ -3441,7 +3434,6 @@ void main() { "DrawElements index range out of bounds"); BufferSlice slice{}; - auto indexBufferShared = MG_State::pGLContext->GetBufferObject(indexBuffer->GetExternalIndex()); MOBILEGL_ASSERT(indexBufferShared != nullptr, "UploadAndBindIndexBuffer failed to resolve shared EBO"); if (substituteRestart) { // The whole buffer is rewritten, not just this draw's range, so that every element @@ -4068,7 +4060,10 @@ void main() { auto& vis = m_vertexInputStateFactory->GetOrCreateVertexInputState(vao); const Uint64 vertexLayoutHash = vis.layoutHash; const Uint64 renderPassHash = renderPassEntry.hash; - const Uint renderStateVersion = MG_State::pGLContext->GetRenderStateParametersVersion(); + // The pipeline-relevant subset only: glViewport / glScissor / glBlendColor / glStencilMask + // and friends are dynamic state or not pipeline state at all, and keying the memo on the + // all-state counter made any of them evict a perfectly good VkPipeline. + const Uint renderStateVersion = MG_State::pGLContext->GetPipelineStateVersion(); for (Uint32 i = 0; i < m_pipelineMemoCount; ++i) { const PipelineMemoEntry& entry = m_pipelineMemo[i]; if (entry.pipeline != VK_NULL_HANDLE && entry.mode == mode && @@ -4708,7 +4703,7 @@ void main() { drawFbo->GetObjectVersion() != snap.fboVersion) { return false; } - if (MG_State::pGLContext->GetRenderStateParametersVersion() != snap.renderStateVersion || + if (MG_State::pGLContext->GetPipelineStateVersion() != snap.renderStateVersion || MG_State::pGLContext->GetTextureBindGeneration() != snap.bindGeneration) { return false; } @@ -5172,7 +5167,7 @@ void main() { snap.drawFbo = drawFbo.get(); snap.fboVersion = drawFbo->GetObjectVersion(); snap.drawFboIsDefault = drawFbo->IsDefaultFramebuffer(); - snap.renderStateVersion = MG_State::pGLContext->GetRenderStateParametersVersion(); + snap.renderStateVersion = MG_State::pGLContext->GetPipelineStateVersion(); snap.bindGeneration = MG_State::pGLContext->GetTextureBindGeneration(); snap.baseTransformFlags = GetShaderTransformFlags(m_swapchainObject.GetPreTransform()).GetRaw(); snap.resolvedTransformFlags = transformFlags.GetRaw(); diff --git a/MobileGL/MG_State/GLState/Core.cpp b/MobileGL/MG_State/GLState/Core.cpp index 08a51e02..f2ca7f40 100644 --- a/MobileGL/MG_State/GLState/Core.cpp +++ b/MobileGL/MG_State/GLState/Core.cpp @@ -393,6 +393,10 @@ namespace MobileGL::MG_State { } // RenderState + Uint GLContext::GetPipelineStateVersion() const { + return m_renderState.GetPipelineStateVersion(); + } + Uint GLContext::GetRenderStateParametersVersion() const { return m_renderState.GetVersion(); } diff --git a/MobileGL/MG_State/GLState/Core.h b/MobileGL/MG_State/GLState/Core.h index f3a8e620..b462a992 100644 --- a/MobileGL/MG_State/GLState/Core.h +++ b/MobileGL/MG_State/GLState/Core.h @@ -159,6 +159,8 @@ namespace MobileGL { // RenderState Uint GetRenderStateParametersVersion() const; + // Only the pipeline-relevant subset - see RenderState::m_pipelineStateVersion. + Uint GetPipelineStateVersion() const; const RenderStateParameters& GetRenderStateParameters() const; void SetViewport(IntVec4 viewport); // x, y, width, height const IntVec4& GetViewport() const; // x, y, width, height diff --git a/MobileGL/MG_State/GLState/RenderState/RenderState.cpp b/MobileGL/MG_State/GLState/RenderState/RenderState.cpp index 74bb7603..06a07ecd 100644 --- a/MobileGL/MG_State/GLState/RenderState/RenderState.cpp +++ b/MobileGL/MG_State/GLState/RenderState/RenderState.cpp @@ -37,6 +37,10 @@ namespace MobileGL { return m_version; } + Uint RenderState::GetPipelineStateVersion() const { + return m_pipelineStateVersion; + } + const RenderStateParameters& RenderState::GetAllParameters() const { return m_parameters; } @@ -122,7 +126,7 @@ namespace MobileGL { if (m_parameters.PolygonModeFront == front && m_parameters.PolygonModeBack == back) return; m_parameters.PolygonModeFront = front; m_parameters.PolygonModeBack = back; - ++m_version; + BumpVersions(); } GLenum RenderState::GetPolygonModeFront() const { @@ -158,7 +162,7 @@ namespace MobileGL { if (m_parameters.PatchVertices == vertices) return; m_parameters.PatchVertices = vertices; - ++m_version; + BumpVersions(); } Uint RenderState::GetPatchVertices() const { @@ -187,7 +191,7 @@ namespace MobileGL { case CapabilityInput::capability: \ if (m_parameters.capability##Enabled == (flag)) break; \ m_parameters.capability##Enabled = (flag); \ - ++m_version; \ + BumpVersions(); \ break; switch (cap) { @@ -220,7 +224,7 @@ namespace MobileGL { blendState.Enabled = enabled; stateChanged = true; } - if (stateChanged) ++m_version; + if (stateChanged) BumpVersions(); break; } default: // not supported currently @@ -276,7 +280,7 @@ namespace MobileGL { if (m_parameters.BlendStates[index].Enabled == enabled) return; m_parameters.BlendStates[index].Enabled = enabled; - ++m_version; + BumpVersions(); } Bool RenderState::IsCapabilityEnabledIndexed(CapabilityInput cap, Uint index) const { @@ -308,7 +312,7 @@ namespace MobileGL { stateChanged = true; } if (!stateChanged) return; - ++m_version; + BumpVersions(); } void RenderState::GetBlendFunc(BlendFactor& srcRGB, BlendFactor& dstRGB, BlendFactor& srcAlpha, @@ -334,7 +338,7 @@ namespace MobileGL { blendState.DstFactorRGB = dstRGB; blendState.SrcFactorAlpha = srcAlpha; blendState.DstFactorAlpha = dstAlpha; - ++m_version; + BumpVersions(); } void RenderState::GetBlendFuncIndexed(Uint index, BlendFactor& srcRGB, BlendFactor& dstRGB, @@ -360,7 +364,7 @@ namespace MobileGL { stateChanged = true; } if (!stateChanged) return; - ++m_version; + BumpVersions(); } void RenderState::GetBlendEquation(BlendEquation& color, BlendEquation& alpha) const { @@ -379,7 +383,7 @@ namespace MobileGL { } blendState.ColorEquation = color; blendState.AlphaEquation = alpha; - ++m_version; + BumpVersions(); } void RenderState::GetBlendEquationIndexed(Uint index, BlendEquation& color, BlendEquation& alpha) const { @@ -395,7 +399,7 @@ namespace MobileGL { if (m_parameters.LogicOp == logicOp) return; m_parameters.LogicOp = logicOp; - ++m_version; + BumpVersions(); } LogicOperation RenderState::GetLogicOp() const { @@ -407,7 +411,7 @@ namespace MobileGL { if (m_parameters.DepthFunc == func) return; m_parameters.DepthFunc = func; - ++m_version; + BumpVersions(); } DepthTestFunc RenderState::GetDepthFunc() const { @@ -418,7 +422,7 @@ namespace MobileGL { if (m_parameters.DepthMask == flag) return; m_parameters.DepthMask = flag; - ++m_version; + BumpVersions(); } Bool RenderState::GetDepthMask() const { @@ -429,10 +433,15 @@ namespace MobileGL { StencilFaceState& state = m_parameters.StencilStates[GetStencilFaceIndex(face)]; if (state.Func == func && state.Ref == ref && state.ValueMask == mask) return; + // Only Func is baked into the pipeline; Ref and ValueMask are dynamic state + // (VK_DYNAMIC_STATE_STENCIL_REFERENCE / _COMPARE_MASK), so glStencilFunc changing + // only the reference must not evict a cached pipeline. + const Bool pipelineRelevantChange = state.Func != func; state.Func = func; state.Ref = ref; state.ValueMask = mask; ++m_version; + if (pipelineRelevantChange) ++m_pipelineStateVersion; } void RenderState::SetStencilMask(StencilFace face, Uint32 mask) { @@ -454,7 +463,7 @@ namespace MobileGL { state.FailOp = fail; state.PassDepthFailOp = depthFail; state.PassDepthPassOp = depthPass; - ++m_version; + BumpVersions(); } const StencilFaceState& RenderState::GetStencilState(StencilFace face) const { @@ -471,7 +480,7 @@ namespace MobileGL { changed = true; } } - if (changed) ++m_version; + if (changed) BumpVersions(); } BoolVec4 RenderState::GetColorMask() const { @@ -482,7 +491,7 @@ namespace MobileGL { void RenderState::SetColorMaskIndexed(Uint index, BoolVec4 mask) { if (m_parameters.ColorMasks[index] == mask) return; m_parameters.ColorMasks[index] = mask; - ++m_version; + BumpVersions(); } BoolVec4 RenderState::GetColorMaskIndexed(Uint index) const { @@ -550,7 +559,7 @@ namespace MobileGL { m_parameters.SampleCoverageValue = value; m_parameters.SampleCoverageInvert = invert; - ++m_version; + BumpVersions(); } Float RenderState::GetSampleCoverageValue() const { @@ -565,7 +574,7 @@ namespace MobileGL { if (m_parameters.SampleMaskValue == mask) return; m_parameters.SampleMaskValue = mask; - ++m_version; + BumpVersions(); } Uint32 RenderState::GetSampleMaskValue() const { @@ -639,7 +648,7 @@ namespace MobileGL { if (m_parameters.CullFaceModeSetting == mode) return; m_parameters.CullFaceModeSetting = mode; - ++m_version; + BumpVersions(); } CullFaceMode RenderState::GetCullFaceMode() const { @@ -650,7 +659,7 @@ namespace MobileGL { if (m_parameters.FrontFaceModeSetting == mode) return; m_parameters.FrontFaceModeSetting = mode; - ++m_version; + BumpVersions(); } FrontFaceMode RenderState::GetFrontFaceMode() const { @@ -661,7 +670,7 @@ namespace MobileGL { if (m_parameters.ProvokingVertexModeSetting == mode) return; m_parameters.ProvokingVertexModeSetting = mode; - ++m_version; + BumpVersions(); } ProvokingVertexMode RenderState::GetProvokingVertexMode() const { diff --git a/MobileGL/MG_State/GLState/RenderState/RenderState.h b/MobileGL/MG_State/GLState/RenderState/RenderState.h index 65e22393..b272fc2e 100644 --- a/MobileGL/MG_State/GLState/RenderState/RenderState.h +++ b/MobileGL/MG_State/GLState/RenderState/RenderState.h @@ -312,6 +312,8 @@ namespace MobileGL { RenderState(); Uint GetVersion() const; + // Version of the pipeline-relevant subset only - see m_pipelineStateVersion. + Uint GetPipelineStateVersion() const; const RenderStateParameters& GetAllParameters() const; // Rasterization @@ -418,7 +420,21 @@ namespace MobileGL { const IntVec4& GetScissorBox() const; // x, y, width, height private: + // Bump both: any state change invalidates the draw snapshot, and this one also + // changes the VkPipeline (or its DirectGLES equivalent). + void BumpVersions() { + ++m_version; + ++m_pipelineStateVersion; + } + Uint16 m_version = 0; + // Only the subset of render state that a backend bakes INTO a pipeline object. + // Viewport, scissor, depth range, blend colour, line width, polygon offset, stencil + // write mask, the clear values, hints and the point-size family are all either + // dynamic pipeline state or not pipeline state at all, so changing one of them must + // not evict a cached pipeline. Keeping one counter for both made a glViewport call + // knock the next draw off the pipeline memo AND the draw fast path. + Uint16 m_pipelineStateVersion = 0; RenderStateParameters m_parameters; // Pixel Store