From 4a4ef50752cfad5578133a1b842090e67b58168b Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Thu, 30 Mar 2023 11:29:44 -0700 Subject: [PATCH] Revert "[Impeller] Absorb DrawPaints at the beginning of EntityPasses (#40675)" This reverts commit 29c2845fd952562cb74a6d2fee61338d8c5ddabb. --- impeller/aiks/aiks_unittests.cc | 12 ------ impeller/aiks/canvas.cc | 12 ------ impeller/aiks/paint_pass_delegate.cc | 2 +- impeller/entity/entity_pass.cc | 56 +++++++++------------------- impeller/entity/entity_pass.h | 9 +---- impeller/renderer/render_target.h | 11 ++---- 6 files changed, 23 insertions(+), 79 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 8b2ac5fdada21..037b7ef5abe48 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -1993,17 +1993,5 @@ TEST_P(AiksTest, OpacityPeepHoleApplicationTest) { ASSERT_TRUE(delegate->CanCollapseIntoParentPass(entity_pass.get())); } -TEST_P(AiksTest, DrawPaintAbsorbsClears) { - Canvas canvas; - canvas.DrawPaint({.color = Color::Red(), .blend_mode = BlendMode::kSource}); - canvas.DrawPaint( - {.color = Color::CornflowerBlue(), .blend_mode = BlendMode::kSource}); - - Picture picture = canvas.EndRecordingAsPicture(); - - ASSERT_EQ(picture.pass->GetElementCount(), 0u); - ASSERT_EQ(picture.pass->GetClearColor(), Color::CornflowerBlue()); -} - } // namespace testing } // namespace impeller diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index 0a76ed999501b..1ee329a6613dc 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -167,18 +167,6 @@ void Canvas::DrawPath(const Path& path, const Paint& paint) { } void Canvas::DrawPaint(const Paint& paint) { - bool is_clear = - paint.blend_mode == BlendMode::kSource || - (paint.blend_mode == BlendMode::kSourceOver && paint.color.alpha == 1); - if (xformation_stack_.size() == 1 && // If we're recording the root pass, - GetCurrentPass().GetElementCount() == 0 && // and this is the first item, - is_clear // and the backdrop is being replaced - ) { - // Then we can absorb this drawPaint as the clear color of the pass. - GetCurrentPass().SetClearColor(paint.color); - return; - } - Entity entity; entity.SetTransformation(GetCurrentTransformation()); entity.SetStencilDepth(GetStencilDepth()); diff --git a/impeller/aiks/paint_pass_delegate.cc b/impeller/aiks/paint_pass_delegate.cc index 5e9f6966f89ab..b1126ce48d20c 100644 --- a/impeller/aiks/paint_pass_delegate.cc +++ b/impeller/aiks/paint_pass_delegate.cc @@ -85,7 +85,7 @@ bool OpacityPeepholePassDelegate::CanCollapseIntoParentPass( // command wrapped in save layer. This would indicate something like an // Opacity or FadeTransition wrapping a very simple widget, like in the // CupertinoPicker. - if (entity_pass->GetElementCount() > 3) { + if (entity_pass->GetEntityCount() > 3) { // Single paint command with a save layer would be: // 1. clip // 2. draw command diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index f78ae074b9809..3e2587cb1d11d 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -144,8 +144,7 @@ EntityPass* EntityPass::AddSubpass(std::unique_ptr pass) { static EntityPassTarget CreateRenderTarget(ContentContext& renderer, ISize size, - bool readable, - const Color& clear_color) { + bool readable) { auto context = renderer.GetContext(); /// All of the load/store actions are managed by `InlinePassContext` when @@ -164,7 +163,7 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer, .resolve_storage_mode = StorageMode::kDevicePrivate, .load_action = LoadAction::kDontCare, .store_action = StoreAction::kMultisampleResolve, - .clear_color = clear_color}, // color_attachment_config + }, // color_attachment_config RenderTarget::AttachmentConfig{ .storage_mode = readable ? StorageMode::kDevicePrivate : StorageMode::kDeviceTransient, @@ -204,18 +203,13 @@ uint32_t EntityPass::GetTotalPassReads(ContentContext& renderer) const { bool EntityPass::Render(ContentContext& renderer, const RenderTarget& render_target) const { - if (render_target.GetColorAttachments().empty()) { - VALIDATION_LOG << "The root RenderTarget must have a color attachment."; - return false; - } - StencilCoverageStack stencil_coverage_stack = {StencilCoverageLayer{ .coverage = Rect::MakeSize(render_target.GetRenderTargetSize()), .stencil_depth = 0}}; if (GetTotalPassReads(renderer) > 0) { - auto offscreen_target = CreateRenderTarget( - renderer, render_target.GetRenderTargetSize(), true, clear_color_); + auto offscreen_target = + CreateRenderTarget(renderer, render_target.GetRenderTargetSize(), true); if (!OnRender(renderer, // renderer offscreen_target.GetRenderTarget() @@ -275,25 +269,18 @@ bool EntityPass::Render(ContentContext& renderer, return true; } - // Set up the clear color of the root pass. - auto color0 = render_target.GetColorAttachments().find(0)->second; - color0.clear_color = clear_color_; - - auto root_render_target = render_target; - root_render_target.SetColorAttachment(color0, 0); - EntityPassTarget pass_target( - root_render_target, + render_target, renderer.GetDeviceCapabilities().SupportsReadFromResolve()); - return OnRender( // - renderer, // renderer - root_render_target.GetRenderTargetSize(), // root_pass_size - pass_target, // pass_target - Point(), // global_pass_position - Point(), // local_pass_position - 0, // pass_depth - stencil_coverage_stack); // stencil_coverage_stack + return OnRender( // + renderer, // renderer + render_target.GetRenderTargetSize(), // root_pass_size + pass_target, // pass_target + Point(), // global_pass_position + Point(), // local_pass_position + 0, // pass_depth + stencil_coverage_stack); // stencil_coverage_stack } EntityPass::EntityResult EntityPass::GetEntityForElement( @@ -399,10 +386,9 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( subpass_coverage->Intersection(Rect::MakeSize(root_pass_size)); auto subpass_target = - CreateRenderTarget(renderer, // - ISize(subpass_coverage->size), // - subpass->GetTotalPassReads(renderer) > 0, // - clear_color_); + CreateRenderTarget(renderer, // + ISize(subpass_coverage->size), // + subpass->GetTotalPassReads(renderer) > 0); auto subpass_texture = subpass_target.GetRenderTarget().GetRenderTargetTexture(); @@ -715,7 +701,7 @@ bool EntityPass::IterateUntilSubpass( return false; } -size_t EntityPass::GetElementCount() const { +size_t EntityPass::GetEntityCount() const { return elements_.size(); } @@ -753,14 +739,6 @@ void EntityPass::SetBlendMode(BlendMode blend_mode) { cover_whole_screen_ = Entity::IsBlendModeDestructive(blend_mode); } -void EntityPass::SetClearColor(Color clear_color) { - clear_color_ = clear_color; -} - -Color EntityPass::GetClearColor() const { - return clear_color_; -} - void EntityPass::SetBackdropFilter(std::optional proc) { if (superpass_) { VALIDATION_LOG << "Backdrop filters cannot be set on EntityPasses that " diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index faad3cb74e0e2..d0b92f0e38731 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -77,8 +77,8 @@ class EntityPass { /// @return Returns whether a subpass was encountered. bool IterateUntilSubpass(const std::function& iterator); - /// @brief Return the number of elements on this pass. - size_t GetElementCount() const; + /// @brief Return the number of entities on this pass. + size_t GetEntityCount() const; void SetTransformation(Matrix xformation); @@ -86,10 +86,6 @@ class EntityPass { void SetBlendMode(BlendMode blend_mode); - void SetClearColor(Color clear_color); - - Color GetClearColor() const; - void SetBackdropFilter(std::optional proc); std::optional GetSubpassCoverage( @@ -208,7 +204,6 @@ class EntityPass { size_t stencil_depth_ = 0u; BlendMode blend_mode_ = BlendMode::kSourceOver; bool cover_whole_screen_ = false; - Color clear_color_ = Color::BlackTransparent(); /// These values are incremented whenever something is added to the pass that /// requires reading from the backdrop texture. Currently, this can happen in diff --git a/impeller/renderer/render_target.h b/impeller/renderer/render_target.h index 14562c6cc71b6..a7310ed7ce80d 100644 --- a/impeller/renderer/render_target.h +++ b/impeller/renderer/render_target.h @@ -23,7 +23,6 @@ class RenderTarget final { StorageMode storage_mode; LoadAction load_action; StoreAction store_action; - Color clear_color; }; struct AttachmentConfigMSAA { @@ -31,27 +30,23 @@ class RenderTarget final { StorageMode resolve_storage_mode; LoadAction load_action; StoreAction store_action; - Color clear_color; }; static constexpr AttachmentConfig kDefaultColorAttachmentConfig = { .storage_mode = StorageMode::kDevicePrivate, .load_action = LoadAction::kClear, - .store_action = StoreAction::kStore, - .clear_color = Color::BlackTransparent()}; + .store_action = StoreAction::kStore}; static constexpr AttachmentConfigMSAA kDefaultColorAttachmentConfigMSAA = { .storage_mode = StorageMode::kDeviceTransient, .resolve_storage_mode = StorageMode::kDevicePrivate, .load_action = LoadAction::kClear, - .store_action = StoreAction::kMultisampleResolve, - .clear_color = Color::BlackTransparent()}; + .store_action = StoreAction::kMultisampleResolve}; static constexpr AttachmentConfig kDefaultStencilAttachmentConfig = { .storage_mode = StorageMode::kDeviceTransient, .load_action = LoadAction::kClear, - .store_action = StoreAction::kDontCare, - .clear_color = Color::BlackTransparent()}; + .store_action = StoreAction::kDontCare}; static RenderTarget CreateOffscreen( const Context& context,