From 0989c206e4d291ea3e235b46ecfcbc26d30e23c3 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Wed, 11 May 2022 16:29:28 -0700 Subject: [PATCH] [Impeller] Fix remaining problems with advanced blends (flutter/engine#33254) --- .../ci/licenses_golden/licenses_flutter | 1 + engine/src/flutter/impeller/aiks/canvas.cc | 55 ++++++++++--------- engine/src/flutter/impeller/aiks/canvas.h | 3 +- engine/src/flutter/impeller/entity/BUILD.gn | 1 + .../contents/filters/inputs/filter_input.cc | 11 +++- .../contents/filters/inputs/filter_input.h | 3 + .../filters/inputs/filter_input_unittests.cc | 29 ++++++++++ .../filters/inputs/texture_filter_input.cc | 9 ++- .../filters/inputs/texture_filter_input.h | 9 ++- .../flutter/impeller/entity/entity_pass.cc | 24 +++++--- .../entity/shaders/texture_blend_screen.frag | 6 +- 11 files changed, 109 insertions(+), 42 deletions(-) create mode 100644 engine/src/flutter/impeller/entity/contents/filters/inputs/filter_input_unittests.cc diff --git a/engine/src/flutter/ci/licenses_golden/licenses_flutter b/engine/src/flutter/ci/licenses_golden/licenses_flutter index 9a77424a702..3d9b617e08f 100644 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter @@ -501,6 +501,7 @@ FILE: ../../../flutter/impeller/entity/contents/filters/inputs/filter_contents_f FILE: ../../../flutter/impeller/entity/contents/filters/inputs/filter_contents_filter_input.h FILE: ../../../flutter/impeller/entity/contents/filters/inputs/filter_input.cc FILE: ../../../flutter/impeller/entity/contents/filters/inputs/filter_input.h +FILE: ../../../flutter/impeller/entity/contents/filters/inputs/filter_input_unittests.cc FILE: ../../../flutter/impeller/entity/contents/filters/inputs/texture_filter_input.cc FILE: ../../../flutter/impeller/entity/contents/filters/inputs/texture_filter_input.h FILE: ../../../flutter/impeller/entity/contents/linear_gradient_contents.cc diff --git a/engine/src/flutter/impeller/aiks/canvas.cc b/engine/src/flutter/impeller/aiks/canvas.cc index 3022887f70a..68867021f50 100644 --- a/engine/src/flutter/impeller/aiks/canvas.cc +++ b/engine/src/flutter/impeller/aiks/canvas.cc @@ -39,6 +39,21 @@ void Canvas::Save() { Save(false); } +void Canvas::Save(bool create_subpass, Entity::BlendMode blend_mode) { + auto entry = CanvasStackEntry{}; + entry.xformation = xformation_stack_.back().xformation; + entry.stencil_depth = xformation_stack_.back().stencil_depth; + if (create_subpass) { + entry.is_subpass = true; + auto subpass = std::make_unique(); + subpass->SetBlendMode(blend_mode); + current_pass_ = GetCurrentPass().AddSubpass(std::move(subpass)); + current_pass_->SetTransformation(xformation_stack_.back().xformation); + current_pass_->SetStencilDepth(xformation_stack_.back().stencil_depth); + } + xformation_stack_.emplace_back(std::move(entry)); +} + bool Canvas::Restore() { FML_DCHECK(xformation_stack_.size() > 0); if (xformation_stack_.size() == 1) { @@ -137,23 +152,6 @@ void Canvas::DrawCircle(Point center, Scalar radius, Paint paint) { std::move(paint)); } -void Canvas::SaveLayer(Paint paint, std::optional bounds) { - Save(true); - GetCurrentPass().SetBlendMode(paint.blend_mode); - - GetCurrentPass().SetDelegate( - std::make_unique(paint, bounds)); - - if (bounds.has_value()) { - // Render target switches due to a save layer can be elided. In such cases - // where passes are collapsed into their parent, the clipping effect to - // the size of the render target that would have been allocated will be - // absent. Explicitly add back a clip to reproduce that behavior. Since - // clips never require a render target switch, this is a cheap operation. - ClipPath(PathBuilder{}.AddRect(bounds.value()).TakePath()); - } -} - void Canvas::ClipPath(Path path, Entity::ClipOperation clip_op) { auto contents = std::make_shared(); contents->SetPath(std::move(path)); @@ -264,17 +262,20 @@ size_t Canvas::GetStencilDepth() const { return xformation_stack_.back().stencil_depth; } -void Canvas::Save(bool create_subpass) { - auto entry = CanvasStackEntry{}; - entry.xformation = xformation_stack_.back().xformation; - entry.stencil_depth = xformation_stack_.back().stencil_depth; - if (create_subpass) { - entry.is_subpass = true; - current_pass_ = GetCurrentPass().AddSubpass(std::make_unique()); - current_pass_->SetTransformation(xformation_stack_.back().xformation); - current_pass_->SetStencilDepth(xformation_stack_.back().stencil_depth); +void Canvas::SaveLayer(Paint paint, std::optional bounds) { + Save(true, paint.blend_mode); + + GetCurrentPass().SetDelegate( + std::make_unique(paint, bounds)); + + if (bounds.has_value()) { + // Render target switches due to a save layer can be elided. In such cases + // where passes are collapsed into their parent, the clipping effect to + // the size of the render target that would have been allocated will be + // absent. Explicitly add back a clip to reproduce that behavior. Since + // clips never require a render target switch, this is a cheap operation. + ClipPath(PathBuilder{}.AddRect(bounds.value()).TakePath()); } - xformation_stack_.emplace_back(std::move(entry)); } void Canvas::DrawTextFrame(TextFrame text_frame, Point position, Paint paint) { diff --git a/engine/src/flutter/impeller/aiks/canvas.h b/engine/src/flutter/impeller/aiks/canvas.h index 71486def8e9..b53bcdcda79 100644 --- a/engine/src/flutter/impeller/aiks/canvas.h +++ b/engine/src/flutter/impeller/aiks/canvas.h @@ -104,7 +104,8 @@ class Canvas { size_t GetStencilDepth() const; - void Save(bool create_subpass); + void Save(bool create_subpass, + Entity::BlendMode = Entity::BlendMode::kSourceOver); void RestoreClip(); diff --git a/engine/src/flutter/impeller/entity/BUILD.gn b/engine/src/flutter/impeller/entity/BUILD.gn index ec4dd551047..c821b4ba83f 100644 --- a/engine/src/flutter/impeller/entity/BUILD.gn +++ b/engine/src/flutter/impeller/entity/BUILD.gn @@ -88,6 +88,7 @@ impeller_component("entity_unittests") { testonly = true sources = [ + "contents/filters/inputs/filter_input_unittests.cc", "entity_playground.cc", "entity_playground.h", "entity_unittests.cc", diff --git a/engine/src/flutter/impeller/entity/contents/filters/inputs/filter_input.cc b/engine/src/flutter/impeller/entity/contents/filters/inputs/filter_input.cc index f69986c8b82..bf0e586711c 100644 --- a/engine/src/flutter/impeller/entity/contents/filters/inputs/filter_input.cc +++ b/engine/src/flutter/impeller/entity/contents/filters/inputs/filter_input.cc @@ -4,6 +4,8 @@ #include "impeller/entity/contents/filters/inputs/filter_input.h" +#include + #include "flutter/fml/logging.h" #include "impeller/entity/contents/filters/filter_contents.h" #include "impeller/entity/contents/filters/inputs/contents_filter_input.h" @@ -26,13 +28,18 @@ FilterInput::Ref FilterInput::Make(Variant input) { } if (auto texture = std::get_if>(&input)) { - return std::static_pointer_cast( - std::shared_ptr(new TextureFilterInput(*texture))); + return Make(*texture, Matrix()); } FML_UNREACHABLE(); } +FilterInput::Ref FilterInput::Make(std::shared_ptr texture, + Matrix local_transform) { + return std::shared_ptr( + new TextureFilterInput(texture, local_transform)); +} + FilterInput::Vector FilterInput::Make(std::initializer_list inputs) { FilterInput::Vector result; result.reserve(inputs.size()); diff --git a/engine/src/flutter/impeller/entity/contents/filters/inputs/filter_input.h b/engine/src/flutter/impeller/entity/contents/filters/inputs/filter_input.h index 6203a0d0245..f34e6e6fa0f 100644 --- a/engine/src/flutter/impeller/entity/contents/filters/inputs/filter_input.h +++ b/engine/src/flutter/impeller/entity/contents/filters/inputs/filter_input.h @@ -38,6 +38,9 @@ class FilterInput { static FilterInput::Ref Make(Variant input); + static FilterInput::Ref Make(std::shared_ptr input, + Matrix local_transform); + static FilterInput::Vector Make(std::initializer_list inputs); virtual Variant GetInput() const = 0; diff --git a/engine/src/flutter/impeller/entity/contents/filters/inputs/filter_input_unittests.cc b/engine/src/flutter/impeller/entity/contents/filters/inputs/filter_input_unittests.cc new file mode 100644 index 00000000000..be245e5a526 --- /dev/null +++ b/engine/src/flutter/impeller/entity/contents/filters/inputs/filter_input_unittests.cc @@ -0,0 +1,29 @@ +// 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 +#include "flutter/testing/testing.h" +#include "gtest/gtest.h" +#include "impeller/entity/contents/filters/inputs/filter_input.h" +#include "impeller/entity/entity.h" +#include "impeller/geometry/geometry_unittests.h" + +namespace impeller { +namespace testing { + +TEST(FilterInputTest, CanSetLocalTransformForTexture) { + std::shared_ptr texture = nullptr; + auto input = + FilterInput::Make(texture, Matrix::MakeTranslation({1.0, 0.0, 0.0})); + Entity e; + e.SetTransformation(Matrix::MakeTranslation({0.0, 2.0, 0.0})); + + ASSERT_MATRIX_NEAR(input->GetLocalTransform(e), + Matrix::MakeTranslation({1.0, 0.0, 0.0})); + ASSERT_MATRIX_NEAR(input->GetTransform(e), + Matrix::MakeTranslation({1.0, 2.0, 0.0})); +} + +} // namespace testing +} // namespace impeller diff --git a/engine/src/flutter/impeller/entity/contents/filters/inputs/texture_filter_input.cc b/engine/src/flutter/impeller/entity/contents/filters/inputs/texture_filter_input.cc index 316ced8bc80..e614a1dd206 100644 --- a/engine/src/flutter/impeller/entity/contents/filters/inputs/texture_filter_input.cc +++ b/engine/src/flutter/impeller/entity/contents/filters/inputs/texture_filter_input.cc @@ -6,8 +6,9 @@ namespace impeller { -TextureFilterInput::TextureFilterInput(std::shared_ptr texture) - : texture_(texture) {} +TextureFilterInput::TextureFilterInput(std::shared_ptr texture, + Matrix local_transform) + : texture_(texture), local_transform_(local_transform) {} TextureFilterInput::~TextureFilterInput() = default; @@ -27,4 +28,8 @@ std::optional TextureFilterInput::GetCoverage( .TransformBounds(GetTransform(entity)); } +Matrix TextureFilterInput::GetLocalTransform(const Entity& entity) const { + return local_transform_; +} + } // namespace impeller diff --git a/engine/src/flutter/impeller/entity/contents/filters/inputs/texture_filter_input.h b/engine/src/flutter/impeller/entity/contents/filters/inputs/texture_filter_input.h index 72f57ba0ff3..4700f30cc2b 100644 --- a/engine/src/flutter/impeller/entity/contents/filters/inputs/texture_filter_input.h +++ b/engine/src/flutter/impeller/entity/contents/filters/inputs/texture_filter_input.h @@ -6,6 +6,8 @@ #include "impeller/entity/contents/filters/inputs/filter_input.h" +#include "impeller/geometry/matrix.h" + namespace impeller { class TextureFilterInput final : public FilterInput { @@ -22,10 +24,15 @@ class TextureFilterInput final : public FilterInput { // |FilterInput| std::optional GetCoverage(const Entity& entity) const override; + // |FilterInput| + Matrix GetLocalTransform(const Entity& entity) const override; + private: - TextureFilterInput(std::shared_ptr texture); + TextureFilterInput(std::shared_ptr texture, + Matrix local_transform = Matrix()); std::shared_ptr texture_; + Matrix local_transform_; friend FilterInput; }; diff --git a/engine/src/flutter/impeller/entity/entity_pass.cc b/engine/src/flutter/impeller/entity/entity_pass.cc index ef037d4219d..5ba104f9e2d 100644 --- a/engine/src/flutter/impeller/entity/entity_pass.cc +++ b/engine/src/flutter/impeller/entity/entity_pass.cc @@ -119,10 +119,12 @@ EntityPass* EntityPass::AddSubpass(std::unique_ptr pass) { } FML_DCHECK(pass->superpass_ == nullptr); pass->superpass_ = this; - auto subpass_pointer = pass.get(); + if (pass->blend_mode_ > Entity::BlendMode::kLastPipelineBlendMode) { contains_advanced_blends_ = true; } + + auto subpass_pointer = pass.get(); elements_.emplace_back(std::move(pass)); return subpass_pointer; } @@ -137,11 +139,17 @@ bool EntityPass::Render(ContentContext& renderer, } auto command_buffer = renderer.GetContext()->CreateRenderCommandBuffer(); - auto render_pass = command_buffer->CreateRenderPass(offscreen_target); + command_buffer->SetLabel("EntityPass Root Command Buffer"); + auto render_pass = command_buffer->CreateRenderPass(render_target); + render_pass->SetLabel("EntityPass Root Render Pass"); { + auto size_rect = + Rect::MakeSize(Size(offscreen_target.GetRenderTargetSize())); auto contents = std::make_shared(); + contents->SetPath(PathBuilder{}.AddRect(size_rect).TakePath()); contents->SetTexture(offscreen_target.GetRenderTargetTexture()); + contents->SetSourceRect(size_rect); Entity entity; entity.SetContents(contents); @@ -307,11 +315,13 @@ bool EntityPass::RenderInternal(ContentContext& renderer, } auto color0 = render_target.GetColorAttachments().find(0)->second; - auto input = FilterInput::Make({ - element_entity.GetContents(), - color0.resolve_texture ? color0.resolve_texture : color0.texture, - }); - element_entity.SetContents(FilterContents::MakeBlend(blend_mode_, input)); + FilterInput::Vector inputs = { + FilterInput::Make(element_entity.GetContents()), + FilterInput::Make( + color0.resolve_texture ? color0.resolve_texture : color0.texture, + element_entity.GetTransformation().Invert())}; + element_entity.SetContents( + FilterContents::MakeBlend(element_entity.GetBlendMode(), inputs)); element_entity.SetBlendMode(Entity::BlendMode::kSource); } diff --git a/engine/src/flutter/impeller/entity/shaders/texture_blend_screen.frag b/engine/src/flutter/impeller/entity/shaders/texture_blend_screen.frag index eb5faac0b69..421125a2d54 100644 --- a/engine/src/flutter/impeller/entity/shaders/texture_blend_screen.frag +++ b/engine/src/flutter/impeller/entity/shaders/texture_blend_screen.frag @@ -26,9 +26,11 @@ vec4 Unpremultiply(vec4 color) { } void main() { - vec4 dst = texture(texture_sampler_dst, v_dst_texture_coords); + vec4 dst = SampleWithBorder(texture_sampler_dst, v_dst_texture_coords); vec4 d = Unpremultiply(dst); vec4 src = SampleWithBorder(texture_sampler_src, v_src_texture_coords); vec4 s = Unpremultiply(src); - frag_color = 1 - ((1 - s) * (1 - d)); + + vec3 color = d.rgb + s.rgb - (d.rgb * s.rgb); + frag_color = vec4(color, d.a - s.a + s.a); }