[Fix] (MG_Backend): stop a renderbuffer blit reading a freed image layout

VkRenderPassManager kept m_renderbufferResources on FastSTL's open-addressing
UnorderedMap while BlitFramebuffer caches a raw pointer into one of its elements -
ResolveColorBlitBinding stores &rbResource->layout - and then calls
MaterializePendingClearForRenderbuffer, which looks that same resource up again.
FastSTL's operator[] runs its load-factor check before find_key and reallocates
the whole bucket array when occupancy crosses it, so even a plain lookup relocates
every element; erase only tombstones and never lowers the occupancy, so the
doubling keeps firing. After a relocation the cached pointer names freed storage
still holding the pre-clear VK_IMAGE_LAYOUT_UNDEFINED, BlitFramebuffer takes its
"source image layout is undefined" early return, and the blit is silently dropped
- glReadPixels then returns the zero-filled fresh allocation.

That is why the failures looked arbitrary: which iteration breaks is pure
arithmetic on the table's occupancy, and the observed set (GL_R8 at k=0,1,3,7,
GL_R16 at k=6, GL_RG16 at k=4) is exactly the doubling ladder. Padding the map
with unrelated live renderbuffers moves the failures to the positions the model
predicts and every previously failing format then passes, so nothing else hides
behind it.

Reordering the materialize ahead of the resolves - the fix ReadPixels got, see the
note at its call site - does not cover this, because BlitFramebuffer resolves two
bindings and the second resolve still runs after the first pointer is taken. The
depth blit, GetOrCreateRenderPass's depthRenderbufferResource and
ReadDepthStencilPixels cache the same kind of pointer, so the invariant belongs in
the container rather than in a per-call-site ordering rule. m_textureResources was
already node-based for exactly this reason; this is the map that was left behind.

Fixes renderbuffers_storage_multisample on DirectVulkan.
This commit is contained in:
BZLZHH
2026-08-05 08:24:01 -04:00
parent 4ce808b9f2
commit 25b9370815
@@ -16,6 +16,7 @@
#include "MG_State/GLState/FramebufferState/FramebufferObject.h"
#include <Includes.h>
#include <unordered_map>
#include <vk_mem_alloc.h>
namespace MobileGL::MG_Backend::DirectVulkan {
@@ -314,7 +315,27 @@ namespace MobileGL::MG_Backend::DirectVulkan {
Uint64 deferredAtFrame = 0;
};
UnorderedMap<MG_State::GLState::RenderbufferObject*, RenderbufferResource> m_renderbufferResources;
// Node-based std::unordered_map, deliberately not FastSTL's open-addressing UnorderedMap:
// callers cache a RenderbufferResource* - or a bare &resource->layout - and then make further
// calls that touch this map. BlitFramebuffer is the one that bit: it resolves the source and
// destination colour bindings (ResolveColorBlitBinding caches &rbResource->layout), then
// materializes the source's pending clear, which looks that same resource up again. FastSTL's
// operator[] runs its load-factor check before find_key and reallocates the whole bucket array
// when occupancy crosses it, so even a plain lookup relocates every element; erase only
// tombstones and never decrements the occupancy, so the doubling keeps firing. After a
// relocation the cached pointer names freed storage still holding the pre-clear
// VK_IMAGE_LAYOUT_UNDEFINED, and BlitFramebuffer bails out at "source image layout is
// undefined", silently dropping the blit - renderbuffers_storage_multisample read back zero
// instead of the clear colour on exactly the iterations that grew the table.
//
// Reordering the materialize ahead of the resolves - the fix ReadPixels got - does not cover
// this: the destination resolve still runs after the source pointer is taken. The depth blit,
// GetOrCreateRenderPass's depthRenderbufferResource and ReadDepthStencilPixels cache the same
// kind of pointer, so the invariant belongs in the container rather than in a per-call-site
// ordering rule. m_textureResources is node-based for the same reason. This buys stability
// across rehash and insert only - erase still invalidates the erased element, which is safe
// here because a renderbuffer that is an FBO attachment is held alive by that attachment.
std::unordered_map<MG_State::GLState::RenderbufferObject*, RenderbufferResource> m_renderbufferResources;
UnorderedMap<MG_State::GLState::RenderbufferObject*, PendingRenderbufferClear> m_pendingRenderbufferClears;
Vector<DeferredRenderbufferRelease> m_deferredRenderbufferReleases;
// Supported sample counts per attachment format, so per-draw resource lookups