Skip to content

Commit 4f9706e

Browse files
iskakaushikschwa423
authored andcommitted
[Reland] Add rects to accumulator rather than bounds (flutter#37435) (flutter#37451)
* [Reland] Add rects to accumulator rather than bounds (flutter#37435) When the accumulator is an `RTreeBoundsAccumulator` rather than a `RectBoundsAccumulator` just accumulating the bounds results in incorrect results as the `rtree` would need to be aware of the constituent non-overlapping rectangles. This would work fine for `RectBoundsAccumulator` as it would just adjust its bounds based on the passed rects. Fixes: flutter/flutter#113251 * Add a test for nested display list rtree
1 parent 87f2799 commit 4f9706e

File tree

4 files changed

+52
-3
lines changed

4 files changed

+52
-3
lines changed

display_list/display_list_unittests.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1650,6 +1650,26 @@ TEST(DisplayList, RTreeOfSaveLayerFilterScene) {
16501650
test_rtree(rtree, {19, 19, 51, 51}, rects, {0, 1});
16511651
}
16521652

1653+
TEST(DisplayList, NestedDisplayListRTreesAreSparse) {
1654+
DisplayListBuilder nested_dl_builder;
1655+
nested_dl_builder.drawRect({10, 10, 20, 20});
1656+
nested_dl_builder.drawRect({50, 50, 60, 60});
1657+
auto nested_display_list = nested_dl_builder.Build();
1658+
1659+
DisplayListBuilder builder;
1660+
builder.drawDisplayList(nested_display_list);
1661+
auto display_list = builder.Build();
1662+
1663+
auto rtree = display_list->rtree();
1664+
std::vector<SkRect> rects = {
1665+
{10, 10, 20, 20},
1666+
{50, 50, 60, 60},
1667+
};
1668+
1669+
// Hitting both sub-dl drawRect calls
1670+
test_rtree(rtree, {19, 19, 51, 51}, rects, {0, 1});
1671+
}
1672+
16531673
TEST(DisplayList, RemoveUnnecessarySaveRestorePairs) {
16541674
{
16551675
DisplayListBuilder builder;

display_list/display_list_utils.cc

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,23 @@ void DisplayListBoundsCalculator::drawPicture(const sk_sp<SkPicture> picture,
672672
}
673673
void DisplayListBoundsCalculator::drawDisplayList(
674674
const sk_sp<DisplayList> display_list) {
675-
AccumulateOpBounds(display_list->bounds(), kDrawDisplayListFlags);
675+
const SkRect bounds = display_list->bounds();
676+
switch (accumulator_.type()) {
677+
case BoundsAccumulatorType::kRect:
678+
AccumulateOpBounds(bounds, kDrawDisplayListFlags);
679+
return;
680+
case BoundsAccumulatorType::kRTree:
681+
std::list<SkRect> rects =
682+
display_list->rtree()->searchNonOverlappingDrawnRects(bounds);
683+
for (const SkRect& rect : rects) {
684+
// TODO (https://github.com/flutter/flutter/issues/114919): Attributes
685+
// are not necessarily `kDrawDisplayListFlags`.
686+
AccumulateOpBounds(rect, kDrawDisplayListFlags);
687+
}
688+
return;
689+
}
690+
691+
FML_UNREACHABLE();
676692
}
677693
void DisplayListBoundsCalculator::drawTextBlob(const sk_sp<SkTextBlob> blob,
678694
SkScalar x,

display_list/display_list_utils.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,11 @@ class ClipBoundsDispatchHelper : public virtual Dispatcher,
337337
void intersect(const SkRect& clipBounds, bool is_aa);
338338
};
339339

340+
enum class BoundsAccumulatorType {
341+
kRect,
342+
kRTree,
343+
};
344+
340345
class BoundsAccumulator {
341346
public:
342347
/// function definition for modifying the bounds of a rectangle
@@ -393,6 +398,8 @@ class BoundsAccumulator {
393398
virtual bool restore(
394399
std::function<bool(const SkRect& original, SkRect& modified)> map,
395400
const SkRect* clip = nullptr) = 0;
401+
402+
virtual BoundsAccumulatorType type() const = 0;
396403
};
397404

398405
class RectBoundsAccumulator final : public virtual BoundsAccumulator {
@@ -414,6 +421,10 @@ class RectBoundsAccumulator final : public virtual BoundsAccumulator {
414421
return rect_.bounds();
415422
}
416423

424+
BoundsAccumulatorType type() const override {
425+
return BoundsAccumulatorType::kRect;
426+
}
427+
417428
private:
418429
class AccumulationRect {
419430
public:
@@ -455,6 +466,10 @@ class RTreeBoundsAccumulator final : public virtual BoundsAccumulator {
455466

456467
sk_sp<DlRTree> rtree() const;
457468

469+
BoundsAccumulatorType type() const override {
470+
return BoundsAccumulatorType::kRTree;
471+
}
472+
458473
private:
459474
std::vector<SkRect> rects_;
460475
std::vector<size_t> saved_offsets_;

testing/scenario_app/run_ios_tests.sh

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,10 @@ echo "Running simulator tests with Impeller"
5252
echo ""
5353

5454
# Skip testFontRenderingWhenSuppliedWithBogusFont: https://github.com/flutter/flutter/issues/113250
55-
# Skip testOneOverlayAndTwoIntersectingOverlays: https://github.com/flutter/flutter/issues/113251
5655
set -o pipefail && xcodebuild -sdk iphonesimulator \
5756
-scheme Scenarios \
5857
-destination 'platform=iOS Simulator,OS=13.0,name=iPhone 8' \
5958
clean test \
6059
FLUTTER_ENGINE="$FLUTTER_ENGINE" \
6160
-skip-testing "ScenariosUITests/BogusFontTextTest/testFontRenderingWhenSuppliedWithBogusFont" \
62-
-skip-testing "ScenariosUITests/UnobstructedPlatformViewTests/testOneOverlayAndTwoIntersectingOverlays" \
6361
INFOPLIST_FILE="Scenarios/Info_Impeller.plist" # Plist with FLTEnableImpeller=YES

0 commit comments

Comments
 (0)