Skip to content
This repository was archived by the owner on Feb 14, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ group("unittests") {
if (is_mac) {
public_deps += [
"//flutter/impeller/golden_tests:impeller_golden_tests",
"//flutter/shell/platform/darwin/common:availability_version_check_unittests",
"//flutter/shell/platform/darwin/common:framework_common_unittests",
"//flutter/third_party/spring_animation:spring_animation_unittests",
]
Expand Down
1 change: 1 addition & 0 deletions ci/licenses_golden/excluded_files
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@
../../../flutter/shell/platform/common/text_input_model_unittests.cc
../../../flutter/shell/platform/common/text_range_unittests.cc
../../../flutter/shell/platform/darwin/Doxyfile
../../../flutter/shell/platform/darwin/common/availability_version_check_unittests.cc
../../../flutter/shell/platform/darwin/common/framework/Source/flutter_codecs_unittest.mm
../../../flutter/shell/platform/darwin/common/framework/Source/flutter_standard_codec_unittest.mm
../../../flutter/shell/platform/darwin/macos/README.md
Expand Down
4 changes: 4 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -2597,6 +2597,7 @@ ORIGIN: ../../../flutter/shell/platform/common/text_input_model.cc + ../../../fl
ORIGIN: ../../../flutter/shell/platform/common/text_input_model.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/common/text_range.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/common/availability_version_check.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/common/availability_version_check.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/common/buffer_conversions.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/common/buffer_conversions.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/common/command_line.h + ../../../flutter/LICENSE
Expand Down Expand Up @@ -2731,6 +2732,7 @@ ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibilit
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_text_entry.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_text_entry.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/availability_version_check_test.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/connection_collection.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/connection_collection.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/connection_collection_test.mm + ../../../flutter/LICENSE
Expand Down Expand Up @@ -5367,6 +5369,7 @@ FILE: ../../../flutter/shell/platform/common/text_input_model.cc
FILE: ../../../flutter/shell/platform/common/text_input_model.h
FILE: ../../../flutter/shell/platform/common/text_range.h
FILE: ../../../flutter/shell/platform/darwin/common/availability_version_check.cc
FILE: ../../../flutter/shell/platform/darwin/common/availability_version_check.h
FILE: ../../../flutter/shell/platform/darwin/common/buffer_conversions.h
FILE: ../../../flutter/shell/platform/darwin/common/buffer_conversions.mm
FILE: ../../../flutter/shell/platform/darwin/common/command_line.h
Expand Down Expand Up @@ -5502,6 +5505,7 @@ FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_text_entry.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_text_entry.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/availability_version_check_test.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/connection_collection.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/connection_collection.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/connection_collection_test.mm
Expand Down
31 changes: 26 additions & 5 deletions impeller/aiks/aiks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2416,19 +2416,18 @@ TEST_P(AiksTest, ClearColorOptimizationDoesNotApplyForBackdropFilters) {
Picture picture = canvas.EndRecordingAsPicture();

std::optional<Color> actual_color;
bool found_subpass = false;
picture.pass->IterateAllElements([&](EntityPass::Element& element) -> bool {
if (auto subpass = std::get_if<std::unique_ptr<EntityPass>>(&element)) {
actual_color = subpass->get()->GetClearColor();
found_subpass = true;
}
// Fail if the first element isn't a subpass.
return true;
});

ASSERT_TRUE(actual_color.has_value());
if (!actual_color) {
return;
}
ASSERT_EQ(actual_color.value(), Color::BlackTransparent());
EXPECT_TRUE(found_subpass);
EXPECT_FALSE(actual_color.has_value());
}

TEST_P(AiksTest, CollapsedDrawPaintInSubpass) {
Expand Down Expand Up @@ -3645,5 +3644,27 @@ TEST_P(AiksTest, MaskBlurWithZeroSigmaIsSkipped) {
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

TEST_P(AiksTest, SubpassWithClearColorOptimization) {
Canvas canvas;

// Use a non-srcOver blend mode to ensure that we don't detect this as an
// opacity peephole optimization.
canvas.SaveLayer(
{.color = Color::Blue().WithAlpha(0.5), .blend_mode = BlendMode::kSource},
Rect::MakeLTRB(0, 0, 200, 200));
canvas.DrawPaint(
{.color = Color::BlackTransparent(), .blend_mode = BlendMode::kSource});
canvas.Restore();

canvas.SaveLayer(
{.color = Color::Blue(), .blend_mode = BlendMode::kDestinationOver});
canvas.Restore();

// This playground should appear blank on CI since we are only drawing
// transparent black. If the clear color optimization is broken, the texture
// will be filled with NaNs and may produce a magenta texture on macOS or iOS.
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

} // namespace testing
} // namespace impeller
8 changes: 8 additions & 0 deletions impeller/aiks/canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,14 @@ void Canvas::SaveLayer(const Paint& paint,
const std::shared_ptr<ImageFilter>& backdrop_filter) {
Save(true, paint.blend_mode, backdrop_filter);

// The DisplayList bounds/rtree doesn't account for filters applied to parent
// layers, and so sub-DisplayLists are getting culled as if no filters are
// applied.
// See also: https://github.com/flutter/flutter/issues/139294
if (paint.image_filter) {
xformation_stack_.back().cull_rect = std::nullopt;
}

auto& new_layer_pass = GetCurrentPass();
new_layer_pass.SetBoundsLimit(bounds);

Expand Down
18 changes: 18 additions & 0 deletions impeller/aiks/canvas_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "flutter/testing/testing.h"
#include "impeller/aiks/canvas.h"
#include "impeller/aiks/image_filter.h"
#include "impeller/geometry/path_builder.h"

// TODO(zanderso): https://github.com/flutter/flutter/issues/127701
Expand Down Expand Up @@ -336,6 +337,23 @@ TEST(AiksCanvasTest, PathClipDiffAgainstFullyCoveredCullRect) {
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull);
}

TEST(AiksCanvasTest, DisableLocalBoundsRectForFilteredSaveLayers) {
Rect initial_cull = Rect::MakeXYWH(0, 0, 10, 10);

Canvas canvas(initial_cull);
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());

canvas.Save();
canvas.SaveLayer(
Paint{.image_filter = ImageFilter::MakeBlur(
Sigma(10), Sigma(10), FilterContents::BlurStyle::kNormal,
Entity::TileMode::kDecal)});
ASSERT_FALSE(canvas.GetCurrentLocalCullingBounds().has_value());

canvas.Restore();
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
}

} // namespace testing
} // namespace impeller

