From 97160d4db50fc8e7d7a37babfd5ff85c3dd16f16 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 30 Aug 2024 20:20:39 -0700 Subject: [PATCH 01/20] [Impeller] opt into exp canvas. --- common/config.gni | 2 +- impeller/aiks/experimental_canvas.cc | 28 ++++++++++++++++++++-------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/common/config.gni b/common/config.gni index 4f8f690811c38..6c2354e7c9128 100644 --- a/common/config.gni +++ b/common/config.gni @@ -37,7 +37,7 @@ declare_args() { slimpeller = false # Opt into new DL dispatcher that skips AIKS layer - experimental_canvas = false + experimental_canvas = true } # feature_defines_list --------------------------------------------------------- diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index 0ea88adc32511..b7123bc090c0e 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -346,6 +346,14 @@ void ExperimentalCanvas::SaveLayer( Save(total_content_depth); return; } + + maybe_coverage_limit = maybe_coverage_limit->Intersection( + Rect::MakeSize(render_target_.GetRenderTargetSize())); + if (!maybe_coverage_limit.has_value()) { + Save(total_content_depth); + return; + } + auto coverage_limit = maybe_coverage_limit.value(); if (can_distribute_opacity && !backdrop_filter && @@ -371,12 +379,16 @@ void ExperimentalCanvas::SaveLayer( /*flood_input_coverage=*/!!backdrop_filter // ); - if (!maybe_subpass_coverage.has_value() || - maybe_subpass_coverage->IsEmpty()) { + if (!maybe_subpass_coverage.has_value()) { Save(total_content_depth); return; } auto subpass_coverage = maybe_subpass_coverage.value(); + auto subpass_size = ISize(subpass_coverage.GetSize()); + if (subpass_size.IsEmpty()) { + Save(total_content_depth); + return; + } // Backdrop filter state, ignored if there is no BDF. std::shared_ptr backdrop_filter_contents; @@ -419,12 +431,12 @@ void ExperimentalCanvas::SaveLayer( paint_copy.color.alpha *= transform_stack_.back().distributed_opacity; transform_stack_.back().distributed_opacity = 1.0; - render_passes_.push_back(LazyRenderingConfig( - renderer_, // - CreateRenderTarget(renderer_, // - ISize(subpass_coverage.GetSize()), // - Color::BlackTransparent() // - ))); + render_passes_.push_back( + LazyRenderingConfig(renderer_, // + CreateRenderTarget(renderer_, // + subpass_size, // + Color::BlackTransparent() // + ))); save_layer_state_.push_back(SaveLayerState{paint_copy, subpass_coverage}); CanvasStackEntry entry; From dd06f920c7c616fa27f8cf2a42bad987c0f0820e Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 30 Aug 2024 22:01:56 -0700 Subject: [PATCH 02/20] ++ --- impeller/aiks/canvas.h | 1 + impeller/aiks/experimental_canvas.cc | 66 +++++++++++++++++----------- impeller/aiks/experimental_canvas.h | 7 +++ 3 files changed, 49 insertions(+), 25 deletions(-) diff --git a/impeller/aiks/canvas.h b/impeller/aiks/canvas.h index dbc3a1952dd73..84e31615afbb2 100644 --- a/impeller/aiks/canvas.h +++ b/impeller/aiks/canvas.h @@ -37,6 +37,7 @@ struct CanvasStackEntry { size_t num_clips = 0u; Scalar distributed_opacity = 1.0f; Entity::RenderingMode rendering_mode = Entity::RenderingMode::kDirect; + bool skipping = true; }; enum class PointStyle { diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index b7123bc090c0e..955a485f0c206 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -296,7 +296,17 @@ void ExperimentalCanvas::SetupRenderPass() { } } +void ExperimentalCanvas::SkipUntilMatchingRestore() { + auto entry = CanvasStackEntry{}; + entry.skipping = true; + transform_stack_.push_back(entry); +} + void ExperimentalCanvas::Save(uint32_t total_content_depth) { + if (IsSkipping()) { + return SkipUntilMatchingRestore(); + } + auto entry = CanvasStackEntry{}; entry.transform = transform_stack_.back().transform; entry.cull_rect = transform_stack_.back().cull_rect; @@ -307,7 +317,7 @@ void ExperimentalCanvas::Save(uint32_t total_content_depth) { << " after allocating " << total_content_depth; entry.clip_height = transform_stack_.back().clip_height; entry.rendering_mode = Entity::RenderingMode::kDirect; - transform_stack_.emplace_back(entry); + transform_stack_.push_back(entry); } void ExperimentalCanvas::SaveLayer( @@ -322,14 +332,12 @@ void ExperimentalCanvas::SaveLayer( if (!clip_coverage_stack_.HasCoverage()) { // The current clip is empty. This means the pass texture won't be // visible, so skip it. - Save(total_content_depth); - return; + return SkipUntilMatchingRestore(); } auto maybe_current_clip_coverage = clip_coverage_stack_.CurrentClipCoverage(); if (!maybe_current_clip_coverage.has_value()) { - Save(total_content_depth); - return; + return SkipUntilMatchingRestore(); } auto current_clip_coverage = maybe_current_clip_coverage.value(); @@ -343,15 +351,13 @@ void ExperimentalCanvas::SaveLayer( .Intersection(current_clip_coverage); if (!maybe_coverage_limit.has_value() || maybe_coverage_limit->IsEmpty()) { - Save(total_content_depth); - return; + return SkipUntilMatchingRestore(); } maybe_coverage_limit = maybe_coverage_limit->Intersection( Rect::MakeSize(render_target_.GetRenderTargetSize())); if (!maybe_coverage_limit.has_value()) { - Save(total_content_depth); - return; + return SkipUntilMatchingRestore(); } auto coverage_limit = maybe_coverage_limit.value(); @@ -364,10 +370,9 @@ void ExperimentalCanvas::SaveLayer( return; } - std::shared_ptr filter_contents; - if (paint.image_filter) { - filter_contents = paint.image_filter->GetFilterContents(); - } + std::shared_ptr filter_contents = paint.WithImageFilter( + Rect(), transform_stack_.back().transform, + Entity::RenderingMode::kSubpassPrependSnapshotTransform); std::optional maybe_subpass_coverage = ComputeSaveLayerCoverage( bounds.value_or(Rect::MakeMaximum()), @@ -380,14 +385,12 @@ void ExperimentalCanvas::SaveLayer( ); if (!maybe_subpass_coverage.has_value()) { - Save(total_content_depth); - return; + return SkipUntilMatchingRestore(); } auto subpass_coverage = maybe_subpass_coverage.value(); auto subpass_size = ISize(subpass_coverage.GetSize()); if (subpass_size.IsEmpty()) { - Save(total_content_depth); - return; + return SkipUntilMatchingRestore(); } // Backdrop filter state, ignored if there is no BDF. @@ -405,11 +408,11 @@ void ExperimentalCanvas::SaveLayer( return filter; }; - auto input_texture = FlipBackdrop(render_passes_, // - GetGlobalPassPosition(), // - std::numeric_limits::max(), // - clip_coverage_stack_, // - renderer_ // + auto input_texture = FlipBackdrop(render_passes_, // + GetGlobalPassPosition(), // + current_depth_, // + clip_coverage_stack_, // + renderer_ // ); if (!input_texture) { // Validation failures are logged in FlipBackdrop. @@ -483,6 +486,11 @@ bool ExperimentalCanvas::Restore() { return false; } + if (IsSkipping()) { + transform_stack_.pop_back(); + return true; + } + // This check is important to make sure we didn't exceed the depth // that the clips were rendered at while rendering any of the // rendering ops. It is OK for the current depth to equal the @@ -511,12 +519,13 @@ bool ExperimentalCanvas::Restore() { SaveLayerState save_layer_state = save_layer_state_.back(); save_layer_state_.pop_back(); + auto global_pass_position = GetGlobalPassPosition(); std::shared_ptr contents = PaintPassDelegate(save_layer_state.paint) .CreateContentsForSubpassTarget( lazy_render_pass.inline_pass_context->GetTexture(), - Matrix::MakeTranslation(Vector3{-GetGlobalPassPosition()}) * + Matrix::MakeTranslation(Vector3{global_pass_position}) * transform_stack_.back().transform); lazy_render_pass.inline_pass_context->EndPass(); @@ -534,8 +543,7 @@ bool ExperimentalCanvas::Restore() { // // See also this bug: https://github.com/flutter/flutter/issues/144213 Point subpass_texture_position = - (save_layer_state.coverage.GetOrigin() - GetGlobalPassPosition()) - .Round(); + (save_layer_state.coverage.GetOrigin() - global_pass_position).Round(); Entity element_entity; element_entity.SetClipDepth(++current_depth_); @@ -682,6 +690,10 @@ void ExperimentalCanvas::DrawTextFrame( void ExperimentalCanvas::AddRenderEntityToCurrentPass(Entity entity, bool reuse_depth) { + if (IsSkipping()) { + return; + } + entity.SetTransform( Matrix::MakeTranslation(Vector3(-GetGlobalPassPosition())) * entity.GetTransform()); @@ -775,6 +787,10 @@ void ExperimentalCanvas::AddRenderEntityToCurrentPass(Entity entity, } void ExperimentalCanvas::AddClipEntityToCurrentPass(Entity entity) { + if (IsSkipping()) { + return; + } + auto transform = entity.GetTransform(); entity.SetTransform( Matrix::MakeTranslation(Vector3(-GetGlobalPassPosition())) * transform); diff --git a/impeller/aiks/experimental_canvas.h b/impeller/aiks/experimental_canvas.h index debb4baf088fc..7b251f541bfc5 100644 --- a/impeller/aiks/experimental_canvas.h +++ b/impeller/aiks/experimental_canvas.h @@ -95,6 +95,13 @@ class ExperimentalCanvas : public Canvas { return 0; } + bool IsSkipping() { + return transform_stack_.back().skipping; + } + + // Increment on SaveLayers that should be skipped, decrement on restores. + void SkipUntilMatchingRestore(); + ContentContext& renderer_; RenderTarget& render_target_; const bool requires_readback_; From 5181dd12d0587b8a14b24e546d78a7ed9b1f251f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 31 Aug 2024 08:11:33 -0700 Subject: [PATCH 03/20] ++ --- impeller/aiks/experimental_canvas.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/impeller/aiks/experimental_canvas.h b/impeller/aiks/experimental_canvas.h index 7b251f541bfc5..6a556149451f7 100644 --- a/impeller/aiks/experimental_canvas.h +++ b/impeller/aiks/experimental_canvas.h @@ -95,11 +95,11 @@ class ExperimentalCanvas : public Canvas { return 0; } - bool IsSkipping() { - return transform_stack_.back().skipping; - } + /// @brief Whether all entites should be skipped until a corresponding + /// restore. + bool IsSkipping() { return transform_stack_.back().skipping; } - // Increment on SaveLayers that should be skipped, decrement on restores. + /// @brief Skip all rendering/clipping entities until next restore. void SkipUntilMatchingRestore(); ContentContext& renderer_; From 9468843dda42601a55733d8cfaf3234f78a27db7 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 31 Aug 2024 09:14:34 -0700 Subject: [PATCH 04/20] [impeller] fixes for subpass skipping. --- impeller/aiks/experimental_canvas.cc | 39 ++++++++++++++++++---------- impeller/aiks/experimental_canvas.h | 5 +++- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index 955a485f0c206..73b8fe23e5933 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -4,6 +4,7 @@ #include "impeller/aiks/experimental_canvas.h" #include +#include #include "fml/logging.h" #include "fml/trace_event.h" #include "impeller/aiks/canvas.h" @@ -320,25 +321,18 @@ void ExperimentalCanvas::Save(uint32_t total_content_depth) { transform_stack_.push_back(entry); } -void ExperimentalCanvas::SaveLayer( - const Paint& paint, - std::optional bounds, - const std::shared_ptr& backdrop_filter, - ContentBoundsPromise bounds_promise, - uint32_t total_content_depth, - bool can_distribute_opacity) { - TRACE_EVENT0("flutter", "Canvas::saveLayer"); - +std::optional ExperimentalCanvas::ComputeCoverageLimit() const { if (!clip_coverage_stack_.HasCoverage()) { // The current clip is empty. This means the pass texture won't be // visible, so skip it. - return SkipUntilMatchingRestore(); + return std::nullopt; } auto maybe_current_clip_coverage = clip_coverage_stack_.CurrentClipCoverage(); if (!maybe_current_clip_coverage.has_value()) { - return SkipUntilMatchingRestore(); + return std::nullopt; } + auto current_clip_coverage = maybe_current_clip_coverage.value(); // The maximum coverage of the subpass. Subpasses textures should never @@ -351,15 +345,29 @@ void ExperimentalCanvas::SaveLayer( .Intersection(current_clip_coverage); if (!maybe_coverage_limit.has_value() || maybe_coverage_limit->IsEmpty()) { - return SkipUntilMatchingRestore(); + return std::nullopt; } - maybe_coverage_limit = maybe_coverage_limit->Intersection( + return maybe_coverage_limit->Intersection( Rect::MakeSize(render_target_.GetRenderTargetSize())); - if (!maybe_coverage_limit.has_value()) { +} + +void ExperimentalCanvas::SaveLayer( + const Paint& paint, + std::optional bounds, + const std::shared_ptr& backdrop_filter, + ContentBoundsPromise bounds_promise, + uint32_t total_content_depth, + bool can_distribute_opacity) { + TRACE_EVENT0("flutter", "Canvas::saveLayer"); + if (IsSkipping()) { return SkipUntilMatchingRestore(); } + auto maybe_coverage_limit = ComputeCoverageLimit(); + if (!maybe_coverage_limit.has_value()) { + return SkipUntilMatchingRestore(); + } auto coverage_limit = maybe_coverage_limit.value(); if (can_distribute_opacity && !backdrop_filter && @@ -387,6 +395,9 @@ void ExperimentalCanvas::SaveLayer( if (!maybe_subpass_coverage.has_value()) { return SkipUntilMatchingRestore(); } + + // TODO(jonahwilliams): this should round out if there are no image + // filters. auto subpass_coverage = maybe_subpass_coverage.value(); auto subpass_size = ISize(subpass_coverage.GetSize()); if (subpass_size.IsEmpty()) { diff --git a/impeller/aiks/experimental_canvas.h b/impeller/aiks/experimental_canvas.h index 6a556149451f7..55defb2c46c72 100644 --- a/impeller/aiks/experimental_canvas.h +++ b/impeller/aiks/experimental_canvas.h @@ -87,6 +87,9 @@ class ExperimentalCanvas : public Canvas { }; private: + /// @brief Compute the current coverage limit in screen space, or std::nullopt. + std::optional ComputeCoverageLimit() const; + // clip depth of the previous save or 0. size_t GetClipHeightFloor() const { if (transform_stack_.size() > 1) { @@ -115,7 +118,7 @@ class ExperimentalCanvas : public Canvas { void AddClipEntityToCurrentPass(Entity entity) override; bool BlitToOnscreen(); - Point GetGlobalPassPosition() { + Point GetGlobalPassPosition() const { if (save_layer_state_.empty()) { return Point(0, 0); } From e2f5268e08ec1e183a3c73924f22a5e50b5c7582 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 31 Aug 2024 09:15:39 -0700 Subject: [PATCH 05/20] ++ --- impeller/aiks/experimental_canvas.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/impeller/aiks/experimental_canvas.h b/impeller/aiks/experimental_canvas.h index 55defb2c46c72..dbef441422fa6 100644 --- a/impeller/aiks/experimental_canvas.h +++ b/impeller/aiks/experimental_canvas.h @@ -87,7 +87,8 @@ class ExperimentalCanvas : public Canvas { }; private: - /// @brief Compute the current coverage limit in screen space, or std::nullopt. + /// @brief Compute the current coverage limit in screen space, or + /// std::nullopt. std::optional ComputeCoverageLimit() const; // clip depth of the previous save or 0. From d198704511feea24abe2440d8cba03ecca348e21 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 31 Aug 2024 09:44:27 -0700 Subject: [PATCH 06/20] better default. --- impeller/aiks/canvas.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/impeller/aiks/canvas.h b/impeller/aiks/canvas.h index 84e31615afbb2..5d3e27f1561be 100644 --- a/impeller/aiks/canvas.h +++ b/impeller/aiks/canvas.h @@ -37,7 +37,8 @@ struct CanvasStackEntry { size_t num_clips = 0u; Scalar distributed_opacity = 1.0f; Entity::RenderingMode rendering_mode = Entity::RenderingMode::kDirect; - bool skipping = true; + // Whether all entities in the current save should be skipped. + bool skipping = false; }; enum class PointStyle { From 0292df8bc85d138fef4fdc9e7b84da6fd6b5c8b4 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 31 Aug 2024 11:45:37 -0700 Subject: [PATCH 07/20] fix offset alignment. --- impeller/aiks/experimental_canvas.cc | 25 +++++++++++++------------ impeller/aiks/experimental_canvas.h | 2 +- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index 73b8fe23e5933..827ea75a6bb37 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -297,15 +297,16 @@ void ExperimentalCanvas::SetupRenderPass() { } } -void ExperimentalCanvas::SkipUntilMatchingRestore() { +void ExperimentalCanvas::SkipUntilMatchingRestore(size_t total_content_depth) { auto entry = CanvasStackEntry{}; entry.skipping = true; + entry.clip_depth = current_depth_ + total_content_depth; transform_stack_.push_back(entry); } void ExperimentalCanvas::Save(uint32_t total_content_depth) { if (IsSkipping()) { - return SkipUntilMatchingRestore(); + return SkipUntilMatchingRestore(total_content_depth); } auto entry = CanvasStackEntry{}; @@ -361,12 +362,12 @@ void ExperimentalCanvas::SaveLayer( bool can_distribute_opacity) { TRACE_EVENT0("flutter", "Canvas::saveLayer"); if (IsSkipping()) { - return SkipUntilMatchingRestore(); + return SkipUntilMatchingRestore(total_content_depth); } auto maybe_coverage_limit = ComputeCoverageLimit(); if (!maybe_coverage_limit.has_value()) { - return SkipUntilMatchingRestore(); + return SkipUntilMatchingRestore(total_content_depth); } auto coverage_limit = maybe_coverage_limit.value(); @@ -393,7 +394,7 @@ void ExperimentalCanvas::SaveLayer( ); if (!maybe_subpass_coverage.has_value()) { - return SkipUntilMatchingRestore(); + return SkipUntilMatchingRestore(total_content_depth); } // TODO(jonahwilliams): this should round out if there are no image @@ -401,7 +402,7 @@ void ExperimentalCanvas::SaveLayer( auto subpass_coverage = maybe_subpass_coverage.value(); auto subpass_size = ISize(subpass_coverage.GetSize()); if (subpass_size.IsEmpty()) { - return SkipUntilMatchingRestore(); + return SkipUntilMatchingRestore(total_content_depth); } // Backdrop filter state, ignored if there is no BDF. @@ -497,11 +498,6 @@ bool ExperimentalCanvas::Restore() { return false; } - if (IsSkipping()) { - transform_stack_.pop_back(); - return true; - } - // This check is important to make sure we didn't exceed the depth // that the clips were rendered at while rendering any of the // rendering ops. It is OK for the current depth to equal the @@ -519,6 +515,11 @@ bool ExperimentalCanvas::Restore() { << current_depth_ << " <=? " << transform_stack_.back().clip_depth; current_depth_ = transform_stack_.back().clip_depth; + if (IsSkipping()) { + transform_stack_.pop_back(); + return true; + } + if (transform_stack_.back().rendering_mode == Entity::RenderingMode::kSubpassAppendSnapshotTransform || transform_stack_.back().rendering_mode == @@ -536,7 +537,7 @@ bool ExperimentalCanvas::Restore() { PaintPassDelegate(save_layer_state.paint) .CreateContentsForSubpassTarget( lazy_render_pass.inline_pass_context->GetTexture(), - Matrix::MakeTranslation(Vector3{global_pass_position}) * + Matrix::MakeTranslation(Vector3{-global_pass_position}) * transform_stack_.back().transform); lazy_render_pass.inline_pass_context->EndPass(); diff --git a/impeller/aiks/experimental_canvas.h b/impeller/aiks/experimental_canvas.h index dbef441422fa6..fc15aee921167 100644 --- a/impeller/aiks/experimental_canvas.h +++ b/impeller/aiks/experimental_canvas.h @@ -104,7 +104,7 @@ class ExperimentalCanvas : public Canvas { bool IsSkipping() { return transform_stack_.back().skipping; } /// @brief Skip all rendering/clipping entities until next restore. - void SkipUntilMatchingRestore(); + void SkipUntilMatchingRestore(size_t total_content_depth); ContentContext& renderer_; RenderTarget& render_target_; From bf7a3964a0e37bbb793f6fb2dbd97e8cfda0719f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 31 Aug 2024 13:11:36 -0700 Subject: [PATCH 08/20] ++ --- impeller/aiks/experimental_canvas.cc | 68 ++++++++++++++++------------ impeller/aiks/experimental_canvas.h | 4 ++ 2 files changed, 43 insertions(+), 29 deletions(-) diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index 827ea75a6bb37..fcc7c712ac1b4 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -62,7 +62,6 @@ static void ApplyFramebufferBlend(Entity& entity) { static std::shared_ptr FlipBackdrop( std::vector& render_passes, Point global_pass_position, - size_t current_clip_depth, EntityPassClipStack& clip_coverage_stack, ContentContext& renderer) { auto rendering_config = std::move(render_passes.back()); @@ -140,22 +139,6 @@ static std::shared_ptr FlipBackdrop( // applied. clip_coverage_stack.ActivateClipReplay(); - // If there are any pending clips to replay, render any that may affect - // the entity we're about to render. - while (const EntityPassClipStack::ReplayResult* next_replay_clip = - clip_coverage_stack.GetNextReplayResult(current_clip_depth)) { - auto& replay_entity = next_replay_clip->entity; - SetClipScissor( - next_replay_clip->clip_coverage, - *render_passes.back().inline_pass_context->GetRenderPass(0).pass, - global_pass_position); - if (!replay_entity.Render( - renderer, - *render_passes.back().inline_pass_context->GetRenderPass(0).pass)) { - VALIDATION_LOG << "Failed to render entity for clip replay."; - } - } - return input_texture; } @@ -397,10 +380,17 @@ void ExperimentalCanvas::SaveLayer( return SkipUntilMatchingRestore(total_content_depth); } - // TODO(jonahwilliams): this should round out if there are no image - // filters. auto subpass_coverage = maybe_subpass_coverage.value(); - auto subpass_size = ISize(subpass_coverage.GetSize()); + + // When an image filter is present, clamp to avoid flicking due to nearest + // sampled image. For other cases, round out to ensure than any geometry is + // not cut off. + ISize subpass_size; + if (paint.image_filter) { + subpass_size = ISize(subpass_coverage.GetSize()); + } else { + subpass_size = ISize(IRect::RoundOut(subpass_coverage).GetSize()); + } if (subpass_size.IsEmpty()) { return SkipUntilMatchingRestore(total_content_depth); } @@ -422,7 +412,6 @@ void ExperimentalCanvas::SaveLayer( auto input_texture = FlipBackdrop(render_passes_, // GetGlobalPassPosition(), // - current_depth_, // clip_coverage_stack_, // renderer_ // ); @@ -550,8 +539,9 @@ bool ExperimentalCanvas::Restore() { // // We do this in lieu of expanding/rounding out the subpass coverage in // order to keep the bounds wrapping consistently tight around subpass - // elements. Which is necessary to avoid intense flickering in cases - // where a subpass texture has a large blur filter with clamp sampling. + // elements when there are image filters. Which is necessary to avoid + // intense flickering in cases where a subpass texture has a large blur + // filter with clamp sampling. // // See also this bug: https://github.com/flutter/flutter/issues/144213 Point subpass_texture_position = @@ -577,9 +567,9 @@ bool ExperimentalCanvas::Restore() { // to the render target texture so far need to execute before it's bound // for blending (otherwise the blend pass will end up executing before // all the previous commands in the active pass). - auto input_texture = FlipBackdrop( - render_passes_, GetGlobalPassPosition(), - element_entity.GetClipDepth(), clip_coverage_stack_, renderer_); + auto input_texture = + FlipBackdrop(render_passes_, GetGlobalPassPosition(), + clip_coverage_stack_, renderer_); if (!input_texture) { return false; } @@ -705,6 +695,7 @@ void ExperimentalCanvas::AddRenderEntityToCurrentPass(Entity entity, if (IsSkipping()) { return; } + RenderPendingClips(); entity.SetTransform( Matrix::MakeTranslation(Vector3(-GetGlobalPassPosition())) * @@ -761,9 +752,8 @@ void ExperimentalCanvas::AddRenderEntityToCurrentPass(Entity entity, // to the render target texture so far need to execute before it's bound // for blending (otherwise the blend pass will end up executing before // all the previous commands in the active pass). - auto input_texture = - FlipBackdrop(render_passes_, GetGlobalPassPosition(), - entity.GetClipDepth(), clip_coverage_stack_, renderer_); + auto input_texture = FlipBackdrop(render_passes_, GetGlobalPassPosition(), + clip_coverage_stack_, renderer_); if (!input_texture) { return; } @@ -856,6 +846,26 @@ void ExperimentalCanvas::AddClipEntityToCurrentPass(Entity entity) { *render_passes_.back().inline_pass_context->GetRenderPass(0).pass); } +void ExperimentalCanvas::RenderPendingClips() { + // If there are any pending clips to replay, render any that may affect + // the entity we're about to render. + while (const EntityPassClipStack::ReplayResult* next_replay_clip = + clip_coverage_stack_.GetNextReplayResult(current_depth_)) { + auto& replay_entity = next_replay_clip->entity; + + SetClipScissor( + next_replay_clip->clip_coverage, + *render_passes_.back().inline_pass_context->GetRenderPass(0).pass, + GetGlobalPassPosition()); + if (!replay_entity.Render(renderer_, + *render_passes_.back() + .inline_pass_context->GetRenderPass(0) + .pass)) { + VALIDATION_LOG << "Failed to render entity for clip replay."; + } + } +} + bool ExperimentalCanvas::BlitToOnscreen() { auto command_buffer = renderer_.GetContext()->CreateCommandBuffer(); command_buffer->SetLabel("EntityPass Root Command Buffer"); diff --git a/impeller/aiks/experimental_canvas.h b/impeller/aiks/experimental_canvas.h index fc15aee921167..cf554d8f203aa 100644 --- a/impeller/aiks/experimental_canvas.h +++ b/impeller/aiks/experimental_canvas.h @@ -91,6 +91,10 @@ class ExperimentalCanvas : public Canvas { /// std::nullopt. std::optional ComputeCoverageLimit() const; + /// @brief After flipping the backdrop texture, render any clips that effect the current + /// entity based on depth. + void RenderPendingClips(); + // clip depth of the previous save or 0. size_t GetClipHeightFloor() const { if (transform_stack_.size() > 1) { From 08f6361f5f3c550b900614b9df6eb4491c6e30cc Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 31 Aug 2024 13:12:40 -0700 Subject: [PATCH 09/20] ++ --- impeller/aiks/experimental_canvas.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/impeller/aiks/experimental_canvas.h b/impeller/aiks/experimental_canvas.h index cf554d8f203aa..413835707b7ef 100644 --- a/impeller/aiks/experimental_canvas.h +++ b/impeller/aiks/experimental_canvas.h @@ -91,8 +91,8 @@ class ExperimentalCanvas : public Canvas { /// std::nullopt. std::optional ComputeCoverageLimit() const; - /// @brief After flipping the backdrop texture, render any clips that effect the current - /// entity based on depth. + /// @brief After flipping the backdrop texture, render any clips that effect + /// the current entity based on depth. void RenderPendingClips(); // clip depth of the previous save or 0. @@ -104,7 +104,7 @@ class ExperimentalCanvas : public Canvas { } /// @brief Whether all entites should be skipped until a corresponding - /// restore. + /// restore. bool IsSkipping() { return transform_stack_.back().skipping; } /// @brief Skip all rendering/clipping entities until next restore. From c85e2b40ec807a46b9a8a5a0ed2186954c6217c3 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 1 Sep 2024 11:33:25 -0700 Subject: [PATCH 10/20] ++ --- impeller/aiks/experimental_canvas.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index fcc7c712ac1b4..cace5b807d58c 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -478,6 +478,8 @@ void ExperimentalCanvas::SaveLayer( backdrop_entity.Render( renderer_, *render_passes_.back().inline_pass_context->GetRenderPass(0).pass); + + RenderPendingClips(); } } From df4e5e584db2b970b0c745a03595ed6ff62e75c3 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 2 Sep 2024 10:51:48 -0700 Subject: [PATCH 11/20] ++ --- impeller/aiks/experimental_canvas.cc | 9 +++++---- impeller/aiks/experimental_canvas.h | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index cace5b807d58c..ffda1e3068dab 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -419,6 +419,7 @@ void ExperimentalCanvas::SaveLayer( // Validation failures are logged in FlipBackdrop. return; } + FlushPendingClips(); backdrop_filter_contents = backdrop_filter_proc( FilterInput::Make(std::move(input_texture)), @@ -478,8 +479,6 @@ void ExperimentalCanvas::SaveLayer( backdrop_entity.Render( renderer_, *render_passes_.back().inline_pass_context->GetRenderPass(0).pass); - - RenderPendingClips(); } } @@ -697,7 +696,7 @@ void ExperimentalCanvas::AddRenderEntityToCurrentPass(Entity entity, if (IsSkipping()) { return; } - RenderPendingClips(); + FlushPendingClips(); entity.SetTransform( Matrix::MakeTranslation(Vector3(-GetGlobalPassPosition())) * @@ -760,6 +759,8 @@ void ExperimentalCanvas::AddRenderEntityToCurrentPass(Entity entity, return; } + FlushPendingClips(); + // The coverage hint tells the rendered Contents which portion of the // rendered output will actually be used, and so we set this to the // current clip coverage (which is the max clip bounds). The contents may @@ -848,7 +849,7 @@ void ExperimentalCanvas::AddClipEntityToCurrentPass(Entity entity) { *render_passes_.back().inline_pass_context->GetRenderPass(0).pass); } -void ExperimentalCanvas::RenderPendingClips() { +void ExperimentalCanvas::FlushPendingClips() { // If there are any pending clips to replay, render any that may affect // the entity we're about to render. while (const EntityPassClipStack::ReplayResult* next_replay_clip = diff --git a/impeller/aiks/experimental_canvas.h b/impeller/aiks/experimental_canvas.h index 413835707b7ef..6330d0b836380 100644 --- a/impeller/aiks/experimental_canvas.h +++ b/impeller/aiks/experimental_canvas.h @@ -93,7 +93,7 @@ class ExperimentalCanvas : public Canvas { /// @brief After flipping the backdrop texture, render any clips that effect /// the current entity based on depth. - void RenderPendingClips(); + void FlushPendingClips(); // clip depth of the previous save or 0. size_t GetClipHeightFloor() const { From b89405960fe03bdad8253a0c15cd3cb3680e0320 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 3 Sep 2024 11:36:41 -0700 Subject: [PATCH 12/20] adjust flush location. --- impeller/aiks/experimental_canvas.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index ffda1e3068dab..cf5333bf9b491 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -419,7 +419,6 @@ void ExperimentalCanvas::SaveLayer( // Validation failures are logged in FlipBackdrop. return; } - FlushPendingClips(); backdrop_filter_contents = backdrop_filter_proc( FilterInput::Make(std::move(input_texture)), @@ -469,6 +468,8 @@ void ExperimentalCanvas::SaveLayer( clip_coverage_stack_.PushSubpass(subpass_coverage, GetClipHeight()); if (backdrop_filter_contents) { + FlushPendingClips(); + // Render the backdrop entity. Entity backdrop_entity; backdrop_entity.SetContents(std::move(backdrop_filter_contents)); From f50102e56145dbc40d83050410e91a73c84347a6 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 3 Sep 2024 19:49:51 -0700 Subject: [PATCH 13/20] ++ --- impeller/aiks/experimental_canvas.cc | 4 +--- impeller/display_list/dl_dispatcher.cc | 8 +++++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index cf5333bf9b491..8ac1bf73e4b82 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -419,6 +419,7 @@ void ExperimentalCanvas::SaveLayer( // Validation failures are logged in FlipBackdrop. return; } + FlushPendingClips(); backdrop_filter_contents = backdrop_filter_proc( FilterInput::Make(std::move(input_texture)), @@ -468,8 +469,6 @@ void ExperimentalCanvas::SaveLayer( clip_coverage_stack_.PushSubpass(subpass_coverage, GetClipHeight()); if (backdrop_filter_contents) { - FlushPendingClips(); - // Render the backdrop entity. Entity backdrop_entity; backdrop_entity.SetContents(std::move(backdrop_filter_contents)); @@ -759,7 +758,6 @@ void ExperimentalCanvas::AddRenderEntityToCurrentPass(Entity entity, if (!input_texture) { return; } - FlushPendingClips(); // The coverage hint tells the rendered Contents which portion of the diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index 18be21f0a4999..97a49038aeb36 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/impeller/display_list/dl_dispatcher.cc @@ -54,8 +54,9 @@ namespace impeller { // The watchdog object allocated here will automatically double-check // the depth usage at any exit point to the function, or any other // point at which it falls out of scope. -#define AUTO_DEPTH_WATCHER(d) \ - DepthWatcher _watcher(__FILE__, __LINE__, GetCanvas(), d) +#define AUTO_DEPTH_WATCHER(d) \ + DepthWatcher _watcher(__FILE__, __LINE__, GetCanvas(), \ + paint_.mask_blur_descriptor.has_value(), d) // While the AUTO_DEPTH_WATCHER macro will check the depth usage at // any exit point from the dispatch function, sometimes the dispatch @@ -75,11 +76,12 @@ struct DepthWatcher { DepthWatcher(const std::string& file, int line, const impeller::Canvas& canvas, + bool has_mask_blur, int allowed) : file_(file), line_(line), canvas_(canvas), - allowed_(allowed), + allowed_(has_mask_blur ? allowed : (allowed + 1)), old_depth_(canvas.GetOpDepth()), old_max_(canvas.GetMaxOpDepth()) {} From be9be268b0bbf909c9a1f09273e18c09bfae657a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 4 Sep 2024 18:02:33 -0700 Subject: [PATCH 14/20] back to old clip stack. --- impeller/aiks/experimental_canvas.cc | 38 ++++++++++------------------ impeller/aiks/experimental_canvas.h | 4 --- 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index 8ac1bf73e4b82..f16ca562121fe 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -3,8 +3,10 @@ // found in the LICENSE file. #include "impeller/aiks/experimental_canvas.h" + #include #include + #include "fml/logging.h" #include "fml/trace_event.h" #include "impeller/aiks/canvas.h" @@ -137,7 +139,18 @@ static std::shared_ptr FlipBackdrop( // Restore any clips that were recorded before the backdrop filter was // applied. - clip_coverage_stack.ActivateClipReplay(); + auto& replay_entities = clip_coverage_stack.GetReplayEntities(); + for (const auto& replay : replay_entities) { + SetClipScissor( + clip_coverage_stack.CurrentClipCoverage(), + *render_passes.back().inline_pass_context->GetRenderPass(0).pass, + global_pass_position); + if (!replay.entity.Render( + renderer, + *render_passes.back().inline_pass_context->GetRenderPass(0).pass)) { + VALIDATION_LOG << "Failed to render entity for clip restore."; + } + } return input_texture; } @@ -419,7 +432,6 @@ void ExperimentalCanvas::SaveLayer( // Validation failures are logged in FlipBackdrop. return; } - FlushPendingClips(); backdrop_filter_contents = backdrop_filter_proc( FilterInput::Make(std::move(input_texture)), @@ -696,7 +708,6 @@ void ExperimentalCanvas::AddRenderEntityToCurrentPass(Entity entity, if (IsSkipping()) { return; } - FlushPendingClips(); entity.SetTransform( Matrix::MakeTranslation(Vector3(-GetGlobalPassPosition())) * @@ -758,7 +769,6 @@ void ExperimentalCanvas::AddRenderEntityToCurrentPass(Entity entity, if (!input_texture) { return; } - FlushPendingClips(); // The coverage hint tells the rendered Contents which portion of the // rendered output will actually be used, and so we set this to the @@ -848,26 +858,6 @@ void ExperimentalCanvas::AddClipEntityToCurrentPass(Entity entity) { *render_passes_.back().inline_pass_context->GetRenderPass(0).pass); } -void ExperimentalCanvas::FlushPendingClips() { - // If there are any pending clips to replay, render any that may affect - // the entity we're about to render. - while (const EntityPassClipStack::ReplayResult* next_replay_clip = - clip_coverage_stack_.GetNextReplayResult(current_depth_)) { - auto& replay_entity = next_replay_clip->entity; - - SetClipScissor( - next_replay_clip->clip_coverage, - *render_passes_.back().inline_pass_context->GetRenderPass(0).pass, - GetGlobalPassPosition()); - if (!replay_entity.Render(renderer_, - *render_passes_.back() - .inline_pass_context->GetRenderPass(0) - .pass)) { - VALIDATION_LOG << "Failed to render entity for clip replay."; - } - } -} - bool ExperimentalCanvas::BlitToOnscreen() { auto command_buffer = renderer_.GetContext()->CreateCommandBuffer(); command_buffer->SetLabel("EntityPass Root Command Buffer"); diff --git a/impeller/aiks/experimental_canvas.h b/impeller/aiks/experimental_canvas.h index 6330d0b836380..282c71b536410 100644 --- a/impeller/aiks/experimental_canvas.h +++ b/impeller/aiks/experimental_canvas.h @@ -91,10 +91,6 @@ class ExperimentalCanvas : public Canvas { /// std::nullopt. std::optional ComputeCoverageLimit() const; - /// @brief After flipping the backdrop texture, render any clips that effect - /// the current entity based on depth. - void FlushPendingClips(); - // clip depth of the previous save or 0. size_t GetClipHeightFloor() const { if (transform_stack_.size() > 1) { From 06cbda0953a1e54878330795bc0c7ca446a710da Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 4 Sep 2024 18:11:30 -0700 Subject: [PATCH 15/20] use dcheck --- impeller/aiks/experimental_canvas.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index f16ca562121fe..6c85a2bb2a786 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -310,7 +310,7 @@ void ExperimentalCanvas::Save(uint32_t total_content_depth) { entry.cull_rect = transform_stack_.back().cull_rect; entry.clip_depth = current_depth_ + total_content_depth; entry.distributed_opacity = transform_stack_.back().distributed_opacity; - FML_CHECK(entry.clip_depth <= transform_stack_.back().clip_depth) + FML_DCHECK(entry.clip_depth <= transform_stack_.back().clip_depth) << entry.clip_depth << " <=? " << transform_stack_.back().clip_depth << " after allocating " << total_content_depth; entry.clip_height = transform_stack_.back().clip_height; @@ -460,7 +460,7 @@ void ExperimentalCanvas::SaveLayer( entry.transform = transform_stack_.back().transform; entry.cull_rect = transform_stack_.back().cull_rect; entry.clip_depth = current_depth_ + total_content_depth; - FML_CHECK(entry.clip_depth <= transform_stack_.back().clip_depth) + FML_DCHECK(entry.clip_depth <= transform_stack_.back().clip_depth) << entry.clip_depth << " <=? " << transform_stack_.back().clip_depth << " after allocating " << total_content_depth; entry.clip_height = transform_stack_.back().clip_height; @@ -513,7 +513,7 @@ bool ExperimentalCanvas::Restore() { // to be overly conservative, but we need to jump the depth to // the clip depth so that the next rendering op will get a // larger depth (it will pre-increment the current_depth_ value). - FML_CHECK(current_depth_ <= transform_stack_.back().clip_depth) + FML_DCHECK(current_depth_ <= transform_stack_.back().clip_depth) << current_depth_ << " <=? " << transform_stack_.back().clip_depth; current_depth_ = transform_stack_.back().clip_depth; @@ -747,7 +747,7 @@ void ExperimentalCanvas::AddRenderEntityToCurrentPass(Entity entity, // We can render at a depth up to and including the depth of the currently // active clips and we will still be clipped out, but we cannot render at // a depth that is greater than the current clips or we will not be clipped. - FML_CHECK(current_depth_ <= transform_stack_.back().clip_depth) + FML_DCHECK(current_depth_ <= transform_stack_.back().clip_depth) << current_depth_ << " <=? " << transform_stack_.back().clip_depth; entity.SetClipDepth(current_depth_); @@ -819,7 +819,7 @@ void ExperimentalCanvas::AddClipEntityToCurrentPass(Entity entity) { // to know if a clip will actually be used in advance of storing it in // the DisplayList buffer. // See https://github.com/flutter/flutter/issues/147021 - FML_CHECK(current_depth_ <= transform_stack_.back().clip_depth) + FML_DCHECK(current_depth_ <= transform_stack_.back().clip_depth) << current_depth_ << " <=? " << transform_stack_.back().clip_depth; entity.SetClipDepth(transform_stack_.back().clip_depth); From 1c0c46cb32311fd4946a1925f3ae2c4aafc3e78b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 4 Sep 2024 19:12:46 -0700 Subject: [PATCH 16/20] ++ --- impeller/display_list/dl_dispatcher.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index 97a49038aeb36..2ab56520b3f8c 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/impeller/display_list/dl_dispatcher.cc @@ -81,7 +81,7 @@ struct DepthWatcher { : file_(file), line_(line), canvas_(canvas), - allowed_(has_mask_blur ? allowed : (allowed + 1)), + allowed_(has_mask_blur ? allowed + 1 : allowed), old_depth_(canvas.GetOpDepth()), old_max_(canvas.GetMaxOpDepth()) {} From c168b96b98b6f0bea01c40123d00ab5c6ced5d0d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 5 Sep 2024 09:01:18 -0700 Subject: [PATCH 17/20] ++ --- impeller/aiks/experimental_canvas.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index 6c85a2bb2a786..4aa52f76bd1bc 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -142,7 +142,7 @@ static std::shared_ptr FlipBackdrop( auto& replay_entities = clip_coverage_stack.GetReplayEntities(); for (const auto& replay : replay_entities) { SetClipScissor( - clip_coverage_stack.CurrentClipCoverage(), + replay.clip_coverage, *render_passes.back().inline_pass_context->GetRenderPass(0).pass, global_pass_position); if (!replay.entity.Render( From 69884f9a183bd40e2abee7dbfae9fb19747ceefc Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 5 Sep 2024 09:48:12 -0700 Subject: [PATCH 18/20] ++ --- testing/dart/painting_test.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/dart/painting_test.dart b/testing/dart/painting_test.dart index 5c90cc6512107..fc39f9fdb7dc6 100644 --- a/testing/dart/painting_test.dart +++ b/testing/dart/painting_test.dart @@ -89,6 +89,7 @@ void main() { final SceneBuilder sceneBuilder = SceneBuilder(); final Picture redClippedPicture = makePicture((Canvas canvas) { + canvas.drawPaint(Paint()..color = const Color(0xFFFFFFFF)); canvas.clipRect(const Rect.fromLTRB(10, 10, 200, 200)); canvas.clipRect(const Rect.fromLTRB(11, 10, 300, 200)); canvas.drawPaint(Paint()..color = const Color(0xFFFF0000)); From 424852083479e26cb79537d1ebb0c3d3ff6ec502 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 6 Sep 2024 09:35:35 -0700 Subject: [PATCH 19/20] round out differences. --- impeller/aiks/canvas.h | 2 ++ impeller/aiks/experimental_canvas.cc | 26 ++++++++++++++++---------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/impeller/aiks/canvas.h b/impeller/aiks/canvas.h index 5d3e27f1561be..60d8e5d341331 100644 --- a/impeller/aiks/canvas.h +++ b/impeller/aiks/canvas.h @@ -39,6 +39,8 @@ struct CanvasStackEntry { Entity::RenderingMode rendering_mode = Entity::RenderingMode::kDirect; // Whether all entities in the current save should be skipped. bool skipping = false; + // Whether subpass coverage was rounded out to pixel coverage, or if false truncated. + bool did_round_out = false; }; enum class PointStyle { diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index 4aa52f76bd1bc..4a73f2cb132ad 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -398,10 +398,13 @@ void ExperimentalCanvas::SaveLayer( // When an image filter is present, clamp to avoid flicking due to nearest // sampled image. For other cases, round out to ensure than any geometry is // not cut off. + // See also this bug: https://github.com/flutter/flutter/issues/144213 ISize subpass_size; + bool did_round_out = false; if (paint.image_filter) { subpass_size = ISize(subpass_coverage.GetSize()); } else { + did_round_out = true; subpass_size = ISize(IRect::RoundOut(subpass_coverage).GetSize()); } if (subpass_size.IsEmpty()) { @@ -465,6 +468,7 @@ void ExperimentalCanvas::SaveLayer( << " after allocating " << total_content_depth; entry.clip_height = transform_stack_.back().clip_height; entry.rendering_mode = Entity::RenderingMode::kSubpassAppendSnapshotTransform; + entry.did_round_out = did_round_out; transform_stack_.emplace_back(entry); // The current clip aiks clip culling can not handle image filters. @@ -549,16 +553,18 @@ bool ExperimentalCanvas::Restore() { // sampling, so aligning here is important for avoiding visual nearest // sampling errors caused by limited floating point precision when // straddling a half pixel boundary. - // - // We do this in lieu of expanding/rounding out the subpass coverage in - // order to keep the bounds wrapping consistently tight around subpass - // elements when there are image filters. Which is necessary to avoid - // intense flickering in cases where a subpass texture has a large blur - // filter with clamp sampling. - // - // See also this bug: https://github.com/flutter/flutter/issues/144213 - Point subpass_texture_position = - (save_layer_state.coverage.GetOrigin() - global_pass_position).Round(); + Point subpass_texture_position; + if (transform_stack_.back().did_round_out) { + // Subpass coverage was rounded out, origin potentially moved "down" by + // as much as a pixel. + subpass_texture_position = + (save_layer_state.coverage.GetOrigin() - global_pass_position).Floor(); + } else { + // Subpass coverage was truncated. Pick the closest phyiscal pixel. + subpass_texture_position = + (save_layer_state.coverage.GetOrigin() - global_pass_position) + .Round(); + } Entity element_entity; element_entity.SetClipDepth(++current_depth_); From 3938253ee2f3168f7ad19f6c289fe16ae48328fe Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 6 Sep 2024 09:38:01 -0700 Subject: [PATCH 20/20] round in vs round out. --- impeller/aiks/canvas.h | 3 ++- impeller/aiks/experimental_canvas.cc | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/impeller/aiks/canvas.h b/impeller/aiks/canvas.h index 60d8e5d341331..0b0c3554c451d 100644 --- a/impeller/aiks/canvas.h +++ b/impeller/aiks/canvas.h @@ -39,7 +39,8 @@ struct CanvasStackEntry { Entity::RenderingMode rendering_mode = Entity::RenderingMode::kDirect; // Whether all entities in the current save should be skipped. bool skipping = false; - // Whether subpass coverage was rounded out to pixel coverage, or if false truncated. + // Whether subpass coverage was rounded out to pixel coverage, or if false + // truncated. bool did_round_out = false; }; diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index 4a73f2cb132ad..b262e8dbb3a77 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -398,7 +398,11 @@ void ExperimentalCanvas::SaveLayer( // When an image filter is present, clamp to avoid flicking due to nearest // sampled image. For other cases, round out to ensure than any geometry is // not cut off. + // // See also this bug: https://github.com/flutter/flutter/issues/144213 + // + // TODO(jonahwilliams): this could still round out for filters that use decal + // sampling mode. ISize subpass_size; bool did_round_out = false; if (paint.image_filter) { @@ -558,7 +562,8 @@ bool ExperimentalCanvas::Restore() { // Subpass coverage was rounded out, origin potentially moved "down" by // as much as a pixel. subpass_texture_position = - (save_layer_state.coverage.GetOrigin() - global_pass_position).Floor(); + (save_layer_state.coverage.GetOrigin() - global_pass_position) + .Floor(); } else { // Subpass coverage was truncated. Pick the closest phyiscal pixel. subpass_texture_position =