From a74b15b01a9abe302371c781daf3209af8a22851 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 26 Oct 2023 11:16:07 -0700 Subject: [PATCH 1/8] [Impeller] remove giant closure. --- impeller/entity/entity_pass.cc | 292 ++++++++++++++++----------------- impeller/entity/entity_pass.h | 11 +- 2 files changed, 153 insertions(+), 150 deletions(-) diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 4d6897b5b3445..d85e9e18d49ff 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -12,13 +12,10 @@ #include "flutter/fml/closure.h" #include "flutter/fml/logging.h" -#include "flutter/fml/macros.h" #include "flutter/fml/trace_event.h" #include "impeller/base/strings.h" #include "impeller/base/validation.h" -#include "impeller/core/allocator.h" #include "impeller/core/formats.h" -#include "impeller/core/texture.h" #include "impeller/entity/contents/clip_contents.h" #include "impeller/entity/contents/content_context.h" #include "impeller/entity/contents/filters/color_filter_contents.h" @@ -28,10 +25,7 @@ #include "impeller/entity/entity.h" #include "impeller/entity/inline_pass_context.h" #include "impeller/geometry/color.h" -#include "impeller/geometry/path_builder.h" -#include "impeller/renderer/command.h" #include "impeller/renderer/command_buffer.h" -#include "impeller/renderer/render_pass.h" #ifdef IMPELLER_DEBUG #include "impeller/entity/contents/checkerboard_contents.h" @@ -704,6 +698,146 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( return EntityPass::EntityResult::Success(element_entity); } +bool EntityPass::RenderElement(Entity& element_entity, + size_t clip_depth_floor, + InlinePassContext& pass_context, + int32_t pass_depth, + ContentContext& renderer, + ClipCoverageStack& clip_coverage_stack, + Point global_pass_position) const { + auto result = pass_context.GetRenderPass(pass_depth); + if (!result.pass) { + // Failure to produce a render pass should be explained by specific errors + // in `InlinePassContext::GetRenderPass()`, so avoid log spam and don't + // append a validation log here. + return false; + } + + // If the pass context returns a backdrop texture, we need to draw it to the + // current pass. We do this because it's faster and takes significantly less + // memory than storing/loading large MSAA textures. Also, it's not possible to + // blit the non-MSAA resolve texture of the previous pass to MSAA textures + // (let alone a transient one). + if (result.backdrop_texture) { + auto size_rect = Rect::MakeSize(result.pass->GetRenderTargetSize()); + auto msaa_backdrop_contents = TextureContents::MakeRect(size_rect); + msaa_backdrop_contents->SetStencilEnabled(false); + msaa_backdrop_contents->SetLabel("MSAA backdrop"); + msaa_backdrop_contents->SetSourceRect(size_rect); + msaa_backdrop_contents->SetTexture(result.backdrop_texture); + + Entity msaa_backdrop_entity; + msaa_backdrop_entity.SetContents(std::move(msaa_backdrop_contents)); + msaa_backdrop_entity.SetBlendMode(BlendMode::kSource); + if (!msaa_backdrop_entity.Render(renderer, *result.pass)) { + VALIDATION_LOG << "Failed to render MSAA backdrop filter entity."; + return false; + } + } + + auto current_clip_coverage = clip_coverage_stack.back().coverage; + if (current_clip_coverage.has_value()) { + // Entity transforms are relative to the current pass position, so we need + // to check clip coverage in the same space. + current_clip_coverage->origin -= global_pass_position; + } + + if (!element_entity.ShouldRender(current_clip_coverage)) { + return true; // Nothing to render. + } + + auto clip_coverage = element_entity.GetClipCoverage(current_clip_coverage); + if (clip_coverage.coverage.has_value()) { + clip_coverage.coverage->origin += global_pass_position; + } + + // 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 + // optionally use this hint to avoid unnecessary rendering work. + if (element_entity.GetContents()->GetCoverageHint().has_value()) { + // If the element already has a coverage hint (because its an advanced + // blend), then we need to intersect the clip coverage hint with the + // existing coverage hint. + element_entity.GetContents()->SetCoverageHint( + current_clip_coverage->Intersection( + element_entity.GetContents()->GetCoverageHint().value())); + } else { + element_entity.GetContents()->SetCoverageHint(current_clip_coverage); + } + + switch (clip_coverage.type) { + case Contents::ClipCoverage::Type::kNoChange: + break; + case Contents::ClipCoverage::Type::kAppend: { + auto op = clip_coverage_stack.back().coverage; + clip_coverage_stack.push_back( + ClipCoverageLayer{.coverage = clip_coverage.coverage, + .clip_depth = element_entity.GetClipDepth() + 1}); + FML_DCHECK(clip_coverage_stack.back().clip_depth == + clip_coverage_stack.front().clip_depth + + clip_coverage_stack.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 true; + } + } break; + case Contents::ClipCoverage::Type::kRestore: { + if (clip_coverage_stack.back().clip_depth <= + element_entity.GetClipDepth()) { + // Drop clip restores that will do nothing. + return true; + } + + auto restoration_index = element_entity.GetClipDepth() - + clip_coverage_stack.front().clip_depth; + FML_DCHECK(restoration_index < clip_coverage_stack.size()); + + // We only need to restore the area that covers the coverage of the + // clip rect at target depth + 1. + std::optional restore_coverage = + (restoration_index + 1 < clip_coverage_stack.size()) + ? clip_coverage_stack[restoration_index + 1].coverage + : std::nullopt; + if (restore_coverage.has_value()) { + // Make the coverage rectangle relative to the current pass. + restore_coverage->origin -= global_pass_position; + } + clip_coverage_stack.resize(restoration_index + 1); + + if (!clip_coverage_stack.back().coverage.has_value()) { + // Running this restore op won't make anything renderable, so skip it. + return true; + } + + auto restore_contents = + static_cast(element_entity.GetContents().get()); + restore_contents->SetRestoreCoverage(restore_coverage); + + } break; + } + +#ifdef IMPELLER_ENABLE_CAPTURE + { + auto element_entity_coverage = element_entity.GetCoverage(); + if (element_entity_coverage.has_value()) { + element_entity_coverage->origin += global_pass_position; + element_entity.GetCapture().AddRect("Coverage", *element_entity_coverage, + {.readonly = true}); + } + } +#endif + + element_entity.SetClipDepth(element_entity.GetClipDepth() - clip_depth_floor); + if (!element_entity.Render(renderer, *result.pass)) { + VALIDATION_LOG << "Failed to render entity."; + return false; + } + return true; +} + bool EntityPass::OnRender( ContentContext& renderer, Capture& capture, @@ -735,144 +869,6 @@ bool EntityPass::OnRender( pass_context.GetRenderPass(pass_depth); } - auto render_element = [&clip_depth_floor, &pass_context, &pass_depth, - &renderer, &clip_coverage_stack, - &global_pass_position](Entity& element_entity) { - auto result = pass_context.GetRenderPass(pass_depth); - - if (!result.pass) { - // Failure to produce a render pass should be explained by specific errors - // in `InlinePassContext::GetRenderPass()`, so avoid log spam and don't - // append a validation log here. - return false; - } - - // If the pass context returns a texture, we need to draw it to the current - // pass. We do this because it's faster and takes significantly less memory - // than storing/loading large MSAA textures. - // Also, it's not possible to blit the non-MSAA resolve texture of the - // previous pass to MSAA textures (let alone a transient one). - if (result.backdrop_texture) { - auto size_rect = Rect::MakeSize(result.pass->GetRenderTargetSize()); - auto msaa_backdrop_contents = TextureContents::MakeRect(size_rect); - msaa_backdrop_contents->SetStencilEnabled(false); - msaa_backdrop_contents->SetLabel("MSAA backdrop"); - msaa_backdrop_contents->SetSourceRect(size_rect); - msaa_backdrop_contents->SetTexture(result.backdrop_texture); - - Entity msaa_backdrop_entity; - msaa_backdrop_entity.SetContents(std::move(msaa_backdrop_contents)); - msaa_backdrop_entity.SetBlendMode(BlendMode::kSource); - if (!msaa_backdrop_entity.Render(renderer, *result.pass)) { - VALIDATION_LOG << "Failed to render MSAA backdrop filter entity."; - return false; - } - } - - auto current_clip_coverage = clip_coverage_stack.back().coverage; - if (current_clip_coverage.has_value()) { - // Entity transforms are relative to the current pass position, so we need - // to check clip coverage in the same space. - current_clip_coverage->origin -= global_pass_position; - } - - if (!element_entity.ShouldRender(current_clip_coverage)) { - return true; // Nothing to render. - } - - auto clip_coverage = element_entity.GetClipCoverage(current_clip_coverage); - if (clip_coverage.coverage.has_value()) { - clip_coverage.coverage->origin += global_pass_position; - } - - // 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 - // optionally use this hint to avoid unnecessary rendering work. - if (element_entity.GetContents()->GetCoverageHint().has_value()) { - // If the element already has a coverage hint (because its an advanced - // blend), then we need to intersect the clip coverage hint with the - // existing coverage hint. - element_entity.GetContents()->SetCoverageHint( - current_clip_coverage->Intersection( - element_entity.GetContents()->GetCoverageHint().value())); - } else { - element_entity.GetContents()->SetCoverageHint(current_clip_coverage); - } - - switch (clip_coverage.type) { - case Contents::ClipCoverage::Type::kNoChange: - break; - case Contents::ClipCoverage::Type::kAppend: { - auto op = clip_coverage_stack.back().coverage; - clip_coverage_stack.push_back( - ClipCoverageLayer{.coverage = clip_coverage.coverage, - .clip_depth = element_entity.GetClipDepth() + 1}); - FML_DCHECK(clip_coverage_stack.back().clip_depth == - clip_coverage_stack.front().clip_depth + - clip_coverage_stack.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 true; - } - } break; - case Contents::ClipCoverage::Type::kRestore: { - if (clip_coverage_stack.back().clip_depth <= - element_entity.GetClipDepth()) { - // Drop clip restores that will do nothing. - return true; - } - - auto restoration_index = element_entity.GetClipDepth() - - clip_coverage_stack.front().clip_depth; - FML_DCHECK(restoration_index < clip_coverage_stack.size()); - - // We only need to restore the area that covers the coverage of the - // clip rect at target depth + 1. - std::optional restore_coverage = - (restoration_index + 1 < clip_coverage_stack.size()) - ? clip_coverage_stack[restoration_index + 1].coverage - : std::nullopt; - if (restore_coverage.has_value()) { - // Make the coverage rectangle relative to the current pass. - restore_coverage->origin -= global_pass_position; - } - clip_coverage_stack.resize(restoration_index + 1); - - if (!clip_coverage_stack.back().coverage.has_value()) { - // Running this restore op won't make anything renderable, so skip it. - return true; - } - - auto restore_contents = static_cast( - element_entity.GetContents().get()); - restore_contents->SetRestoreCoverage(restore_coverage); - - } break; - } - -#ifdef IMPELLER_ENABLE_CAPTURE - { - auto element_entity_coverage = element_entity.GetCoverage(); - if (element_entity_coverage.has_value()) { - element_entity_coverage->origin += global_pass_position; - element_entity.GetCapture().AddRect( - "Coverage", *element_entity_coverage, {.readonly = true}); - } - } -#endif - - element_entity.SetClipDepth(element_entity.GetClipDepth() - - clip_depth_floor); - if (!element_entity.Render(renderer, *result.pass)) { - VALIDATION_LOG << "Failed to render entity."; - return false; - } - return true; - }; - if (backdrop_filter_proc_) { if (!backdrop_filter_contents) { VALIDATION_LOG @@ -889,7 +885,8 @@ bool EntityPass::OnRender( Matrix::MakeTranslation(Vector3(-local_pass_position))); backdrop_entity.SetClipDepth(clip_depth_floor); - render_element(backdrop_entity); + RenderElement(backdrop_entity, clip_depth_floor, pass_context, pass_depth, + renderer, clip_coverage_stack, global_pass_position); } bool is_collapsing_clear_colors = !collapsed_parent_pass && @@ -983,8 +980,9 @@ bool EntityPass::OnRender( //-------------------------------------------------------------------------- /// Render the Element. /// - - if (!render_element(result.entity)) { + if (!RenderElement(result.entity, clip_depth_floor, pass_context, + pass_depth, renderer, clip_coverage_stack, + global_pass_position)) { // Specific validation logs are handled in `render_element()`. return false; } diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index 1c7bc2cf29db5..9293d41949771 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -9,15 +9,12 @@ #include #include -#include "flutter/fml/macros.h" -#include "impeller/core/texture.h" #include "impeller/entity/contents/contents.h" #include "impeller/entity/contents/filters/filter_contents.h" #include "impeller/entity/entity.h" #include "impeller/entity/entity_pass_delegate.h" #include "impeller/entity/inline_pass_context.h" #include "impeller/renderer/render_target.h" -#include "impeller/typographer/lazy_glyph_atlas.h" namespace impeller { @@ -189,6 +186,14 @@ class EntityPass { static EntityResult Skip() { return {{}, kSkip}; } }; + bool RenderElement(Entity& element_entity, + size_t clip_depth_floor, + InlinePassContext& pass_context, + int32_t pass_depth, + ContentContext& renderer, + ClipCoverageStack& clip_coverage_stack, + Point global_pass_position) const; + EntityResult GetEntityForElement(const EntityPass::Element& element, ContentContext& renderer, Capture& capture, From fa8ebf8e057381693ad2a31a6088e68553885003 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 27 Oct 2023 09:52:18 -0700 Subject: [PATCH 2/8] conditionally save stencil --- impeller/aiks/canvas.cc | 1 + impeller/entity/entity_pass.cc | 22 ++++++++++------------ impeller/entity/entity_pass.h | 10 ++++++++++ impeller/entity/inline_pass_context.cc | 10 ++++++---- impeller/entity/inline_pass_context.h | 2 ++ 5 files changed, 29 insertions(+), 16 deletions(-) diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index 2308d5385fb4a..e8e18e4d5d4e6 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -379,6 +379,7 @@ void Canvas::ClipGeometry(std::unique_ptr geometry, entity.SetClipDepth(GetClipDepth()); GetCurrentPass().AddEntity(entity); + GetCurrentPass().ContainsClip(); ++xformation_stack_.back().clip_depth; xformation_stack_.back().contains_clips = true; diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index d85e9e18d49ff..e94e0c0c626f8 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -359,7 +359,7 @@ bool EntityPass::Render(ContentContext& renderer, // there's no need to set up a stencil attachment on the root render target. if (!supports_onscreen_backdrop_reads && reads_from_onscreen_backdrop) { auto offscreen_target = CreateRenderTarget( - renderer, root_render_target.GetRenderTargetSize(), true, + renderer, root_render_target.GetRenderTargetSize(), clips_applied_, GetClearColor(render_target.GetRenderTargetSize())); if (!OnRender(renderer, // renderer @@ -500,7 +500,6 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( //-------------------------------------------------------------------------- /// Setup entity element. /// - if (const auto& entity = std::get_if(&element)) { element_entity = *entity; element_entity.SetCapture(capture.CreateChild("Entity")); @@ -512,16 +511,15 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( Matrix::MakeTranslation(Vector3(-global_pass_position)) * element_entity.GetTransformation()); } + return EntityPass::EntityResult::Success(element_entity); } //-------------------------------------------------------------------------- /// Setup subpass element. /// - - else if (const auto& subpass_ptr = - std::get_if>(&element)) { + if (const auto& subpass_ptr = + std::get_if>(&element)) { auto subpass = subpass_ptr->get(); - if (subpass->delegate_->CanElide()) { return EntityPass::EntityResult::Skip(); } @@ -691,11 +689,10 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( element_entity.SetTransformation(subpass_texture_capture.AddMatrix( "Transform", Matrix::MakeTranslation(Vector3(subpass_coverage->origin - global_pass_position)))); - } else { - FML_UNREACHABLE(); - } - return EntityPass::EntityResult::Success(element_entity); + return EntityPass::EntityResult::Success(element_entity); + } + FML_UNREACHABLE(); } bool EntityPass::RenderElement(Entity& element_entity, @@ -854,8 +851,9 @@ bool EntityPass::OnRender( TRACE_EVENT0("impeller", "EntityPass::OnRender"); auto context = renderer.GetContext(); - InlinePassContext pass_context( - context, pass_target, GetTotalPassReads(renderer), collapsed_parent_pass); + InlinePassContext pass_context(context, pass_target, + GetTotalPassReads(renderer), clips_applied_, + collapsed_parent_pass); if (!pass_context.IsValid()) { VALIDATION_LOG << SPrintF("Pass context invalid (Depth=%d)", pass_depth); return false; diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index 9293d41949771..65a6285d94d84 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -162,6 +162,10 @@ class EntityPass { std::optional GetElementsCoverage( std::optional coverage_limit) const; + void ContainsClip() { + clips_applied_ = true; + } + private: struct EntityResult { enum Status { @@ -297,6 +301,12 @@ class EntityPass { uint32_t advanced_blend_reads_from_pass_texture_ = 0; uint32_t backdrop_filter_reads_from_pass_texture_ = 0; + /// The number of clips applied in the current entity pass. + /// + /// If this value is zero, then the stencil buffer does not have to be stored + /// between backdrop filters. + bool clips_applied_ = false; + uint32_t GetTotalPassReads(ContentContext& renderer) const; BackdropFilterProc backdrop_filter_proc_ = nullptr; diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index 4253e75987729..694ca32c7fc38 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -18,10 +18,12 @@ InlinePassContext::InlinePassContext( std::shared_ptr context, EntityPassTarget& pass_target, uint32_t pass_texture_reads, + bool contains_clips, std::optional collapsed_parent_pass) : context_(std::move(context)), pass_target_(pass_target), total_pass_reads_(pass_texture_reads), + contains_clips_(contains_clips), is_collapsed_(collapsed_parent_pass.has_value()) { if (collapsed_parent_pass.has_value()) { pass_ = collapsed_parent_pass.value().pass; @@ -142,15 +144,15 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass( // Only clear the stencil if this is the very first pass of the // layer. - stencil->load_action = - pass_count_ > 0 ? LoadAction::kLoad : LoadAction::kClear; + stencil->load_action = (pass_count_ > 0 && contains_clips_) + ? LoadAction::kLoad + : LoadAction::kClear; // If we're on the last pass of the layer, there's no need to store the // stencil because nothing needs to read it. - stencil->store_action = pass_count_ == total_pass_reads_ + stencil->store_action = (pass_count_ == total_pass_reads_ || !contains_clips_) ? StoreAction::kDontCare : StoreAction::kStore; pass_target_.target_.SetStencilAttachment(stencil.value()); - pass_target_.target_.SetColorAttachment(color0, 0); pass_ = command_buffer_->CreateRenderPass(pass_target_.GetRenderTarget()); diff --git a/impeller/entity/inline_pass_context.h b/impeller/entity/inline_pass_context.h index cf828f35a2dea..0e6c14a7b14a2 100644 --- a/impeller/entity/inline_pass_context.h +++ b/impeller/entity/inline_pass_context.h @@ -22,6 +22,7 @@ class InlinePassContext { std::shared_ptr context, EntityPassTarget& pass_target, uint32_t pass_texture_reads, + bool contains_clips, std::optional collapsed_parent_pass = std::nullopt); ~InlinePassContext(); @@ -41,6 +42,7 @@ class InlinePassContext { std::shared_ptr pass_; uint32_t pass_count_ = 0; uint32_t total_pass_reads_ = 0; + bool contains_clips_ = false; // Whether this context is collapsed into a parent entity pass. bool is_collapsed_ = false; From 46b416f99c2522f39ad2801e986aed7991c59bd9 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 27 Oct 2023 13:48:48 -0700 Subject: [PATCH 3/8] testing --- impeller/aiks/canvas.cc | 3 +- impeller/entity/entity.h | 2 + impeller/entity/entity_pass.cc | 176 +++++-------------------- impeller/entity/entity_pass.h | 27 ++-- impeller/entity/inline_pass_context.cc | 17 +-- impeller/entity/inline_pass_context.h | 2 - 6 files changed, 57 insertions(+), 170 deletions(-) diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index e8e18e4d5d4e6..8ffe2b1cbc446 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -377,9 +377,9 @@ void Canvas::ClipGeometry(std::unique_ptr geometry, entity.SetTransformation(GetCurrentTransformation()); entity.SetContents(std::move(contents)); entity.SetClipDepth(GetClipDepth()); + entity.record = true; GetCurrentPass().AddEntity(entity); - GetCurrentPass().ContainsClip(); ++xformation_stack_.back().clip_depth; xformation_stack_.back().contains_clips = true; @@ -417,6 +417,7 @@ void Canvas::RestoreClip() { // takes up the full render target. entity.SetContents(std::make_shared()); entity.SetClipDepth(GetClipDepth()); + entity.record = true; GetCurrentPass().AddEntity(entity); } diff --git a/impeller/entity/entity.h b/impeller/entity/entity.h index 26fa0b3974e14..a8ec6821f74a4 100644 --- a/impeller/entity/entity.h +++ b/impeller/entity/entity.h @@ -114,6 +114,8 @@ class Entity { void SetCapture(Capture capture) const; + bool record = false; + private: Matrix transformation_; std::shared_ptr contents_; diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index c98e9507d35ab..2e34ee09030cf 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -255,8 +255,7 @@ void EntityPass::AddSubpassInline(std::unique_ptr pass) { static RenderTarget::AttachmentConfig GetDefaultStencilConfig(bool readable) { return RenderTarget::AttachmentConfig{ - .storage_mode = readable ? StorageMode::kDevicePrivate - : StorageMode::kDeviceTransient, + .storage_mode = StorageMode::kDeviceTransient, .load_action = LoadAction::kDontCare, .store_action = StoreAction::kDontCare, }; @@ -361,8 +360,12 @@ bool EntityPass::Render(ContentContext& renderer, // and then blit the results onto the onscreen texture. If using this branch, // there's no need to set up a stencil attachment on the root render target. if (!supports_onscreen_backdrop_reads && reads_from_onscreen_backdrop) { + if (superpass_) { + FML_LOG(ERROR) << "Offscreen has superpass that reads from backdrop"; + } + auto offscreen_target = CreateRenderTarget( - renderer, root_render_target.GetRenderTargetSize(), clips_applied_, + renderer, root_render_target.GetRenderTargetSize(), true, GetClearColor(render_target.GetRenderTargetSize())); if (!OnRender(renderer, // renderer @@ -506,6 +509,11 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( if (const auto& entity = std::get_if(&element)) { element_entity = *entity; element_entity.SetCapture(capture.CreateChild("Entity")); + + if (element_entity.record) { + rendered_clip_entities_.push_back({element_entity, element_entity.GetTransformation()}); + } + if (!global_pass_position.IsZero()) { // If the pass image is going to be rendered with a non-zero position, // apply the negative translation to entity copies before rendering them @@ -833,146 +841,7 @@ bool EntityPass::RenderElement(Entity& element_entity, #endif element_entity.SetClipDepth(element_entity.GetClipDepth() - clip_depth_floor); - if (!element_entity.Render(renderer, *result.pass)) { - VALIDATION_LOG << "Failed to render entity."; - return false; - } - return true; -} - -bool EntityPass::RenderElement(Entity& element_entity, - size_t clip_depth_floor, - InlinePassContext& pass_context, - int32_t pass_depth, - ContentContext& renderer, - ClipCoverageStack& clip_coverage_stack, - Point global_pass_position) const { - auto result = pass_context.GetRenderPass(pass_depth); - if (!result.pass) { - // Failure to produce a render pass should be explained by specific errors - // in `InlinePassContext::GetRenderPass()`, so avoid log spam and don't - // append a validation log here. - return false; - } - - // If the pass context returns a backdrop texture, we need to draw it to the - // current pass. We do this because it's faster and takes significantly less - // memory than storing/loading large MSAA textures. Also, it's not possible to - // blit the non-MSAA resolve texture of the previous pass to MSAA textures - // (let alone a transient one). - if (result.backdrop_texture) { - auto size_rect = Rect::MakeSize(result.pass->GetRenderTargetSize()); - auto msaa_backdrop_contents = TextureContents::MakeRect(size_rect); - msaa_backdrop_contents->SetStencilEnabled(false); - msaa_backdrop_contents->SetLabel("MSAA backdrop"); - msaa_backdrop_contents->SetSourceRect(size_rect); - msaa_backdrop_contents->SetTexture(result.backdrop_texture); - - Entity msaa_backdrop_entity; - msaa_backdrop_entity.SetContents(std::move(msaa_backdrop_contents)); - msaa_backdrop_entity.SetBlendMode(BlendMode::kSource); - if (!msaa_backdrop_entity.Render(renderer, *result.pass)) { - VALIDATION_LOG << "Failed to render MSAA backdrop filter entity."; - return false; - } - } - - auto current_clip_coverage = clip_coverage_stack.back().coverage; - if (current_clip_coverage.has_value()) { - // Entity transforms are relative to the current pass position, so we need - // to check clip coverage in the same space. - current_clip_coverage->origin -= global_pass_position; - } - - if (!element_entity.ShouldRender(current_clip_coverage)) { - return true; // Nothing to render. - } - - auto clip_coverage = element_entity.GetClipCoverage(current_clip_coverage); - if (clip_coverage.coverage.has_value()) { - clip_coverage.coverage->origin += global_pass_position; - } - - // 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 - // optionally use this hint to avoid unnecessary rendering work. - if (element_entity.GetContents()->GetCoverageHint().has_value()) { - // If the element already has a coverage hint (because its an advanced - // blend), then we need to intersect the clip coverage hint with the - // existing coverage hint. - element_entity.GetContents()->SetCoverageHint( - current_clip_coverage->Intersection( - element_entity.GetContents()->GetCoverageHint().value())); - } else { - element_entity.GetContents()->SetCoverageHint(current_clip_coverage); - } - - switch (clip_coverage.type) { - case Contents::ClipCoverage::Type::kNoChange: - break; - case Contents::ClipCoverage::Type::kAppend: { - auto op = clip_coverage_stack.back().coverage; - clip_coverage_stack.push_back( - ClipCoverageLayer{.coverage = clip_coverage.coverage, - .clip_depth = element_entity.GetClipDepth() + 1}); - FML_DCHECK(clip_coverage_stack.back().clip_depth == - clip_coverage_stack.front().clip_depth + - clip_coverage_stack.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 true; - } - } break; - case Contents::ClipCoverage::Type::kRestore: { - if (clip_coverage_stack.back().clip_depth <= - element_entity.GetClipDepth()) { - // Drop clip restores that will do nothing. - return true; - } - auto restoration_index = element_entity.GetClipDepth() - - clip_coverage_stack.front().clip_depth; - FML_DCHECK(restoration_index < clip_coverage_stack.size()); - - // We only need to restore the area that covers the coverage of the - // clip rect at target depth + 1. - std::optional restore_coverage = - (restoration_index + 1 < clip_coverage_stack.size()) - ? clip_coverage_stack[restoration_index + 1].coverage - : std::nullopt; - if (restore_coverage.has_value()) { - // Make the coverage rectangle relative to the current pass. - restore_coverage->origin -= global_pass_position; - } - clip_coverage_stack.resize(restoration_index + 1); - - if (!clip_coverage_stack.back().coverage.has_value()) { - // Running this restore op won't make anything renderable, so skip it. - return true; - } - - auto restore_contents = - static_cast(element_entity.GetContents().get()); - restore_contents->SetRestoreCoverage(restore_coverage); - - } break; - } - -#ifdef IMPELLER_ENABLE_CAPTURE - { - auto element_entity_coverage = element_entity.GetCoverage(); - if (element_entity_coverage.has_value()) { - element_entity_coverage->origin += global_pass_position; - element_entity.GetCapture().AddRect("Coverage", *element_entity_coverage, - {.readonly = true}); - } - } -#endif - - element_entity.SetClipDepth(element_entity.GetClipDepth() - clip_depth_floor); if (!element_entity.Render(renderer, *result.pass)) { VALIDATION_LOG << "Failed to render entity."; return false; @@ -996,9 +865,8 @@ bool EntityPass::OnRender( TRACE_EVENT0("impeller", "EntityPass::OnRender"); auto context = renderer.GetContext(); - InlinePassContext pass_context(context, pass_target, - GetTotalPassReads(renderer), clips_applied_, - collapsed_parent_pass); + InlinePassContext pass_context( + context, pass_target, GetTotalPassReads(renderer), collapsed_parent_pass); if (!pass_context.IsValid()) { VALIDATION_LOG << SPrintF("Pass context invalid (Depth=%d)", pass_depth); return false; @@ -1012,6 +880,24 @@ bool EntityPass::OnRender( pass_context.GetRenderPass(pass_depth); } + if (superpass_) { + auto pending_clip_entities = superpass_->CollectParentClipEntities(); + for (auto entity : pending_clip_entities) { + if (!global_pass_position.IsZero()) { + // If the pass image is going to be rendered with a non-zero position, + // apply the negative translation to entity copies before rendering them + // so that they'll end up rendering to the correct on-screen position. + entity.entity.SetTransformation( + Matrix::MakeTranslation(Vector3(-global_pass_position)) * + entity.original_transform); + } + auto result = pass_context.GetRenderPass(pass_depth); + if (!entity.entity.Render(renderer, *result.pass)) { + FML_LOG(ERROR) << "ERROR"; + } + } + } + if (backdrop_filter_proc_) { if (!backdrop_filter_contents) { VALIDATION_LOG diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index 65a6285d94d84..70499e56c0f56 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -4,6 +4,7 @@ #pragma once +#include #include #include #include @@ -162,10 +163,6 @@ class EntityPass { std::optional GetElementsCoverage( std::optional coverage_limit) const; - void ContainsClip() { - clips_applied_ = true; - } - private: struct EntityResult { enum Status { @@ -282,6 +279,13 @@ class EntityPass { /// evaluated and recorded to an `EntityPassTarget` by the `OnRender` method. std::vector elements_; + struct EntityPair { + Entity entity; + Matrix original_transform; + }; + + mutable std::vector rendered_clip_entities_; + EntityPass* superpass_ = nullptr; Matrix xformation_; size_t clip_depth_ = 0u; @@ -290,6 +294,15 @@ class EntityPass { bool enable_offscreen_debug_checkerboard_ = false; std::optional bounds_limit_; + std::vector CollectParentClipEntities() { + std::vector result; + if (superpass_) { + result = superpass_->CollectParentClipEntities(); + } + result.insert(result.end(), rendered_clip_entities_.begin(), rendered_clip_entities_.end()); + return result; + } + /// These values are incremented whenever something is added to the pass that /// requires reading from the backdrop texture. Currently, this can happen in /// the following scenarios: @@ -301,12 +314,6 @@ class EntityPass { uint32_t advanced_blend_reads_from_pass_texture_ = 0; uint32_t backdrop_filter_reads_from_pass_texture_ = 0; - /// The number of clips applied in the current entity pass. - /// - /// If this value is zero, then the stencil buffer does not have to be stored - /// between backdrop filters. - bool clips_applied_ = false; - uint32_t GetTotalPassReads(ContentContext& renderer) const; BackdropFilterProc backdrop_filter_proc_ = nullptr; diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index 694ca32c7fc38..b45ec0a5d0db1 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -18,12 +18,10 @@ InlinePassContext::InlinePassContext( std::shared_ptr context, EntityPassTarget& pass_target, uint32_t pass_texture_reads, - bool contains_clips, std::optional collapsed_parent_pass) : context_(std::move(context)), pass_target_(pass_target), total_pass_reads_(pass_texture_reads), - contains_clips_(contains_clips), is_collapsed_(collapsed_parent_pass.has_value()) { if (collapsed_parent_pass.has_value()) { pass_ = collapsed_parent_pass.value().pass; @@ -142,16 +140,11 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass( return {}; } - // Only clear the stencil if this is the very first pass of the - // layer. - stencil->load_action = (pass_count_ > 0 && contains_clips_) - ? LoadAction::kLoad - : LoadAction::kClear; - // If we're on the last pass of the layer, there's no need to store the - // stencil because nothing needs to read it. - stencil->store_action = (pass_count_ == total_pass_reads_ || !contains_clips_) - ? StoreAction::kDontCare - : StoreAction::kStore; + if (total_pass_reads_ > 0) { + FML_LOG(ERROR) << "clear despite read."; + } + stencil->load_action = LoadAction::kClear; + stencil->store_action = StoreAction::kDontCare; pass_target_.target_.SetStencilAttachment(stencil.value()); pass_target_.target_.SetColorAttachment(color0, 0); diff --git a/impeller/entity/inline_pass_context.h b/impeller/entity/inline_pass_context.h index 0e6c14a7b14a2..cf828f35a2dea 100644 --- a/impeller/entity/inline_pass_context.h +++ b/impeller/entity/inline_pass_context.h @@ -22,7 +22,6 @@ class InlinePassContext { std::shared_ptr context, EntityPassTarget& pass_target, uint32_t pass_texture_reads, - bool contains_clips, std::optional collapsed_parent_pass = std::nullopt); ~InlinePassContext(); @@ -42,7 +41,6 @@ class InlinePassContext { std::shared_ptr pass_; uint32_t pass_count_ = 0; uint32_t total_pass_reads_ = 0; - bool contains_clips_ = false; // Whether this context is collapsed into a parent entity pass. bool is_collapsed_ = false; From c8f6ca38472dd170a2bde4c2abd81ac3fc16707d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 31 Oct 2023 13:05:45 -0700 Subject: [PATCH 4/8] ++ --- impeller/aiks/canvas.cc | 2 -- impeller/entity/entity.h | 2 -- impeller/entity/entity_pass.cc | 48 ++++++++++++++------------ impeller/entity/entity_pass.h | 34 +++++++++--------- impeller/entity/inline_pass_context.cc | 4 --- impeller/entity/inline_pass_context.h | 2 +- 6 files changed, 44 insertions(+), 48 deletions(-) diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index 8ffe2b1cbc446..2308d5385fb4a 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -377,7 +377,6 @@ void Canvas::ClipGeometry(std::unique_ptr geometry, entity.SetTransformation(GetCurrentTransformation()); entity.SetContents(std::move(contents)); entity.SetClipDepth(GetClipDepth()); - entity.record = true; GetCurrentPass().AddEntity(entity); @@ -417,7 +416,6 @@ void Canvas::RestoreClip() { // takes up the full render target. entity.SetContents(std::make_shared()); entity.SetClipDepth(GetClipDepth()); - entity.record = true; GetCurrentPass().AddEntity(entity); } diff --git a/impeller/entity/entity.h b/impeller/entity/entity.h index a8ec6821f74a4..26fa0b3974e14 100644 --- a/impeller/entity/entity.h +++ b/impeller/entity/entity.h @@ -114,8 +114,6 @@ class Entity { void SetCapture(Capture capture) const; - bool record = false; - private: Matrix transformation_; std::shared_ptr contents_; diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 2e34ee09030cf..acef370725764 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -510,10 +510,6 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( element_entity = *entity; element_entity.SetCapture(capture.CreateChild("Entity")); - if (element_entity.record) { - rendered_clip_entities_.push_back({element_entity, element_entity.GetTransformation()}); - } - if (!global_pass_position.IsZero()) { // If the pass image is going to be rendered with a non-zero position, // apply the negative translation to entity copies before rendering them @@ -729,6 +725,13 @@ bool EntityPass::RenderElement(Entity& element_entity, // blit the non-MSAA resolve texture of the previous pass to MSAA textures // (let alone a transient one). if (result.backdrop_texture) { + // Restore any clips that were recorded before the backdrop filter was applied. + for (const auto& entity : clip_replay_->GetReplayEntities()) { + if (!entity.Render(renderer, *result.pass)) { + VALIDATION_LOG << "Failed to render entity for clip restore."; + } + } + auto size_rect = Rect::MakeSize(result.pass->GetRenderTargetSize()); auto msaa_backdrop_contents = TextureContents::MakeRect(size_rect); msaa_backdrop_contents->SetStencilEnabled(false); @@ -841,7 +844,7 @@ bool EntityPass::RenderElement(Entity& element_entity, #endif element_entity.SetClipDepth(element_entity.GetClipDepth() - clip_depth_floor); - + clip_replay_->RecordEntity(element_entity, clip_coverage.type); if (!element_entity.Render(renderer, *result.pass)) { VALIDATION_LOG << "Failed to render entity."; return false; @@ -880,24 +883,6 @@ bool EntityPass::OnRender( pass_context.GetRenderPass(pass_depth); } - if (superpass_) { - auto pending_clip_entities = superpass_->CollectParentClipEntities(); - for (auto entity : pending_clip_entities) { - if (!global_pass_position.IsZero()) { - // If the pass image is going to be rendered with a non-zero position, - // apply the negative translation to entity copies before rendering them - // so that they'll end up rendering to the correct on-screen position. - entity.entity.SetTransformation( - Matrix::MakeTranslation(Vector3(-global_pass_position)) * - entity.original_transform); - } - auto result = pass_context.GetRenderPass(pass_depth); - if (!entity.entity.Render(renderer, *result.pass)) { - FML_LOG(ERROR) << "ERROR"; - } - } - } - if (backdrop_filter_proc_) { if (!backdrop_filter_contents) { VALIDATION_LOG @@ -1204,4 +1189,21 @@ void EntityPass::SetEnableOffscreenCheckerboard(bool enabled) { enable_offscreen_debug_checkerboard_ = enabled; } +void EntityPassClipReplay::RecordEntity(Entity entity, + Contents::ClipCoverage::Type type) { + switch (type) { + case Contents::ClipCoverage::Type::kNoChange: + return; + case Contents::ClipCoverage::Type::kAppend: + rendered_clip_entities_.push_back(entity); + case Contents::ClipCoverage::Type::kRestore: + rendered_clip_entities_.pop_back(); + break; + } +} + +const std::vector& EntityPassClipReplay::GetReplayEntities() const { + return rendered_clip_entities_; +} + } // namespace impeller diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index 70499e56c0f56..87257c44af4cb 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -20,6 +20,7 @@ namespace impeller { class ContentContext; +class EntityPassClipReplay; class EntityPass { public: @@ -279,13 +280,6 @@ class EntityPass { /// evaluated and recorded to an `EntityPassTarget` by the `OnRender` method. std::vector elements_; - struct EntityPair { - Entity entity; - Matrix original_transform; - }; - - mutable std::vector rendered_clip_entities_; - EntityPass* superpass_ = nullptr; Matrix xformation_; size_t clip_depth_ = 0u; @@ -293,15 +287,7 @@ class EntityPass { bool flood_clip_ = false; bool enable_offscreen_debug_checkerboard_ = false; std::optional bounds_limit_; - - std::vector CollectParentClipEntities() { - std::vector result; - if (superpass_) { - result = superpass_->CollectParentClipEntities(); - } - result.insert(result.end(), rendered_clip_entities_.begin(), rendered_clip_entities_.end()); - return result; - } + std::unique_ptr clip_replay_ = std::make_unique(); /// These values are incremented whenever something is added to the pass that /// requires reading from the backdrop texture. Currently, this can happen in @@ -326,4 +312,20 @@ class EntityPass { EntityPass& operator=(const EntityPass&) = delete; }; +/// @brief A class that tracks all clips that have been recorded in the current +/// entity pass stencil. +/// +/// These clips are replayed when restoring the backdrop so that the +/// stencil buffer is left in an identical state. +class EntityPassClipReplay { + public: + /// @brief Record the entity based on the provided coverage [type]. + void RecordEntity(Entity entity, Contents::ClipCoverage::Type type); + + const std::vector& GetReplayEntities() const; + + private: + std::vector rendered_clip_entities_; +}; + } // namespace impeller diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index b45ec0a5d0db1..c1f9f59649c78 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -21,7 +21,6 @@ InlinePassContext::InlinePassContext( std::optional collapsed_parent_pass) : context_(std::move(context)), pass_target_(pass_target), - total_pass_reads_(pass_texture_reads), is_collapsed_(collapsed_parent_pass.has_value()) { if (collapsed_parent_pass.has_value()) { pass_ = collapsed_parent_pass.value().pass; @@ -140,9 +139,6 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass( return {}; } - if (total_pass_reads_ > 0) { - FML_LOG(ERROR) << "clear despite read."; - } stencil->load_action = LoadAction::kClear; stencil->store_action = StoreAction::kDontCare; pass_target_.target_.SetStencilAttachment(stencil.value()); diff --git a/impeller/entity/inline_pass_context.h b/impeller/entity/inline_pass_context.h index cf828f35a2dea..0c89bd5630e75 100644 --- a/impeller/entity/inline_pass_context.h +++ b/impeller/entity/inline_pass_context.h @@ -40,7 +40,7 @@ class InlinePassContext { std::shared_ptr command_buffer_; std::shared_ptr pass_; uint32_t pass_count_ = 0; - uint32_t total_pass_reads_ = 0; + // Whether this context is collapsed into a parent entity pass. bool is_collapsed_ = false; From 5bed1c2b6b1e9836595cf8fbe5b25f2b97c86f5f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 31 Oct 2023 13:06:06 -0700 Subject: [PATCH 5/8] ++ --- impeller/entity/entity_pass.cc | 3 ++- impeller/entity/entity_pass.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index acef370725764..122b5be28df6f 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -725,7 +725,8 @@ bool EntityPass::RenderElement(Entity& element_entity, // blit the non-MSAA resolve texture of the previous pass to MSAA textures // (let alone a transient one). if (result.backdrop_texture) { - // Restore any clips that were recorded before the backdrop filter was applied. + // Restore any clips that were recorded before the backdrop filter was + // applied. for (const auto& entity : clip_replay_->GetReplayEntities()) { if (!entity.Render(renderer, *result.pass)) { VALIDATION_LOG << "Failed to render entity for clip restore."; diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index 87257c44af4cb..fef80bafdbc8e 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -287,7 +287,8 @@ class EntityPass { bool flood_clip_ = false; bool enable_offscreen_debug_checkerboard_ = false; std::optional bounds_limit_; - std::unique_ptr clip_replay_ = std::make_unique(); + std::unique_ptr clip_replay_ = + std::make_unique(); /// These values are incremented whenever something is added to the pass that /// requires reading from the backdrop texture. Currently, this can happen in From 96589db67aefb2f6ebbe2115f727e9d7b1b5862c Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 31 Oct 2023 13:11:21 -0700 Subject: [PATCH 6/8] ++ --- impeller/entity/entity_pass.cc | 32 ++++++++++++++------------------ impeller/entity/entity_pass.h | 7 +++++-- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 122b5be28df6f..af57c061f8726 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -253,13 +253,12 @@ void EntityPass::AddSubpassInline(std::unique_ptr pass) { pass->advanced_blend_reads_from_pass_texture_; } -static RenderTarget::AttachmentConfig GetDefaultStencilConfig(bool readable) { - return RenderTarget::AttachmentConfig{ - .storage_mode = StorageMode::kDeviceTransient, - .load_action = LoadAction::kDontCare, - .store_action = StoreAction::kDontCare, - }; -} +static const constexpr RenderTarget::AttachmentConfig kDefaultStencilConfig = + RenderTarget::AttachmentConfig{ + .storage_mode = StorageMode::kDeviceTransient, + .load_action = LoadAction::kDontCare, + .store_action = StoreAction::kDontCare, + }; static EntityPassTarget CreateRenderTarget(ContentContext& renderer, ISize size, @@ -284,8 +283,8 @@ 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 - GetDefaultStencilConfig(readable) // stencil_attachment_config + .clear_color = clear_color}, // color_attachment_config + kDefaultStencilConfig // stencil_attachment_config ); } else { target = RenderTarget::CreateOffscreen( @@ -298,8 +297,8 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer, .load_action = LoadAction::kDontCare, .store_action = StoreAction::kDontCare, .clear_color = clear_color, - }, // color_attachment_config - GetDefaultStencilConfig(readable) // stencil_attachment_config + }, // color_attachment_config + kDefaultStencilConfig // stencil_attachment_config ); } @@ -360,10 +359,6 @@ bool EntityPass::Render(ContentContext& renderer, // and then blit the results onto the onscreen texture. If using this branch, // there's no need to set up a stencil attachment on the root render target. if (!supports_onscreen_backdrop_reads && reads_from_onscreen_backdrop) { - if (superpass_) { - FML_LOG(ERROR) << "Offscreen has superpass that reads from backdrop"; - } - auto offscreen_target = CreateRenderTarget( renderer, root_render_target.GetRenderTargetSize(), true, GetClearColor(render_target.GetRenderTargetSize())); @@ -468,8 +463,7 @@ bool EntityPass::Render(ContentContext& renderer, *renderer.GetContext(), *renderer.GetRenderTargetCache(), color0.texture->GetSize(), renderer.GetContext()->GetCapabilities()->SupportsOffscreenMSAA(), - "ImpellerOnscreen", - GetDefaultStencilConfig(reads_from_onscreen_backdrop)); + "ImpellerOnscreen", kDefaultStencilConfig); } // Set up the clear color of the root pass. @@ -1190,7 +1184,9 @@ void EntityPass::SetEnableOffscreenCheckerboard(bool enabled) { enable_offscreen_debug_checkerboard_ = enabled; } -void EntityPassClipReplay::RecordEntity(Entity entity, +EntityPassClipReplay::EntityPassClipReplay() {} + +void EntityPassClipReplay::RecordEntity(const Entity& entity, Contents::ClipCoverage::Type type) { switch (type) { case Contents::ClipCoverage::Type::kNoChange: diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index fef80bafdbc8e..eb1e98961d42b 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -4,7 +4,6 @@ #pragma once -#include #include #include #include @@ -320,8 +319,12 @@ class EntityPass { /// stencil buffer is left in an identical state. class EntityPassClipReplay { public: + EntityPassClipReplay(); + + ~EntityPassClipReplay() = default; + /// @brief Record the entity based on the provided coverage [type]. - void RecordEntity(Entity entity, Contents::ClipCoverage::Type type); + void RecordEntity(const Entity& entity, Contents::ClipCoverage::Type type); const std::vector& GetReplayEntities() const; From 0ad0fbc247d6a58529d72a8abbd0e0256770606d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 31 Oct 2023 13:46:48 -0700 Subject: [PATCH 7/8] ++ --- impeller/entity/entity_pass.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index af57c061f8726..b5e90fe0b5239 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -721,7 +721,8 @@ bool EntityPass::RenderElement(Entity& element_entity, if (result.backdrop_texture) { // Restore any clips that were recorded before the backdrop filter was // applied. - for (const auto& entity : clip_replay_->GetReplayEntities()) { + auto& replay_entities = clip_replay_->GetReplayEntities(); + for (const auto& entity : replay_entities) { if (!entity.Render(renderer, *result.pass)) { VALIDATION_LOG << "Failed to render entity for clip restore."; } @@ -1193,6 +1194,7 @@ void EntityPassClipReplay::RecordEntity(const Entity& entity, return; case Contents::ClipCoverage::Type::kAppend: rendered_clip_entities_.push_back(entity); + break; case Contents::ClipCoverage::Type::kRestore: rendered_clip_entities_.pop_back(); break; From 7ef468bcada74865b2a0757882e9df07ad1bf6b6 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 31 Oct 2023 16:39:16 -0700 Subject: [PATCH 8/8] bdero review. --- impeller/entity/entity_pass.cc | 22 ++++++++++------------ impeller/entity/entity_pass.h | 12 ++++++------ 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index b5e90fe0b5239..4ace9d4f8304b 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -262,7 +262,6 @@ static const constexpr RenderTarget::AttachmentConfig kDefaultStencilConfig = static EntityPassTarget CreateRenderTarget(ContentContext& renderer, ISize size, - bool readable, const Color& clear_color) { auto context = renderer.GetContext(); @@ -359,9 +358,9 @@ bool EntityPass::Render(ContentContext& renderer, // and then blit the results onto the onscreen texture. If using this branch, // there's no need to set up a stencil attachment on the root render target. if (!supports_onscreen_backdrop_reads && reads_from_onscreen_backdrop) { - auto offscreen_target = CreateRenderTarget( - renderer, root_render_target.GetRenderTargetSize(), true, - GetClearColor(render_target.GetRenderTargetSize())); + auto offscreen_target = + CreateRenderTarget(renderer, root_render_target.GetRenderTargetSize(), + GetClearColor(render_target.GetRenderTargetSize())); if (!OnRender(renderer, // renderer capture, // capture @@ -621,10 +620,9 @@ 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->GetClearColor(subpass_size)); // clear_color if (!subpass_target.IsValid()) { VALIDATION_LOG << "Subpass render target is invalid."; @@ -1185,10 +1183,10 @@ void EntityPass::SetEnableOffscreenCheckerboard(bool enabled) { enable_offscreen_debug_checkerboard_ = enabled; } -EntityPassClipReplay::EntityPassClipReplay() {} +EntityPassClipRecorder::EntityPassClipRecorder() {} -void EntityPassClipReplay::RecordEntity(const Entity& entity, - Contents::ClipCoverage::Type type) { +void EntityPassClipRecorder::RecordEntity(const Entity& entity, + Contents::ClipCoverage::Type type) { switch (type) { case Contents::ClipCoverage::Type::kNoChange: return; @@ -1201,7 +1199,7 @@ void EntityPassClipReplay::RecordEntity(const Entity& entity, } } -const std::vector& EntityPassClipReplay::GetReplayEntities() const { +const std::vector& EntityPassClipRecorder::GetReplayEntities() const { return rendered_clip_entities_; } diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index eb1e98961d42b..90d2eb51adece 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -19,7 +19,7 @@ namespace impeller { class ContentContext; -class EntityPassClipReplay; +class EntityPassClipRecorder; class EntityPass { public: @@ -286,8 +286,8 @@ class EntityPass { bool flood_clip_ = false; bool enable_offscreen_debug_checkerboard_ = false; std::optional bounds_limit_; - std::unique_ptr clip_replay_ = - std::make_unique(); + std::unique_ptr clip_replay_ = + std::make_unique(); /// These values are incremented whenever something is added to the pass that /// requires reading from the backdrop texture. Currently, this can happen in @@ -317,11 +317,11 @@ class EntityPass { /// /// These clips are replayed when restoring the backdrop so that the /// stencil buffer is left in an identical state. -class EntityPassClipReplay { +class EntityPassClipRecorder { public: - EntityPassClipReplay(); + EntityPassClipRecorder(); - ~EntityPassClipReplay() = default; + ~EntityPassClipRecorder() = default; /// @brief Record the entity based on the provided coverage [type]. void RecordEntity(const Entity& entity, Contents::ClipCoverage::Type type);