From 6d8b704e2a185c82430a339d70508742887a962f Mon Sep 17 00:00:00 2001
From: Geoff Lang <geofflang@chromium.org>
Date: Thu, 19 Mar 2026 15:17:08 -0400
Subject: [PATCH] Metal: Remove TextureMtl::mFormat

TextureMtl::mFormat is supposed to represent the format of the native
storage but it was updated to the last format set on any mip level, even
if it is not in the native storage. This is extremely error prone, a lot
of the Metal texturing code relied on it being properly set.

Remove mFormat and query it from the native storage or the specific
image desc.

Bug: chromium:493256564
Change-Id: I629c009b34c7ef7ca5fa7a97f5845accf22b13b8
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/7684363
Commit-Queue: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
---

diff --git a/src/libANGLE/renderer/metal/TextureMtl.h b/src/libANGLE/renderer/metal/TextureMtl.h
index 65283cd..2dfd58b 100644
--- a/src/libANGLE/renderer/metal/TextureMtl.h
+++ b/src/libANGLE/renderer/metal/TextureMtl.h
@@ -182,15 +182,14 @@
                                     int layer,
                                     GLenum format);
 
-    const mtl::Format &getFormat() const { return mFormat; }
-
   private:
     void deallocateNativeStorage(bool keepImages, bool keepSamplerStateAndFormat = false);
     angle::Result createNativeStorage(const gl::Context *context,
                                       gl::TextureType type,
                                       GLuint mips,
                                       GLuint samples,
-                                      const gl::Extents &size);
+                                      const gl::Extents &size,
+                                      const mtl::Format &format);
     angle::Result onBaseMaxLevelsChanged(const gl::Context *context);
     angle::Result ensureSamplerStateCreated(const gl::Context *context);
     // Ensure image at given index is created:
@@ -328,10 +327,10 @@
 
     angle::Result generateMipmapCPU(const gl::Context *context);
 
-    bool needsFormatViewForPixelLocalStorage(const ShPixelLocalStorageOptions &) const;
+    bool needsFormatViewForPixelLocalStorage(const ShPixelLocalStorageOptions &,
+                                             const mtl::Format &format) const;
     bool isImmutableOrPBuffer() const;
 
-    mtl::Format mFormat;
     egl::Surface *mBoundSurface = nullptr;
     class NativeTextureWrapper;
     class NativeTextureWrapperWithViewSupport;
diff --git a/src/libANGLE/renderer/metal/TextureMtl.mm b/src/libANGLE/renderer/metal/TextureMtl.mm
index 9301862..0dcbddd 100644
--- a/src/libANGLE/renderer/metal/TextureMtl.mm
+++ b/src/libANGLE/renderer/metal/TextureMtl.mm
@@ -777,8 +777,8 @@
 class TextureMtl::NativeTextureWrapper : angle::NonCopyable
 {
   public:
-    NativeTextureWrapper(mtl::TextureRef texture, GLuint baseGLLevel)
-        : mNativeTexture(std::move(texture)), mBaseGLLevel(baseGLLevel)
+    NativeTextureWrapper(mtl::TextureRef texture, GLuint baseGLLevel, const mtl::Format &format)
+        : mNativeTexture(std::move(texture)), mBaseGLLevel(baseGLLevel), mFormat(format)
     {
         ASSERT(mNativeTexture && mNativeTexture->valid());
     }
@@ -810,6 +810,7 @@
                                  getNativeLevel(glLevel), slice, dataOut);
     }
 
+    const mtl::Format &getFormat() const { return mFormat; }
     GLuint getBaseGLLevel() const { return mBaseGLLevel; }
     // Get max addressable GL level that this texture supports.
     GLuint getMaxSupportedGLLevel() const { return mBaseGLLevel + mipmapLevels() - 1; }
@@ -855,14 +856,17 @@
   protected:
     mtl::TextureRef mNativeTexture;
     const GLuint mBaseGLLevel;
+    const mtl::Format mFormat;
 };
 
 // This class extends NativeTextureWrapper with support for view creation
 class TextureMtl::NativeTextureWrapperWithViewSupport : public NativeTextureWrapper
 {
   public:
-    NativeTextureWrapperWithViewSupport(mtl::TextureRef texture, GLuint baseGLLevel)
-        : NativeTextureWrapper(std::move(texture), baseGLLevel)
+    NativeTextureWrapperWithViewSupport(mtl::TextureRef texture,
+                                        GLuint baseGLLevel,
+                                        const mtl::Format &format)
+        : NativeTextureWrapper(std::move(texture), baseGLLevel, format)
     {}
 
     // Create a view of one slice at a level.
@@ -952,7 +956,6 @@
     if (!keepSamplerStateAndFormat)
     {
         mMetalSamplerState = nil;
-        mFormat            = mtl::Format();
     }
 }
 
@@ -976,9 +979,9 @@
     ANGLE_CHECK(contextMtl, desc.format.valid(), gl::err::kInternalError, GL_INVALID_OPERATION);
     angle::FormatID angleFormatId =
         angle::Format::InternalFormatToID(desc.format.info->sizedInternalFormat);
-    mFormat = contextMtl->getPixelFormat(angleFormatId);
+    mtl::Format format = contextMtl->getPixelFormat(angleFormatId);
 
-    ANGLE_TRY(createNativeStorage(context, mState.getType(), mips, 0, desc.size));
+    ANGLE_TRY(createNativeStorage(context, mState.getType(), mips, 0, desc.size, format));
 
     // Transfer data from defined images to actual texture object
     int numCubeFaces = static_cast<int>(mNativeTextureStorage->cubeFaces());
@@ -1019,44 +1022,46 @@
                                               gl::TextureType type,
                                               GLuint mips,
                                               GLuint samples,
