From dfd92b7732198de288d9fb7453e8ef52b8979ec2 Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" <98614782+auto-submit[bot]@users.noreply.github.com> Date: Tue, 27 Jan 2026 17:25:47 +0000 Subject: [PATCH] Reverts "Fixes metal vec3 uniform padding (#181340)" (#181552) Reverts: flutter/flutter#181340 Initiated by: gaaclarke Reason for reverting: golden failures and known logical errors that didn't have test coverage Original PR Author: gaaclarke Reviewed By: {walley892, b-luk} This change reverts the following previous change: fixes https://github.com/flutter/flutter/issues/180873 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md Co-authored-by: auto-submit[bot] --- .../impeller/compiler/compiler_test.cc | 1 - .../flutter/impeller/compiler/reflector.cc | 34 ++------ .../impeller/compiler/runtime_stage_data.cc | 4 +- engine/src/flutter/impeller/compiler/types.h | 7 +- .../flutter/impeller/core/runtime_types.cc | 23 +---- .../src/flutter/impeller/core/runtime_types.h | 16 +--- .../dl_runtime_effect_impeller.cc | 2 +- .../contents/runtime_effect_contents.cc | 83 +++++++++---------- .../entity/contents/runtime_effect_contents.h | 16 ++-- .../impeller/entity/entity_unittests.cc | 9 +- .../impeller/runtime_stage/runtime_stage.cc | 15 +--- .../runtime_stage/runtime_stage_types.fbs | 4 +- .../runtime_stage/runtime_stage_unittests.cc | 31 ++----- .../lib/ui/painting/fragment_program.cc | 5 +- .../testing/dart/fragment_shader_test.dart | 46 +++++----- 15 files changed, 105 insertions(+), 191 deletions(-) diff --git a/engine/src/flutter/impeller/compiler/compiler_test.cc b/engine/src/flutter/impeller/compiler/compiler_test.cc index 683e6bea578..dd4603fd683 100644 --- a/engine/src/flutter/impeller/compiler/compiler_test.cc +++ b/engine/src/flutter/impeller/compiler/compiler_test.cc @@ -106,7 +106,6 @@ bool CompilerTestBase::CanCompileAndReflect( entry_point_name); Reflector::Options reflector_options; - reflector_options.target_platform = GetParam(); reflector_options.header_file_name = ReflectionHeaderName(fixture_name); reflector_options.shader_name = "shader_name"; diff --git a/engine/src/flutter/impeller/compiler/reflector.cc b/engine/src/flutter/impeller/compiler/reflector.cc index c63c21a83ef..d5f220a3710 100644 --- a/engine/src/flutter/impeller/compiler/reflector.cc +++ b/engine/src/flutter/impeller/compiler/reflector.cc @@ -26,7 +26,6 @@ #include "impeller/geometry/matrix.h" #include "impeller/geometry/scalar.h" #include "impeller/runtime_stage/runtime_stage.h" -#include "runtime_stage_types_flatbuffers.h" #include "spirv_common.hpp" namespace impeller { @@ -373,27 +372,6 @@ std::shared_ptr Reflector::GenerateRuntimeStageData() uniform_description.columns = spir_type.columns; uniform_description.bit_width = spir_type.width; uniform_description.array_elements = GetArrayElements(spir_type); - - if (TargetPlatformIsMetal(options_.target_platform) && - uniform_description.type == spirv_cross::SPIRType::BaseType::Float) { - // Metal aligns float3 to 16 bytes. - // Metal aligns float3x3 COLUMNS to 16 bytes. - // For float3: Size 12. Padding 4. Stride 16. - // For float3x3: Size 36. Padding 12 (4 per col). Stride 48. - - if (spir_type.vecsize == 3 && - (spir_type.columns == 1 || spir_type.columns == 3)) { - for (size_t c = 0; c < spir_type.columns; c++) { - for (size_t v = 0; v < 3; v++) { - uniform_description.padding_layout.push_back( - fb::PaddingType::kFloat); - } - uniform_description.padding_layout.push_back( - fb::PaddingType::kPadding); - } - } - } - FML_CHECK(data->backend != RuntimeStageBackend::kVulkan || spir_type.basetype == spirv_cross::SPIRType::BaseType::SampledImage) @@ -418,7 +396,7 @@ std::shared_ptr Reflector::GenerateRuntimeStageData() size_t binding = compiler_->get_decoration(ubo.id, spv::Decoration::DecorationBinding); auto members = ReadStructMembers(ubo.type_id); - std::vector padding_layout; + std::vector struct_layout; size_t float_count = 0; for (size_t i = 0; i < members.size(); i += 1) { @@ -429,7 +407,7 @@ std::shared_ptr Reflector::GenerateRuntimeStageData() size_t padding_count = (member.size + sizeof(float) - 1) / sizeof(float); while (padding_count > 0) { - padding_layout.push_back(fb::PaddingType::kPadding); + struct_layout.push_back(0); padding_count--; } break; @@ -440,18 +418,18 @@ std::shared_ptr Reflector::GenerateRuntimeStageData() // 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++) { - padding_layout.push_back(fb::PaddingType::kFloat); + struct_layout.push_back(1); } for (auto j = 0u; j < member.element_padding / sizeof(float); j++) { - padding_layout.push_back(fb::PaddingType::kPadding); + 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) { - padding_layout.push_back(fb::PaddingType::kFloat); + struct_layout.push_back(1); member_float_count--; } } @@ -468,7 +446,7 @@ std::shared_ptr Reflector::GenerateRuntimeStageData() .location = binding, .binding = binding, .type = spirv_cross::SPIRType::Struct, - .padding_layout = std::move(padding_layout), + .struct_layout = std::move(struct_layout), .struct_float_count = float_count, }); } diff --git a/engine/src/flutter/impeller/compiler/runtime_stage_data.cc b/engine/src/flutter/impeller/compiler/runtime_stage_data.cc index ab184cc3182..b745ec94673 100644 --- a/engine/src/flutter/impeller/compiler/runtime_stage_data.cc +++ b/engine/src/flutter/impeller/compiler/runtime_stage_data.cc @@ -332,8 +332,8 @@ std::unique_ptr RuntimeStageData::CreateStageFlatbuffer( desc->array_elements = uniform.array_elements.value(); } - for (const auto& byte_type : uniform.padding_layout) { - desc->padding_layout.push_back(static_cast(byte_type)); + for (const auto& byte_type : uniform.struct_layout) { + desc->struct_layout.push_back(static_cast(byte_type)); } desc->struct_float_count = uniform.struct_float_count; diff --git a/engine/src/flutter/impeller/compiler/types.h b/engine/src/flutter/impeller/compiler/types.h index f683b3554e8..2776ef432b9 100644 --- a/engine/src/flutter/impeller/compiler/types.h +++ b/engine/src/flutter/impeller/compiler/types.h @@ -12,7 +12,6 @@ #include #include -#include "runtime_stage_types_flatbuffers.h" #include "shaderc/shaderc.hpp" #include "spirv_cross.hpp" #include "spirv_msl.hpp" @@ -56,11 +55,7 @@ struct UniformDescription { size_t columns = 0u; size_t bit_width = 0u; std::optional array_elements = std::nullopt; - /// The layout of padding bytes in the uniform buffer. - /// The format matches the values in the flatbuffer - /// UniformDescription::padding_layout. - /// \see RuntimeEffectContents::EmplaceUniform - std::vector padding_layout = {}; + std::vector struct_layout = {}; size_t struct_float_count = 0u; }; diff --git a/engine/src/flutter/impeller/core/runtime_types.cc b/engine/src/flutter/impeller/core/runtime_types.cc index 4370b89dcbd..08811275967 100644 --- a/engine/src/flutter/impeller/core/runtime_types.cc +++ b/engine/src/flutter/impeller/core/runtime_types.cc @@ -4,35 +4,16 @@ #include "impeller/core/runtime_types.h" -#include "flutter/fml/logging.h" - namespace impeller { -size_t RuntimeUniformDescription::GetDartSize() const { - // Struct uniforms aren't yet supported, they only exist as Vulkan - // collections. - FML_DCHECK(type != kStruct); +size_t RuntimeUniformDescription::GetSize() const { size_t size = dimensions.rows * dimensions.cols * bit_width / 8u; if (array_elements.value_or(0) > 0) { // Covered by check on the line above. // NOLINTNEXTLINE(bugprone-unchecked-optional-access) size *= array_elements.value(); } - return size; -} - -size_t RuntimeUniformDescription::GetGPUSize() const { - size_t size = 0; - if (padding_layout.empty()) { - size = dimensions.rows * dimensions.cols * bit_width / 8u; - } else { - size = sizeof(float) * padding_layout.size(); - } - if (array_elements.value_or(0) > 0) { - // Covered by check on the line above. - // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - size *= array_elements.value(); - } + size += sizeof(float) * struct_layout.size(); return size; } diff --git a/engine/src/flutter/impeller/core/runtime_types.h b/engine/src/flutter/impeller/core/runtime_types.h index 163a434d444..84eb052991a 100644 --- a/engine/src/flutter/impeller/core/runtime_types.h +++ b/engine/src/flutter/impeller/core/runtime_types.h @@ -38,11 +38,6 @@ struct RuntimeUniformDimensions { size_t cols = 0; }; -enum class RuntimePaddingType : uint8_t { - kPadding = 0, - kFloat = 1, -}; - struct RuntimeUniformDescription { std::string name; size_t location = 0u; @@ -52,16 +47,11 @@ struct RuntimeUniformDescription { RuntimeUniformDimensions dimensions = {}; size_t bit_width = 0u; std::optional array_elements; - std::vector padding_layout = {}; + std::vector struct_layout = {}; size_t struct_float_count = 0u; - /// @brief Computes the total number of bytes that this uniform requires for - /// representation in the Dart float buffer. - size_t GetDartSize() const; - - /// @brief Computes the total number of bytes that this uniform requires for - /// representation in the GPU. - size_t GetGPUSize() const; + /// @brief Computes the total number of bytes that this uniform requires. + size_t GetSize() const; }; } // namespace impeller diff --git a/engine/src/flutter/impeller/display_list/dl_runtime_effect_impeller.cc b/engine/src/flutter/impeller/display_list/dl_runtime_effect_impeller.cc index 73e360f65b0..c85085fc67b 100644 --- a/engine/src/flutter/impeller/display_list/dl_runtime_effect_impeller.cc +++ b/engine/src/flutter/impeller/display_list/dl_runtime_effect_impeller.cc @@ -38,7 +38,7 @@ size_t DlRuntimeEffectImpeller::uniform_size() const { size_t total = 0; for (const auto& uniform : runtime_stage_->GetUniforms()) { - total += uniform.GetGPUSize(); + total += uniform.GetSize(); } return total; } diff --git a/engine/src/flutter/impeller/entity/contents/runtime_effect_contents.cc b/engine/src/flutter/impeller/entity/contents/runtime_effect_contents.cc index 2cce0a5c3eb..54a891d1d62 100644 --- a/engine/src/flutter/impeller/entity/contents/runtime_effect_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/runtime_effect_contents.cc @@ -24,48 +24,35 @@ namespace impeller { -// static -BufferView RuntimeEffectContents::EmplaceUniform( - const uint8_t* source_data, - HostBuffer& data_host_buffer, - const RuntimeUniformDescription& uniform) { - size_t minimum_uniform_alignment = - data_host_buffer.GetMinimumUniformAlignment(); - size_t alignment = std::max(uniform.bit_width / 8, minimum_uniform_alignment); +namespace { +constexpr char kPaddingType = 0; +constexpr char kFloatType = 1; +} // namespace - if (uniform.padding_layout.empty()) { - return data_host_buffer.Emplace(source_data, uniform.GetGPUSize(), - alignment); +// static +BufferView RuntimeEffectContents::EmplaceVulkanUniform( + const std::shared_ptr>& input_data, + HostBuffer& data_host_buffer, + const RuntimeUniformDescription& uniform, + size_t minimum_uniform_alignment) { + // TODO(jonahwilliams): rewrite this to emplace directly into + // HostBuffer. + std::vector uniform_buffer; + uniform_buffer.reserve(uniform.struct_layout.size()); + size_t uniform_byte_index = 0u; + for (char byte_type : uniform.struct_layout) { + if (byte_type == kPaddingType) { + uniform_buffer.push_back(0.f); + } else { + FML_DCHECK(byte_type == kFloatType); + uniform_buffer.push_back(reinterpret_cast( + input_data->data())[uniform_byte_index++]); + } } - // If the uniform has a padding layout, we need to repack the data. - // We can do this by using the EmplaceProc to write directly to the - // HostBuffer. return data_host_buffer.Emplace( - uniform.GetGPUSize(), alignment, - [&uniform, source_data](uint8_t* destination) { - size_t count = uniform.array_elements.value_or(1); - if (count == 0) { - // Make sure to run at least once. - count = 1; - } - size_t uniform_byte_index = 0u; - size_t struct_float_index = 0u; - auto* float_destination = reinterpret_cast(destination); - auto* float_source = reinterpret_cast(source_data); - - for (size_t i = 0; i < count; i++) { - for (RuntimePaddingType byte_type : uniform.padding_layout) { - if (byte_type == RuntimePaddingType::kPadding) { - float_destination[struct_float_index++] = 0.f; - } else { - FML_DCHECK(byte_type == RuntimePaddingType::kFloat); - float_destination[struct_float_index++] = - float_source[uniform_byte_index++]; - } - } - } - }); + reinterpret_cast(uniform_buffer.data()), + sizeof(float) * uniform_buffer.size(), minimum_uniform_alignment); } void RuntimeEffectContents::SetRuntimeStage( @@ -297,8 +284,12 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, << "Uniform " << uniform.name << " had unexpected type kFloat for Vulkan backend."; - BufferView buffer_view = EmplaceUniform( - uniform_data_->data() + buffer_offset, data_host_buffer, uniform); + size_t alignment = + std::max(uniform.bit_width / 8, + data_host_buffer.GetMinimumUniformAlignment()); + BufferView buffer_view = + data_host_buffer.Emplace(uniform_data_->data() + buffer_offset, + uniform.GetSize(), alignment); ShaderUniformSlot uniform_slot; uniform_slot.name = uniform.name.c_str(); @@ -307,7 +298,7 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, DescriptorType::kUniformBuffer, uniform_slot, std::move(metadata), std::move(buffer_view)); buffer_index++; - buffer_offset += uniform.GetDartSize(); + buffer_offset += uniform.GetSize(); buffer_location++; break; } @@ -318,10 +309,12 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, uniform_slot.binding = uniform.location; uniform_slot.name = uniform.name.c_str(); - pass.BindResource( - ShaderStage::kFragment, DescriptorType::kUniformBuffer, - uniform_slot, nullptr, - EmplaceUniform(uniform_data_->data(), data_host_buffer, uniform)); + pass.BindResource(ShaderStage::kFragment, + DescriptorType::kUniformBuffer, uniform_slot, + nullptr, + EmplaceVulkanUniform( + uniform_data_, data_host_buffer, uniform, + data_host_buffer.GetMinimumUniformAlignment())); } } } diff --git a/engine/src/flutter/impeller/entity/contents/runtime_effect_contents.h b/engine/src/flutter/impeller/entity/contents/runtime_effect_contents.h index 1985d31000f..0084a3bc252 100644 --- a/engine/src/flutter/impeller/entity/contents/runtime_effect_contents.h +++ b/engine/src/flutter/impeller/entity/contents/runtime_effect_contents.h @@ -37,17 +37,11 @@ class RuntimeEffectContents final : public ColorSourceContents { bool BootstrapShader(const ContentContext& renderer) const; // Visible for testing - /// Copies the uniform data into the host buffer. - /// - /// If the `uniform` has a `padding_layout`, it is used to repack the data. - /// - /// @param source_data The pointer to the start of the uniform data in the - /// source. - /// @param host_buffer The host buffer to emplace the uniform data into. - /// @param uniform The description of the uniform being emplaced. - static BufferView EmplaceUniform(const uint8_t* source_data, - HostBuffer& host_buffer, - const RuntimeUniformDescription& uniform); + static BufferView EmplaceVulkanUniform( + const std::shared_ptr>& input_data, + HostBuffer& host_buffer, + const RuntimeUniformDescription& uniform, + size_t minimum_uniform_alignment); private: bool RegisterShader(const ContentContext& renderer) const; diff --git a/engine/src/flutter/impeller/entity/entity_unittests.cc b/engine/src/flutter/impeller/entity/entity_unittests.cc index aae5107e8d9..86c39c5db8d 100644 --- a/engine/src/flutter/impeller/entity/entity_unittests.cc +++ b/engine/src/flutter/impeller/entity/entity_unittests.cc @@ -1922,9 +1922,12 @@ TEST_P(EntityTest, RuntimeEffectSetsRightSizeWhenUniformIsStruct) { uniform_data->resize(sizeof(FragUniforms)); memcpy(uniform_data->data(), &frag_uniforms, sizeof(FragUniforms)); - auto buffer_view = RuntimeEffectContents::EmplaceUniform( - uniform_data->data(), GetContentContext()->GetTransientsDataBuffer(), - runtime_stage->GetUniforms()[0]); + auto buffer_view = RuntimeEffectContents::EmplaceVulkanUniform( + uniform_data, GetContentContext()->GetTransientsDataBuffer(), + runtime_stage->GetUniforms()[0], + GetContentContext() + ->GetTransientsDataBuffer() + .GetMinimumUniformAlignment()); // 16 bytes: // 8 bytes for iResolution diff --git a/engine/src/flutter/impeller/runtime_stage/runtime_stage.cc b/engine/src/flutter/impeller/runtime_stage/runtime_stage.cc index 732477d4cd6..6d0bfb31d49 100644 --- a/engine/src/flutter/impeller/runtime_stage/runtime_stage.cc +++ b/engine/src/flutter/impeller/runtime_stage/runtime_stage.cc @@ -81,18 +81,9 @@ absl::StatusOr RuntimeStage::Create( static_cast(i->rows()), static_cast(i->columns())}; desc.bit_width = i->bit_width(); desc.array_elements = i->array_elements(); - if (i->padding_layout()) { - for (const auto& byte_type : *i->padding_layout()) { - impeller::RuntimePaddingType type; - switch (byte_type) { - case fb::PaddingType::kPadding: - type = impeller::RuntimePaddingType::kPadding; - break; - case fb::PaddingType::kFloat: - type = impeller::RuntimePaddingType::kFloat; - break; - } - desc.padding_layout.push_back(type); + if (i->struct_layout()) { + for (const auto& byte_type : *i->struct_layout()) { + desc.struct_layout.push_back(static_cast(byte_type)); } } desc.struct_float_count = i->struct_float_count(); diff --git a/engine/src/flutter/impeller/runtime_stage/runtime_stage_types.fbs b/engine/src/flutter/impeller/runtime_stage/runtime_stage_types.fbs index c39f52a22cc..941184b7d6c 100644 --- a/engine/src/flutter/impeller/runtime_stage/runtime_stage_types.fbs +++ b/engine/src/flutter/impeller/runtime_stage/runtime_stage_types.fbs @@ -29,7 +29,7 @@ enum UniformDataType:uint32 { // A struct is made up solely of 4 byte floats and 4-byte paddings between // them. // This enum describes whether a particular byte is a float or padding. -enum PaddingType:uint8 { +enum StructByteType:uint8 { kPadding = 0, kFloat = 1, } @@ -43,7 +43,7 @@ table UniformDescription { rows: uint64; columns: uint64; array_elements: uint64; - padding_layout: [PaddingType]; + struct_layout: [StructByteType]; struct_float_count: uint64; } diff --git a/engine/src/flutter/impeller/runtime_stage/runtime_stage_unittests.cc b/engine/src/flutter/impeller/runtime_stage/runtime_stage_unittests.cc index 40a67af01a7..c66bb133bc5 100644 --- a/engine/src/flutter/impeller/runtime_stage/runtime_stage_unittests.cc +++ b/engine/src/flutter/impeller/runtime_stage/runtime_stage_unittests.cc @@ -94,7 +94,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 0u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { auto uni = stage->GetUniform("u_alpha"); @@ -103,7 +102,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 1u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { auto uni = stage->GetUniform("u_sparkle_color"); @@ -112,7 +110,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 2u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { auto uni = stage->GetUniform("u_sparkle_alpha"); @@ -121,7 +118,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 3u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { auto uni = stage->GetUniform("u_blur"); @@ -130,7 +126,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 4u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { auto uni = stage->GetUniform("u_radius_scale"); @@ -139,7 +134,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 6u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { auto uni = stage->GetUniform("u_max_radius"); @@ -148,7 +142,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 7u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { auto uni = stage->GetUniform("u_resolution_scale"); @@ -157,7 +150,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 8u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { auto uni = stage->GetUniform("u_noise_scale"); @@ -166,7 +158,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 9u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { auto uni = stage->GetUniform("u_noise_phase"); @@ -175,7 +166,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 10u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { @@ -185,7 +175,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 11u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { auto uni = stage->GetUniform("u_circle2"); @@ -194,7 +183,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 12u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { auto uni = stage->GetUniform("u_circle3"); @@ -203,7 +191,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 13u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { auto uni = stage->GetUniform("u_rotation1"); @@ -212,7 +199,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 14u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { auto uni = stage->GetUniform("u_rotation2"); @@ -221,7 +207,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 15u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } { auto uni = stage->GetUniform("u_rotation3"); @@ -230,7 +215,6 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { EXPECT_EQ(uni->dimensions.cols, 1u); EXPECT_EQ(uni->location, 16u); EXPECT_EQ(uni->type, RuntimeUniformType::kFloat); - EXPECT_TRUE(uni->padding_layout.empty()); } break; } @@ -250,15 +234,14 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { // introduced. // This means 36 * 4 = 144 bytes total. - EXPECT_EQ(uni->GetGPUSize(), 144u); - std::vector layout(uni->GetGPUSize() / sizeof(float), - RuntimePaddingType::kFloat); - layout[5] = RuntimePaddingType::kPadding; - layout[6] = RuntimePaddingType::kPadding; - layout[7] = RuntimePaddingType::kPadding; - layout[23] = RuntimePaddingType::kPadding; + EXPECT_EQ(uni->GetSize(), 144u); + std::vector layout(uni->GetSize() / sizeof(float), 1); + layout[5] = 0; + layout[6] = 0; + layout[7] = 0; + layout[23] = 0; - EXPECT_THAT(uni->padding_layout, ::testing::ElementsAreArray(layout)); + EXPECT_THAT(uni->struct_layout, ::testing::ElementsAreArray(layout)); break; } } diff --git a/engine/src/flutter/lib/ui/painting/fragment_program.cc b/engine/src/flutter/lib/ui/painting/fragment_program.cc index e2ea133a857..ae705901f8f 100644 --- a/engine/src/flutter/lib/ui/painting/fragment_program.cc +++ b/engine/src/flutter/lib/ui/painting/fragment_program.cc @@ -82,8 +82,7 @@ Dart_Handle ConvertUniformDescriptionToMap( FML_DCHECK(!Dart_IsError(result)); } { // 2 - Dart_Handle size = - Dart_NewIntegerFromUint64(uniform_description.GetGPUSize()); + Dart_Handle size = Dart_NewIntegerFromUint64(uniform_description.GetSize()); FML_DCHECK(!Dart_IsError(size)); [[maybe_unused]] Dart_Handle result = Dart_ListSetAt(keys, 2, Dart_NewStringFromCString("size")); @@ -162,7 +161,7 @@ std::string FragmentProgram::initFromAsset(const std::string& asset_name) { impeller::RuntimeUniformType::kSampledImage) { sampled_image_count++; } else { - other_uniforms_bytes += uniform_description.GetGPUSize(); + other_uniforms_bytes += uniform_description.GetSize(); } } diff --git a/engine/src/flutter/testing/dart/fragment_shader_test.dart b/engine/src/flutter/testing/dart/fragment_shader_test.dart index d07cdda5db9..3a6fe2570b6 100644 --- a/engine/src/flutter/testing/dart/fragment_shader_test.dart +++ b/engine/src/flutter/testing/dart/fragment_shader_test.dart @@ -695,31 +695,39 @@ void main() async { shader.dispose(); }); - test('FragmentShader shader with array uniforms renders correctly', () async { - final FragmentProgram program = await FragmentProgram.fromAsset('uniform_arrays.frag.iplr'); + test( + 'FragmentShader shader with array uniforms renders correctly', + () async { + final FragmentProgram program = await FragmentProgram.fromAsset('uniform_arrays.frag.iplr'); - final FragmentShader shader = program.fragmentShader(); - for (var i = 0; i < 20; i++) { - shader.setFloat(i, i.toDouble()); - } + final FragmentShader shader = program.fragmentShader(); + for (var i = 0; i < 20; i++) { + shader.setFloat(i, i.toDouble()); + } - await _expectShaderRendersGreen(shader); - shader.dispose(); - }); + await _expectShaderRendersGreen(shader); + shader.dispose(); + }, + skip: Platform.executableArguments.contains('--impeller-backend=metal'), + ); - test('FragmentShader shader with mat2 uniform renders correctly', () async { - final FragmentProgram program = await FragmentProgram.fromAsset('uniform_mat2.frag.iplr'); + test( + 'FragmentShader shader with mat2 uniform renders correctly', + () async { + final FragmentProgram program = await FragmentProgram.fromAsset('uniform_mat2.frag.iplr'); - final FragmentShader shader = program.fragmentShader(); + final FragmentShader shader = program.fragmentShader(); - shader.setFloat(0, 4.0); // m00 - shader.setFloat(1, 8.0); // m01 - shader.setFloat(2, 16.0); // m10 - shader.setFloat(3, 32.0); // m11 + shader.setFloat(0, 4.0); // m00 + shader.setFloat(1, 8.0); // m01 + shader.setFloat(2, 16.0); // m10 + shader.setFloat(3, 32.0); // m11 - await _expectShaderRendersGreen(shader); - shader.dispose(); - }); + await _expectShaderRendersGreen(shader); + shader.dispose(); + }, + skip: Platform.executableArguments.contains('--impeller-backend=metal'), + ); _runImpellerTest( 'ImageFilter.shader errors if shader does not have correct uniform layout',