From 12518fac13322e8dd471fc5334f9d37afd4983e8 Mon Sep 17 00:00:00 2001
From: Jonah Williams <jonahwilliams@google.com>
Date: Thu, 16 Jan 2025 16:48:01 -0800
Subject: [PATCH] [Impellerc] correctly pad arrays of vec3s in reflector.
 (#161697)

When processing the metadata for a struct with an array, incorporate the
padding into the layout. Previously we would register a vec3[n] array as
being 4*n bytes (including the padding), but when uploading this would
result in us writing data into the padding and then leave the rest
uninitialized. now we correctly insert a padding element between 3 byte
elements.

Fixes https://github.com/flutter/flutter/issues/161645
---
 .../flutter/impeller/compiler/reflector.cc    | 24 +++++++++++++++----
 .../fixtures/shaders/general_shaders/BUILD.gn |  1 +
 .../shaders/general_shaders/vec3_uniform.frag | 15 ++++++++++++
 .../testing/dart/fragment_shader_test.dart    | 18 ++++++++++++++
 4 files changed, 53 insertions(+), 5 deletions(-)
 create mode 100644 engine/src/flutter/lib/ui/fixtures/shaders/general_shaders/vec3_uniform.frag

diff --git a/engine/src/flutter/impeller/compiler/reflector.cc b/engine/src/flutter/impeller/compiler/reflector.cc
index c71619a660..ba8ecb3569 100644
--- a/engine/src/flutter/impeller/compiler/reflector.cc
+++ b/engine/src/flutter/impeller/compiler/reflector.cc
@@ -398,11 +398,25 @@ std::shared_ptr<RuntimeStageData::Shader> Reflector::GenerateRuntimeStageData()
           break;
         }
         case StructMember::UnderlyingType::kFloat: {
-          size_t member_float_count = member.byte_length / sizeof(float);
-          float_count += member_float_count;
-          while (member_float_count > 0) {
-            struct_layout.push_back(1);
-            member_float_count--;
+          if (member.array_elements > 1) {
+            // For each array element member, insert 1 layout property per byte
+            // and 0 layout property per byte of padding
+            for (auto i = 0; i < member.array_elements; i++) {
+              for (auto j = 0u; j < member.size / sizeof(float); j++) {
+                struct_layout.push_back(1);
+              }
+              for (auto j = 0u; j < member.element_padding / sizeof(float);
+                   j++) {
+                struct_layout.push_back(0);
+              }
+            }
+          } else {
+            size_t member_float_count = member.byte_length / sizeof(float);
+            float_count += member_float_count;
+            while (member_float_count > 0) {
+              struct_layout.push_back(1);
+              member_float_count--;
+            }
           }
           break;
         }
diff --git a/engine/src/flutter/lib/ui/fixtures/shaders/general_shaders/BUILD.gn b/engine/src/flutter/lib/ui/fixtures/shaders/general_shaders/BUILD.gn
index d14c68b0ab..f6f81f46a6 100644
--- a/engine/src/flutter/lib/ui/fixtures/shaders/general_shaders/BUILD.gn
+++ b/engine/src/flutter/lib/ui/fixtures/shaders/general_shaders/BUILD.gn
@@ -20,6 +20,7 @@ if (enable_unittests) {
     "filter_shader.frag",
     "missing_size.frag",
     "missing_texture.frag",
+    "vec3_uniform.frag",
   ]
 
   group("general_shaders") {
diff --git a/engine/src/flutter/lib/ui/fixtures/shaders/general_shaders/vec3_uniform.frag b/engine/src/flutter/lib/ui/fixtures/shaders/general_shaders/vec3_uniform.frag
new file mode 100644
index 0000000000..2b1cadd8fb
--- /dev/null
+++ b/engine/src/flutter/lib/ui/fixtures/shaders/general_shaders/vec3_uniform.frag
@@ -0,0 +1,15 @@
+#version 320 es
+
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+precision highp float;
+
+uniform vec3[4] color_array;
+
+out vec4 fragColor;
+
+void main() {
+  fragColor = vec4(color_array[3].xyz, 1);
+}
diff --git a/engine/src/flutter/testing/dart/fragment_shader_test.dart b/engine/src/flutter/testing/dart/fragment_shader_test.dart
index c2f0dec0f5..4a76a09c1d 100644
--- a/engine/src/flutter/testing/dart/fragment_shader_test.dart
+++ b/engine/src/flutter/testing/dart/fragment_shader_test.dart
@@ -376,6 +376,24 @@ void main() async {
     }
   });
 
+  test('Shader Compiler appropriately pads vec3 uniform arrays', () async {
+    if (!impellerEnabled) {
+      print('Skipped for Skia');
+      return;
+    }
+
+    final FragmentProgram program = await FragmentProgram.fromAsset('vec3_uniform.frag.iplr');
+    final FragmentShader shader = program.fragmentShader();
+
+    // Set the last vec3 in the uniform array to green. The shader will read this
+    // value, and if the uniforms were padded correctly will render green.
+    shader.setFloat(12, 0);
+    shader.setFloat(13, 1.0);
+    shader.setFloat(14, 0);
+
+    await _expectShaderRendersGreen(shader);
+  });
+
   test('ImageFilter.shader can be applied to canvas operations', () async {
     if (!impellerEnabled) {
       print('Skipped for Skia');