diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index b9cd19a12afcd..49af55d4e6af2 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -10,6 +10,7 @@ #include "impeller/base/validation.h" #include "impeller/core/allocator.h" #include "impeller/core/formats.h" +#include "impeller/entity/contents/clip_contents.h" #include "impeller/entity/contents/framebuffer_blend_contents.h" #include "impeller/entity/contents/text_contents.h" #include "impeller/entity/entity.h" @@ -431,6 +432,12 @@ void ExperimentalCanvas::SaveLayer( entry.rendering_mode = Entity::RenderingMode::kSubpassAppendSnapshotTransform; transform_stack_.emplace_back(entry); + // The current clip aiks clip culling can not handle image filters. + // Remove this once we've migrated to exp canvas and removed it. + if (paint.image_filter) { + transform_stack_.back().cull_rect = std::nullopt; + } + // Start non-collapsed subpasses with a fresh clip coverage stack limited by // the subpass coverage. This is important because image filters applied to // save layers may transform the subpass texture after it's rendered, @@ -752,6 +759,7 @@ void ExperimentalCanvas::AddClipEntityToCurrentPass(Entity entity) { auto transform = entity.GetTransform(); entity.SetTransform( Matrix::MakeTranslation(Vector3(-GetGlobalPassPosition())) * transform); + // Ideally the clip depth would be greater than the current rendering // depth because any rendering calls that follow this clip operation will // pre-increment the depth and then be rendering above our clip depth, diff --git a/impeller/entity/contents/clip_contents.cc b/impeller/entity/contents/clip_contents.cc index f13f452734732..cd1fc5c863cff 100644 --- a/impeller/entity/contents/clip_contents.cc +++ b/impeller/entity/contents/clip_contents.cc @@ -53,8 +53,11 @@ Contents::ClipCoverage ClipContents::GetClipCoverage( case Entity::ClipOperation::kDifference: // This can be optimized further by considering cases when the bounds of // the current stencil will shrink. - return {.type = ClipCoverage::Type::kAppend, - .coverage = current_clip_coverage}; + return { + .type = ClipCoverage::Type::kAppend, // + .is_difference_or_non_square = true, // + .coverage = current_clip_coverage // + }; case Entity::ClipOperation::kIntersect: if (!geometry_) { return {.type = ClipCoverage::Type::kAppend, .coverage = std::nullopt}; @@ -64,8 +67,9 @@ Contents::ClipCoverage ClipContents::GetClipCoverage( return {.type = ClipCoverage::Type::kAppend, .coverage = std::nullopt}; } return { - .type = ClipCoverage::Type::kAppend, - .coverage = current_clip_coverage->Intersection(coverage.value()), + .type = ClipCoverage::Type::kAppend, // + .is_difference_or_non_square = !geometry_->IsAxisAlignedRect(), // + .coverage = current_clip_coverage->Intersection(coverage.value()), // }; } FML_UNREACHABLE(); @@ -91,19 +95,6 @@ bool ClipContents::Render(const ContentContext& renderer, using VS = ClipPipeline::VertexShader; - if (clip_op_ == Entity::ClipOperation::kIntersect && - geometry_->IsAxisAlignedRect() && - entity.GetTransform().IsTranslationScaleOnly()) { - std::optional coverage = - geometry_->GetCoverage(entity.GetTransform()); - if (coverage.has_value() && - coverage->Contains(Rect::MakeSize(pass.GetRenderTargetSize()))) { - // Skip axis-aligned intersect clips that cover the whole render target - // since they won't draw anything to the depth buffer. - return true; - } - } - VS::FrameInfo info; info.depth = GetShaderClipDepth(entity); diff --git a/impeller/entity/contents/contents.h b/impeller/entity/contents/contents.h index c26d1d56a49c8..8bffbc66a2951 100644 --- a/impeller/entity/contents/contents.h +++ b/impeller/entity/contents/contents.h @@ -38,6 +38,17 @@ class Contents { enum class Type { kNoChange, kAppend, kRestore }; Type type = Type::kNoChange; + // TODO(jonahwilliams): this should probably use the Entity::ClipOperation + // enum, but that has transitive import errors. + bool is_difference_or_non_square = false; + + /// @brief This coverage is the outer coverage of the clip. + /// + /// For example, if the clip is a circular clip, this is the rectangle that + /// contains the circle and not the rectangle that is contained within the + /// circle. This means that we cannot use the coverage alone to determine if + /// a clip can be culled, and instead also use the somewhat hacky + /// "is_difference_or_non_square" field. std::optional coverage = std::nullopt; }; diff --git a/impeller/entity/entity_pass_clip_stack.cc b/impeller/entity/entity_pass_clip_stack.cc index eab0565b3d382..17df6966315e5 100644 --- a/impeller/entity/entity_pass_clip_stack.cc +++ b/impeller/entity/entity_pass_clip_stack.cc @@ -60,7 +60,7 @@ EntityPassClipStack::ClipStateResult EntityPassClipStack::ApplyClipState( case Contents::ClipCoverage::Type::kNoChange: break; case Contents::ClipCoverage::Type::kAppend: { - auto op = CurrentClipCoverage(); + auto maybe_coverage = CurrentClipCoverage(); // Compute the previous clip height. size_t previous_clip_height = 0; @@ -72,6 +72,24 @@ EntityPassClipStack::ClipStateResult EntityPassClipStack::ApplyClipState( previous_clip_height = clip_height_floor; } + if (!maybe_coverage.has_value()) { + // Running this append op won't impact the clip buffer because the + // whole screen is already being clipped, so skip it. + return result; + } + auto op = maybe_coverage.value(); + + // If the new clip coverage is bigger than the existing coverage for + // intersect clips, we do not need to change the clip region. + if (!global_clip_coverage.is_difference_or_non_square && + global_clip_coverage.coverage.has_value() && + global_clip_coverage.coverage.value().Contains(op)) { + subpass_state.clip_coverage.push_back(ClipCoverageLayer{ + .coverage = op, .clip_height = previous_clip_height + 1}); + + return result; + } + subpass_state.clip_coverage.push_back( ClipCoverageLayer{.coverage = global_clip_coverage.coverage, .clip_height = previous_clip_height + 1}); @@ -81,11 +99,6 @@ EntityPassClipStack::ClipStateResult EntityPassClipStack::ApplyClipState( subpass_state.clip_coverage.front().clip_height + subpass_state.clip_coverage.size() - 1); - if (!op.has_value()) { - // Running this append op won't impact the clip buffer because the - // whole screen is already being clipped, so skip it. - return result; - } } break; case Contents::ClipCoverage::Type::kRestore: { ClipRestoreContents* restore_contents = diff --git a/impeller/entity/entity_pass_unittests.cc b/impeller/entity/entity_pass_unittests.cc index fd25599393a4e..2c24eab20c3fb 100644 --- a/impeller/entity/entity_pass_unittests.cc +++ b/impeller/entity/entity_pass_unittests.cc @@ -130,6 +130,115 @@ TEST(EntityPassClipStackTest, AppendAndRestoreClipCoverage) { EXPECT_EQ(recorder.GetReplayEntities().size(), 0u); } +// Append two clip coverages, the second is larger the first. This +// should result in the second clip not requiring any update. +TEST(EntityPassClipStackTest, AppendLargerClipCoverage) { + EntityPassClipStack recorder = + EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100)); + + ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u); + + // Push a clip. + Entity entity; + EntityPassClipStack::ClipStateResult result = recorder.ApplyClipState( + Contents::ClipCoverage{ + .type = Contents::ClipCoverage::Type::kAppend, + .coverage = Rect::MakeLTRB(50, 50, 55, 55), + }, + entity, 0, Point(0, 0)); + EXPECT_TRUE(result.should_render); + EXPECT_TRUE(result.clip_did_change); + + // Push a clip with larger coverage than the previous state. + result = recorder.ApplyClipState( + Contents::ClipCoverage{ + .type = Contents::ClipCoverage::Type::kAppend, + .coverage = Rect::MakeLTRB(0, 0, 100, 100), + }, + entity, 0, Point(0, 0)); + + EXPECT_FALSE(result.should_render); + EXPECT_FALSE(result.clip_did_change); +} + +// Since clip entities return the outer coverage we can only cull axis aligned +// rectangles and intersect clips. +TEST(EntityPassClipStackTest, + AppendLargerClipCoverageWithDifferenceOrNonSquare) { + EntityPassClipStack recorder = + EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100)); + + ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u); + + // Push a clip. + Entity entity; + EntityPassClipStack::ClipStateResult result = recorder.ApplyClipState( + Contents::ClipCoverage{ + .type = Contents::ClipCoverage::Type::kAppend, + .coverage = Rect::MakeLTRB(50, 50, 55, 55), + }, + entity, 0, Point(0, 0)); + EXPECT_TRUE(result.should_render); + EXPECT_TRUE(result.clip_did_change); + + // Push a clip with larger coverage than the previous state. + result = recorder.ApplyClipState( + Contents::ClipCoverage{ + .type = Contents::ClipCoverage::Type::kAppend, + .is_difference_or_non_square = true, + .coverage = Rect::MakeLTRB(0, 0, 100, 100), + }, + entity, 0, Point(0, 0)); + + EXPECT_TRUE(result.should_render); + EXPECT_TRUE(result.clip_did_change); +} + +TEST(EntityPassClipStackTest, AppendDecreasingSizeClipCoverage) { + EntityPassClipStack recorder = + EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100)); + + ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u); + + // Push Clips that shrink in size. All should be applied. + Entity entity; + + for (auto i = 1; i < 20; i++) { + EntityPassClipStack::ClipStateResult result = recorder.ApplyClipState( + Contents::ClipCoverage{ + .type = Contents::ClipCoverage::Type::kAppend, + .coverage = Rect::MakeLTRB(i, i, 100 - i, 100 - i), + }, + entity, 0, Point(0, 0)); + EXPECT_TRUE(result.should_render); + EXPECT_TRUE(result.clip_did_change); + EXPECT_EQ(recorder.CurrentClipCoverage(), + Rect::MakeLTRB(i, i, 100 - i, 100 - i)); + } +} + +TEST(EntityPassClipStackTest, AppendIncreasingSizeClipCoverage) { + EntityPassClipStack recorder = + EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100)); + + ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u); + + // Push Clips that grow in size. All should be skipped. + Entity entity; + + for (auto i = 1; i < 20; i++) { + EntityPassClipStack::ClipStateResult result = recorder.ApplyClipState( + Contents::ClipCoverage{ + .type = Contents::ClipCoverage::Type::kAppend, + .coverage = Rect::MakeLTRB(0 - i, 0 - i, 100 + i, 100 + i), + }, + entity, 0, Point(0, 0)); + EXPECT_FALSE(result.should_render); + EXPECT_FALSE(result.clip_did_change); + EXPECT_EQ(recorder.CurrentClipCoverage(), Rect::MakeLTRB(0, 0, 100, 100)); + } +} + TEST(EntityPassClipStackTest, UnbalancedRestore) { EntityPassClipStack recorder = EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100));