From 5bb0c9951910c64d8d2ca1251790ac625be82686 Mon Sep 17 00:00:00 2001 From: Le Hoang Quyen Date: Tue, 15 Sep 2026 23:03:40 +0800 Subject: [PATCH] MSL: Zero-initialize widened synthetic fragment outputs In ANGLE's MSL translator, sub-4-component fragment shader outputs (e.g., float, vec2, ivec3, uvec2) are widened to 4-component vectors in RewritePipelines to satisfy Metal render pipeline color attachment output semantics. However, SaturateScalarOrVectorCommon previously only copied the first dim channels, leaving synthetic channels [dim..saturation-1] uninitialized in the emitted originalToModified conversion function. Because the external struct variable is uninitialized and default Metal write mask is MTLColorWriteMaskAll, uninitialized GPU thread-register residue was written to active color attachments and could be harvested via glReadPixels. This CL explicitly zero-initializes the widened synthetic channels in SaturateScalarOrVectorCommon for pipeline outputs (original-to-modified). Bug: chromium:536445815 Change-Id: I391c89dfd7d11a483737e6c2575f170dad2df6c6 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/8409967 Reviewed-by: Shahbaz Youssefi Commit-Queue: Quyen Le Auto-Submit: Quyen Le --- src/compiler/translator/msl/ModifyStruct.cpp | 13 +++ .../angle_end2end_tests_expectations.txt | 6 ++ src/tests/gl_tests/GLSLTest.cpp | 94 +++++++++++++++++++ 3 files changed, 113 insertions(+) diff --git a/src/compiler/translator/msl/ModifyStruct.cpp b/src/compiler/translator/msl/ModifyStruct.cpp index 4ba2532a76..aee1570be2 100644 --- a/src/compiler/translator/msl/ModifyStruct.cpp +++ b/src/compiler/translator/msl/ModifyStruct.cpp @@ -882,6 +882,19 @@ bool SaturateScalarOrVectorCommon(ConvertStructState &state, }); } + // Zero-initialize components added by saturation for pipeline outputs (original-to-modified) + // to avoid indeterminate GPU register residue leaking into color attachments. + if (state.config.convertType == ConvertType::OriginalToModified) + { + for (uint8_t d = dim; d < saturation; ++d) + { + state.addConversion([=](Access::Env &, OriginalAccess &, ModifiedAccess &m) { + auto &m_ = AccessIndex(m, d); + return Access{*CreateZeroNode(m_.getType()), m_}; + }); + } + } + return true; } diff --git a/src/tests/angle_end2end_tests_expectations.txt b/src/tests/angle_end2end_tests_expectations.txt index 628e7cf463..f666ec7e4b 100644 --- a/src/tests/angle_end2end_tests_expectations.txt +++ b/src/tests/angle_end2end_tests_expectations.txt @@ -83,6 +83,12 @@ 523165431 GLES : GLSLTest.FragDataPassedToFunctionNoDrawBuffers/* = SKIP 523165431 GLES : GLSLTest.FragDataPassedToFunctionOutNoDrawBuffers/* = SKIP +// Fragment output with missing channels is not clearly defined in WebGL standard +562053522 VULKAN : WebGL2GLSLTest.FragmentOutputMissingChannels/* = SKIP +562053522 OPENGL : WebGL2GLSLTest.FragmentOutputMissingChannels/* = SKIP +562053522 GLES : WebGL2GLSLTest.FragmentOutputMissingChannels/* = SKIP +562053522 D3D11 : WebGL2GLSLTest.FragmentOutputMissingChannels/* = SKIP + // Fails AST validation 342866374 : GLSLTest_ES31.PerVertexRedefinition/*_ForceInitShaderVariables* = SKIP diff --git a/src/tests/gl_tests/GLSLTest.cpp b/src/tests/gl_tests/GLSLTest.cpp index 29c33f47b6..91691e2c75 100644 --- a/src/tests/gl_tests/GLSLTest.cpp +++ b/src/tests/gl_tests/GLSLTest.cpp @@ -7154,6 +7154,100 @@ void main() EXPECT_PIXEL_NEAR(0, 0, 255, 127, 0, 255, 1); } +// Test that sub-4-component fragment outputs zero-initialize missing channels (or keep cleared +// values on non-widening backends). +TEST_P(WebGL2GLSLTest, FragmentOutputMissingChannels) +{ + // Test 1: out float -> writes R (0.8), G, B, A must be either 0 (widened) or cleared values + // (51, 76, 102) + { + glClearColor(0.1f, 0.2f, 0.3f, 0.4f); + glClear(GL_COLOR_BUFFER_BIT); + + constexpr char kFS[] = R"(#version 300 es +precision highp float; +layout(location = 0) out float color; +void main() +{ + color = 0.8; +})"; + + ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), kFS); + drawQuad(program, essl3_shaders::PositionAttrib(), 0.5f, 1.0f, true); + + GLColor pixel; + glReadPixels(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &pixel); + EXPECT_GL_NO_ERROR(); + EXPECT_NEAR(pixel.R, 204, 1); + EXPECT_TRUE(pixel.G == 0 || std::abs(pixel.G - 51) <= 1); + EXPECT_TRUE(pixel.B == 0 || std::abs(pixel.B - 76) <= 1); + EXPECT_TRUE(pixel.A == 0 || std::abs(pixel.A - 102) <= 1); + } + + // Test 2: out vec2 -> writes R (0.8), G (0.6), B, A must be either 0 (widened) or cleared + // values (76, 102) + { + glClearColor(0.1f, 0.2f, 0.3f, 0.4f); + glClear(GL_COLOR_BUFFER_BIT); + + constexpr char kFS[] = R"(#version 300 es +precision highp float; +layout(location = 0) out vec2 color; +void main() +{ + color = vec2(0.8, 0.6); +})"; + + ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), kFS); + drawQuad(program, essl3_shaders::PositionAttrib(), 0.5f, 1.0f, true); + + GLColor pixel; + glReadPixels(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &pixel); + EXPECT_GL_NO_ERROR(); + EXPECT_NEAR(pixel.R, 204, 1); + EXPECT_NEAR(pixel.G, 153, 1); + EXPECT_TRUE(pixel.B == 0 || std::abs(pixel.B - 76) <= 1); + EXPECT_TRUE(pixel.A == 0 || std::abs(pixel.A - 102) <= 1); + } + + // Test 3: out uvec3 -> writes R (12), G (34), B (56), A must be either 0 (widened) or cleared + // value (4) + { + GLTexture tex; + glBindTexture(GL_TEXTURE_2D, tex); + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8UI, getWindowWidth(), getWindowHeight(), 0, + GL_RGBA_INTEGER, 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); + + const GLuint clearColor[4] = {1u, 2u, 3u, 4u}; + glClearBufferuiv(GL_COLOR, 0, clearColor); + + constexpr char kFS[] = R"(#version 300 es +precision highp int; +layout(location = 0) out uvec3 color; +void main() +{ + color = uvec3(12u, 34u, 56u); +})"; + + ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), kFS); + drawQuad(program, essl3_shaders::PositionAttrib(), 0.5f, 1.0f, true); + + uint8_t pixel[4] = {}; + glReadPixels(0, 0, 1, 1, GL_RGBA_INTEGER, GL_UNSIGNED_BYTE, pixel); + EXPECT_GL_NO_ERROR(); + EXPECT_EQ(pixel[0], 12); + EXPECT_EQ(pixel[1], 34); + EXPECT_EQ(pixel[2], 56); + EXPECT_TRUE(pixel[3] == 0 || pixel[3] == 4) + << " pixel[3]=" << static_cast(pixel[3]); + } +} + // Verify that functions without return statements return zero-initialized vec4 TEST_P(WebGL2GLSLTest, MissingReturnZeroInitVec4) { -- 2.55.0