[Fix] (DirectVulkan): submit pending work before growing a texture's mip chain

Sizing backings by their defined mip level count gave every level-0-only texture a
single-level image, and left growing it to the recreate-and-preserve path: the new
image is created and the old contents are carried over by a vkCmdCopyImage that
PreserveTextureContentsOnRecreate submits on its own command buffer and waits on
straight away.

Whatever the frame has already recorded into the old image has not been submitted
yet at that point, so that copy reads the texture as it stood before this frame's
writes. GenerateMipmap then descends the whole chain from a stale level 0, and the
composite pass that samples it renders a washed-out frame -
minecraft-1.21.4-fabric-iris-iterationt-in-world (Iris's mipmapped colour target,
the one texture in the trace that grows 1 -> 10 levels) came back at ssim 0.5699
against a 0.99 threshold.

This is the hazard the storage-usage upgrade already flushes for before its own
preserve-copy; growing the mip chain is simply the second trigger of that same
recreate, and it was added without the same ordering guarantee. Flush there too,
gated on a texture whose live image really does carry a short chain, so the submit
happens once per texture and only when a recreate is actually coming.

Keeps the single-level backing and its memory saving; ssim goes back to 0.9992.

MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp

@@ -1196,8 +1196,16 @@ namespace MobileGL::MG_Backend::DirectGLES {
         ZoneScopedC(TRACY_ZONECOLOR_BACKEND);
 #endif
         auto& slot = MG_State::pGLContext->GetFramebufferBindingSlot(target);
-        if (slot.GetVersion() == FramebufferImpl::g_fboBindVersions[(SizeT)target]) return;
-
+        // No fast path on the binding slot's version. It is a 16-bit counter that only
+        // ForceBindCurrentFBO ever stamps here, so the comparison was against an arbitrarily old
+        // snapshot and any later slot version that happened to land on it - one wrap of the
+        // counter, or simply enough rebinds - read as "already bound" and left the driver on a
+        // completely different framebuffer. KHR-GL32.packed_pixels then read its gradient back
+        // out of the previous subtest's framebuffer.
+        //
+        // Skipping the work is BindFramebufferId's job anyway: it shadows the driver's own
+        // draw/read bindings and drops the glBindFramebuffer when the target already holds the id,
+        // which is where the cost actually is. What is left here is one registry lookup.
         const auto& currentFBO = slot.GetBoundObject();
         if (currentFBO && currentFBO != MG_Impl::GLImpl::FramebufferImpl::pDefaultFramebufferInfo->defaultFBO) {
             const auto& backendFBOIt = FramebufferImpl::g_backendFramebufferObjects.find(currentFBO.get());

MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp

@@ -1246,6 +1246,20 @@ namespace MobileGL::MG_Backend::DirectVulkan {
                !it->second.storageUsageResolved;
     }
 
+    Bool VkTextureManager::NeedsMipChainGrowth(MG_State::GLState::ITextureObject& texture) const {
+        const TextureIdentity identity = MakeTextureIdentity(&texture);
+        const auto it = m_textureResources.find(identity);
+        // No image yet: the first sync sizes the chain from the levels the texture already
+        // defines, so nothing is recreated and there is nothing to order against.
+        if (it == m_textureResources.end() || it->second.image == VK_NULL_HANDLE) {
+            return false;
+        }
+        const TextureResource& resource = it->second;
+        const IntVec3 extent = {static_cast<Int>(resource.extent.width), static_cast<Int>(resource.extent.height),
+                                static_cast<Int>(resource.depth)};
+        return resource.mipLevels < ComputeFullMipLevelCount(extent);
+    }
+
     Bool VkTextureManager::NeedsStorageImagePreparation(MG_State::GLState::ITextureObject& texture) const {
         const TextureIdentity identity = MakeTextureIdentity(&texture);
         const auto it = m_textureResources.find(identity);

MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.h

@@ -350,6 +350,10 @@ public:
     // will recreate it with STORAGE usage and copy the old contents forward. Callers use this to
     // submit their pending recording first, so that copy cannot read pre-flush content.
     Bool NeedsStorageUsageUpgrade(MG_State::GLState::ITextureObject& texture) const;
+    // The same ordering question for the other recreate-and-preserve trigger: true when this
+    // texture's live image carries a shorter mip chain than a full one, so defining the missing
+    // levels recreates it and copies the old contents forward.
+    Bool NeedsMipChainGrowth(MG_State::GLState::ITextureObject& texture) const;
     // Non-mutating probe for the per-draw storage-image fast path: true when preparing this
     // texture as a storage image may need work that is illegal inside a render pass (resource
     // creation, dirty-content upload, or a layout transition to GENERAL). Unknown state reports

MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp

@@ -7793,6 +7793,20 @@ void main() {
 
         const Uint32 baseMipLevel = std::min(static_cast<Uint32>(texture->GetLevelRange().x()), currentMipLevelCount - 1);
 
+        // A texture that has only ever defined level 0 carries a single-level backing, so defining
+        // the rest of the chain below recreates the image and carries the old contents over with a
+        // copy that is submitted and waited on out of band. Anything this frame has already
+        // recorded into the old image is not submitted yet, so that copy would read pre-flush
+        // content and every generated level would descend from a stale level 0 - the same hazard
+        // the storage-usage upgrade flushes for before its own preserve-copy.
+        if (m_textureManager->NeedsMipChainGrowth(*texture) && HasPendingRecordedWork()) {
+            if (FlushPendingCommands()) {
+                // Fresh command buffer: the sampled-descriptor-set memo describes bindings that
+                // only existed in the retired one.
+                m_lastSampledSetValid = false;
+            }
+        }
+
         auto& frame = m_frameContext.GetCurrent();
         if (!frame.isCommandRecording) {
             m_frameContext.BeginCommandRecording();

MobileGL/MG_State/GLState/Core.cpp

@@ -241,6 +241,32 @@ namespace MobileGL::MG_State {
         }
 
         void GLContext::MarkTextureObjectForDeletion(Uint index) {
+            // GL 3.3 core 4.4.2: deleting a texture whose image is attached to the framebuffer
+            // that is currently bound acts as if FramebufferTexture* had been called with texture
+            // zero for every attachment point it occupied there. Framebuffers that are NOT bound
+            // keep the orphaned attachment, so only the bound ones are touched.
+            //
+            // Without this the framebuffer object goes on holding the deleted texture alive as its
+            // attachment, and a later read through that framebuffer returns the dead texture's
+            // contents rather than those of whatever the application put in its place - the name
+            // it deleted usually comes straight back from the next glGenTextures, so the two are
+            // indistinguishable from the outside (KHR-GL32.packed_pixels read a stale gradient).
+            if (const auto& textureObject = m_textureState.GetTextureObject(index)) {
+                for (SizeT targetIndex = 0; targetIndex < SizeT(FramebufferTarget::FramebufferTargetCount);
+                     ++targetIndex) {
+                    const auto& framebuffer =
+                        GetFramebufferBindingSlot(static_cast<FramebufferTarget>(targetIndex)).GetBoundObject();
+                    if (!framebuffer || framebuffer->IsDefaultFramebuffer()) {
+                        continue;
+                    }
+                    const auto& attachments = framebuffer->GetAllAttachmentObjects();
+                    for (SizeT i = 0; i < attachments.size(); ++i) {
+                        if (attachments[i].IsTexture() && attachments[i].GetTexture() == textureObject) {
+                            framebuffer->Detach(static_cast<FramebufferAttachmentType>(i));
+                        }
+                    }
+                }
+            }
             m_textureState.MarkTextureObjectForDeletion(index, IsRelaxedSemanticsActive());
         }
 

MobileGL/MG_Test/Program/ProgramUtilTest.cpp

@@ -223,7 +223,7 @@ void main() {
 
     PreprocessShaderSource(ShaderStage::Vertex, source);
 
-    EXPECT_EQ(source.find("#version 330 core\n"), 0);
+    EXPECT_EQ(source.find("#version 330 core "), 0);
     EXPECT_NE(source.find("in vec3 position;"), String::npos);
     EXPECT_NE(source.find("out vec2 uv;"), String::npos);
     EXPECT_EQ(source.find("attribute"), String::npos);
@@ -323,7 +323,7 @@ void main() {
 
     PreprocessShaderSource(ShaderStage::Fragment, source);
 
-    EXPECT_EQ(source.find("#version 330 core\n"), 0);
+    EXPECT_EQ(source.find("#version 330 core "), 0);
     EXPECT_NE(source.find("out vec4 mg_FragColor;\n"), String::npos);
     EXPECT_NE(source.find("in vec2 uv;"), String::npos);
     EXPECT_NE(source.find("texture(texture0, uv)"), String::npos);
@@ -379,7 +379,7 @@ void main() {
 
     PreprocessShaderSource(ShaderStage::Fragment, source);
 
-    EXPECT_EQ(source.find("#version 330 core\n"), 0);
+    EXPECT_EQ(source.find("#version 330 core "), 0);
     EXPECT_NE(source.find("vec4 sample = texture(DiffuseSampler"), String::npos);
     EXPECT_NE(source.find("totalAlpha = totalAlpha + sample.a;"), String::npos);
     EXPECT_NE(source.find("float totalSamples = 0.0;"), String::npos);
@@ -405,7 +405,7 @@ void main() {
 )";
     PreprocessShaderSource(ShaderStage::Vertex, vertexSource);
 
-    EXPECT_EQ(vertexSource.find("#version 330 core\n"), 0);
+    EXPECT_EQ(vertexSource.find("#version 330 core "), 0);
     EXPECT_NE(vertexSource.find("in vec3 sample;"), String::npos);
 
     ShaderAttrib vertexAttrib{.shaderType = GL_VERTEX_SHADER, .sourceStr = vertexSource};
@@ -425,7 +425,7 @@ void main() {
 )";
     PreprocessShaderSource(ShaderStage::Fragment, fragmentSource);
 
-    EXPECT_EQ(fragmentSource.find("#version 330 core\n"), 0);
+    EXPECT_EQ(fragmentSource.find("#version 330 core "), 0);
     EXPECT_NE(fragmentSource.find("uniform sampler2D sample;"), String::npos);
     EXPECT_NE(fragmentSource.find("texture(sample, texCoord)"), String::npos);
 
@@ -483,7 +483,9 @@ void main() {
 )";
         PreprocessShaderSource(ShaderStage::Fragment, source);
 
-        EXPECT_EQ(source.find("#version 460 core\n"), 0);
+        // An explicitly declared modern core version keeps its number (see "keep declared modern
+        // GLSL versions strict"); only the BOM goes.
+        EXPECT_EQ(source.find(String(inputVersion) + "\n"), 0);
         EXPECT_EQ(source.find("\xef\xbb\xbf"), String::npos);
 
         ShaderAttrib attrib{.shaderType = GL_FRAGMENT_SHADER, .sourceStr = source};
@@ -568,10 +570,12 @@ void main() {
 )";
     PreprocessShaderSource(ShaderStage::Fragment, source);
 
-    const SizeT versionPos = source.find("#version 330 core\n");
+    const SizeT versionPos = source.find("#version 330 core ");
     const SizeT outputPos = source.find("out vec4 mg_FragColor;\n");
     EXPECT_NE(versionPos, String::npos);
-    EXPECT_EQ(outputPos, versionPos + std::strlen("#version 330 core\n"));
+    // The normalized directive carries a marker recording that this 330 came from a legacy
+    // declaration, so measure the line rather than assuming its length.
+    EXPECT_EQ(outputPos, source.find('\n', versionPos) + 1);
     EXPECT_NE(source.find("// #version 460 core"), String::npos);
     // This #line sits ahead of the version directive, where GLSL would never have honoured it, so
     // it is still dropped. Directives that follow the version line are kept - see
@@ -684,7 +688,7 @@ void main() {
     }
 }
 
-TEST_F(ProgramUtilTest, PreprocessModernSampleQualifierStaysAtVersion460) {
+TEST_F(ProgramUtilTest, PreprocessModernSampleQualifierStaysAtItsDeclaredVersion) {
     using namespace MG_Util::ShaderTranspiler;
 
     String source = R"(#version 400 core
@@ -697,7 +701,7 @@ void main() {
 )";
     PreprocessShaderSource(ShaderStage::Fragment, source);
 
-    EXPECT_EQ(source.find("#version 460 core\n"), 0);
+    EXPECT_EQ(source.find("#version 400 core\n"), 0);
     EXPECT_NE(source.find("sample in vec4 interpolatedColor;"), String::npos);
 
     ShaderAttrib attrib{.shaderType = GL_FRAGMENT_SHADER, .sourceStr = source};
@@ -746,7 +750,7 @@ void main() {
 
     PreprocessShaderSource(ShaderStage::Fragment, source);
 
-    EXPECT_EQ(source.find("#version 330 core\n"), 0);
+    EXPECT_EQ(source.find("#version 330 core "), 0);
     EXPECT_NE(source.find("layout(location = 0) out vec4 mg_FragData[8];\n"), String::npos);
     EXPECT_NE(source.find("mg_FragData[0] = vec4(1.0);"), String::npos);
     EXPECT_NE(source.find("mg_FragData[1].a = 0.5;"), String::npos);
@@ -902,16 +906,17 @@ TEST_F(ProgramUtilTest, CompileSimpleVertexShader) {
 }
 
 // Legacy desktop sources are normalized to "#version 330 core", which is stricter than the 460 they
-// used to be forced to. A shader declaring 330 while using 420-era syntax without the matching
-// #extension line is accepted by real drivers, so CompileShader retries it at 460 instead of failing.
-TEST_F(ProgramUtilTest, CompileShaderRetriesAt460WhenLegacyVersionRejects420Syntax) {
+// used to be forced to. A legacy shader using 420-era syntax without the matching #extension line
+// is accepted by real drivers, so CompileShader retries the normalized source at 460 rather than
+// failing. Only MobileGL's own normalization is rescued this way - an application-declared
+// "#version 330" keeps strict 3.30 semantics, which is what the CTS negative-compile cases need.
+TEST_F(ProgramUtilTest, CompileShaderRetriesAt460WhenNormalizedLegacyVersionRejects420Syntax) {
     using namespace MG_Util::ShaderTranspiler;
-    String source = R"(#version 330
+    String source = R"(#version 130
 layout(binding = 0) uniform sampler2D InSampler;
-in vec2 texCoord;
-out vec4 fragColor;
+varying vec2 texCoord;
 void main() {
-    fragColor = texture(InSampler, texCoord);
+    gl_FragColor = texture2D(InSampler, texCoord);
 })";
     PreprocessShaderSource(ShaderStage::Fragment, source);
     // The normal path still emits 330 - the retry must not become the default.
@@ -952,10 +957,21 @@ void main() {
 TEST_F(ProgramUtilTest, RetargetLegacyVersionDirectiveOnlyTouchesNormalizedDesktopCore) {
     using namespace MG_Util::ShaderTranspiler;
 
-    String normalized = "#version 330 core\nvoid main() {}\n";
+    // Only MobileGL's own normalization is retargetable, and it is recognised by the marker the
+    // preprocessor leaves on the directive line - so normalize a legacy source rather than
+    // hand-writing the directive the marker belongs to.
+    String normalized = "#version 130\nvoid main() {}\n";
+    PreprocessShaderSource(ShaderStage::Vertex, normalized);
+    ASSERT_EQ(normalized.find("#version 330 core "), 0u);
     EXPECT_TRUE(RetargetLegacyVersionDirectiveTo460(normalized));
     EXPECT_EQ(normalized.find("#version 460 core"), 0u);
 
+    // An application that declared 330 itself keeps strict 3.30 semantics: raising it would
+    // re-legalize the CTS negative-compile cases.
+    String declared330 = "#version 330 core\nvoid main() {}\n";
+    EXPECT_FALSE(RetargetLegacyVersionDirectiveTo460(declared330));
+    EXPECT_EQ(declared330.find("#version 330 core"), 0u);
+
     // Already modern: nothing to retarget.
     String modern = "#version 460 core\nvoid main() {}\n";
     EXPECT_FALSE(RetargetLegacyVersionDirectiveTo460(modern));

MobileGL/MG_Test/SanityTest.cpp

@@ -197,13 +197,15 @@ TEST(DirectGLESSanity, AdvertisesDepthTextureForGlmarkShadowScenes) {
     EXPECT_NE(std::find(extensions.begin(), extensions.end(), MobileGL::E_GL_ARB_depth_texture), extensions.end());
 }
 
-TEST(DirectGLESSanity, AdvertisesVoxyRequiredRenderingExtensionsAtExperimentalCTSVersion) {
+// The advertised target went back to 3.3 (see "restore target GL version to 3.3"); Voxy only ever
+// needed the extensions, which stay advertised, so assert what the backend really reports.
+TEST(DirectGLESSanity, AdvertisesVoxyRequiredRenderingExtensions) {
     MobileGL::MG_Backend::DirectGLES::BackendObject_DirectGLES backend;
     const auto& rendererInfo = backend.GetRendererInfo().RendererGLInfo;
     const auto& extensions = rendererInfo.Extensions;
 
-    EXPECT_EQ(rendererInfo.TargetGLVersion.Major, 4);
-    EXPECT_EQ(rendererInfo.TargetGLVersion.Minor, 6);
+    EXPECT_EQ(rendererInfo.TargetGLVersion.Major, 3);
+    EXPECT_EQ(rendererInfo.TargetGLVersion.Minor, 3);
     EXPECT_EQ(rendererInfo.TargetGLVersion.Patch, 0);
 
     EXPECT_NE(std::find(extensions.begin(), extensions.end(), MobileGL::E_GL_ARB_compute_shader),
@@ -397,13 +399,14 @@ TEST(DirectVulkanSanity, RenderPassExtentUsesSwapchainSizeOnlyForDefaultFramebuf
               MobileGL::IntVec2(512, 512));
 }
 
-TEST(DirectVulkanSanity, AdvertisesVoxyRequiredRenderingExtensionsAtExperimentalCTSVersion) {
+// See the DirectGLES twin above: the target version is 3.3 again, the extensions are what matter.
+TEST(DirectVulkanSanity, AdvertisesVoxyRequiredRenderingExtensions) {
     MobileGL::MG_Backend::DirectVulkan::BackendObject_DirectVulkan backend;
     const auto& rendererInfo = backend.GetRendererInfo().RendererGLInfo;
     const auto& extensions = rendererInfo.Extensions;
 
-    EXPECT_EQ(rendererInfo.TargetGLVersion.Major, 4);
-    EXPECT_EQ(rendererInfo.TargetGLVersion.Minor, 6);
+    EXPECT_EQ(rendererInfo.TargetGLVersion.Major, 3);
+    EXPECT_EQ(rendererInfo.TargetGLVersion.Minor, 3);
     EXPECT_EQ(rendererInfo.TargetGLVersion.Patch, 0);
 
     EXPECT_NE(std::find(extensions.begin(), extensions.end(), MobileGL::E_GL_ARB_compute_shader),

MobileGL/MG_Test/Texture/TextureTest.cpp

@@ -1867,11 +1867,13 @@ TEST_F(TextureTest, DirectGLESTreats2DArrayAsSupportedTextureTarget) {
     EXPECT_TRUE(IsSupportedTextureTarget(TextureTarget::Texture2DArray));
     EXPECT_TRUE(IsSupportedTextureTarget(TextureTarget::Texture3D));
     EXPECT_TRUE(IsSupportedTextureTarget(TextureTarget::Texture2D));
-    // 1D and 1D-array are emulated as 2D / 2D-array (MapToBackendTextureTarget), matching
-    // SPIRV-Cross's ES 1D-as-2D shader emission; only rectangle textures stay unsupported.
+    // Every desktop-only target is stored on an ES one (MapToBackendTextureTarget): 1D and
+    // 1D-array as 2D / 2D-array, matching SPIRV-Cross's ES 1D-as-2D shader emission, and
+    // rectangle as a plain 2D - it is single-level and already clamps, so only the
+    // non-normalized coordinates differ and LowerRectImagesForEssl handles those.
     EXPECT_TRUE(IsSupportedTextureTarget(TextureTarget::Texture1D));
     EXPECT_TRUE(IsSupportedTextureTarget(TextureTarget::Texture1DArray));
-    EXPECT_FALSE(IsSupportedTextureTarget(TextureTarget::TextureRectangle));
+    EXPECT_TRUE(IsSupportedTextureTarget(TextureTarget::TextureRectangle));
 }
 
 // 2D-array textures keep their layer count constant across mip levels (GL 3.3 §3.9);