From 5242f8c4150e57281ea7cfb12c2226a491194cad Mon Sep 17 00:00:00 2001
From: Le Hoang Quyen <[email protected]>
Date: Wed, 20 May 2026 00:35:19 +0800
Subject: [PATCH] Metal: Fix uninitialized textures on invalidate.

- Added a front-end feature `set_need_init_on_invalidation` to mark
  attachments as needing initialization on invalidation.
- Added `markAttachmentsUninitialized` in `Framebuffer.cpp` to set the
  init state to `MayNeedInit` and flag dirty bits.
- Updated `discard`, `invalidate`, and `invalidateSub` in
  `Framebuffer.cpp` to use this new method.
- Added `overrideInvalidateAttachments` helper in `Framebuffer.cpp` to
  filter packed depth/stencil attachments when the feature is enabled.
- Optimized `invalidateSub` to treat sub-area invalidations as no-ops
  in robust resource init mode if the feature is enabled.
- Enabled `set_need_init_on_invalidation` in Metal backend.
- Added a metal feature to force non-zero texture creations for
  testing purpose.
- Added a new test `ClearThenInvalidateThenReadBack` in
  `RobustResourceInitTest.cpp`.

We only apply the fix to the Metal backend for now. The D3D11 and GL
backends will be next.

Bug: chromium:513043537
Change-Id: I344bff69ec8c5ca48d385b867397ed384031feaf
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/7859172
Reviewed-by: Geoff Lang <[email protected]>
Reviewed-by: Shahbaz Youssefi <[email protected]>
Commit-Queue: Quyen Le <[email protected]>
---

diff --git a/include/platform/autogen/FeaturesMtl_autogen.h b/include/platform/autogen/FeaturesMtl_autogen.h
index 3efcc45..202f71c 100644
--- a/include/platform/autogen/FeaturesMtl_autogen.h
+++ b/include/platform/autogen/FeaturesMtl_autogen.h
@@ -338,6 +338,12 @@
         &members,
     };
 
+    FeatureInfo allocateNonZeroTextures = {
+        "allocateNonZeroTextures",
+        FeatureCategory::MetalFeatures,
+        &members,
+    };
+
 };
 
 inline FeaturesMtl::FeaturesMtl()  = default;
diff --git a/include/platform/autogen/FrontendFeatures_autogen.h b/include/platform/autogen/FrontendFeatures_autogen.h
index 4431bb7..9faa0a7 100644
--- a/include/platform/autogen/FrontendFeatures_autogen.h
+++ b/include/platform/autogen/FrontendFeatures_autogen.h
@@ -225,6 +225,12 @@
         &members,
     };
 
+    FeatureInfo setNeedInitOnInvalidation = {
+        "setNeedInitOnInvalidation",
+        FeatureCategory::FrontendFeatures,
+        &members,
+    };
+
 };
 
 inline FrontendFeatures::FrontendFeatures()  = default;
diff --git a/include/platform/frontend_features.json b/include/platform/frontend_features.json
index 6f79c21..4b99462 100644
--- a/include/platform/frontend_features.json
+++ b/include/platform/frontend_features.json
@@ -270,6 +270,13 @@
             "description": [
                 "Allow #extension ... : disable after non preprocessor tokens in WebGL."
             ]
+        },
+        {
+            "name": "set_need_init_on_invalidation",
+            "category": "Features",
+            "description": [
+                "Set Texture's InitState to MayNeedInit inside invalidateFramebuffer or discardFramebuffer."
+            ]
         }
     ]
 }
diff --git a/include/platform/mtl_features.json b/include/platform/mtl_features.json
index 386e3a8..78f0cbc 100644
--- a/include/platform/mtl_features.json
+++ b/include/platform/mtl_features.json
@@ -411,6 +411,13 @@
                 "Inject statements into loop bodies when needed to ensure infinite loops are optimized out."
             ],
             "issue": "http://crbug.com/1513738"
+        },
+        {
+            "name": "allocate_non_zero_textures",
+            "category": "Features",
+            "description": [
+                "Clear allocated textures to non-zero values to help detect uninitialized reads."
+            ]
         }
     ]
 }
