[Impeller] fix array uniforms on GLES backend. (flutter#170710)

Fixes flutter#170647


Correctly account for the size of array elements for runtime effect
shaders on the GLES backend.

Author: Jonah Williams

engine/src/flutter/impeller/entity/contents/runtime_effect_contents.cc

@@ -88,9 +88,12 @@ static std::unique_ptr<ShaderMetadata> MakeShaderMetadata(
   std::unique_ptr<ShaderMetadata> metadata = std::make_unique<ShaderMetadata>();
   metadata->name = uniform.name;
   metadata->members.emplace_back(ShaderStructMemberMetadata{
-      .type = GetShaderType(uniform.type),
-      .size = uniform.GetSize(),
-      .byte_length = uniform.bit_width / 8,
+      .type = GetShaderType(uniform.type),  //
+      .size = uniform.dimensions.rows * uniform.dimensions.cols *
+              (uniform.bit_width / 8u),  //
+      .byte_length =
+          (uniform.bit_width / 8u) * uniform.array_elements.value_or(1),  //
+      .array_elements = uniform.array_elements                            //
   });
 
   return metadata;

engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.cc

@@ -375,7 +375,17 @@ bool BufferBindingsGLES::BindUniformBufferV2(
       continue;
     }
 
+    // The reflector/runtime stage data is confused as to whether 0 means
+    // no elements or whether it is not an array. Specifically:
+    //   * The built-in generated header files use std::nullopt to mean not
+    //   an array. Setting the array_elements count to 1 generates incorrect
+    //   code that tries to create 1 length arrays.
+    //   * The runtime stage flatbuffer serializes the std::nullopt as 0,
+    //     and thus needs to treat array length of 0 as a scalar element.
     size_t element_count = member.array_elements.value_or(1);
+    if (element_count == 0) {
+      element_count = 1;
+    }
     size_t element_stride = member.byte_length / element_count;
     auto* buffer_data =
         reinterpret_cast<const GLfloat*>(buffer_ptr + member.offset);

engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.h

@@ -18,6 +18,7 @@ namespace impeller {
 
 namespace testing {
 FML_TEST_CLASS(BufferBindingsGLESTest, BindUniformData);
+FML_TEST_CLASS(BufferBindingsGLESTest, BindArrayData);
 }  // namespace testing
 
 //------------------------------------------------------------------------------
@@ -51,6 +52,7 @@ class BufferBindingsGLES {
 
  private:
   FML_FRIEND_TEST(testing::BufferBindingsGLESTest, BindUniformData);
+  FML_FRIEND_TEST(testing::BufferBindingsGLESTest, BindArrayData);
   //----------------------------------------------------------------------------
   /// @brief      The arguments to glVertexAttribPointer.
   ///

engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc

@@ -33,7 +33,8 @@ TEST(BufferBindingsGLESTest, BindUniformData) {
                                              .name = "foobar",
                                              .offset = 0,
                                              .size = sizeof(float),
-                                             .byte_length = sizeof(float)}}};
+                                             .byte_length = sizeof(float),
+                                             .array_elements = 0}}};
   std::shared_ptr<ReactorGLES> reactor;
   std::shared_ptr<Allocation> backing_store = std::make_shared<Allocation>();
   ASSERT_TRUE(backing_store->Truncate(Bytes{sizeof(float)}));
@@ -47,5 +48,40 @@ TEST(BufferBindingsGLESTest, BindUniformData) {
                                        Range{0, 1}));
 }
 
+TEST(BufferBindingsGLESTest, BindArrayData) {
+  BufferBindingsGLES bindings;
+  absl::flat_hash_map<std::string, GLint> uniform_bindings;
+  uniform_bindings["SHADERMETADATA.FOOBAR[0]"] = 1;
+  bindings.SetUniformBindings(std::move(uniform_bindings));
+  auto mock_gles_impl = std::make_unique<MockGLESImpl>();
+
+  EXPECT_CALL(*mock_gles_impl, Uniform1fv(_, _, _)).Times(1);
+
+  std::shared_ptr<MockGLES> mock_gl = MockGLES::Init(std::move(mock_gles_impl));
+  std::vector<BufferResource> bound_buffers;
+  std::vector<TextureAndSampler> bound_textures;
+
+  ShaderMetadata shader_metadata = {
+      .name = "shader_metadata",
+      .members = {ShaderStructMemberMetadata{.type = ShaderType::kFloat,
+                                             .name = "foobar",
+                                             .offset = 0,
+                                             .size = sizeof(float),
+                                             .byte_length = sizeof(float) * 4,
+                                             .array_elements = 4}}};
+  std::shared_ptr<ReactorGLES> reactor;
+  std::shared_ptr<Allocation> backing_store = std::make_shared<Allocation>();
+  ASSERT_TRUE(backing_store->Truncate(Bytes{sizeof(float) * 4}));
+  DeviceBufferGLES device_buffer(
+      DeviceBufferDescriptor{.size = sizeof(float) * 4}, reactor,
+      backing_store);
+  BufferView buffer_view(&device_buffer, Range(0, sizeof(float)));
+  bound_buffers.push_back(BufferResource(&shader_metadata, buffer_view));
+
+  EXPECT_TRUE(bindings.BindUniformData(mock_gl->GetProcTable(), bound_textures,
+                                       bound_buffers, Range{0, 0},
+                                       Range{0, 1}));
+}
+
 }  // namespace testing
 }  // namespace impeller