From 4d6904184d573357f47cd85c15d658cd063b7e85 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 4 Oct 2026 23:24:06 -0400 Subject: [PATCH] [Fix] (ShaderTranspiler): deduplicate image types after Vulkan image rewrites --- MobileGL/MG_Test/Program/ProgramUtilTest.cpp | 49 +++++++++++++++++++ .../ShaderTranspiler/ShaderCompiler.cpp | 15 ++++-- 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp index 1498af42..0dddfb1f 100644 --- a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp @@ -123,10 +123,12 @@ TEST_F(ProgramUtilTest, UnformattedFloatStorageImagesKeepIntegerAtomicImagesType layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in; layout(rgba16, binding = 0) uniform image2D floatImage; layout(r32ui, binding = 1) uniform uimage2D atomicImage; +layout(rgba32f, binding = 2) uniform image2D otherFloatImages[2]; void main() { ivec2 coordinate = ivec2(gl_GlobalInvocationID.xy); imageStore(floatImage, coordinate, imageLoad(floatImage, coordinate)); + imageStore(otherFloatImages[1], coordinate, imageLoad(otherFloatImages[0], coordinate)); imageAtomicAdd(atomicImage, coordinate, 1u); } )"; @@ -174,6 +176,10 @@ void main() { << outputText; EXPECT_EQ(countOccurrences(outputText, "OpCapability StorageImageWriteWithoutFormat"), 1u) << outputText; + EXPECT_EQ(countOccurrences(outputText, "2D 0 0 0 2 Unknown"), 1u) << outputText; + EXPECT_NE(outputText.find("Binding 0"), String::npos) << outputText; + EXPECT_NE(outputText.find("Binding 1"), String::npos) << outputText; + EXPECT_NE(outputText.find("Binding 2"), String::npos) << outputText; EXPECT_TRUE(tools.Validate(outputBinary)); Vector secondOutputBinary; @@ -181,6 +187,49 @@ void main() { EXPECT_EQ(secondOutputBinary, outputBinary); } +TEST_F(ProgramUtilTest, LowerRectImagesMergesWithExisting2DImageTypes) { + using namespace MG_Util::ShaderTranspiler; + const String spirvText = R"( + OpCapability Shader + OpCapability ImageRect + OpMemoryModel Logical GLSL450 + OpEntryPoint GLCompute %main "main" + OpExecutionMode %main LocalSize 1 1 1 + OpDecorate %rect DescriptorSet 0 + OpDecorate %rect Binding 0 + OpDecorate %flat DescriptorSet 0 + OpDecorate %flat Binding 1 + %void = OpTypeVoid + %uint = OpTypeInt 32 0 + %rectType = OpTypeImage %uint Rect 0 0 0 2 R32ui + %flatType = OpTypeImage %uint 2D 0 0 0 2 R32ui + %rectPtr = OpTypePointer UniformConstant %rectType + %flatPtr = OpTypePointer UniformConstant %flatType + %rect = OpVariable %rectPtr UniformConstant + %flat = OpVariable %flatPtr UniformConstant + %mainType = OpTypeFunction %void + %main = OpFunction %void None %mainType + %entry = OpLabel + %r = OpLoad %rectType %rect + %f = OpLoad %flatType %flat + OpReturn + OpFunctionEnd +)"; + spvtools::SpirvTools tools(SPV_ENV_VULKAN_1_1); + Vector inputBinary; + ASSERT_TRUE(tools.Assemble(spirvText, &inputBinary)); + Vector outputBinary; + ASSERT_TRUE(ShaderCompiler::LowerRectImages(inputBinary, outputBinary)); + EXPECT_TRUE(tools.Validate(outputBinary)); + String outputText; + ASSERT_TRUE(tools.Disassemble(outputBinary, &outputText)); + EXPECT_NE(outputText.find("Binding 0"), String::npos); + EXPECT_NE(outputText.find("Binding 1"), String::npos); + const auto firstImage = outputText.find("OpTypeImage"); + ASSERT_NE(firstImage, String::npos); + EXPECT_EQ(outputText.find("OpTypeImage", firstImage + 1), String::npos); +} + TEST_F(ProgramUtilTest, UnformattedFloatStorageImagesKeepFloatAtomicImageTypesTyped) { using namespace MG_Util::ShaderTranspiler; diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp index d8164601..c9e28dc8 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp @@ -1813,6 +1813,8 @@ namespace MobileGL { using namespace spvtools; Optimizer optimizer(SPV_ENV_VULKAN_1_1); optimizer.RegisterPass(NormalizeRectCoordinatesPass::CreateNormalizeRectCoordinatesPass()); + // Rect and 2D images of the same format become the same type after lowering. + optimizer.RegisterPass(CreateRemoveDuplicatesPass()); return RunOptimizerChecked("LowerRectImages", optimizer, inputBinary, outputBinary, true, enableSpirvValidation); } @@ -2103,11 +2105,14 @@ namespace MobileGL { } outputBinary.insert(outputBinary.begin() + static_cast(capabilityInsertOffset), addedCapabilities.begin(), addedCapabilities.end()); - // Hand-rolled word walk, so no Optimizer wrapper ever sees this rewrite; - // check the modified module explicitly in validating lanes. - ValidateOrLatch("UseUnformattedFloatStorageImagesForVulkan", outputBinary, - enableSpirvValidation); - return true; + // Erasing the format can make distinct image types identical. Use the same + // cleanup as the image-widening/lowering paths to merge their dependent types + // and update every use before validating the rewritten module. + spvtools::Optimizer optimizer(SPV_ENV_VULKAN_1_1); + optimizer.RegisterPass(spvtools::CreateRemoveDuplicatesPass()); + Vector rewrittenBinary = std::move(outputBinary); + return RunOptimizerChecked("UseUnformattedFloatStorageImagesForVulkan", optimizer, + rewrittenBinary, outputBinary, true, enableSpirvValidation); } namespace {