diff --git a/src/libANGLE/Framebuffer.cpp b/src/libANGLE/Framebuffer.cpp
index a8d5c85..f1d13ab 100644
--- a/src/libANGLE/Framebuffer.cpp
+++ b/src/libANGLE/Framebuffer.cpp
@@ -337,6 +337,22 @@
     return !stencilAttachment || stencilAttachment->getStencilSize() == 8;
 }
 
+angle::Result CheckAttachmentEnclosed(const Context *context,
+                                      const FramebufferAttachment &attachment,
+                                      const Rectangle &area,
+                                      bool *enclosedOut)
+{
+    *enclosedOut = true;
+    if (attachment.isAttached())
+    {
+        ANGLE_TRY(attachment.ensureSizeResolved(context));
+
+        Extents size = attachment.getSize();
+        *enclosedOut = area.encloses(Rectangle(0, 0, size.width, size.height));
+    }
+    return angle::Result::Continue;
+}
+
 }  // anonymous namespace
 
 FramebufferStatus FramebufferStatus::Complete()
@@ -1606,6 +1622,15 @@
     // can be no-ops, so we should probably do that to ensure consistency.
     // TODO(jmadill): WebGL behaviour, and robust resource init behaviour without WebGL.
 
+    if (context->getFrontendFeatures().setNeedInitOnInvalidation.enabled &&
+        context->isRobustResourceInitEnabled())
+    {
+        // We don't need to override the attachments list like in invalidate() because, unlike
+        // invalidate(), discard() allows a packed depth/stencil attachment's data to become
+        // undefined even if the attachments list only contains the depth or stencil aspect.
+        markAttachmentsUninitialized(context, count, attachments);
+    }
+
     return mImpl->discard(context, count, attachments);
 }
 
@@ -1617,6 +1642,15 @@
     // can be no-ops, so we should probably do that to ensure consistency.
     // TODO(jmadill): WebGL behaviour, and robust resource init behaviour without WebGL.
 
+    if (context->getFrontendFeatures().setNeedInitOnInvalidation.enabled &&
+        context->isRobustResourceInitEnabled())
+    {
+        auto overrideAttachments = overrideInvalidateAttachments(count, attachments);
+        markAttachmentsUninitialized(context, overrideAttachments.size(),
+                                     overrideAttachments.data());
+        return mImpl->invalidate(context, overrideAttachments.size(), overrideAttachments.data());
+    }
+
     return mImpl->invalidate(context, count, attachments);
 }
 