Expand Down
1 change: 0 additions & 1 deletion impeller/aiks/paint_pass_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#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 {

Expand Down
4 changes: 2 additions & 2 deletions impeller/entity/contents/clip_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ Contents::StencilCoverage ClipContents::GetStencilCoverage(

bool ClipContents::ShouldRender(
const Entity& entity,
const std::optional<Rect>& stencil_coverage) const {
const std::optional<Rect> stencil_coverage) const {
return true;
}

Expand Down Expand Up @@ -163,7 +163,7 @@ Contents::StencilCoverage ClipRestoreContents::GetStencilCoverage(

bool ClipRestoreContents::ShouldRender(
const Entity& entity,
const std::optional<Rect>& stencil_coverage) const {
const std::optional<Rect> stencil_coverage) const {
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions impeller/entity/contents/clip_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class ClipContents final : public Contents {

// |Contents|
bool ShouldRender(const Entity& entity,
const std::optional<Rect>& stencil_coverage) const override;
const std::optional<Rect> stencil_coverage) const override;

// |Contents|
bool Render(const ContentContext& renderer,
Expand Down Expand Up @@ -76,7 +76,7 @@ class ClipRestoreContents final : public Contents {

// |Contents|
bool ShouldRender(const Entity& entity,
const std::optional<Rect>& stencil_coverage) const override;
const std::optional<Rect> stencil_coverage) const override;

// |Contents|
bool Render(const ContentContext& renderer,
Expand Down
3 changes: 1 addition & 2 deletions impeller/entity/contents/contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,10 @@ bool Contents::ApplyColorFilter(
}

bool Contents::ShouldRender(const Entity& entity,
const std::optional<Rect>& stencil_coverage) const {
const std::optional<Rect> stencil_coverage) const {
if (!stencil_coverage.has_value()) {
return false;
}

auto coverage = GetCoverage(entity);
if (!coverage.has_value()) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion impeller/entity/contents/contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ class Contents {
const std::string& label = "Snapshot") const;

virtual bool ShouldRender(const Entity& entity,
const std::optional<Rect>& stencil_coverage) const;
const std::optional<Rect> stencil_coverage) const;

//----------------------------------------------------------------------------
/// @brief Return the color source's intrinsic size, if available.
Expand Down
4 changes: 4 additions & 0 deletions impeller/entity/entity.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ Contents::StencilCoverage Entity::GetStencilCoverage(
}

bool Entity::ShouldRender(const std::optional<Rect>& stencil_coverage) const {
#ifdef IMPELLER_CONTENT_CULLING
return contents_->ShouldRender(*this, stencil_coverage);
#else
return true;
#endif // IMPELLER_CONTENT_CULLING
}

void Entity::SetContents(std::shared_ptr<Contents> contents) {
Expand Down
34 changes: 21 additions & 13 deletions impeller/entity/entity_pass.cc
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ bool EntityPass::Render(ContentContext& renderer,
if (!supports_onscreen_backdrop_reads && reads_from_onscreen_backdrop) {
auto offscreen_target = CreateRenderTarget(
renderer, root_render_target.GetRenderTargetSize(), true,
GetClearColor(render_target.GetRenderTargetSize()));
GetClearColorOrDefault(render_target.GetRenderTargetSize()));

if (!OnRender(renderer, // renderer
capture, // capture
Expand Down Expand Up @@ -475,7 +475,8 @@ bool EntityPass::Render(ContentContext& renderer,
}

// Set up the clear color of the root pass.
color0.clear_color = GetClearColor(render_target.GetRenderTargetSize());
color0.clear_color =
GetClearColorOrDefault(render_target.GetRenderTargetSize());
root_render_target.SetColorAttachment(color0, 0);

EntityPassTarget pass_target(
Expand Down Expand Up @@ -628,10 +629,10 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
}

auto subpass_target = CreateRenderTarget(
renderer, // renderer
subpass_size, // size
subpass->GetTotalPassReads(renderer) > 0, // readable
subpass->GetClearColor(subpass_size)); // clear_color
renderer, // renderer
subpass_size, // size
subpass->GetTotalPassReads(renderer) > 0, // readable
subpass->GetClearColorOrDefault(subpass_size)); // clear_color

if (!subpass_target.IsValid()) {
VALIDATION_LOG << "Subpass render target is invalid.";
Expand Down Expand Up @@ -722,8 +723,7 @@ bool EntityPass::OnRender(
}
auto clear_color_size = pass_target.GetRenderTarget().GetRenderTargetSize();

if (!collapsed_parent_pass &&
!GetClearColor(clear_color_size).IsTransparent()) {
if (!collapsed_parent_pass && GetClearColor(clear_color_size).has_value()) {
// Force the pass context to create at least one new pass if the clear color
// is present.
pass_context.GetRenderPass(pass_depth);
Expand Down Expand Up @@ -1140,21 +1140,29 @@ void EntityPass::SetBlendMode(BlendMode blend_mode) {
flood_clip_ = Entity::IsBlendModeDestructive(blend_mode);
}

Color EntityPass::GetClearColor(ISize target_size) const {
Color result = Color::BlackTransparent();
Color EntityPass::GetClearColorOrDefault(ISize size) const {
return GetClearColor(size).value_or(Color::BlackTransparent());
}

std::optional<Color> EntityPass::GetClearColor(ISize target_size) const {
if (backdrop_filter_proc_) {
return result;
return std::nullopt;
}

std::optional<Color> result = std::nullopt;
for (const Element& element : elements_) {
auto [entity_color, blend_mode] =
ElementAsBackgroundColor(element, target_size);
if (!entity_color.has_value()) {
break;
}
result = result.Blend(entity_color.value(), blend_mode);
result = result.value_or(Color::BlackTransparent())
.Blend(entity_color.value(), blend_mode);
}
return result.Premultiply();
if (result.has_value()) {
return result->Premultiply();
}
return result;
}

void EntityPass::SetBackdropFilter(BackdropFilterProc proc) {
Expand Down
8 changes: 7 additions & 1 deletion impeller/entity/entity_pass.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,13 @@ class EntityPass {

void SetBlendMode(BlendMode blend_mode);

Color GetClearColor(ISize size = ISize::Infinite()) const;
/// @brief Return the premultiplied clear color of the pass entities, if any.
std::optional<Color> GetClearColor(ISize size = ISize::Infinite()) const;

/// @brief Return the premultiplied clear color of the pass entities.
///
/// If the entity pass has no clear color, this will return transparent black.
Color GetClearColorOrDefault(ISize size = ISize::Infinite()) const;

void SetBackdropFilter(BackdropFilterProc proc);

Expand Down
14 changes: 14 additions & 0 deletions impeller/entity/entity_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1607,6 +1607,20 @@ TEST_P(EntityTest, SolidFillShouldRenderIsCorrect) {
}
}

TEST_P(EntityTest, DoesNotCullEntitiesByDefault) {
auto fill = std::make_shared<SolidColorContents>();
fill->SetColor(Color::CornflowerBlue());
fill->SetGeometry(
Geometry::MakeRect(Rect::MakeLTRB(-1000, -1000, -900, -900)));

Entity entity;
entity.SetContents(fill);

// Even though the entity is offscreen, this should still render because we do
// not compute the coverage intersection by default.
EXPECT_TRUE(entity.ShouldRender(Rect::MakeLTRB(0, 0, 100, 100)));
}

TEST_P(EntityTest, ClipContentsShouldRenderIsCorrect) {
// For clip ops, `ShouldRender` should always return true.

Expand Down
19 changes: 19 additions & 0 deletions shell/platform/darwin/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,25 @@ source_set("availability_version_check") {
public_configs = [ "//flutter:config" ]
}

test_fixtures("availability_version_check_fixtures") {
fixtures = []
}

executable("availability_version_check_unittests") {
testonly = true

sources = [ "availability_version_check_unittests.cc" ]

deps = [
":availability_version_check",
":availability_version_check_fixtures",
"//flutter/fml",
"//flutter/testing",
]

public_configs = [ "//flutter:config" ]
}

# Shared framework headers end up in the same folder as platform-specific
# framework headers when consumed by clients, so the include paths assume they
# are next to each other.
Expand Down
Loading