From 023a852855d0be2398b637f31bf711dd6bb60bc3 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 28 May 2024 15:43:28 -0700 Subject: [PATCH 1/4] [Impeller] make sure buffers are 4 aligned for foreground color blending --- impeller/core/host_buffer.h | 9 +++++---- .../entity/contents/filters/blend_filter_contents.cc | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/impeller/core/host_buffer.h b/impeller/core/host_buffer.h index bb7af2de42acf..6858433781839 100644 --- a/impeller/core/host_buffer.h +++ b/impeller/core/host_buffer.h @@ -91,10 +91,11 @@ class HostBuffer { /// template >> - [[nodiscard]] BufferView Emplace(const BufferType& buffer) { - return Emplace(reinterpret_cast(&buffer), // buffer - sizeof(BufferType), // size - alignof(BufferType) // alignment + [[nodiscard]] BufferView Emplace(const BufferType& buffer, + size_t alignment = 0) { + return Emplace(reinterpret_cast(&buffer), // buffer + sizeof(BufferType), // size + std::max(alignment, alignof(BufferType)) // alignment ); } diff --git a/impeller/entity/contents/filters/blend_filter_contents.cc b/impeller/entity/contents/filters/blend_filter_contents.cc index cced93ac3b015..f7f3f2d94cd6a 100644 --- a/impeller/entity/contents/filters/blend_filter_contents.cc +++ b/impeller/entity/contents/filters/blend_filter_contents.cc @@ -867,7 +867,7 @@ std::optional BlendFilterContents::CreateFramebufferAdvancedBlend( } auto blit_pass = cmd_buffer->CreateBlitPass(); auto buffer_view = renderer.GetTransientsBuffer().Emplace( - foreground_color->Premultiply().ToR8G8B8A8()); + foreground_color->Premultiply().ToR8G8B8A8(), /*alignment=*/4); blit_pass->AddCopy(std::move(buffer_view), foreground_texture); if (!blit_pass->EncodeCommands( From f90c3bb7bd40bd2f0ff8758fbef9be3d5f2d14a4 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 28 May 2024 15:46:00 -0700 Subject: [PATCH 2/4] updated docstring --- impeller/core/host_buffer.h | 1 + 1 file changed, 1 insertion(+) diff --git a/impeller/core/host_buffer.h b/impeller/core/host_buffer.h index 6858433781839..1480d79b2a533 100644 --- a/impeller/core/host_buffer.h +++ b/impeller/core/host_buffer.h @@ -84,6 +84,7 @@ class HostBuffer { /// host buffer. /// /// @param[in] buffer The buffer data. + /// @param[in] alignment Minimum alignment of the data being emplaced. /// /// @tparam BufferType The type of the buffer data. /// From 2b0c98a9fa15aec13cfd08e42be77b0b3e2ca773 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 28 May 2024 17:08:32 -0700 Subject: [PATCH 3/4] added test --- impeller/entity/BUILD.gn | 1 + .../blend_filter_contents_unittests.cc | 68 +++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 impeller/entity/contents/filters/blend_filter_contents_unittests.cc diff --git a/impeller/entity/BUILD.gn b/impeller/entity/BUILD.gn index fbf419da1abf5..5b1bbdb1770ae 100644 --- a/impeller/entity/BUILD.gn +++ b/impeller/entity/BUILD.gn @@ -239,6 +239,7 @@ impeller_component("entity_unittests") { sources = [ "contents/clip_contents_unittests.cc", + "contents/filters/blend_filter_contents_unittests.cc", "contents/filters/gaussian_blur_filter_contents_unittests.cc", "contents/filters/inputs/filter_input_unittests.cc", "contents/filters/matrix_filter_contents_unittests.cc", diff --git a/impeller/entity/contents/filters/blend_filter_contents_unittests.cc b/impeller/entity/contents/filters/blend_filter_contents_unittests.cc new file mode 100644 index 0000000000000..977eae47a26e4 --- /dev/null +++ b/impeller/entity/contents/filters/blend_filter_contents_unittests.cc @@ -0,0 +1,68 @@ +// 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. + +#include "flutter/testing/testing.h" +#include "gmock/gmock.h" +#include "impeller/entity/contents/content_context.h" +#include "impeller/entity/contents/filters/blend_filter_contents.h" +#include "impeller/entity/entity_playground.h" + +namespace impeller { +namespace testing { + +class BlendFilterContentsTest : public EntityPlayground { + public: + /// Create a texture that has been cleared to transparent black. + std::shared_ptr MakeTexture(ISize size) { + std::shared_ptr command_buffer = + GetContentContext()->GetContext()->CreateCommandBuffer(); + if (!command_buffer) { + return nullptr; + } + + auto render_target = GetContentContext()->MakeSubpass( + "Clear Subpass", size, command_buffer, + [](const ContentContext&, RenderPass&) { return true; }); + + if (!GetContentContext() + ->GetContext() + ->GetCommandQueue() + ->Submit(/*buffers=*/{command_buffer}) + .ok()) { + return nullptr; + } + + if (render_target.ok()) { + return render_target.value().GetRenderTargetTexture(); + } + return nullptr; + } +}; +INSTANTIATE_PLAYGROUND_SUITE(BlendFilterContentsTest); + +// https://github.com/flutter/flutter/issues/149216 +TEST_P(BlendFilterContentsTest, AdvancedBlendColorAlignsColorTo4) { + std::shared_ptr texture = MakeTexture(ISize(100, 100)); + BlendFilterContents filter_contents; + filter_contents.SetInputs({FilterInput::Make(texture)}); + filter_contents.SetForegroundColor(Color(1.0, 0.0, 0.0, 1.0)); + filter_contents.SetBlendMode(BlendMode::kColorDodge); + + std::shared_ptr renderer = GetContentContext(); + // Add random byte to get the HostBuffer in a bad alignment. + uint8_t byte = 0xff; + BufferView buffer_view = + renderer->GetTransientsBuffer().Emplace(&byte, /*length=*/1, /*align=*/1); + EXPECT_EQ(buffer_view.range.offset, 4u); + EXPECT_EQ(buffer_view.range.length, 1u); + Entity entity; + + std::optional result = filter_contents.GetEntity( + *renderer, entity, /*coverage_hint=*/std::nullopt); + + EXPECT_TRUE(result.has_value()); +} + +} // namespace testing +} // namespace impeller From 95c5ea38dbd66ccddb4c856aa085f6583bfe1f8f Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 28 May 2024 18:19:25 -0700 Subject: [PATCH 4/4] license --- ci/licenses_golden/excluded_files | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index 610d517ed482a..cf4b729a7f9a2 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -145,6 +145,7 @@ ../../../flutter/impeller/display_list/skia_conversions_unittests.cc ../../../flutter/impeller/docs ../../../flutter/impeller/entity/contents/clip_contents_unittests.cc +../../../flutter/impeller/entity/contents/filters/blend_filter_contents_unittests.cc ../../../flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc ../../../flutter/impeller/entity/contents/filters/inputs/filter_input_unittests.cc ../../../flutter/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc