[Fix, Test] (GLImpl, DirectGLES): deallocate zero-sized multisample images instead of defining them

MobileGL/MG_Backend/DirectGLES/Managers.cpp

@@ -2657,25 +2657,51 @@ namespace MobileGL::MG_Backend::DirectGLES {
                         const auto backendSamples = static_cast<GLsizei>(ClampSamplesToBackendSupport(
                             GetFormatCapabilityTargetIndex(targetInternal), textureMipmapObject->GetFormat(),
                             glFormat, static_cast<Int>(stateTextureObject->GetSamples())));
-                        switch (targetInternal) {
+                        // ES 3.1 8.19 requires width/height (and depth, for the array target) >= 1,
-                        case TextureTarget::Texture2DMultisample:
+                        // so a degenerate size has nothing to allocate and must not reach the
-                            g_GLESFuncs.glTexStorage2DMultisample(
+                        // driver. The frontend deallocates such an image rather than defining it
-                                target, backendSamples, glInternalFormat,
+                        // (GL 4.6 core 8.8), so this is belt and braces for any path that still
-                                static_cast<GLsizei>(baseSize.x()), static_cast<GLsizei>(baseSize.y()),
+                        // syncs one.
-                                stateTextureObject->HasFixedSampleLocations() ? GL_TRUE : GL_FALSE);
+                        const Bool hasAllocatableSize =
-                            break;
+                            baseSize.x() >= 1 && baseSize.y() >= 1 &&
-                        case TextureTarget::Texture2DMultisampleArray:
+                            (targetInternal != TextureTarget::Texture2DMultisampleArray || baseSize.z() >= 1);
-                            g_GLESFuncs.glTexStorage3DMultisample(
+                        if (!hasAllocatableSize) {
-                                target, backendSamples, glInternalFormat,
+                            MGLOG_D("Skipping multisample storage for texture %u: degenerate size (%d, %d, %d)",
-                                static_cast<GLsizei>(baseSize.x()), static_cast<GLsizei>(baseSize.y()),
+                                    m_backendTextureId, baseSize.x(), baseSize.y(), baseSize.z());
-                                static_cast<GLsizei>(baseSize.z()),
+                        } else {
-                                stateTextureObject->HasFixedSampleLocations() ? GL_TRUE : GL_FALSE);
+                            switch (targetInternal) {
-                            break;
+                            case TextureTarget::Texture2DMultisample:
-                        default:
+                                g_GLESFuncs.glTexStorage2DMultisample(
-                            MOBILEGL_ASSERT(false, "Unexpected multisample target: %d", static_cast<Int>(targetInternal));
+                                    target, backendSamples, glInternalFormat,
-                            break;
+                                    static_cast<GLsizei>(baseSize.x()), static_cast<GLsizei>(baseSize.y()),
+                                    stateTextureObject->HasFixedSampleLocations() ? GL_TRUE : GL_FALSE);
+                                break;
+                            case TextureTarget::Texture2DMultisampleArray:
+                                g_GLESFuncs.glTexStorage3DMultisample(
+                                    target, backendSamples, glInternalFormat,
+                                    static_cast<GLsizei>(baseSize.x()), static_cast<GLsizei>(baseSize.y()),
+                                    static_cast<GLsizei>(baseSize.z()),
+                                    stateTextureObject->HasFixedSampleLocations() ? GL_TRUE : GL_FALSE);
+                                break;
+                            default:
+                                MOBILEGL_ASSERT(false, "Unexpected multisample target: %d",
+                                                static_cast<Int>(targetInternal));
+                                break;
+                            }
+                            m_backendStorageImmutable = true;
                         }
-                        m_backendStorageImmutable = true;
+                        // The one storage branch that cleared the ES error queue without ever
+                        // draining it again, so anything this call raised was left for an
+                        // unrelated later query to trip over. Paired with its two siblings now.
+                        DebugImpl::ErrorLopper::Loop([file = __FILE__, line = __LINE__, func = __func__, target,
+                                                      glInternalFormat, backendSamples](GLenum err) {
+                            MGLOG_D("%s(%s:%d) ES error: %s. glTexStorage*Multisample: target=%s, internalformat=%s, "
+                                    "samples=%d",
+                                    func, file, line, MG_Util::ConvertGLEnumToString(err).c_str(),
+                                    MG_Util::ConvertGLEnumToString(target).c_str(),
+                                    MG_Util::ConvertGLEnumToString(glInternalFormat).c_str(),
+                                    static_cast<Int>(backendSamples));
+                        });
                         for (const auto& uploadTarget : uploadTargets) {
                             for (SizeT level = 0; level < mipmapCount; ++level) {
                                 textureMipmapObject->MarkStorageDirty(uploadTarget, level, false);

MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp

@@ -590,6 +590,20 @@ namespace MobileGL::MG_Impl::GLImpl {
                             "AllocateMultisampleTextureStorage requires mipmap-backed storage");
 
             auto* textureMipmapObject = static_cast<MG_State::GLState::TextureObjectMipmap*>(textureObject.get());
+            // GL 4.6 core 8.8: a zero-sized image DEALLOCATES the image rather than defining an
+            // empty one. Only the multisample pair cares, and it cares a great deal: the CTS's
+            // per-case state reset clears both DEFAULT multisample textures this way on every
+            // texture unit, and a "defined" 0x0 default texture stops being skipped by
+            // IsUndefinedDefaultTexture - it then joins the per-draw sync and bind passes on
+            // every unit the reset touched, and reaches an ES glTexStorage*Multisample(..., 0, 0)
+            // that ES 3.1 8.19 makes INVALID_VALUE on every driver there is. A proxy target holds
+            // no image at all, only the query result, so it keeps recording what was asked for.
+            if ((width <= 0 || height <= 0 || depth <= 0) &&
+                !TextureImpl::IsProxyTextureTarget(textureUploadTarget)) {
+                textureObject->SetInternalFormat(TextureInternalFormat::Unknown);
+                textureMipmapObject->TruncateMipmapLevels(textureUploadTarget, 0);
+                return;
+            }
             textureObject->SetInternalFormat(textureInternalFormat);
             textureObject->SetSamples(samples);
             textureObject->SetFixedSampleLocations(fixedsamplelocations == GL_TRUE);
@@ -4717,6 +4731,22 @@ namespace MobileGL::MG_Impl::GLImpl {
         TextureStorage3D(textureObject->GetExternalIndex(), levels, internalformat, width, height, depth);
     }
 
+    // Unlike glTexImage*Multisample, where a zero-sized image is a legal deallocation (see
+    // AllocateMultisampleTextureStorage), the immutable forms take a strictly positive size: GL
+    // 4.6 core 8.19 makes width, height or depth < 1 INVALID_VALUE. Without this the shared
+    // _State helper would deallocate the image and TexStorageMultisample_State would then freeze
+    // the now-imageless texture as immutable.
+    static Bool ValidateTexStorageMultisampleSize(GLsizei width, GLsizei height, GLsizei depth, const char* caller) {
+        if (width >= 1 && height >= 1 && depth >= 1) {
+            return true;
+        }
+        MG_State::pGLContext->RecordError(
+            ErrorCode::InvalidValue,
+            MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", caller,
+                                         "Immutable multisample storage requires width, height and depth >= 1."));
+        return false;
+    }
+
     // The multisample storage forms allocate exactly what the glTexImage*Multisample ones do, and
     // then freeze it: TEXTURE_IMMUTABLE_FORMAT becomes TRUE and a second call is INVALID_OPERATION
     // (GL 4.6 core 8.19). Only the allocation was shared before, so a multisample texture stayed
@@ -4737,6 +4767,7 @@ namespace MobileGL::MG_Impl::GLImpl {
         const TextureTarget textureTarget = MG_Util::ConvertGLEnumToTextureTarget(target);
         auto& activeUnit = MG_State::pGLContext->GetTextureUnitObject(MG_State::pGLContext->GetActiveTextureUnit());
         if (!ValidateTextureMutable(activeUnit.GetBindingSlot(textureTarget).GetBoundObject(), __func__)) return;
+        if (!ValidateTexStorageMultisampleSize(width, height, 1, __func__)) return;
         TexStorageMultisample_State(
             target, TexImage2DMultisample_State(target, samples, internalformat, width, height, fixedsamplelocations),
             __func__);
@@ -4747,6 +4778,7 @@ namespace MobileGL::MG_Impl::GLImpl {
         const TextureTarget textureTarget = MG_Util::ConvertGLEnumToTextureTarget(target);
         auto& activeUnit = MG_State::pGLContext->GetTextureUnitObject(MG_State::pGLContext->GetActiveTextureUnit());
         if (!ValidateTextureMutable(activeUnit.GetBindingSlot(textureTarget).GetBoundObject(), __func__)) return;
+        if (!ValidateTexStorageMultisampleSize(width, height, depth, __func__)) return;
         TexStorageMultisample_State(target,
                                     TexImage3DMultisample_State(target, samples, internalformat, width, height, depth,
                                                                 fixedsamplelocations),

MobileGL/MG_Test/Texture/TextureTest.cpp

@@ -2975,6 +2975,58 @@ TEST_F(TextureTest, CtsStyleStateResetOnDefaultTexturesLeavesNoError) {
     EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR) << "GL_TEXTURE_2D_MULTISAMPLE_ARRAY reset failed";
 }
 
+// Clean is not enough: per GL 4.6 core 8.8 that zero-sized reset has to DEALLOCATE the image,
+// not define an empty one. gluStateReset runs it on both default multisample textures on every
+// texture unit of a 3.2+ context, and a default texture left 'defined' afterwards stops being
+// skipped by IsUndefinedDefaultTexture - it then joins the per-draw sync and bind passes on
+// every unit the reset touched and reaches an ES glTexStorage*Multisample(..., 0, 0), which ES
+// 3.1 8.19 rejects on every driver.
+TEST_F(TextureTest, ZeroSizedMultisampleTexImageDeallocatesTheImage) {
+    MG_Impl::GLImpl::ActiveTexture(GL_TEXTURE0);
+
+    MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D_MULTISAMPLE, 0);
+    const auto& defaultMultisample = MG_State::pGLContext->GetTextureUnitObject(0)
+                                         .GetBindingSlot(TextureTarget::Texture2DMultisample)
+                                         .GetBoundObject();
+    MG_Impl::GLImpl::TexImage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE, 1, GL_RGBA8, 4, 4, GL_TRUE);
+    ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+    ASSERT_FALSE(MG_State::GLState::IsUndefinedDefaultTexture(defaultMultisample.get()));
+
+    MG_Impl::GLImpl::TexImage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE, 1, GL_RGBA8, 0, 0, GL_TRUE);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+    EXPECT_TRUE(MG_State::GLState::IsUndefinedDefaultTexture(defaultMultisample.get()));
+
+    // The array target's reset also passes zero LAYERS, which deallocates just the same.
+    MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D_MULTISAMPLE_ARRAY, 0);
+    const auto& defaultMultisampleArray = MG_State::pGLContext->GetTextureUnitObject(0)
+                                              .GetBindingSlot(TextureTarget::Texture2DMultisampleArray)
+                                              .GetBoundObject();
+    MG_Impl::GLImpl::TexImage3DMultisample(GL_TEXTURE_2D_MULTISAMPLE_ARRAY, 1, GL_RGBA8, 4, 4, 2, GL_TRUE);
+    ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+    ASSERT_FALSE(MG_State::GLState::IsUndefinedDefaultTexture(defaultMultisampleArray.get()));
+
+    MG_Impl::GLImpl::TexImage3DMultisample(GL_TEXTURE_2D_MULTISAMPLE_ARRAY, 1, GL_RGBA8, 4, 4, 0, GL_TRUE);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+    EXPECT_TRUE(MG_State::GLState::IsUndefinedDefaultTexture(defaultMultisampleArray.get()));
+
+    // The immutable forms do NOT share that leniency: GL 4.6 core 8.19 makes a size below 1
+    // INVALID_VALUE, and freezing an imageless texture as immutable would be unrecoverable.
+    GLuint texture = 0;
+    MG_Impl::GLImpl::GenTextures(1, &texture);
+    MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D_MULTISAMPLE, texture);
+    ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+    MG_Impl::GLImpl::TexStorage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE, 1, GL_RGBA8, 0, 0, GL_TRUE);
+    ExpectSingleGlError(GL_INVALID_VALUE);
+    EXPECT_FALSE(MG_State::pGLContext->GetTextureUnitObject(0)
+                     .GetBindingSlot(TextureTarget::Texture2DMultisample)
+                     .GetBoundObject()
+                     ->IsImmutable());
+
+    MG_Impl::GLImpl::DeleteTextures(1, &texture);
+    MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D_MULTISAMPLE, 0);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+}
+
 // ---- GL CTS packed_pixels / texture_swizzle readback root-cause regressions --------------------
 
 TEST_F(TextureTest, NormalizeLegacySizedFormatsMapToCanonicalShadowLayouts) {