From fc8b10a964a5b5a1641d8153f644ca9fd981fae7 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Wed, 26 Apr 2023 12:36:33 -0700 Subject: [PATCH 1/2] [Impeller] Coerce opaque ColorSourceContents to Source --- impeller/aiks/aiks_unittests.cc | 27 ++++++++++ impeller/aiks/color_source.cc | 1 + impeller/core/texture.cc | 12 ++++- impeller/core/texture.h | 9 +++- .../contents/conical_gradient_contents.cc | 12 +++++ .../contents/conical_gradient_contents.h | 3 ++ impeller/entity/contents/contents.cc | 4 ++ impeller/entity/contents/contents.h | 6 +++ .../contents/linear_gradient_contents.cc | 12 +++++ .../contents/linear_gradient_contents.h | 3 ++ .../contents/radial_gradient_contents.cc | 12 +++++ .../contents/radial_gradient_contents.h | 3 ++ .../entity/contents/solid_color_contents.cc | 4 ++ .../entity/contents/solid_color_contents.h | 3 ++ .../contents/sweep_gradient_contents.cc | 12 +++++ .../entity/contents/sweep_gradient_contents.h | 3 ++ .../entity/contents/tiled_texture_contents.cc | 11 ++++ .../entity/contents/tiled_texture_contents.h | 3 ++ impeller/entity/entity_pass.cc | 5 ++ impeller/entity/entity_unittests.cc | 53 +++++++++++++++++++ 20 files changed, 194 insertions(+), 4 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 3a1a529f7d448..7089ff35961a2 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -2136,5 +2136,32 @@ TEST_P(AiksTest, CanRenderOffscreenCheckerboard) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +TEST_P(AiksTest, OpaqueEntitiesGetCoercedToSource) { + Canvas canvas; + canvas.Scale(Vector2(1.618, 1.618)); + canvas.DrawCircle(Point(), 10, + { + .color = Color::CornflowerBlue(), + .blend_mode = BlendMode::kSourceOver, + }); + Picture picture = canvas.EndRecordingAsPicture(); + + // Extract the SolidColorSource. + Entity entity; + std::shared_ptr contents; + picture.pass->IterateAllEntities([&e = entity, &contents](Entity& entity) { + if (ScalarNearlyEqual(entity.GetTransformation().GetScale().x, 1.618f)) { + e = entity; + contents = + std::static_pointer_cast(entity.GetContents()); + return false; + } + return true; + }); + + ASSERT_TRUE(contents->IsOpaque()); + ASSERT_EQ(entity.GetBlendMode(), BlendMode::kSource); +} + } // namespace testing } // namespace impeller diff --git a/impeller/aiks/color_source.cc b/impeller/aiks/color_source.cc index f5644b216e30e..9571151a1492a 100644 --- a/impeller/aiks/color_source.cc +++ b/impeller/aiks/color_source.cc @@ -17,6 +17,7 @@ #include "impeller/entity/contents/solid_color_contents.h" #include "impeller/entity/contents/sweep_gradient_contents.h" #include "impeller/entity/contents/tiled_texture_contents.h" +#include "impeller/geometry/color.h" #include "impeller/geometry/matrix.h" #include "impeller/geometry/scalar.h" #include "impeller/runtime_stage/runtime_stage.h" diff --git a/impeller/core/texture.cc b/impeller/core/texture.cc index d884d355cf42b..756c78f10e9e1 100644 --- a/impeller/core/texture.cc +++ b/impeller/core/texture.cc @@ -14,7 +14,8 @@ Texture::~Texture() = default; bool Texture::SetContents(const uint8_t* contents, size_t length, - size_t slice) { + size_t slice, + bool is_opaque) { if (!IsSliceValid(slice)) { VALIDATION_LOG << "Invalid slice for texture."; return false; @@ -23,11 +24,13 @@ bool Texture::SetContents(const uint8_t* contents, return false; } intent_ = TextureIntent::kUploadFromHost; + is_opaque_ = is_opaque; return true; } bool Texture::SetContents(std::shared_ptr mapping, - size_t slice) { + size_t slice, + bool is_opaque) { if (!IsSliceValid(slice)) { VALIDATION_LOG << "Invalid slice for texture."; return false; @@ -39,9 +42,14 @@ bool Texture::SetContents(std::shared_ptr mapping, return false; } intent_ = TextureIntent::kUploadFromHost; + is_opaque_ = is_opaque; return true; } +bool Texture::IsOpaque() const { + return is_opaque_; +} + size_t Texture::GetMipCount() const { return GetTextureDescriptor().mip_count; } diff --git a/impeller/core/texture.h b/impeller/core/texture.h index 5d3852c137f8b..462888b5926a7 100644 --- a/impeller/core/texture.h +++ b/impeller/core/texture.h @@ -22,15 +22,19 @@ class Texture { [[nodiscard]] bool SetContents(const uint8_t* contents, size_t length, - size_t slice = 0); + size_t slice = 0, + bool is_opaque = false); [[nodiscard]] bool SetContents(std::shared_ptr mapping, - size_t slice = 0); + size_t slice = 0, + bool is_opaque = false); virtual bool IsValid() const = 0; virtual ISize GetSize() const = 0; + bool IsOpaque() const; + size_t GetMipCount() const; const TextureDescriptor& GetTextureDescriptor() const; @@ -59,6 +63,7 @@ class Texture { private: TextureIntent intent_ = TextureIntent::kRenderToTexture; const TextureDescriptor desc_; + bool is_opaque_ = false; bool IsSliceValid(size_t slice) const; diff --git a/impeller/entity/contents/conical_gradient_contents.cc b/impeller/entity/contents/conical_gradient_contents.cc index 37d348482c1e9..b65964db879e2 100644 --- a/impeller/entity/contents/conical_gradient_contents.cc +++ b/impeller/entity/contents/conical_gradient_contents.cc @@ -51,6 +51,18 @@ void ConicalGradientContents::SetFocus(std::optional focus, focus_radius_ = radius; } +bool ConicalGradientContents::IsOpaque() const { + if (GetOpacity() < 1) { + return false; + } + for (auto color : colors_) { + if (!color.IsOpaque()) { + return false; + } + } + return true; +} + bool ConicalGradientContents::Render(const ContentContext& renderer, const Entity& entity, RenderPass& pass) const { diff --git a/impeller/entity/contents/conical_gradient_contents.h b/impeller/entity/contents/conical_gradient_contents.h index d46ae86a0eabf..d773a0e9a2d91 100644 --- a/impeller/entity/contents/conical_gradient_contents.h +++ b/impeller/entity/contents/conical_gradient_contents.h @@ -24,6 +24,9 @@ class ConicalGradientContents final : public ColorSourceContents { ~ConicalGradientContents() override; + // |Contents| + bool IsOpaque() const override; + // |Contents| bool Render(const ContentContext& renderer, const Entity& entity, diff --git a/impeller/entity/contents/contents.cc b/impeller/entity/contents/contents.cc index 3e498c0b12f31..da74dee6638f9 100644 --- a/impeller/entity/contents/contents.cc +++ b/impeller/entity/contents/contents.cc @@ -45,6 +45,10 @@ Contents::Contents() = default; Contents::~Contents() = default; +bool Contents::IsOpaque() const { + return false; +} + Contents::StencilCoverage Contents::GetStencilCoverage( const Entity& entity, const std::optional& current_stencil_coverage) const { diff --git a/impeller/entity/contents/contents.h b/impeller/entity/contents/contents.h index 7ed44adcda0f2..b4d6289add92e 100644 --- a/impeller/entity/contents/contents.h +++ b/impeller/entity/contents/contents.h @@ -56,6 +56,12 @@ class Contents { /// @brief Get the screen space bounding rectangle that this contents affects. virtual std::optional GetCoverage(const Entity& entity) const = 0; + /// @brief Whether this Contents only emits opaque source colors from the + /// fragment stage. This value does not account for any entity + /// properties (e.g. the blend mode), clips/visibility culling, or + /// inherited opacity. + virtual bool IsOpaque() const; + /// @brief Given the current screen space bounding rectangle of the stencil, /// return the expected stencil coverage after this draw call. This /// should only be implemented for contents that may write to the diff --git a/impeller/entity/contents/linear_gradient_contents.cc b/impeller/entity/contents/linear_gradient_contents.cc index cb76d16231dad..bdfc3407b2dc6 100644 --- a/impeller/entity/contents/linear_gradient_contents.cc +++ b/impeller/entity/contents/linear_gradient_contents.cc @@ -44,6 +44,18 @@ void LinearGradientContents::SetTileMode(Entity::TileMode tile_mode) { tile_mode_ = tile_mode; } +bool LinearGradientContents::IsOpaque() const { + if (GetOpacity() < 1) { + return false; + } + for (auto color : colors_) { + if (!color.IsOpaque()) { + return false; + } + } + return true; +} + bool LinearGradientContents::Render(const ContentContext& renderer, const Entity& entity, RenderPass& pass) const { diff --git a/impeller/entity/contents/linear_gradient_contents.h b/impeller/entity/contents/linear_gradient_contents.h index 4a393bfadade9..93d10eaf2591b 100644 --- a/impeller/entity/contents/linear_gradient_contents.h +++ b/impeller/entity/contents/linear_gradient_contents.h @@ -25,6 +25,9 @@ class LinearGradientContents final : public ColorSourceContents { ~LinearGradientContents() override; + // |Contents| + bool IsOpaque() const override; + // |Contents| bool Render(const ContentContext& renderer, const Entity& entity, diff --git a/impeller/entity/contents/radial_gradient_contents.cc b/impeller/entity/contents/radial_gradient_contents.cc index 76d989f00ff99..e7bd713524938 100644 --- a/impeller/entity/contents/radial_gradient_contents.cc +++ b/impeller/entity/contents/radial_gradient_contents.cc @@ -45,6 +45,18 @@ const std::vector& RadialGradientContents::GetStops() const { return stops_; } +bool RadialGradientContents::IsOpaque() const { + if (GetOpacity() < 1) { + return false; + } + for (auto color : colors_) { + if (!color.IsOpaque()) { + return false; + } + } + return true; +} + bool RadialGradientContents::Render(const ContentContext& renderer, const Entity& entity, RenderPass& pass) const { diff --git a/impeller/entity/contents/radial_gradient_contents.h b/impeller/entity/contents/radial_gradient_contents.h index 6cb611d1ee68a..1114b26dfe6db 100644 --- a/impeller/entity/contents/radial_gradient_contents.h +++ b/impeller/entity/contents/radial_gradient_contents.h @@ -24,6 +24,9 @@ class RadialGradientContents final : public ColorSourceContents { ~RadialGradientContents() override; + // |Contents| + bool IsOpaque() const override; + // |Contents| bool Render(const ContentContext& renderer, const Entity& entity, diff --git a/impeller/entity/contents/solid_color_contents.cc b/impeller/entity/contents/solid_color_contents.cc index 9c14169833330..1c5b032cd4a37 100644 --- a/impeller/entity/contents/solid_color_contents.cc +++ b/impeller/entity/contents/solid_color_contents.cc @@ -24,6 +24,10 @@ Color SolidColorContents::GetColor() const { return color_.WithAlpha(color_.alpha * GetOpacity()); } +bool SolidColorContents::IsOpaque() const { + return GetColor().IsOpaque(); +} + std::optional SolidColorContents::GetCoverage( const Entity& entity) const { if (GetColor().IsTransparent()) { diff --git a/impeller/entity/contents/solid_color_contents.h b/impeller/entity/contents/solid_color_contents.h index 5348b906afa15..b034c6dbdc221 100644 --- a/impeller/entity/contents/solid_color_contents.h +++ b/impeller/entity/contents/solid_color_contents.h @@ -34,6 +34,9 @@ class SolidColorContents final : public ColorSourceContents { Color GetColor() const; + // |Contents| + bool IsOpaque() const override; + // |Contents| std::optional GetCoverage(const Entity& entity) const override; diff --git a/impeller/entity/contents/sweep_gradient_contents.cc b/impeller/entity/contents/sweep_gradient_contents.cc index 3ea26e378e9dc..a70ff716d0092 100644 --- a/impeller/entity/contents/sweep_gradient_contents.cc +++ b/impeller/entity/contents/sweep_gradient_contents.cc @@ -50,6 +50,18 @@ const std::vector& SweepGradientContents::GetStops() const { return stops_; } +bool SweepGradientContents::IsOpaque() const { + if (GetOpacity() < 1) { + return false; + } + for (auto color : colors_) { + if (!color.IsOpaque()) { + return false; + } + } + return true; +} + bool SweepGradientContents::Render(const ContentContext& renderer, const Entity& entity, RenderPass& pass) const { diff --git a/impeller/entity/contents/sweep_gradient_contents.h b/impeller/entity/contents/sweep_gradient_contents.h index 94c448cc22961..ed776dee9d617 100644 --- a/impeller/entity/contents/sweep_gradient_contents.h +++ b/impeller/entity/contents/sweep_gradient_contents.h @@ -25,6 +25,9 @@ class SweepGradientContents final : public ColorSourceContents { ~SweepGradientContents() override; + // |Contents| + bool IsOpaque() const override; + // |Contents| bool Render(const ContentContext& renderer, const Entity& entity, diff --git a/impeller/entity/contents/tiled_texture_contents.cc b/impeller/entity/contents/tiled_texture_contents.cc index e825e2acaf982..ce485480eb666 100644 --- a/impeller/entity/contents/tiled_texture_contents.cc +++ b/impeller/entity/contents/tiled_texture_contents.cc @@ -93,6 +93,17 @@ bool TiledTextureContents::UsesEmulatedTileMode( !TileModeToAddressMode(y_tile_mode_, capabilities).has_value(); } +// |Contents| +bool TiledTextureContents::IsOpaque() const { + if (GetOpacity() < 1) { + return false; + } + if (color_filter_.has_value()) { + return false; + } + return texture_->IsOpaque(); +} + bool TiledTextureContents::Render(const ContentContext& renderer, const Entity& entity, RenderPass& pass) const { diff --git a/impeller/entity/contents/tiled_texture_contents.h b/impeller/entity/contents/tiled_texture_contents.h index 32fbfaa0e4a10..929c0c9e6f5ce 100644 --- a/impeller/entity/contents/tiled_texture_contents.h +++ b/impeller/entity/contents/tiled_texture_contents.h @@ -27,6 +27,9 @@ class TiledTextureContents final : public ColorSourceContents { using ColorFilterProc = std::function(FilterInput::Ref)>; + // |Contents| + bool IsOpaque() const override; + // |Contents| bool Render(const ContentContext& renderer, const Entity& entity, diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 0b67412fed0a1..b7c7050e890eb 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -48,6 +48,11 @@ void EntityPass::SetDelegate(std::unique_ptr delegate) { } void EntityPass::AddEntity(Entity entity) { + if (entity.GetBlendMode() == BlendMode::kSourceOver && + entity.GetContents()->IsOpaque()) { + entity.SetBlendMode(BlendMode::kSource); + } + if (entity.GetBlendMode() > Entity::kLastPipelineBlendMode) { advanced_blend_reads_from_pass_texture_ += 1; } diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index f1285c3393ef7..007c6757a1696 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -15,15 +15,18 @@ #include "gtest/gtest.h" #include "impeller/entity/contents/atlas_contents.h" #include "impeller/entity/contents/clip_contents.h" +#include "impeller/entity/contents/conical_gradient_contents.h" #include "impeller/entity/contents/contents.h" #include "impeller/entity/contents/filters/blend_filter_contents.h" #include "impeller/entity/contents/filters/color_filter_contents.h" #include "impeller/entity/contents/filters/filter_contents.h" #include "impeller/entity/contents/filters/inputs/filter_input.h" #include "impeller/entity/contents/linear_gradient_contents.h" +#include "impeller/entity/contents/radial_gradient_contents.h" #include "impeller/entity/contents/rrect_shadow_contents.h" #include "impeller/entity/contents/runtime_effect_contents.h" #include "impeller/entity/contents/solid_color_contents.h" +#include "impeller/entity/contents/sweep_gradient_contents.h" #include "impeller/entity/contents/text_contents.h" #include "impeller/entity/contents/texture_contents.h" #include "impeller/entity/contents/tiled_texture_contents.h" @@ -2547,5 +2550,55 @@ TEST_P(EntityTest, CoverageForStrokePathWithNegativeValuesInTransform) { ASSERT_RECT_NEAR(coverage.value(), Rect::MakeXYWH(102.5, 342.5, 85, 155)); } +TEST_P(EntityTest, SolidColorContentsIsOpaque) { + SolidColorContents contents; + contents.SetColor(Color::CornflowerBlue()); + ASSERT_TRUE(contents.IsOpaque()); + contents.SetColor(Color::CornflowerBlue().WithAlpha(0.5)); + ASSERT_FALSE(contents.IsOpaque()); +} + +TEST_P(EntityTest, ConicalGradientContentsIsOpaque) { + ConicalGradientContents contents; + contents.SetColors({Color::CornflowerBlue()}); + ASSERT_TRUE(contents.IsOpaque()); + contents.SetColors({Color::CornflowerBlue().WithAlpha(0.5)}); + ASSERT_FALSE(contents.IsOpaque()); +} + +TEST_P(EntityTest, LinearGradientContentsIsOpaque) { + LinearGradientContents contents; + contents.SetColors({Color::CornflowerBlue()}); + ASSERT_TRUE(contents.IsOpaque()); + contents.SetColors({Color::CornflowerBlue().WithAlpha(0.5)}); + ASSERT_FALSE(contents.IsOpaque()); +} + +TEST_P(EntityTest, RadialGradientContentsIsOpaque) { + RadialGradientContents contents; + contents.SetColors({Color::CornflowerBlue()}); + ASSERT_TRUE(contents.IsOpaque()); + contents.SetColors({Color::CornflowerBlue().WithAlpha(0.5)}); + ASSERT_FALSE(contents.IsOpaque()); +} + +TEST_P(EntityTest, SweepGradientContentsIsOpaque) { + RadialGradientContents contents; + contents.SetColors({Color::CornflowerBlue()}); + ASSERT_TRUE(contents.IsOpaque()); + contents.SetColors({Color::CornflowerBlue().WithAlpha(0.5)}); + ASSERT_FALSE(contents.IsOpaque()); +} + +TEST_P(EntityTest, TiledTextureContentsIsOpaque) { + auto bay_bridge = CreateTextureForFixture("bay_bridge.jpg"); + TiledTextureContents contents; + contents.SetTexture(bay_bridge); + // This is a placeholder test. Images currently never decompress as opaque + // (whether in Flutter or the playground), and so this should currently always + // return false in practice. + ASSERT_FALSE(contents.IsOpaque()); +} + } // namespace testing } // namespace impeller From 0ae51ca8bc4c3a1e9b05da801127ef3b5fe05723 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Wed, 26 Apr 2023 13:46:42 -0700 Subject: [PATCH 2/2] Fix opacity peephole bugs --- impeller/aiks/paint_pass_delegate.cc | 7 +++++-- impeller/entity/entity.cc | 23 +++++++++++++++++++++++ impeller/entity/entity.h | 4 ++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/impeller/aiks/paint_pass_delegate.cc b/impeller/aiks/paint_pass_delegate.cc index 5e9f6966f89ab..e942222a11d6d 100644 --- a/impeller/aiks/paint_pass_delegate.cc +++ b/impeller/aiks/paint_pass_delegate.cc @@ -7,6 +7,7 @@ #include "impeller/entity/contents/contents.h" #include "impeller/entity/contents/texture_contents.h" #include "impeller/entity/entity_pass.h" +#include "impeller/geometry/color.h" #include "impeller/geometry/path_builder.h" namespace impeller { @@ -72,6 +73,8 @@ bool OpacityPeepholePassDelegate::CanElide() { // |EntityPassDelgate| bool OpacityPeepholePassDelegate::CanCollapseIntoParentPass( EntityPass* entity_pass) { + // OpacityPeepholePassDelegate will only get used if the pass's blend mode is + // SourceOver, so no need to check here. if (paint_.color.alpha <= 0.0 || paint_.color.alpha >= 1.0 || paint_.image_filter.has_value() || paint_.color_filter.has_value()) { return false; @@ -97,7 +100,7 @@ bool OpacityPeepholePassDelegate::CanCollapseIntoParentPass( auto had_subpass = entity_pass->IterateUntilSubpass( [&all_coverages, &all_can_accept](Entity& entity) { auto contents = entity.GetContents(); - if (!contents->CanInheritOpacity(entity)) { + if (!entity.CanInheritOpacity()) { all_can_accept = false; return false; } @@ -119,7 +122,7 @@ bool OpacityPeepholePassDelegate::CanCollapseIntoParentPass( } auto alpha = paint_.color.alpha; entity_pass->IterateUntilSubpass([&alpha](Entity& entity) { - entity.GetContents()->SetInheritedOpacity(alpha); + entity.SetInheritedOpacity(alpha); return true; }); return true; diff --git a/impeller/entity/entity.cc b/impeller/entity/entity.cc index a3bb12cc9b5ec..40ef6117504ca 100644 --- a/impeller/entity/entity.cc +++ b/impeller/entity/entity.cc @@ -12,6 +12,7 @@ #include "impeller/entity/contents/filters/filter_contents.h" #include "impeller/entity/contents/texture_contents.h" #include "impeller/entity/entity_pass.h" +#include "impeller/geometry/color.h" #include "impeller/geometry/vector.h" #include "impeller/renderer/render_pass.h" @@ -101,6 +102,28 @@ BlendMode Entity::GetBlendMode() const { return blend_mode_; } +bool Entity::CanInheritOpacity() const { + if (!contents_) { + return false; + } + if (!((blend_mode_ == BlendMode::kSource && contents_->IsOpaque()) || + blend_mode_ == BlendMode::kSourceOver)) { + return false; + } + return contents_->CanInheritOpacity(*this); +} + +bool Entity::SetInheritedOpacity(Scalar alpha) { + if (!CanInheritOpacity()) { + return false; + } + if (blend_mode_ == BlendMode::kSource && contents_->IsOpaque()) { + blend_mode_ = BlendMode::kSourceOver; + } + contents_->SetInheritedOpacity(alpha); + return true; +} + /// @brief Returns true if the blend mode is "destrictive", meaning that even /// fully transparent source colors would result in the destination /// getting changed. diff --git a/impeller/entity/entity.h b/impeller/entity/entity.h index 440d1b35a7d2d..5577c486c4c8a 100644 --- a/impeller/entity/entity.h +++ b/impeller/entity/entity.h @@ -88,6 +88,10 @@ class Entity { static bool IsBlendModeDestructive(BlendMode blend_mode); + bool CanInheritOpacity() const; + + bool SetInheritedOpacity(Scalar alpha); + private: Matrix transformation_; std::shared_ptr contents_;