@@ -1655,7 +1689,7 @@
     {
         ASSERT(HasSupportedStencilBitCount(glState.getDrawFramebuffer()));
 
-        const auto &depthStencil       = glState.getDepthStencilState();
+        const auto &depthStencil = glState.getDepthStencilState();
         // The least significant |stencilBits| of stencil mask state specify a
         // mask. Check only those bits, ignoring any masked high bits.
         // Only the stencil write mask can affect which stencil bits are cleared. Clears are always
@@ -1697,6 +1731,43 @@
     // can be no-ops, so we should probably do that to ensure consistency.
     // TODO(jmadill): Make a invalidate no-op in WebGL 2.0.
 
+    if (context->getFrontendFeatures().setNeedInitOnInvalidation.enabled &&
+        context->isRobustResourceInitEnabled())
+    {
+        DrawBufferMask colorMask;
+        bool invalidateDepth   = false;
+        bool invalidateStencil = false;
+
+        for (size_t i = 0; i < count; ++i)
+        {
+            GLenum attachment = attachments[i];
+            if (attachment >= GL_COLOR_ATTACHMENT0 &&
+                attachment < GL_COLOR_ATTACHMENT0 + IMPLEMENTATION_MAX_DRAW_BUFFERS)
+            {
+                colorMask.set(attachment - GL_COLOR_ATTACHMENT0);
+            }
+            if (attachment == GL_DEPTH_ATTACHMENT || attachment == GL_DEPTH_STENCIL_ATTACHMENT)
+            {
+                invalidateDepth = true;
+            }
+            if (attachment == GL_STENCIL_ATTACHMENT || attachment == GL_DEPTH_STENCIL_ATTACHMENT)
+            {
+                invalidateStencil = true;
+            }
+        }
+
+        bool allEnclosed = false;

diff --git a/src/tests/angle_end2end_tests_expectations.txt b/src/tests/angle_end2end_tests_expectations.txt
index 120b907..6f13d2b 100644
--- a/src/tests/angle_end2end_tests_expectations.txt
+++ b/src/tests/angle_end2end_tests_expectations.txt
@@ -572,6 +572,7 @@
 496259841 WIN D3D11 : RobustResourceInitTestES3.InvalidateThenReadBack/* = SKIP
 496259841 WIN D3D11 : RobustResourceInitTestES3.DrawThenInvalidateThenReadBack/* = SKIP
 496259841 WIN D3D11 : RobustResourceInitTestES3.DrawThenInvalidateThenVerifyDepthStencil/* = SKIP
+514927166 WIN D3D11 : RobustResourceInitTestES3.ClearThenInvalidateThenReadBack/* = SKIP
 512896813 WIN D3D11 : LineLoopTestES3.LineLoopDrawArraysInstancedBaseInstance/* = SKIP
 512896813 WIN D3D11 : LineLoopTestES3.LineLoopDrawElementsInstancedBaseVertexBaseInstance/* = SKIP
 514615434 D3D11 : DrawBaseVertexBaseInstanceTest.NegativeBaseVertex/* = SKIP
diff --git a/src/tests/gl_tests/RobustResourceInitTest.cpp b/src/tests/gl_tests/RobustResourceInitTest.cpp
index 6935603..0c5d42c 100644
--- a/src/tests/gl_tests/RobustResourceInitTest.cpp
+++ b/src/tests/gl_tests/RobustResourceInitTest.cpp
@@ -809,6 +809,55 @@
     EXPECT_GL_NO_ERROR();
 }
 
+// Calling invalidate after clear should either read back the result of the clear call (invalidate
+// didn't happen) or transparent black (the init color).
+TEST_P(RobustResourceInitTestES3, ClearThenInvalidateThenReadBack)
+{
+    ANGLE_SKIP_TEST_IF(!hasGLExtension());
+
+    for (int i = 0; i < 10; ++i)
+    {
+        GLTexture tex;
+        GLRenderbuffer rb;
+        GLFramebuffer fbo;
+        glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+
+        if (i % 2 == 0)
+        {
+            glBindTexture(GL_TEXTURE_2D, tex);
+            glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, kWidth, kHeight);
+            glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+            glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+            glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, tex, 0);
+        }
+        else
+        {
+            glBindRenderbuffer(GL_RENDERBUFFER, rb);
+            glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8, kWidth, kHeight);
+            glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, rb);
+        }
+        ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_FRAMEBUFFER);
+
+        // Clear to red
+        glClearColor(1.0f, 0.0f, 0.0f, 1.0f);
+        glClear(GL_COLOR_BUFFER_BIT);
+
+        // Invalidate
+        std::array<GLenum, 1> attachments = {GL_COLOR_ATTACHMENT0};
+        glInvalidateFramebuffer(GL_FRAMEBUFFER, 1, attachments.data());
+
+        const int w = getWindowWidth();
+        const int h = getWindowHeight();
+
+        // Read back, ensure the content is either red or transparent black
+        GLColor invalidated(100, 100, 100, 100);
+        glReadPixels(w / 2, h / 2, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &invalidated);
+        EXPECT_TRUE(invalidated == GLColor::red || invalidated == GLColor::transparentBlack)
+            << "At iteration " << i << ", color is " << invalidated;
+        EXPECT_GL_NO_ERROR();
+    }
+}
+
 // Calling invalidate after draw should either read back the result of the draw call (invalidate
 // didn't happen) or the init depth/stencil values.
 TEST_P(RobustResourceInitTestES3, DrawThenInvalidateThenReadBackDepthStencil)
@@ -3372,6 +3421,7 @@
 GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(RobustResourceInitTestES3);
 ANGLE_INSTANTIATE_TEST_ES3_AND(RobustResourceInitTestES3,
                                ES3_METAL().enable(Feature::EmulateDontCareLoadWithRandomClear),
+                               ES3_METAL().enable(Feature::AllocateNonZeroTextures),
                                ES3_VULKAN().enable(Feature::AllocateNonZeroMemory));
 
 GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(RobustResourceInitTestES31);