From 518e9c7796f0eb2cabef159307d452cc519771f8 Mon Sep 17 00:00:00 2001
From: Swung0x48 <swung0x48@outlook.com>
Date: Fri, 21 Aug 2026 06:57:59 -0400
Subject: [PATCH] [Fix, Test] (DirectGLES): read an image array subscript
 unsigned literal as the element index it is

---
 MobileGL/MG_Backend/DirectGLES/Utils.cpp      | 16 ++++++-
 .../Backend/DirectGLES/EsslShaderPassTest.cpp | 44 +++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.cpp b/MobileGL/MG_Backend/DirectGLES/Utils.cpp
index 7237a556..cfdadae7 100644
--- a/MobileGL/MG_Backend/DirectGLES/Utils.cpp
+++ b/MobileGL/MG_Backend/DirectGLES/Utils.cpp
@@ -1032,10 +1032,24 @@ namespace MobileGL::MG_Backend::DirectGLES {
         namespace {
             // The digits of an array extent or of an element subscript, or -1 for "not a plain
             // decimal literal".
+            //
+            // One trailing `u`/`U` is PART of the literal rather than grounds for rejection.
+            // SPIRV-Cross prints an index in the type SPIR-V gave it, and
+            // LegalizeResourceArrayIndexPass mints its per-element constants in the type of the
+            // index it replaced (ConstantLikeIndex reads that index's own type_id), so an image
+            // array reached through anything unsigned - `for (uint i = 0u; i < 4u; ++i)`, or any
+            // expression on gl_LocalInvocationIndex, which is uint by definition - arrives here
+            // spelled `g_image[0u]`. Reading that as "not a literal" declined the array and left
+            // it on one layout(binding = N), which hands its elements the consecutive units
+            // N, N+1, ... - exactly the silently-wrong-units defect the split exists to remove.
             Int ParseNonNegativeIntLiteral(const String& text) {
                 if (text.empty()) return -1;
+                SizeT digitCount = text.size();
+                if (text[digitCount - 1] == 'u' || text[digitCount - 1] == 'U') --digitCount;
+                if (digitCount == 0) return -1;
                 Int value = 0;
-                for (const char c : text) {
+                for (SizeT i = 0; i < digitCount; ++i) {
+                    const char c = text[i];
                     if (c < '0' || c > '9') return -1;
                     value = value * 10 + (c - '0');
                     if (value > 4096) return -1; // no image array is anywhere near this
diff --git a/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp b/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp
index 09d03378..831532e1 100644
--- a/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp
+++ b/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp
@@ -685,6 +685,50 @@ void main()
     ASSERT_EQ(outOfRangeDeclined.size(), 1u);
 }
 
+// A uint subscript IS a literal element index. SPIRV-Cross prints an index in the type SPIR-V
+// gave it and LegalizeResourceArrayIndexPass mints its per-element constants in the type of the
+// index it replaced, so an array walked by anything unsigned - a `uint` loop counter, or
+// anything derived from gl_LocalInvocationIndex, which is uint by definition - reaches this pass
+// spelled `g_image[0u]`. Refusing the `u` declined the array and left every element on the
+// consecutive units one binding hands out, silently.
+TEST(RemapImageArrayElementUnitsTest, AUintSubscriptIsStillALiteralElementIndex) {
+    const String source = R"(#version 320 es
+layout(local_size_x = 1) in;
+layout(rgba32f, binding = 0) uniform writeonly highp image2D g_image[3];
+void main()
+{
+    imageStore(g_image[0u], ivec2(0), vec4(1.0));
+    imageStore(g_image[2U], ivec2(0), vec4(2.0));
+}
+)";
+    Vector<String> declined;
+    const String out = RemapImageArrayElementUnits(source, {Plan("g_image", {0, 4, 8})}, &declined);
+    EXPECT_TRUE(declined.empty()) << (declined.empty() ? String() : declined[0]);
+
+    EXPECT_TRUE(Contains(out, "binding = 0) uniform writeonly highp image2D " + Elem("g_image", 0) + ";")) << out;
+    EXPECT_TRUE(Contains(out, "binding = 4) uniform writeonly highp image2D " + Elem("g_image", 1) + ";")) << out;
+    EXPECT_TRUE(Contains(out, "binding = 8) uniform writeonly highp image2D " + Elem("g_image", 2) + ";")) << out;
+    EXPECT_TRUE(Contains(out, "imageStore(" + Elem("g_image", 0) + ", ivec2(0), vec4(1.0))")) << out;
+    EXPECT_TRUE(Contains(out, "imageStore(" + Elem("g_image", 2) + ", ivec2(0), vec4(2.0))")) << out;
+    EXPECT_FALSE(Contains(out, "g_image[")) << out;
+}
+
+// ...and the suffix is not a licence to accept anything else that ends in one: `iu` is not a
+// literal, and neither is a bare `u`.
+TEST(RemapImageArrayElementUnitsTest, ASuffixAloneDoesNotMakeAnExpressionALiteral) {
+    const String source = R"(#version 320 es
+layout(rgba32f, binding = 0) uniform writeonly highp image2D g_image[2];
+void main()
+{
+    highp int iu = 1;
+    imageStore(g_image[iu], ivec2(0), vec4(1.0));
+}
+)";
+    Vector<String> declined;
+    EXPECT_EQ(RemapImageArrayElementUnits(source, {Plan("g_image", {0, 4})}, &declined), source);
+    ASSERT_EQ(declined.size(), 1u);
+}
+
 // A use the pass cannot see a subscript on has no element index to rewrite, so splitting the
 // array out from under it would leave it naming a declaration that no longer exists. Decline,
 // loudly, and change nothing.