From 60b868ed1c80a8ab8645d9f27d5165464a581100 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 24 Mar 2024 13:26:40 -0700 Subject: [PATCH 1/2] [Impeller] fix unbalanced restores. --- impeller/entity/BUILD.gn | 1 + impeller/entity/entity_pass.cc | 8 +++- impeller/entity/entity_pass_unittests.cc | 52 ++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 impeller/entity/entity_pass_unittests.cc diff --git a/impeller/entity/BUILD.gn b/impeller/entity/BUILD.gn index b13e8f81526d8..a917b91b781cf 100644 --- a/impeller/entity/BUILD.gn +++ b/impeller/entity/BUILD.gn @@ -272,6 +272,7 @@ impeller_component("entity_unittests") { "contents/tiled_texture_contents_unittests.cc", "contents/vertices_contents_unittests.cc", "entity_pass_target_unittests.cc", + "entity_pass_unittests.cc", "entity_playground.cc", "entity_playground.h", "entity_unittests.cc", diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 353fccf8cd5f9..b470189a242a9 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -869,7 +869,9 @@ bool EntityPass::RenderElement(Entity& element_entity, if constexpr (ContentContext::kEnableStencilThenCover) { // Skip all clip restores when stencil-then-cover is enabled. - clip_replay_->RecordEntity(element_entity, clip_coverage.type); + if (clip_coverage_stack.back().coverage.has_value()) { + clip_replay_->RecordEntity(element_entity, clip_coverage.type); + } return true; } @@ -1269,7 +1271,9 @@ void EntityPassClipRecorder::RecordEntity(const Entity& entity, rendered_clip_entities_.push_back(entity.Clone()); break; case Contents::ClipCoverage::Type::kRestore: - rendered_clip_entities_.pop_back(); + if (!rendered_clip_entities_.empty()) { + rendered_clip_entities_.pop_back(); + } break; } } diff --git a/impeller/entity/entity_pass_unittests.cc b/impeller/entity/entity_pass_unittests.cc new file mode 100644 index 0000000000000..d1f05448e938b --- /dev/null +++ b/impeller/entity/entity_pass_unittests.cc @@ -0,0 +1,52 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/testing/testing.h" +#include "gtest/gtest.h" +#include "impeller/entity/entity_pass.h" + +namespace impeller { +namespace testing { + +TEST(EntityPassClipRecorderTest, CanPushAndPopEntities) { + EntityPassClipRecorder recorder = EntityPassClipRecorder(); + + EXPECT_TRUE(recorder.GetReplayEntities().empty()); + + Entity entity; + recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kAppend); + EXPECT_EQ(recorder.GetReplayEntities().size(), 1u); + + recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kAppend); + EXPECT_EQ(recorder.GetReplayEntities().size(), 2u); + + recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kRestore); + EXPECT_EQ(recorder.GetReplayEntities().size(), 1u); + + recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kRestore); + EXPECT_TRUE(recorder.GetReplayEntities().empty()); +} + +TEST(EntityPassClipRecorderTest, CanPopEntitiesSafely) { + EntityPassClipRecorder recorder = EntityPassClipRecorder(); + + EXPECT_TRUE(recorder.GetReplayEntities().empty()); + + Entity entity; + recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kRestore); + EXPECT_TRUE(recorder.GetReplayEntities().empty()); +} + +TEST(EntityPassClipRecorderTest, CanAppendNoChange) { + EntityPassClipRecorder recorder = EntityPassClipRecorder(); + + EXPECT_TRUE(recorder.GetReplayEntities().empty()); + + Entity entity; + recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kNoChange); + EXPECT_TRUE(recorder.GetReplayEntities().empty()); +} + +} // namespace testing +} // namespace impeller From 7d56494982e807eb1dd134e94bf6fa1c5e41a4e7 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 25 Mar 2024 08:56:19 -0700 Subject: [PATCH 2/2] update licenses --- ci/licenses_golden/excluded_files | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index e89bbf56616b0..c9c50cea46b57 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -152,6 +152,7 @@ ../../../flutter/impeller/entity/contents/tiled_texture_contents_unittests.cc ../../../flutter/impeller/entity/contents/vertices_contents_unittests.cc ../../../flutter/impeller/entity/entity_pass_target_unittests.cc +../../../flutter/impeller/entity/entity_pass_unittests.cc ../../../flutter/impeller/entity/entity_unittests.cc ../../../flutter/impeller/entity/geometry/geometry_unittests.cc ../../../flutter/impeller/entity/render_target_cache_unittests.cc