From a7d7e2bc442c3f427aa8f612f8e6cdcde4d7808e Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 20 Sep 2026 21:34:14 -0400 Subject: [PATCH] [MG_Backend] (DirectGLES/DirectVulkan): the stencil-only scratch attachment and the aspect staging buffer's lifetime Three follow-ups the first pass needed: DirectGLES glGetTexImage has to attach a STENCIL_INDEX texture at GL_STENCIL_ATTACHMENT. It went through the colour branch, which left the scratch READ framebuffer incomplete for a stencil-only texture, so the readback bailed out before the new stencil branch could run. ScratchFBOImpl::EnsureDepthAttachment2D gains the stencil-only point; the shadow remembers which of the three points it holds so a later depth attach is not deduped against it. DirectVulkan's aspect staging buffer is recorded into the current frame's command buffer and outlives the glBlitFramebuffer call that records it, so it retires with the frame. VkBufferManager::DeferRelease is the manager's own deferred-release entry point and moves to its public section for that. --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 13 +++++++++---- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 17 +++++++++++++---- MobileGL/MG_Backend/DirectGLES/Managers.h | 6 +++++- .../DirectVulkan/Renderer/VkBufferManager.h | 7 ++++++- 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index ce65d146..1cfd643e 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -15293,12 +15293,17 @@ namespace MobileGL::MG_Backend::DirectGLES { MG_Util::ConvertGLEnumToTextureUploadTarget(target)); // GL_DEPTH_STENCIL can't be attached as a color attachment (glCheckFramebufferStatus // would report it incomplete); it has its own combined depth+stencil attachment point. - // glReadBuffer only selects among color attachments, so it does not apply here. - if (format == GL_DEPTH_STENCIL || format == GL_DEPTH_COMPONENT) { + // A STENCIL_INDEX texture has a third one, GL_STENCIL_ATTACHMENT: attaching it as a + // depth attachment (or as a color one) leaves the scratch FBO incomplete, and the read + // that follows writes nothing at all. glReadBuffer only selects among color attachments, + // so it does not apply to any of the three. + const Bool depthStencilAttachment = + format == GL_DEPTH_STENCIL || format == GL_DEPTH_COMPONENT || format == GL_STENCIL_INDEX; + if (depthStencilAttachment) { ScratchFBOImpl::EnsureDepthAttachment2D( tempFB, GL_READ_FRAMEBUFFER, backendTexId, backendAttachTarget == GL_UNKNOWN_MGL ? target : backendAttachTarget, level, - /*withStencil=*/format == GL_DEPTH_STENCIL); + /*withStencil=*/format == GL_DEPTH_STENCIL, /*stencilOnly=*/format == GL_STENCIL_INDEX); } else if (backendAttachTarget == GL_TEXTURE_3D || backendAttachTarget == GL_TEXTURE_2D_ARRAY || backendAttachTarget == GL_TEXTURE_CUBE_MAP_ARRAY) { // ES cannot attach 3D/array textures through glFramebufferTexture2D; layer 0 here, and @@ -15313,7 +15318,7 @@ namespace MobileGL::MG_Backend::DirectGLES { tempFB, GL_READ_FRAMEBUFFER, backendTexId, backendAttachTarget == GL_UNKNOWN_MGL ? target : backendAttachTarget, level); } - if (format != GL_DEPTH_STENCIL && format != GL_DEPTH_COMPONENT) { + if (!depthStencilAttachment) { MGLOG_D("GetTexImage: glReadBuffer(GL_COLOR_ATTACHMENT0)"); ScratchFBOImpl::EnsureReadBuffer(tempFB, GL_COLOR_ATTACHMENT0); } diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 0c27fed9..04bd7a2e 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -11596,6 +11596,7 @@ namespace MobileGL::MG_Backend::DirectGLES { fb.depthLevel = 0; fb.depthLayer = -1; fb.depthHasStencil = false; + fb.depthIsStencilOnly = false; fb.attachmentsKnown = true; } @@ -11629,6 +11630,7 @@ namespace MobileGL::MG_Backend::DirectGLES { fb.depthLevel = 0; fb.depthLayer = -1; fb.depthHasStencil = false; + fb.depthIsStencilOnly = false; } } // namespace @@ -11708,14 +11710,15 @@ namespace MobileGL::MG_Backend::DirectGLES { } void EnsureDepthAttachment2D(ScratchFramebuffer& fb, GLenum fbTarget, Uint tex, GLenum texTarget, GLint level, - Bool withStencil) { + Bool withStencil, Bool stencilOnly) { PrepareForUse(fb, fbTarget); if (fb.colorTex != 0) { g_GLESFuncs.glFramebufferTexture2D(fbTarget, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, 0, 0); RecordNoColor(fb); } if (fb.depthTex == tex && fb.depthTarget == texTarget && fb.depthLevel == level && - fb.depthLayer < 0 && fb.depthHasStencil == withStencil) { + fb.depthLayer < 0 && fb.depthHasStencil == withStencil && + fb.depthIsStencilOnly == stencilOnly) { return; } if (fb.depthTex != 0) { @@ -11724,8 +11727,12 @@ namespace MobileGL::MG_Backend::DirectGLES { g_GLESFuncs.glFramebufferTexture2D(fbTarget, GL_DEPTH_STENCIL_ATTACHMENT, GL_TEXTURE_2D, 0, 0); } DrainPendingGLErrors(); + // A stencil-only texture has no depth point to attach at, and a depth one has no + // stencil point; GL_DEPTH_STENCIL_ATTACHMENT is only the combined texture's form. g_GLESFuncs.glFramebufferTexture2D(fbTarget, - withStencil ? GL_DEPTH_STENCIL_ATTACHMENT : GL_DEPTH_ATTACHMENT, + stencilOnly ? GL_STENCIL_ATTACHMENT + : (withStencil ? GL_DEPTH_STENCIL_ATTACHMENT + : GL_DEPTH_ATTACHMENT), texTarget, tex, level); if (g_GLESFuncs.glGetError() != GL_NO_ERROR) { RecordNoDepth(fb); @@ -11736,6 +11743,7 @@ namespace MobileGL::MG_Backend::DirectGLES { fb.depthLayer = -1; fb.depthLevel = level; fb.depthHasStencil = withStencil; + fb.depthIsStencilOnly = stencilOnly; } // The layer-point sibling of EnsureColorAttachmentLayer above, for a view's mip chain in @@ -11749,7 +11757,7 @@ namespace MobileGL::MG_Backend::DirectGLES { RecordNoColor(fb); } if (fb.depthTex == tex && fb.depthTarget == 0 && fb.depthLevel == level && fb.depthLayer == layer && - !fb.depthHasStencil) { + !fb.depthHasStencil && !fb.depthIsStencilOnly) { return; } if (fb.depthTex != 0) { @@ -11766,6 +11774,7 @@ namespace MobileGL::MG_Backend::DirectGLES { fb.depthLevel = level; fb.depthLayer = layer; fb.depthHasStencil = false; + fb.depthIsStencilOnly = false; } void EnsureNoColorAttachment(ScratchFramebuffer& fb, GLenum fbTarget) { diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 1ab27256..cbe08694 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -2567,6 +2567,10 @@ namespace MobileGL::MG_Backend::DirectGLES { // >= 0 => attached via glFramebufferTextureLayer. GLint depthLayer = -1; Bool depthHasStencil = false; + // The point holds a STENCIL_INDEX texture, so it was made at + // GL_STENCIL_ATTACHMENT - a third attach point that neither depth form + // may be deduped against. + Bool depthIsStencilOnly = false; // Per-FBO read/draw buffer state (0 = unknown, set on first use). GLenum readBuffer = 0; GLenum drawBuffer = 0; @@ -2584,7 +2588,7 @@ namespace MobileGL::MG_Backend::DirectGLES { void EnsureColorAttachment2D(ScratchFramebuffer& fb, GLenum fbTarget, Uint tex, GLenum texTarget, GLint level); void EnsureColorAttachmentLayer(ScratchFramebuffer& fb, GLenum fbTarget, Uint tex, GLint level, GLint layer); void EnsureDepthAttachment2D(ScratchFramebuffer& fb, GLenum fbTarget, Uint tex, GLenum texTarget, GLint level, - Bool withStencil); + Bool withStencil, Bool stencilOnly = false); void EnsureDepthAttachmentLayer(ScratchFramebuffer& fb, GLenum fbTarget, Uint tex, GLint level, GLint layer); void EnsureNoColorAttachment(ScratchFramebuffer& fb, GLenum fbTarget); void EnsureNoDepthAttachment(ScratchFramebuffer& fb, GLenum fbTarget); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h index 6894afa6..2b3af03e 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h @@ -204,6 +204,12 @@ namespace MobileGL::MG_Backend::DirectVulkan { // Busy = potentially referenced by GPU work that has not been fenced yet // (including commands recorded for the current, unsubmitted frame). Bool IsResourceBusy(const VkBufferResource& resource) const; + // Hand the manager a buffer to destroy once the frame that recorded commands + // naming it has completed. The renderer's blit path needs a device-local + // scratch store for one recorded operation, which cannot be a stack local: + // the glBlitFramebuffer that records the commands returns long before the + // command buffer is submitted. + void DeferRelease(VkBufferObject&& buffer); private: #if MOBILEGL_BUILD_DISAGGREGATED @@ -237,7 +243,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { #if MOBILEGL_BUILD_DISAGGREGATED Bool StagedWireRangeCopy(WireBufferResource& resource, const void* data, SizeT offset, SizeT size); #endif - void DeferRelease(VkBufferObject&& buffer); void CollectDeferredReleases(Uint32 frameIndex); void DestroyAllDeferredReleases(); void TrackLiveResource(const SharedPtr& resource);