[Fix] (DirectGLES): route default-framebuffer binds through the FBO-binding shadow - the raw glBindFramebuffer(0) in BindCurrentFBO/SyncAndBindFramebufferObject left the shadow claiming the previous user FBO, false-skipping its next re-bind and letting scoped guards restore a stale binding (caught by adversarial review); also scrub buffer-binding shadows when VAO client-attribute staging buffers are deleted, include cube-map arrays in the pack-image-params gate, and make the delete-recording test hook assertion-unwind safe

This commit is contained in:
2026-07-21 19:54:54 -04:00
parent 122da27249
commit e87063e90c
4 changed files with 48 additions and 9 deletions
+11 -7
View File
@@ -1002,8 +1002,10 @@ namespace MobileGL::MG_Backend::DirectGLES {
}
} else {
MGLOG_D("Binding default framebuffer as %s FBO", (target == FramebufferTarget::Read ? "READ" : "DRAW"));
g_GLESFuncs.glBindFramebuffer(target == FramebufferTarget::Draw ? GL_DRAW_FRAMEBUFFER : GL_READ_FRAMEBUFFER,
0);
// Through the shadow: a raw bind here would leave the shadow claiming
// the previous user FBO, false-skipping its next re-bind.
FramebufferImpl::BindFramebufferId(
target == FramebufferTarget::Draw ? GL_DRAW_FRAMEBUFFER : GL_READ_FRAMEBUFFER, 0);
}
}
@@ -1013,8 +1015,8 @@ namespace MobileGL::MG_Backend::DirectGLES {
ZoneScopedC(TRACY_ZONECOLOR_BACKEND);
#endif
if (!framebuffer || framebuffer == MG_Impl::GLImpl::FramebufferImpl::pDefaultFramebufferInfo->defaultFBO) {
g_GLESFuncs.glBindFramebuffer(target == FramebufferTarget::Draw ? GL_DRAW_FRAMEBUFFER : GL_READ_FRAMEBUFFER,
0);
FramebufferImpl::BindFramebufferId(
target == FramebufferTarget::Draw ? GL_DRAW_FRAMEBUFFER : GL_READ_FRAMEBUFFER, 0);
return;
}
@@ -3993,9 +3995,11 @@ namespace MobileGL::MG_Backend::DirectGLES {
// accepted and repacks on the CPU.
if (convertible) {
// GL_PACK_IMAGE_HEIGHT/GL_PACK_SKIP_IMAGES only apply to 3D/array image
// readbacks; 2D targets must ignore them (GL 3.3 section 6.1.4).
const Bool applyPackImageParams =
backendAttachTarget == GL_TEXTURE_3D || backendAttachTarget == GL_TEXTURE_2D_ARRAY;
// readbacks (cube-map arrays address as arrays); 2D targets must ignore
// them (GL 3.3 section 6.1.4).
const Bool applyPackImageParams = backendAttachTarget == GL_TEXTURE_3D ||
backendAttachTarget == GL_TEXTURE_2D_ARRAY ||
backendAttachTarget == GL_TEXTURE_CUBE_MAP_ARRAY;
// 3D/array images read back every slice, but the FBO path can only read one layer:
// multi-slice reads are served from the CPU shadow (slice-major, tight layout).
const GLsizei sliceCount = std::max(size.z(), 1);
@@ -863,6 +863,16 @@ namespace MobileGL::MG_Backend::DirectGLES {
g_boundPixelUnpackBufferKnown = false;
}
void NoteBufferIdDeleted(Uint id) {
if (id == 0) {
return;
}
if (g_boundArrayBufferKnown && g_boundArrayBufferId == id) {
InvalidateArrayBufferBindingCache();
}
ScrubBufferBindingShadowsForId(id);
}
namespace {
// Shadow of the GL indexed buffer bindings so redundant glBindBufferBase/Range
// (same index + id + range) are skipped. isBase distinguishes a whole-buffer
@@ -1202,6 +1212,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
}
for (auto& bufferId : m_clientAttributeBufferIds) {
if (bufferId != 0) {
BufferImpl::NoteBufferIdDeleted(bufferId);
g_GLESFuncs.glDeleteBuffers(1, &bufferId);
bufferId = 0;
}
@@ -189,6 +189,10 @@ namespace MobileGL::MG_Backend::DirectGLES {
void BindPixelPackBufferId(Uint id);
void BindPixelUnpackBufferId(Uint id);
void InvalidatePixelBufferBindingCaches();
// A GL buffer id is being deleted by code outside BufferImpl (e.g. the VAO
// client-attribute staging buffers): scrub every buffer-binding shadow that
// could false-skip when the name is recycled.
void NoteBufferIdDeleted(Uint id);
// 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/
+22 -2
View File
@@ -1680,13 +1680,22 @@ namespace {
if (!g_deletedTextureIds) return;
for (GLsizei i = 0; i < count; ++i) g_deletedTextureIds->push_back(textures[i]);
}
// Clears the recording hook even when a gtest assertion unwinds the test body
// (a dangling pointer to the dead stack vector would corrupt later tests).
struct ScopedDeletedTextureRecording {
explicit ScopedDeletedTextureRecording(MobileGL::Vector<GLuint>& sink) { g_deletedTextureIds = &sink; }
~ScopedDeletedTextureRecording() { g_deletedTextureIds = nullptr; }
ScopedDeletedTextureRecording(const ScopedDeletedTextureRecording&) = delete;
ScopedDeletedTextureRecording& operator=(const ScopedDeletedTextureRecording&) = delete;
};
} // namespace
TEST(DirectGLESBackendTexture, DestructorDeletesIdAndScrubsBindingCache) {
using namespace MobileGL::MG_Backend::DirectGLES;
ScopedDirectGLESTextureBindings scoped; // installs glGenTextures/glBindTexture mocks + resets caches
MobileGL::Vector<GLuint> deleted;
g_deletedTextureIds = &deleted;
ScopedDeletedTextureRecording recording(deleted);
auto functions = g_GLESFuncs;
functions.glDeleteTextures = SG_DeleteTextures;
SetGLESFuncsTable(functions);
@@ -1714,5 +1723,16 @@ TEST(DirectGLESBackendTexture, DestructorDeletesIdAndScrubsBindingCache) {
--TextureImpl::g_textureContextGeneration; // restore for later tests
EXPECT_EQ(deleted.size(), 1u);
}
g_deletedTextureIds = nullptr;
}
TEST(DirectGLESStateGuards, DefaultFramebufferBindGoesThroughShadow) {
using namespace MobileGL::MG_Backend::DirectGLES;
ScopedStateGuardMocks mocks;
// The regression this guards against: binding framebuffer 0 raw while the
// shadow keeps a user-FBO id makes the next re-bind of that FBO false-skip.
FramebufferImpl::BindFramebufferId(GL_DRAW_FRAMEBUFFER, 7);
FramebufferImpl::BindFramebufferId(GL_DRAW_FRAMEBUFFER, 0); // default-FBO path must use this API
FramebufferImpl::BindFramebufferId(GL_DRAW_FRAMEBUFFER, 7); // must reach the driver again
EXPECT_EQ(mocks.log.Count("BindFramebuffer:"), 3u);
}