From b483f13d5e0ce67719d04736b1765bc191155e2e Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Thu, 22 Feb 2024 00:33:16 -0800 Subject: [PATCH 1/8] [Impeller] Enable depth buffer clipping & stencil-then-cover path rendering. --- impeller/aiks/aiks_unittests.cc | 11 +++++++++-- impeller/entity/contents/color_source_contents.h | 1 + impeller/entity/contents/content_context.cc | 14 ++++++++------ impeller/entity/contents/content_context.h | 3 ++- impeller/entity/contents/contents.cc | 2 +- impeller/entity/entity.h | 2 +- impeller/entity/entity_unittests.cc | 6 +++++- 7 files changed, 27 insertions(+), 12 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index ef646c0ece078..0834abc3b60f9 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3537,7 +3537,8 @@ TEST_P(AiksTest, CorrectClipDepthAssignedToEntities) { // once we switch to the clip depth approach. auto picture = canvas.EndRecordingAsPicture(); - std::array expected = { + + std::vector expected = { 2, // DrawRRect 4, // ClipRRect -- Has a depth value equal to the max depth of all the // content it affect. In this case, the SaveLayer and all @@ -3546,8 +3547,14 @@ TEST_P(AiksTest, CorrectClipDepthAssignedToEntities) { // contents are rendered, so it should have a depth value // greater than all its contents. 3, // DrawRRect - 5, // Restore (will be removed once we switch to the clip depth approach) }; + + if constexpr (!ContentContext::kEnableStencilThenCover) { + expected.push_back( // + 5 // Restore (no longer necessary when clipping on the depth buffer) + ); + } + std::vector actual; picture.pass->IterateAllElements([&](EntityPass::Element& element) -> bool { diff --git a/impeller/entity/contents/color_source_contents.h b/impeller/entity/contents/color_source_contents.h index 4951494b69030..301435e2f0c87 100644 --- a/impeller/entity/contents/color_source_contents.h +++ b/impeller/entity/contents/color_source_contents.h @@ -167,6 +167,7 @@ class ColorSourceContents : public Contents { /// Cover draw. + options.blend_mode = entity.GetBlendMode(); options.stencil_mode = ContentContextOptions::StencilMode::kCoverCompare; std::optional maybe_cover_area = diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index 270e8173508c2..2ab8d93599fb8 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -480,23 +480,25 @@ fml::StatusOr ContentContext::MakeSubpass( ISize texture_size, const SubpassCallback& subpass_callback, bool msaa_enabled, + bool depth_stencil_enabled, int32_t mip_count) const { const std::shared_ptr& context = GetContext(); RenderTarget subpass_target; + + std::optional depth_stencil_config = + depth_stencil_enabled ? RenderTarget::kDefaultStencilAttachmentConfig + : std::optional(); + if (context->GetCapabilities()->SupportsOffscreenMSAA() && msaa_enabled) { subpass_target = RenderTarget::CreateOffscreenMSAA( *context, *GetRenderTargetCache(), texture_size, /*mip_count=*/mip_count, SPrintF("%s Offscreen", label.c_str()), - RenderTarget::kDefaultColorAttachmentConfigMSAA, - std::nullopt // stencil_attachment_config - ); + RenderTarget::kDefaultColorAttachmentConfigMSAA, depth_stencil_config); } else { subpass_target = RenderTarget::CreateOffscreen( *context, *GetRenderTargetCache(), texture_size, /*mip_count=*/mip_count, SPrintF("%s Offscreen", label.c_str()), - RenderTarget::kDefaultColorAttachmentConfig, // - std::nullopt // stencil_attachment_config - ); + RenderTarget::kDefaultColorAttachmentConfig, depth_stencil_config); } return MakeSubpass(label, subpass_target, subpass_callback); } diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index 67e840716557d..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; @@ -793,6 +793,7 @@ class ContentContext { ISize texture_size, const SubpassCallback& subpass_callback, bool msaa_enabled = true, + bool depth_stencil_enabled = false, int32_t mip_count = 1) const; /// Makes a subpass that will render to `subpass_target`. diff --git a/impeller/entity/contents/contents.cc b/impeller/entity/contents/contents.cc index b71ceca97db2e..7c0c62af98ac0 100644 --- a/impeller/entity/contents/contents.cc +++ b/impeller/entity/contents/contents.cc @@ -101,7 +101,7 @@ std::optional Contents::RenderToSnapshot( entity.GetTransform()); return contents.Render(renderer, sub_entity, pass); }, - msaa_enabled, + msaa_enabled, /*depth_stencil_enabled=*/true, std::min(mip_count, static_cast(subpass_size.MipCount()))); if (!render_target.ok()) { diff --git a/impeller/entity/entity.h b/impeller/entity/entity.h index e0a14fd218ffd..430a0423e5257 100644 --- a/impeller/entity/entity.h +++ b/impeller/entity/entity.h @@ -131,7 +131,7 @@ class Entity { std::shared_ptr contents_; BlendMode blend_mode_ = BlendMode::kSourceOver; uint32_t clip_depth_ = 0u; - uint32_t new_clip_depth_ = 0u; + uint32_t new_clip_depth_ = 1u; mutable Capture capture_; }; diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index 07b065d0a10ea..d882216f63c9b 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -2788,7 +2788,11 @@ TEST_P(EntityTest, FillPathGeometryGetPositionBufferReturnsExpectedMode) { .Close() .TakePath(); GeometryResult result = get_result(path); - EXPECT_EQ(result.mode, GeometryResult::Mode::kNormal); + if constexpr (ContentContext::kEnableStencilThenCover) { + EXPECT_EQ(result.mode, GeometryResult::Mode::kNonZero); + } else { + EXPECT_EQ(result.mode, GeometryResult::Mode::kNormal); + } } } From 8699fb7b04c23cf3248a0d4b0e533d143e36548c Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Thu, 22 Feb 2024 01:59:46 -0800 Subject: [PATCH 2/8] Improve test error --- impeller/entity/entity_unittests.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index d882216f63c9b..20e69bdad4ce4 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -2676,7 +2676,13 @@ TEST_P(EntityTest, AdvancedBlendCoverageHintIsNotResetByEntityPass) { EXPECT_EQ(test_allocator->GetDescriptors()[2].size, ISize(200, 200)); EXPECT_EQ(test_allocator->GetDescriptors()[3].size, ISize(200, 200)); } else { - EXPECT_TRUE(false); + std::stringstream sizes; + for (const auto& desc : test_allocator->GetDescriptors()) { + sizes << "\nISize" << desc.size; + } + EXPECT_TRUE(false) << "Unexpected number of render targets. Total: " + << test_allocator->GetDescriptors().size() + << "\nExpected sizes: " << sizes.str(); } } From 9b191596f0315f71fcba1b49c6ac63047413eb52 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Thu, 22 Feb 2024 02:44:09 -0800 Subject: [PATCH 3/8] Make EntityTest.AdvancedBlendCoverageHintIsNotResetByEntityPass less fragile --- impeller/entity/entity_unittests.cc | 51 ++++++++++------------------- 1 file changed, 17 insertions(+), 34 deletions(-) diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index 20e69bdad4ce4..0d0da4791cc77 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include #include #include #include @@ -2606,7 +2607,9 @@ class TestRenderTargetAllocator : public RenderTargetAllocator { void End() override { RenderTargetAllocator::End(); } - std::vector GetDescriptors() const { return allocated_; } + std::vector GetAllocatedTextureDescriptors() const { + return allocated_; + } void ResetDescriptors() { allocated_.clear(); } @@ -2651,39 +2654,19 @@ TEST_P(EntityTest, AdvancedBlendCoverageHintIsNotResetByEntityPass) { EXPECT_TRUE(pass->Render(content_context, rt)); - if (test_allocator->GetDescriptors().size() == 6u) { - EXPECT_EQ(test_allocator->GetDescriptors()[0].size, ISize(1000, 1000)); - EXPECT_EQ(test_allocator->GetDescriptors()[1].size, ISize(1000, 1000)); - EXPECT_EQ(test_allocator->GetDescriptors()[2].size, ISize(1000, 1000)); - EXPECT_EQ(test_allocator->GetDescriptors()[3].size, ISize(1000, 1000)); - - EXPECT_EQ(test_allocator->GetDescriptors()[4].size, ISize(200, 200)); - EXPECT_EQ(test_allocator->GetDescriptors()[5].size, ISize(200, 200)); - } else if (test_allocator->GetDescriptors().size() == 7u) { - // Onscreen render target. - EXPECT_EQ(test_allocator->GetDescriptors()[0].size, ISize(1000, 1000)); - EXPECT_EQ(test_allocator->GetDescriptors()[1].size, ISize(1000, 1000)); - EXPECT_EQ(test_allocator->GetDescriptors()[2].size, ISize(1000, 1000)); - - EXPECT_EQ(test_allocator->GetDescriptors()[3].size, ISize(200, 200)); - EXPECT_EQ(test_allocator->GetDescriptors()[4].size, ISize(200, 200)); - EXPECT_EQ(test_allocator->GetDescriptors()[5].size, ISize(200, 200)); - EXPECT_EQ(test_allocator->GetDescriptors()[6].size, ISize(200, 200)); - } else if (test_allocator->GetDescriptors().size() == 4u) { - EXPECT_EQ(test_allocator->GetDescriptors()[0].size, ISize(1000, 1000)); - EXPECT_EQ(test_allocator->GetDescriptors()[1].size, ISize(1000, 1000)); - - EXPECT_EQ(test_allocator->GetDescriptors()[2].size, ISize(200, 200)); - EXPECT_EQ(test_allocator->GetDescriptors()[3].size, ISize(200, 200)); - } else { - std::stringstream sizes; - for (const auto& desc : test_allocator->GetDescriptors()) { - sizes << "\nISize" << desc.size; - } - EXPECT_TRUE(false) << "Unexpected number of render targets. Total: " - << test_allocator->GetDescriptors().size() - << "\nExpected sizes: " << sizes.str(); - } + std::vector descriptors = + test_allocator->GetAllocatedTextureDescriptors(); + + auto contains_size = [&descriptors](ISize size) -> bool { + return std::find_if(descriptors.begin(), descriptors.end(), + [&size](auto desc) { return desc.size == size; }) != + descriptors.end(); + }; + + EXPECT_TRUE(contains_size(ISize(1000, 1000))) + << "The root texture wasn't allocated"; + EXPECT_TRUE(contains_size(ISize(200, 200))) + << "The ColorBurned texture wasn't allocated (100x100 scales up 2x)"; } TEST_P(EntityTest, SpecializationConstantsAreAppliedToVariants) { From 00d4b723812836392eace06c97bce6a98dcd3e57 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Thu, 22 Feb 2024 03:34:31 -0800 Subject: [PATCH 4/8] still need clip restores for clip coverage tracking --- impeller/aiks/canvas.cc | 6 ++---- impeller/entity/entity_pass.cc | 5 +++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index 1c8c963c3e31a..2fbfe2b2394d5 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -219,10 +219,8 @@ bool Canvas::Restore() { } transform_stack_.pop_back(); - if constexpr (!ContentContext::kEnableStencilThenCover) { - if (num_clips > 0) { - RestoreClip(); - } + if (num_clips > 0) { + RestoreClip(); } return true; diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 76a5d313e0dae..1e485b9c4781e 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -839,6 +839,11 @@ bool EntityPass::RenderElement(Entity& element_entity, } clip_coverage_stack.resize(restoration_index + 1); + if constexpr (ContentContext::kEnableStencilThenCover) { + // Skip all clip restores when stencil-then-cover is enabled. + return true; + } + if (!clip_coverage_stack.back().coverage.has_value()) { // Running this restore op won't make anything renderable, so skip it. return true; From fbaad59f05d9ea0d7e204c8de3ccfae4380a8324 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Thu, 22 Feb 2024 05:50:19 -0800 Subject: [PATCH 5/8] Rework to make PositionUV case work correctly --- .../entity/contents/color_source_contents.h | 162 +++++++----------- .../contents/conical_gradient_contents.cc | 4 +- .../contents/linear_gradient_contents.cc | 4 +- .../contents/radial_gradient_contents.cc | 4 +- .../contents/runtime_effect_contents.cc | 8 +- .../entity/contents/solid_color_contents.cc | 2 +- .../contents/sweep_gradient_contents.cc | 4 +- .../entity/contents/tiled_texture_contents.cc | 10 +- .../entity/geometry/fill_path_geometry.cc | 34 ++-- impeller/entity/geometry/fill_path_geometry.h | 3 + impeller/entity/geometry/geometry.cc | 4 + impeller/entity/geometry/geometry.h | 2 + impeller/entity/geometry/rect_geometry.h | 2 +- .../entity/geometry/stroke_path_geometry.cc | 5 + .../entity/geometry/stroke_path_geometry.h | 3 + 15 files changed, 121 insertions(+), 130 deletions(-) diff --git a/impeller/entity/contents/color_source_contents.h b/impeller/entity/contents/color_source_contents.h index 301435e2f0c87..a23fae582ab86 100644 --- a/impeller/entity/contents/color_source_contents.h +++ b/impeller/entity/contents/color_source_contents.h @@ -10,6 +10,7 @@ #include "impeller/entity/contents/content_context.h" #include "impeller/entity/contents/contents.h" #include "impeller/entity/geometry/geometry.h" +#include "impeller/entity/geometry/rect_geometry.h" #include "impeller/geometry/matrix.h" namespace impeller { @@ -112,14 +113,76 @@ class ColorSourceContents : public Contents { ContentContextOptions)>; template - bool DrawGeometry(GeometryResult geometry_result, - const ContentContext& renderer, + bool DrawGeometry(const ContentContext& renderer, const Entity& entity, RenderPass& pass, const PipelineBuilderCallback& pipeline_callback, typename VertexShaderT::FrameInfo frame_info, - const BindFragmentCallback& bind_fragment_callback) const { + const BindFragmentCallback& bind_fragment_callback, + bool enable_uvs = false, + Rect texture_coverage = {}, + const Matrix& effect_transform = {}) const { auto options = OptionsFromPassAndEntity(pass, entity); + + GeometryResult::Mode geometry_mode = GetGeometry()->GetResultMode(); + Geometry& geometry = *GetGeometry(); + + const bool is_stencil_then_cover = + geometry_mode == GeometryResult::Mode::kNonZero || + geometry_mode == GeometryResult::Mode::kEvenOdd; + if (is_stencil_then_cover) { + pass.SetStencilReference(0); + + /// Stencil preparation draw. + + GeometryResult stencil_geometry_result = + GetGeometry()->GetPositionBuffer(renderer, entity, pass); + pass.SetVertexBuffer(std::move(stencil_geometry_result.vertex_buffer)); + options.primitive_type = stencil_geometry_result.type; + + options.blend_mode = BlendMode::kDestination; + switch (stencil_geometry_result.mode) { + case GeometryResult::Mode::kNonZero: + pass.SetCommandLabel("Stencil preparation (NonZero)"); + options.stencil_mode = + ContentContextOptions::StencilMode::kStencilNonZeroFill; + break; + case GeometryResult::Mode::kEvenOdd: + pass.SetCommandLabel("Stencil preparation (EvenOdd)"); + options.stencil_mode = + ContentContextOptions::StencilMode::kStencilEvenOddFill; + break; + default: + FML_UNREACHABLE(); + } + pass.SetPipeline(renderer.GetClipPipeline(options)); + ClipPipeline::VertexShader::FrameInfo clip_frame_info; + clip_frame_info.depth = entity.GetShaderClipDepth(); + clip_frame_info.mvp = stencil_geometry_result.transform; + ClipPipeline::VertexShader::BindFrameInfo( + pass, renderer.GetTransientsBuffer().EmplaceUniform(clip_frame_info)); + + if (!pass.Draw().ok()) { + return false; + } + + /// Cover draw. + + options.blend_mode = entity.GetBlendMode(); + options.stencil_mode = ContentContextOptions::StencilMode::kCoverCompare; + std::optional maybe_cover_area = GetGeometry()->GetCoverage({}); + if (!maybe_cover_area.has_value()) { + return true; + } + geometry = RectGeometry(maybe_cover_area.value()); + } + + GeometryResult geometry_result = + enable_uvs + ? geometry.GetPositionUVBuffer(texture_coverage, effect_transform, + renderer, entity, pass) + : geometry.GetPositionBuffer(renderer, entity, pass); + pass.SetVertexBuffer(std::move(geometry_result.vertex_buffer)); options.primitive_type = geometry_result.type; // Take the pre-populated vertex shader uniform struct and set managed @@ -127,66 +190,6 @@ class ColorSourceContents : public Contents { frame_info.depth = entity.GetShaderClipDepth(); frame_info.mvp = geometry_result.transform; - pass.SetVertexBuffer(std::move(geometry_result.vertex_buffer)); - - if constexpr (ContentContext::kEnableStencilThenCover) { - const bool is_stencil_then_cover = - geometry_result.mode == GeometryResult::Mode::kNonZero || - geometry_result.mode == GeometryResult::Mode::kEvenOdd; - if (is_stencil_then_cover) { - pass.SetStencilReference(0); - - /// Stencil preparation draw. - - options.blend_mode = BlendMode::kDestination; - switch (geometry_result.mode) { - case GeometryResult::Mode::kNonZero: - pass.SetCommandLabel("Stencil preparation (NonZero)"); - options.stencil_mode = - ContentContextOptions::StencilMode::kStencilNonZeroFill; - break; - case GeometryResult::Mode::kEvenOdd: - pass.SetCommandLabel("Stencil preparation (EvenOdd)"); - options.stencil_mode = - ContentContextOptions::StencilMode::kStencilEvenOddFill; - break; - default: - FML_UNREACHABLE(); - } - pass.SetPipeline(renderer.GetClipPipeline(options)); - ClipPipeline::VertexShader::FrameInfo clip_frame_info; - clip_frame_info.depth = entity.GetShaderClipDepth(); - clip_frame_info.mvp = geometry_result.transform; - ClipPipeline::VertexShader::BindFrameInfo( - pass, - renderer.GetTransientsBuffer().EmplaceUniform(clip_frame_info)); - - if (!pass.Draw().ok()) { - return false; - } - - /// Cover draw. - - options.blend_mode = entity.GetBlendMode(); - options.stencil_mode = - ContentContextOptions::StencilMode::kCoverCompare; - std::optional maybe_cover_area = - GetGeometry()->GetCoverage(entity.GetTransform()); - if (!maybe_cover_area.has_value()) { - return true; - } - auto points = maybe_cover_area.value().GetPoints(); - auto vertices = - VertexBufferBuilder{} - .AddVertices( - {{points[0]}, {points[1]}, {points[2]}, {points[3]}}) - .CreateVertexBuffer(renderer.GetTransientsBuffer()); - pass.SetVertexBuffer(std::move(vertices)); - - frame_info.mvp = pass.GetOrthographicTransform(); - } - } - // If overdraw prevention is enabled (like when drawing stroke paths), we // increment the stencil buffer as we draw, preventing overlapping fragments // from drawing. Afterwards, we need to append another draw call to clean up @@ -235,39 +238,6 @@ class ColorSourceContents : public Contents { return true; } - template - bool DrawPositions(const ContentContext& renderer, - const Entity& entity, - RenderPass& pass, - const PipelineBuilderCallback& pipeline_callback, - typename VertexShaderT::FrameInfo frame_info, - const BindFragmentCallback& bind_pipeline_callback) const { - GeometryResult geometry_result = - GetGeometry()->GetPositionBuffer(renderer, entity, pass); - - return DrawGeometry(std::move(geometry_result), renderer, - entity, pass, pipeline_callback, - frame_info, bind_pipeline_callback); - } - - template - bool DrawPositionsAndUVs( - Rect texture_coverage, - const Matrix& effect_transform, - const ContentContext& renderer, - const Entity& entity, - RenderPass& pass, - const PipelineBuilderCallback& pipeline_callback, - typename VertexShaderT::FrameInfo frame_info, - const BindFragmentCallback& bind_pipeline_callback) const { - auto geometry_result = GetGeometry()->GetPositionUVBuffer( - texture_coverage, effect_transform, renderer, entity, pass); - - return DrawGeometry(std::move(geometry_result), renderer, - entity, pass, pipeline_callback, - frame_info, bind_pipeline_callback); - } - private: std::shared_ptr geometry_; Matrix inverse_matrix_; diff --git a/impeller/entity/contents/conical_gradient_contents.cc b/impeller/entity/contents/conical_gradient_contents.cc index de3013d14f3f8..9a053a9260571 100644 --- a/impeller/entity/contents/conical_gradient_contents.cc +++ b/impeller/entity/contents/conical_gradient_contents.cc @@ -71,7 +71,7 @@ bool ConicalGradientContents::RenderSSBO(const ContentContext& renderer, [&renderer](ContentContextOptions options) { return renderer.GetConicalGradientSSBOFillPipeline(options); }; - return ColorSourceContents::DrawPositions( + return ColorSourceContents::DrawGeometry( renderer, entity, pass, pipeline_callback, frame_info, [this, &renderer](RenderPass& pass) { FS::FragInfo frag_info; @@ -128,7 +128,7 @@ bool ConicalGradientContents::RenderTexture(const ContentContext& renderer, [&renderer](ContentContextOptions options) { return renderer.GetConicalGradientFillPipeline(options); }; - return ColorSourceContents::DrawPositions( + return ColorSourceContents::DrawGeometry( renderer, entity, pass, pipeline_callback, frame_info, [this, &renderer, &gradient_texture](RenderPass& pass) { FS::FragInfo frag_info; diff --git a/impeller/entity/contents/linear_gradient_contents.cc b/impeller/entity/contents/linear_gradient_contents.cc index 8b7f8ad0bc9f8..99f2c79055f15 100644 --- a/impeller/entity/contents/linear_gradient_contents.cc +++ b/impeller/entity/contents/linear_gradient_contents.cc @@ -75,7 +75,7 @@ bool LinearGradientContents::RenderTexture(const ContentContext& renderer, [&renderer](ContentContextOptions options) { return renderer.GetLinearGradientFillPipeline(options); }; - return ColorSourceContents::DrawPositions( + return ColorSourceContents::DrawGeometry( renderer, entity, pass, pipeline_callback, frame_info, [this, &renderer](RenderPass& pass) { auto gradient_data = CreateGradientBuffer(colors_, stops_); @@ -126,7 +126,7 @@ bool LinearGradientContents::RenderSSBO(const ContentContext& renderer, [&renderer](ContentContextOptions options) { return renderer.GetLinearGradientSSBOFillPipeline(options); }; - return ColorSourceContents::DrawPositions( + return ColorSourceContents::DrawGeometry( renderer, entity, pass, pipeline_callback, frame_info, [this, &renderer](RenderPass& pass) { FS::FragInfo frag_info; diff --git a/impeller/entity/contents/radial_gradient_contents.cc b/impeller/entity/contents/radial_gradient_contents.cc index d60130ced04ea..9946a2425d6df 100644 --- a/impeller/entity/contents/radial_gradient_contents.cc +++ b/impeller/entity/contents/radial_gradient_contents.cc @@ -77,7 +77,7 @@ bool RadialGradientContents::RenderSSBO(const ContentContext& renderer, [&renderer](ContentContextOptions options) { return renderer.GetRadialGradientSSBOFillPipeline(options); }; - return ColorSourceContents::DrawPositions( + return ColorSourceContents::DrawGeometry( renderer, entity, pass, pipeline_callback, frame_info, [this, &renderer](RenderPass& pass) { FS::FragInfo frag_info; @@ -127,7 +127,7 @@ bool RadialGradientContents::RenderTexture(const ContentContext& renderer, [&renderer](ContentContextOptions options) { return renderer.GetRadialGradientFillPipeline(options); }; - return ColorSourceContents::DrawPositions( + return ColorSourceContents::DrawGeometry( renderer, entity, pass, pipeline_callback, frame_info, [this, &renderer, &gradient_texture](RenderPass& pass) { FS::FragInfo frag_info; diff --git a/impeller/entity/contents/runtime_effect_contents.cc b/impeller/entity/contents/runtime_effect_contents.cc index d51b89bf9e94e..423cbdcb95637 100644 --- a/impeller/entity/contents/runtime_effect_contents.cc +++ b/impeller/entity/contents/runtime_effect_contents.cc @@ -144,6 +144,8 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, BindFragmentCallback bind_callback = [this, &renderer, &context, &descriptor_set_layouts]( RenderPass& pass) { + descriptor_set_layouts.clear(); + size_t minimum_sampler_index = 100000000; size_t buffer_index = 0; size_t buffer_offset = 0; @@ -313,9 +315,9 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, runtime_stage_->GetEntrypoint(), options, create_callback); }; - return ColorSourceContents::DrawPositions(renderer, entity, pass, - pipeline_callback, - VS::FrameInfo{}, bind_callback); + return ColorSourceContents::DrawGeometry(renderer, entity, pass, + pipeline_callback, + VS::FrameInfo{}, bind_callback); } } // namespace impeller diff --git a/impeller/entity/contents/solid_color_contents.cc b/impeller/entity/contents/solid_color_contents.cc index a8b46e297052e..083ac7d61ec38 100644 --- a/impeller/entity/contents/solid_color_contents.cc +++ b/impeller/entity/contents/solid_color_contents.cc @@ -58,7 +58,7 @@ bool SolidColorContents::Render(const ContentContext& renderer, [&renderer](ContentContextOptions options) { return renderer.GetSolidFillPipeline(options); }; - return ColorSourceContents::DrawPositions( + return ColorSourceContents::DrawGeometry( renderer, entity, pass, pipeline_callback, frame_info, [](RenderPass& pass) { pass.SetCommandLabel("Solid Fill"); diff --git a/impeller/entity/contents/sweep_gradient_contents.cc b/impeller/entity/contents/sweep_gradient_contents.cc index af43454ae9c2d..3586a14b9fc75 100644 --- a/impeller/entity/contents/sweep_gradient_contents.cc +++ b/impeller/entity/contents/sweep_gradient_contents.cc @@ -85,7 +85,7 @@ bool SweepGradientContents::RenderSSBO(const ContentContext& renderer, [&renderer](ContentContextOptions options) { return renderer.GetSweepGradientSSBOFillPipeline(options); }; - return ColorSourceContents::DrawPositions( + return ColorSourceContents::DrawGeometry( renderer, entity, pass, pipeline_callback, frame_info, [this, &renderer](RenderPass& pass) { FS::FragInfo frag_info; @@ -134,7 +134,7 @@ bool SweepGradientContents::RenderTexture(const ContentContext& renderer, [&renderer](ContentContextOptions options) { return renderer.GetSweepGradientFillPipeline(options); }; - return ColorSourceContents::DrawPositions( + return ColorSourceContents::DrawGeometry( renderer, entity, pass, pipeline_callback, frame_info, [this, &renderer, &gradient_texture](RenderPass& pass) { FS::FragInfo frag_info; diff --git a/impeller/entity/contents/tiled_texture_contents.cc b/impeller/entity/contents/tiled_texture_contents.cc index 5e73d93b96f39..0862345f6e1d1 100644 --- a/impeller/entity/contents/tiled_texture_contents.cc +++ b/impeller/entity/contents/tiled_texture_contents.cc @@ -154,9 +154,8 @@ bool TiledTextureContents::Render(const ContentContext& renderer, [&renderer, &pipeline_method](ContentContextOptions options) { return (renderer.*pipeline_method)(options); }; - return ColorSourceContents::DrawPositionsAndUVs( - Rect::MakeSize(texture_size), GetInverseEffectTransform(), renderer, - entity, pass, pipeline_callback, frame_info, + return ColorSourceContents::DrawGeometry( + renderer, entity, pass, pipeline_callback, frame_info, [this, &renderer, &is_external_texture, &uses_emulated_tile_mode](RenderPass& pass) { auto& host_buffer = renderer.GetTransientsBuffer(); @@ -215,7 +214,10 @@ bool TiledTextureContents::Render(const ContentContext& renderer, } return true; - }); + }, + /*enable_uvs=*/true, + /*texture_coverage=*/Rect::MakeSize(texture_size), + /*effect_transform=*/GetInverseEffectTransform()); } std::optional TiledTextureContents::RenderToSnapshot( diff --git a/impeller/entity/geometry/fill_path_geometry.cc b/impeller/entity/geometry/fill_path_geometry.cc index 959d858f6d73c..e187659ce04b1 100644 --- a/impeller/entity/geometry/fill_path_geometry.cc +++ b/impeller/entity/geometry/fill_path_geometry.cc @@ -15,21 +15,6 @@ FillPathGeometry::FillPathGeometry(const Path& path, std::optional inner_rect) : path_(path), inner_rect_(inner_rect) {} -static GeometryResult::Mode GetGeometryMode(const Path& path) { - if (!ContentContext::kEnableStencilThenCover || path.IsConvex()) { - return GeometryResult::Mode::kNormal; - } - - switch (path.GetFillType()) { - case FillType::kNonZero: - return GeometryResult::Mode::kNonZero; - case FillType::kOdd: - return GeometryResult::Mode::kEvenOdd; - } - - FML_UNREACHABLE(); -} - GeometryResult FillPathGeometry::GetPositionBuffer( const ContentContext& renderer, const Entity& entity, @@ -81,7 +66,7 @@ GeometryResult FillPathGeometry::GetPositionBuffer( .type = PrimitiveType::kTriangleStrip, .vertex_buffer = vertex_buffer, .transform = pass.GetOrthographicTransform() * entity.GetTransform(), - .mode = GetGeometryMode(path_), + .mode = GetResultMode(), }; } @@ -149,10 +134,25 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer( .vertex_buffer = vertex_builder.CreateVertexBuffer(renderer.GetTransientsBuffer()), .transform = pass.GetOrthographicTransform() * entity.GetTransform(), - .mode = GetGeometryMode(path_), + .mode = GetResultMode(), }; } +GeometryResult::Mode FillPathGeometry::GetResultMode() const { + if (!ContentContext::kEnableStencilThenCover || path_.IsConvex()) { + return GeometryResult::Mode::kNormal; + } + + switch (path_.GetFillType()) { + case FillType::kNonZero: + return GeometryResult::Mode::kNonZero; + case FillType::kOdd: + return GeometryResult::Mode::kEvenOdd; + } + + FML_UNREACHABLE(); +} + GeometryVertexType FillPathGeometry::GetVertexType() const { return GeometryVertexType::kPosition; } diff --git a/impeller/entity/geometry/fill_path_geometry.h b/impeller/entity/geometry/fill_path_geometry.h index c93a04b90cc8d..722f7cc661c40 100644 --- a/impeller/entity/geometry/fill_path_geometry.h +++ b/impeller/entity/geometry/fill_path_geometry.h @@ -42,6 +42,9 @@ class FillPathGeometry final : public Geometry { const Entity& entity, RenderPass& pass) const override; + // |Geometry| + GeometryResult::Mode GetResultMode() const override; + Path path_; std::optional inner_rect_; diff --git a/impeller/entity/geometry/geometry.cc b/impeller/entity/geometry/geometry.cc index 437dc45072061..772c0cb27d81f 100644 --- a/impeller/entity/geometry/geometry.cc +++ b/impeller/entity/geometry/geometry.cc @@ -168,6 +168,10 @@ GeometryResult Geometry::GetPositionUVBuffer(Rect texture_coverage, return {}; } +GeometryResult::Mode Geometry::GetResultMode() const { + return GeometryResult::Mode::kNormal; +} + std::shared_ptr Geometry::MakeFillPath( const Path& path, std::optional inner_rect) { diff --git a/impeller/entity/geometry/geometry.h b/impeller/entity/geometry/geometry.h index 732e97ddf31f1..ae30159b83318 100644 --- a/impeller/entity/geometry/geometry.h +++ b/impeller/entity/geometry/geometry.h @@ -128,6 +128,8 @@ class Geometry { const Entity& entity, RenderPass& pass) const = 0; + virtual GeometryResult::Mode GetResultMode() const; + virtual GeometryVertexType GetVertexType() const = 0; virtual std::optional GetCoverage(const Matrix& transform) const = 0; diff --git a/impeller/entity/geometry/rect_geometry.h b/impeller/entity/geometry/rect_geometry.h index 91842a027633e..bed8c23d6c419 100644 --- a/impeller/entity/geometry/rect_geometry.h +++ b/impeller/entity/geometry/rect_geometry.h @@ -21,7 +21,6 @@ class RectGeometry final : public Geometry { // |Geometry| bool IsAxisAlignedRect() const override; - private: // |Geometry| GeometryResult GetPositionBuffer(const ContentContext& renderer, const Entity& entity, @@ -40,6 +39,7 @@ class RectGeometry final : public Geometry { const Entity& entity, RenderPass& pass) const override; + private: Rect rect_; RectGeometry(const RectGeometry&) = delete; diff --git a/impeller/entity/geometry/stroke_path_geometry.cc b/impeller/entity/geometry/stroke_path_geometry.cc index c77045b7eca7e..ba3d2fe8cd567 100644 --- a/impeller/entity/geometry/stroke_path_geometry.cc +++ b/impeller/entity/geometry/stroke_path_geometry.cc @@ -6,6 +6,7 @@ #include "impeller/core/buffer_view.h" #include "impeller/core/formats.h" +#include "impeller/entity/geometry/geometry.h" #include "impeller/entity/texture_fill.vert.h" #include "impeller/geometry/path_builder.h" #include "impeller/geometry/path_component.h" @@ -646,6 +647,10 @@ GeometryResult StrokePathGeometry::GetPositionUVBuffer( }; } +GeometryResult::Mode StrokePathGeometry::GetResultMode() const { + return GeometryResult::Mode::kPreventOverdraw; +} + GeometryVertexType StrokePathGeometry::GetVertexType() const { return GeometryVertexType::kPosition; } diff --git a/impeller/entity/geometry/stroke_path_geometry.h b/impeller/entity/geometry/stroke_path_geometry.h index 08be408ab80ff..254bcc0a03e0e 100644 --- a/impeller/entity/geometry/stroke_path_geometry.h +++ b/impeller/entity/geometry/stroke_path_geometry.h @@ -41,6 +41,9 @@ class StrokePathGeometry final : public Geometry { const Entity& entity, RenderPass& pass) const override; + // |Geometry| + GeometryResult::Mode GetResultMode() const override; + // |Geometry| GeometryVertexType GetVertexType() const override; From 5acd75ec069378d7263977ddb37097ea5e9e60a9 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Thu, 22 Feb 2024 06:04:41 -0800 Subject: [PATCH 6/8] Correct test --- impeller/aiks/aiks_unittests.cc | 37 +++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 0834abc3b60f9..cfe5e36a87866 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -169,6 +169,36 @@ bool GenerateMipmap(const std::shared_ptr& context, return context->GetCommandQueue()->Submit({buffer}).ok(); } +TEST_P(AiksTest, CanRenderMultiContourPaths) { + auto texture = CreateTextureForFixture("table_mountain_nx.png", + /*enable_mipmapping=*/true); + GenerateMipmap(GetContext(), texture, "table_mountain_nx"); + Canvas canvas; + canvas.Scale(GetContentScale()); + canvas.Translate({100.0f, 100.0f, 0}); + Paint paint; + paint.color = Color::White(); + paint.style = Paint::Style::kFill; + + auto draw_path = [&]() { + PathBuilder path_builder; + path_builder.AddCircle({150, 150}, 150); + path_builder.AddRoundedRect(Rect::MakeLTRB(300, 300, 600, 600), 10); + canvas.DrawPath(path_builder.TakePath(), paint); + }; + + // Solid color source. + paint.color_source = ColorSource::MakeColor(); + draw_path(); + + // Image color source. + paint.color_source = ColorSource::MakeImage(texture, Entity::TileMode::kClamp, + Entity::TileMode::kClamp, {}, {}); + draw_path(); + + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + void CanRenderTiledTexture(AiksTest* aiks_test, Entity::TileMode tile_mode, Matrix local_matrix = {}) { @@ -3547,14 +3577,9 @@ TEST_P(AiksTest, CorrectClipDepthAssignedToEntities) { // contents are rendered, so it should have a depth value // greater than all its contents. 3, // DrawRRect + 5, // Restore (no longer necessary when clipping on the depth buffer) }; - if constexpr (!ContentContext::kEnableStencilThenCover) { - expected.push_back( // - 5 // Restore (no longer necessary when clipping on the depth buffer) - ); - } - std::vector actual; picture.pass->IterateAllElements([&](EntityPass::Element& element) -> bool { From 2889d0460b9be9ef61ba31d795f0b59be328eb2d Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Thu, 22 Feb 2024 06:06:15 -0800 Subject: [PATCH 7/8] Remove redundant golden --- impeller/aiks/aiks_unittests.cc | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index cfe5e36a87866..37dc2ba536996 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -169,36 +169,6 @@ bool GenerateMipmap(const std::shared_ptr& context, return context->GetCommandQueue()->Submit({buffer}).ok(); } -TEST_P(AiksTest, CanRenderMultiContourPaths) { - auto texture = CreateTextureForFixture("table_mountain_nx.png", - /*enable_mipmapping=*/true); - GenerateMipmap(GetContext(), texture, "table_mountain_nx"); - Canvas canvas; - canvas.Scale(GetContentScale()); - canvas.Translate({100.0f, 100.0f, 0}); - Paint paint; - paint.color = Color::White(); - paint.style = Paint::Style::kFill; - - auto draw_path = [&]() { - PathBuilder path_builder; - path_builder.AddCircle({150, 150}, 150); - path_builder.AddRoundedRect(Rect::MakeLTRB(300, 300, 600, 600), 10); - canvas.DrawPath(path_builder.TakePath(), paint); - }; - - // Solid color source. - paint.color_source = ColorSource::MakeColor(); - draw_path(); - - // Image color source. - paint.color_source = ColorSource::MakeImage(texture, Entity::TileMode::kClamp, - Entity::TileMode::kClamp, {}, {}); - draw_path(); - - ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); -} - void CanRenderTiledTexture(AiksTest* aiks_test, Entity::TileMode tile_mode, Matrix local_matrix = {}) { From 1fc3551cfab5153e2e06ade3ef9dbc5a0592579f Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Thu, 22 Feb 2024 19:43:01 -0800 Subject: [PATCH 8/8] Turn off StC --- impeller/entity/contents/content_context.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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;