From bbc7b9ca841cc994960e9f79f29e94cc3b12daa6 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 20 Aug 2026 04:21:10 -0400 Subject: [PATCH] [Fix] (DirectGLES): report refused renderbuffer storage and collect dead backend twins on object churn --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 20 +++++++++++++++++ MobileGL/MG_Backend/DirectGLES/Managers.h | 25 +++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index c5d3227b..efb45795 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -5655,6 +5655,14 @@ namespace MobileGL::MG_Backend::DirectGLES { GLenum glInternalFormat, glType, glFormat; TextureImpl::GenerateRenderbufferFormatInfo(internalFormat, &glInternalFormat, &glFormat, &glType); + // The allocation is deferred to here, so an ES driver that refuses it (a + // multi-gigabyte renderbuffer is refused routinely) used to leave m_isInitialized + // true over a renderbuffer with no storage and say nothing at all: the attachment + // then rendered nowhere. Drain first so the check cannot pick up an unrelated stale + // flag, and report GL_OUT_OF_MEMORY to the application. The error lands on whatever + // entry point triggered the sync rather than on glRenderbufferStorage itself, which + // is where the deferred model puts it - still far better than silence. + DebugImpl::ErrorLopper::Clear(); if (samples > 0) { // Same clamp as the multisample texture path: the frontend accepts the count it // advertised, the driver only takes the count it supports for this format, and @@ -5668,6 +5676,18 @@ namespace MobileGL::MG_Backend::DirectGLES { g_GLESFuncs.glRenderbufferStorage(GL_RENDERBUFFER, glInternalFormat, static_cast(width), static_cast(height)); } + if (g_GLESFuncs.glGetError() == GL_OUT_OF_MEMORY) { + MGLOG_E_ONCE("Renderbuffer %u storage allocation ran out of memory: %dx%d, samples=%d, format=%s", + stateRBOObject->GetExternalIndex(), width, height, samples, + MG_Util::ConvertGLEnumToString(glInternalFormat).c_str()); + if (MG_State::pGLContext) { + MG_State::pGLContext->RecordError( + ErrorCode::OutOfMemory, + MakeUnique("DirectGLES", "BackendRenderbufferObject::SyncToBackend", + "The ES driver could not allocate the renderbuffer storage.")); + } + } + DebugImpl::ErrorLopper::Clear(); m_cacheInternalFormat = internalFormat; m_cacheWidth = width; diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 34eb45e0..f60a038b 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -129,7 +129,28 @@ namespace MobileGL::MG_Backend::DirectGLES { // Twin creation is the moment a driver-owned id starts needing a guarded // destructor; cold path, so the once-guard costs nothing per draw. EnsureProcessTeardownSentinel(); + // Sweep BEFORE the entry reference below exists: the map is open-addressed and an + // erase relocates the rest of the probe cluster, so collecting once that reference + // is taken would invalidate it. The sweep is therefore owed from an earlier call + // rather than triggered by this one. + if (m_creationTick >= kCreationGCInterval) { + m_creationTick = 0; + CollectGarbage(); + } + const SizeT entryCountBeforeInsert = m_entries.size(); auto& entry = m_entries[stateObj.get()]; + if (m_entries.size() != entryCountBeforeInsert) { + // A key the registry has never held. Nothing tells the backend that a texture or + // renderbuffer was DELETED - the twin, and the driver storage it owns, lives + // until a collection - and CollectGarbageIfNeeded is ticked only from the + // per-draw sync paths, which a CTS-shaped workload runs about ten times per + // case. 1024 of those ticks then span ~100 cases, so ~100 cases' worth of dead + // (and, for this suite, gigabyte-sized) objects stay allocated at once. Object + // CHURN rather than draw count is what makes the sweep urgent, so a twin the + // registry has never seen ticks it too - and it does so on the path that is + // about to allocate, which is exactly when the memory is needed. + ++m_creationTick; + } if (entry.stateRef.expired()) { // The previous owner of this address is gone and the allocator handed it // to a new object: its twin describes ids the new state object never made. @@ -203,8 +224,12 @@ namespace MobileGL::MG_Backend::DirectGLES { private: static constexpr Uint32 kGCInterval = 1024; + // Creations are far rarer than draws, so this counts in a much smaller unit than + // kGCInterval does. + static constexpr Uint32 kCreationGCInterval = 64; BackendMap m_entries; Uint32 m_gcTick = 0; + Uint32 m_creationTick = 0; Bool m_isCollecting = false; };