-                                              const gl::Extents &size)
+                                              const gl::Extents &size,
+                                              const mtl::Format &format)
 {
     ASSERT(samples == 0 || mips == 0);
     ContextMtl *contextMtl = mtl::GetImpl(context);
 
     // Create actual texture object:
     mSlices              = 1;
-    bool allowFormatView = mFormat.hasDepthAndStencilBits() ||
-                           needsFormatViewForPixelLocalStorage(
-                               contextMtl->getDisplay()->getNativePixelLocalStorageOptions());
+    bool allowFormatView =
+        format.hasDepthAndStencilBits() ||
+        needsFormatViewForPixelLocalStorage(
+            contextMtl->getDisplay()->getNativePixelLocalStorageOptions(), format);
     mtl::TextureRef nativeTextureStorage;
     switch (type)
     {
         case gl::TextureType::_2D:
-            ANGLE_TRY(mtl::Texture::Make2DTexture(
-                contextMtl, mFormat, size.width, size.height, mips,
-                /** renderTargetOnly */ false, allowFormatView, &nativeTextureStorage));
+            ANGLE_TRY(mtl::Texture::Make2DTexture(contextMtl, format, size.width, size.height, mips,
+                                                  /** renderTargetOnly */ false, allowFormatView,
+                                                  &nativeTextureStorage));
             break;
         case gl::TextureType::CubeMap:
             mSlices = 6;
-            ANGLE_TRY(mtl::Texture::MakeCubeTexture(contextMtl, mFormat, size.width, mips,
+            ANGLE_TRY(mtl::Texture::MakeCubeTexture(contextMtl, format, size.width, mips,
                                                     /** renderTargetOnly */ false, allowFormatView,
                                                     &nativeTextureStorage));
             break;
         case gl::TextureType::_3D:
             ANGLE_TRY(mtl::Texture::Make3DTexture(
-                contextMtl, mFormat, size.width, size.height, size.depth, mips,
+                contextMtl, format, size.width, size.height, size.depth, mips,
                 /** renderTargetOnly */ false, allowFormatView, &nativeTextureStorage));
             break;
         case gl::TextureType::_2DArray:
             mSlices = size.depth;
             ANGLE_TRY(mtl::Texture::Make2DArrayTexture(
-                contextMtl, mFormat, size.width, size.height, mips, mSlices,
+                contextMtl, format, size.width, size.height, mips, mSlices,
                 /** renderTargetOnly */ false, allowFormatView, &nativeTextureStorage));
             break;
         case gl::TextureType::_2DMultisample:
             ANGLE_TRY(mtl::Texture::Make2DMSTexture(
-                contextMtl, mFormat, size.width, size.height, samples,
+                contextMtl, format, size.width, size.height, samples,
                 /** renderTargetOnly */ false, allowFormatView, &nativeTextureStorage));
             break;
         default:
@@ -1066,15 +1071,16 @@
     if (mState.getImmutableFormat())
     {
         mNativeTextureStorage = std::make_unique<NativeTextureWrapperWithViewSupport>(
-            std::move(nativeTextureStorage), /*baseGLLevel=*/0);
+            std::move(nativeTextureStorage), /*baseGLLevel=*/0, format);
     }
     else
     {
         mNativeTextureStorage = std::make_unique<NativeTextureWrapperWithViewSupport>(
-            std::move(nativeTextureStorage), /*baseGLLevel=*/mState.getEffectiveBaseLevel());
+            std::move(nativeTextureStorage), /*baseGLLevel=*/mState.getEffectiveBaseLevel(),
+            format);
     }
 
-    ANGLE_TRY(checkForEmulatedChannels(context, mFormat, *mNativeTextureStorage));
+    ANGLE_TRY(checkForEmulatedChannels(context, format, *mNativeTextureStorage));
 
     ANGLE_TRY(createViewFromBaseToMaxLevel());
 
@@ -1091,11 +1097,14 @@
         return angle::Result::Continue;
     }
 

diff --git a/src/tests/gl_tests/TextureTest.cpp b/src/tests/gl_tests/TextureTest.cpp
index acef103..7137425 100644
--- a/src/tests/gl_tests/TextureTest.cpp
+++ b/src/tests/gl_tests/TextureTest.cpp
@@ -2864,6 +2864,39 @@
     EXPECT_EQ(expected, actual);
 }
 
+// Regression test for TextureMtl::mFormat becoming mismatched with the native storage format when
+// updating mips outside of the storage.
+TEST_P(Texture2DTestES3, StaleFormatCacheOutOrRangeMip)
+{
+
+    GLTexture tex;
+    glBindTexture(GL_TEXTURE_2D, tex);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, 4);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+
+    // Create RGBA8 texture with one mip.
+    constexpr GLuint kWidth = 24;
+    std::vector<GLColor> pixels(kWidth, GLColor::red);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, kWidth, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                 pixels.data());
+
+    // Force native storage allocation with a draw
+    ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), essl3_shaders::fs::Red());
+    drawQuad(program, essl3_shaders::PositionAttrib(), 0.5f);
+
+    // Set data on a high mip with a different format causing mFormat to change in TextureMtl
+    glTexImage2D(GL_TEXTURE_2D, 10, GL_R8, 1, 1, 0, GL_RED, GL_UNSIGNED_BYTE, nullptr);
+
+    GLFramebuffer fbo;
+    glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, tex, 0);
+    ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_FRAMEBUFFER);
+
+    // Check that ReadPixels reads RGBA data from mip 0 correctly.
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+}
+
 // Almost mirrors UnitTest_DMSAA_dst_read test from Android skqp test suite
 TEST_P(Texture2DTestES3, UnitTest_DMSAA_dst_read)
 {