From 8ec3fb867a5469ad57badb60b4496862fcd31766 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 29 Feb 2024 13:45:26 -0800 Subject: [PATCH 1/7] [Impeller] testing. --- impeller/aiks/picture.cc | 6 ++++-- impeller/entity/contents/content_context.h | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/impeller/aiks/picture.cc b/impeller/aiks/picture.cc index 77f7fa8dbcfd0..c2cbac29cb0fc 100644 --- a/impeller/aiks/picture.cc +++ b/impeller/aiks/picture.cc @@ -67,7 +67,8 @@ std::shared_ptr Picture::RenderToTexture( "Picture Snapshot MSAA", // label RenderTarget:: kDefaultColorAttachmentConfigMSAA, // color_attachment_config - std::nullopt // stencil_attachment_config + RenderTarget:: + kDefaultStencilAttachmentConfig // stencil_attachment_config ); } else { target = render_target_allocator.CreateOffscreen( @@ -76,7 +77,8 @@ std::shared_ptr Picture::RenderToTexture( /*mip_count=*/1, "Picture Snapshot", // label RenderTarget::kDefaultColorAttachmentConfig, // color_attachment_config - std::nullopt // stencil_attachment_config + RenderTarget:: + kDefaultStencilAttachmentConfig // stencil_attachment_config ); } if (!target.IsValid()) { diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index 6d4fc90298689..d38df9ae03b5a 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -408,7 +408,7 @@ class ContentContext { /// // TODO(bdero): Remove this setting once StC is fully de-risked // https://github.com/flutter/flutter/issues/123671 - static constexpr bool kEnableStencilThenCover = false; + static constexpr bool kEnableStencilThenCover = true; #if IMPELLER_ENABLE_3D std::shared_ptr GetSceneContext() const; From 0dffce4576735ebbe0af62958fe51ec4695be271 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 29 Feb 2024 14:30:17 -0800 Subject: [PATCH 2/7] ++ --- impeller/entity/contents/content_context.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index e45d654bb9784..4021c1f141870 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -487,8 +487,7 @@ fml::StatusOr ContentContext::MakeSubpass( RenderTarget subpass_target; std::optional depth_stencil_config = - depth_stencil_enabled ? RenderTarget::kDefaultStencilAttachmentConfig - : std::optional(); + RenderTarget::kDefaultStencilAttachmentConfig; if (context->GetCapabilities()->SupportsOffscreenMSAA() && msaa_enabled) { subpass_target = GetRenderTargetCache()->CreateOffscreenMSAA( From 72806f3a7db4c4b4389dd78af049893a1135db7a Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Thu, 29 Feb 2024 15:27:03 -0800 Subject: [PATCH 3/7] Update content_context.cc --- impeller/entity/contents/content_context.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index 4021c1f141870..42f1ce6990f03 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -486,8 +486,9 @@ fml::StatusOr ContentContext::MakeSubpass( const std::shared_ptr& context = GetContext(); RenderTarget subpass_target; - std::optional depth_stencil_config = - RenderTarget::kDefaultStencilAttachmentConfig; + std::optional depth_stencil_config = + depth_stencil_enabled ? RenderTarget::kDefaultStencilAttachmentConfig + : std::optional(); if (context->GetCapabilities()->SupportsOffscreenMSAA() && msaa_enabled) { subpass_target = GetRenderTargetCache()->CreateOffscreenMSAA( From 457c16e8797985e5c38ecb2471a6923a9416d098 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 29 Feb 2024 16:42:19 -0800 Subject: [PATCH 4/7] more experiments. --- impeller/entity/contents/content_context.cc | 2 +- impeller/entity/inline_pass_context.cc | 22 ++++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index 42f1ce6990f03..e45d654bb9784 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -486,7 +486,7 @@ fml::StatusOr ContentContext::MakeSubpass( const std::shared_ptr& context = GetContext(); RenderTarget subpass_target; - std::optional depth_stencil_config = + std::optional depth_stencil_config = depth_stencil_enabled ? RenderTarget::kDefaultStencilAttachmentConfig : std::optional(); diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index d39d41394fe34..a9639b075695b 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -146,17 +146,17 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass( color0.store_action = is_msaa ? StoreAction::kMultisampleResolve : StoreAction::kStore; - if (ContentContext::kEnableStencilThenCover) { - auto depth = pass_target_.GetRenderTarget().GetDepthAttachment(); - if (!depth.has_value()) { - VALIDATION_LOG << "Depth attachment unexpectedly missing from the " - "EntityPass render target."; - return {}; - } - depth->load_action = LoadAction::kClear; - depth->store_action = StoreAction::kDontCare; - pass_target_.target_.SetDepthAttachment(depth.value()); - } + // if (ContentContext::kEnableStencilThenCover) { + // auto depth = pass_target_.GetRenderTarget().GetDepthAttachment(); + // if (!depth.has_value()) { + // VALIDATION_LOG << "Depth attachment unexpectedly missing from the " + // "EntityPass render target."; + // return {}; + // } + // depth->load_action = LoadAction::kClear; + // depth->store_action = StoreAction::kDontCare; + // pass_target_.target_.SetDepthAttachment(depth.value()); + // } auto depth = pass_target_.GetRenderTarget().GetDepthAttachment(); auto stencil = pass_target_.GetRenderTarget().GetStencilAttachment(); From 031a1190fa1853ee5310f94aa84f25df6fce76ab Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 29 Feb 2024 16:43:31 -0800 Subject: [PATCH 5/7] ++ --- impeller/entity/entity_pass.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 9da1ae322568b..63c56fe1f5a46 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -764,7 +764,7 @@ bool EntityPass::RenderElement(Entity& element_entity, Entity msaa_backdrop_entity; msaa_backdrop_entity.SetContents(std::move(msaa_backdrop_contents)); msaa_backdrop_entity.SetBlendMode(BlendMode::kSource); - msaa_backdrop_entity.SetNewClipDepth(std::numeric_limits::max()); + msaa_backdrop_entity.SetNewClipDepth(1000); // Just set a big number. if (!msaa_backdrop_entity.Render(renderer, *result.pass)) { VALIDATION_LOG << "Failed to render MSAA backdrop filter entity."; return false; From c7980f52e5fa45b3c217ca2caf0b462af324acd1 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 29 Feb 2024 16:49:14 -0800 Subject: [PATCH 6/7] ++ --- impeller/entity/entity_pass.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 63c56fe1f5a46..cccb25e21bdb8 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -764,7 +764,7 @@ bool EntityPass::RenderElement(Entity& element_entity, Entity msaa_backdrop_entity; msaa_backdrop_entity.SetContents(std::move(msaa_backdrop_contents)); msaa_backdrop_entity.SetBlendMode(BlendMode::kSource); - msaa_backdrop_entity.SetNewClipDepth(1000); // Just set a big number. + msaa_backdrop_entity.SetNewClipDepth(1000); // Just set a big number. if (!msaa_backdrop_entity.Render(renderer, *result.pass)) { VALIDATION_LOG << "Failed to render MSAA backdrop filter entity."; return false; From 2f128f70c4868190fddc450b3e0470efc86136ec Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 4 Mar 2024 16:09:04 -0800 Subject: [PATCH 7/7] revert other changes. --- impeller/entity/contents/content_context.h | 2 +- impeller/entity/entity_pass.cc | 40 +++++++--------------- impeller/entity/inline_pass_context.cc | 23 +++++++------ 3 files changed, 25 insertions(+), 40 deletions(-) diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index d38df9ae03b5a..6d4fc90298689 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -408,7 +408,7 @@ class ContentContext { /// // TODO(bdero): Remove this setting once StC is fully de-risked // https://github.com/flutter/flutter/issues/123671 - static constexpr bool kEnableStencilThenCover = true; + static constexpr bool kEnableStencilThenCover = false; #if IMPELLER_ENABLE_3D std::shared_ptr GetSceneContext() const; diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index cccb25e21bdb8..f85a383fb37da 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -459,29 +459,11 @@ bool EntityPass::Render(ContentContext& renderer, // this method. auto color0 = root_render_target.GetColorAttachments().find(0u)->second; - // If a root stencil was provided by the caller, then verify that it has a - // configuration which can be used to render this pass. auto stencil_attachment = root_render_target.GetStencilAttachment(); auto depth_attachment = root_render_target.GetDepthAttachment(); - if (stencil_attachment.has_value() && depth_attachment.has_value()) { - auto stencil_texture = stencil_attachment->texture; - if (!stencil_texture) { - VALIDATION_LOG << "The root RenderTarget must have a stencil texture."; - return false; - } - - auto stencil_storage_mode = - stencil_texture->GetTextureDescriptor().storage_mode; - if (reads_from_onscreen_backdrop && - stencil_storage_mode == StorageMode::kDeviceTransient) { - VALIDATION_LOG << "The given root RenderTarget stencil needs to be read, " - "but it's marked as transient."; - return false; - } - } - // Setup a new root stencil with an optimal configuration if one wasn't - // provided by the caller. - else { + if (!stencil_attachment.has_value() || !depth_attachment.has_value()) { + // Setup a new root stencil with an optimal configuration if one wasn't + // provided by the caller. root_render_target.SetupDepthStencilAttachments( *renderer.GetContext(), *renderer.GetContext()->GetResourceAllocator(), color0.texture->GetSize(), @@ -739,12 +721,7 @@ bool EntityPass::RenderElement(Entity& element_entity, 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) { + if (result.just_created) { // Restore any clips that were recorded before the backdrop filter was // applied. auto& replay_entities = clip_replay_->GetReplayEntities(); @@ -753,7 +730,14 @@ bool EntityPass::RenderElement(Entity& element_entity, VALIDATION_LOG << "Failed to render entity for clip restore."; } } + } + // 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); @@ -764,7 +748,7 @@ bool EntityPass::RenderElement(Entity& element_entity, Entity msaa_backdrop_entity; msaa_backdrop_entity.SetContents(std::move(msaa_backdrop_contents)); msaa_backdrop_entity.SetBlendMode(BlendMode::kSource); - msaa_backdrop_entity.SetNewClipDepth(1000); // Just set a big number. + msaa_backdrop_entity.SetNewClipDepth(std::numeric_limits::max()); if (!msaa_backdrop_entity.Render(renderer, *result.pass)) { VALIDATION_LOG << "Failed to render MSAA backdrop filter entity."; return false; diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index a9639b075695b..73b15b16a9086 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -146,17 +146,17 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass( color0.store_action = is_msaa ? StoreAction::kMultisampleResolve : StoreAction::kStore; - // if (ContentContext::kEnableStencilThenCover) { - // auto depth = pass_target_.GetRenderTarget().GetDepthAttachment(); - // if (!depth.has_value()) { - // VALIDATION_LOG << "Depth attachment unexpectedly missing from the " - // "EntityPass render target."; - // return {}; - // } - // depth->load_action = LoadAction::kClear; - // depth->store_action = StoreAction::kDontCare; - // pass_target_.target_.SetDepthAttachment(depth.value()); - // } + if (ContentContext::kEnableStencilThenCover) { + auto depth = pass_target_.GetRenderTarget().GetDepthAttachment(); + if (!depth.has_value()) { + VALIDATION_LOG << "Depth attachment unexpectedly missing from the " + "EntityPass render target."; + return {}; + } + depth->load_action = LoadAction::kClear; + depth->store_action = StoreAction::kDontCare; + pass_target_.target_.SetDepthAttachment(depth.value()); + } auto depth = pass_target_.GetRenderTarget().GetDepthAttachment(); auto stencil = pass_target_.GetRenderTarget().GetStencilAttachment(); @@ -187,6 +187,7 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass( " Count=" + std::to_string(pass_count_)); result.pass = pass_; + result.just_created = true; if (!renderer_.GetContext()->GetCapabilities()->SupportsReadFromResolve() && result.backdrop_texture ==