[Perf] (MG_State, MG_Backend): stop glViewport from evicting a cached VkPipeline

RenderState kept one version counter for all render state, and DirectVulkan read
it in three places: the pipeline memo key, the SetupDrawSnapshot fast-path guard,
and that guard's store. So glViewport, glScissor, glBlendColor, glStencilMask,
glClearColor, glPolygonOffset, glLineWidth and the point-size family - none of
which can alter a VkPipeline, all of which an application changes between draws -
knocked the next draw off both fast paths and made it rebuild a pipeline lookup
that was already correct.

The counter is now split. m_version still moves on every state change, because
the draw snapshot really does depend on all of it. m_pipelineStateVersion moves
only for the state a backend bakes into a pipeline object, and it is what the
three DirectVulkan sites read.

The exclusion list is the eight VkDynamicState entries PipelineFactory declares
plus the state that is not pipeline state at all (the clear values, hints, the
point-size family, clamp read colour, the primitive restart index). glStencilFunc
is the one setter that had to be split rather than classified: Func is in the
pipeline payload but Ref and ValueMask are dynamic state, so it bumps the
pipeline version only when Func actually changes.

Capabilities are deliberately NOT in the exclusion list even though several look
like dynamic state: GL_FRAMEBUFFER_SRGB feeds the render-pass hash, depth and
stencil test feed drawUsesDepthStencil, and scissor test, blend, cull face,
polygon offset fill, primitive restart, colour logic op and rasterizer discard
all feed the pipeline payload.

Two smaller draw-path wins ride along, both removing work whose answer was
already in hand. UploadAndBindVertexStreams searched all 32 VAO attribute slots
for the SharedPtr matching a binding's buffer key, once per binding per draw -
but VertexInputStateFactory writes bindingBufferKeys[b] and
bindingAttributeLocations[b] from the same loop iteration, one binding per
attribute with no merging, so the attribute at that location IS the buffer, by
construction. UploadAndBindIndexBuffer round-tripped the element-array buffer's
raw pointer back through the GL name table on every indexed draw, costing a map
lookup and an atomic refcount pair, when the binding slot's SharedPtr was already
in scope forty lines above - where a comment says exactly that about the vertex
path.

Behaviour-neutral by construction and verified as such: a 13355-case subset of
GL30-GL45 covering viewport, scissor, blend, stencil, depth, polygon offset,
clear, multisample, cull, logic op, line width and point state, plus the whole
direct_state_access suite, is identical before and after on both backends - in
the failure list and in the crashed-case set. direct_state_access stays at
Espryt 370/371 and Magma 371/371.
This commit is contained in:
BZLZHH
2026-08-05 12:49:54 -04:00
parent ba81ee114e
commit f3d52faad4
5 changed files with 70 additions and 44 deletions
@@ -3037,17 +3037,6 @@ void main() {
vkBuffers.assign(bindingCount, VK_NULL_HANDLE);
vkOffsets.assign(bindingCount, 0);
auto findBufferByKey = [&](SizeT bufferKey) -> const SharedPtr<MG_State::GLState::BufferObject>* {
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<SizeT>(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<MG_State::GLState::BufferObject>* 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<MG_State::GLState::BufferObject>& 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();