Skip to content

Commit

Permalink
[Reland] Add rects to accumulator rather than bounds (flutter#37435) (f…
Browse files Browse the repository at this point in the history
…lutter#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
  • Loading branch information
iskakaushik authored Nov 9, 2022
1 parent e778ca4 commit f8048be
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 3 deletions.
20 changes: 20 additions & 0 deletions display_list/display_list_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1650,6 +1650,26 @@ TEST(DisplayList, RTreeOfSaveLayerFilterScene) {
test_rtree(rtree, {19, 19, 51, 51}, rects, {0, 1});
}

TEST(DisplayList, NestedDisplayListRTreesAreSparse) {
DisplayListBuilder nested_dl_builder;
nested_dl_builder.drawRect({10, 10, 20, 20});
nested_dl_builder.drawRect({50, 50, 60, 60});
auto nested_display_list = nested_dl_builder.Build();

DisplayListBuilder builder;
builder.drawDisplayList(nested_display_list);
auto display_list = builder.Build();

auto rtree = display_list->rtree();
std::vector<SkRect> rects = {
{10, 10, 20, 20},
{50, 50, 60, 60},
};

// Hitting both sub-dl drawRect calls
test_rtree(rtree, {19, 19, 51, 51}, rects, {0, 1});
}

TEST(DisplayList, RemoveUnnecessarySaveRestorePairs) {
{
DisplayListBuilder builder;
Expand Down
18 changes: 17 additions & 1 deletion display_list/display_list_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,23 @@ void DisplayListBoundsCalculator::drawPicture(const sk_sp<SkPicture> picture,
}
void DisplayListBoundsCalculator::drawDisplayList(
const sk_sp<DisplayList> display_list) {
AccumulateOpBounds(display_list->bounds(), kDrawDisplayListFlags);
const SkRect bounds = display_list->bounds();
switch (accumulator_.type()) {
case BoundsAccumulatorType::kRect:
AccumulateOpBounds(bounds, kDrawDisplayListFlags);
return;
case BoundsAccumulatorType::kRTree:
std::list<SkRect> rects =
display_list->rtree()->searchNonOverlappingDrawnRects(bounds);
for (const SkRect& rect : rects) {
// TODO (https://github.com/flutter/flutter/issues/114919): Attributes
// are not necessarily `kDrawDisplayListFlags`.
AccumulateOpBounds(rect, kDrawDisplayListFlags);
}
return;
}

FML_UNREACHABLE();
}
void DisplayListBoundsCalculator::drawTextBlob(const sk_sp<SkTextBlob> blob,
SkScalar x,
Expand Down
15 changes: 15 additions & 0 deletions display_list/display_list_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,11 @@ class ClipBoundsDispatchHelper : public virtual Dispatcher,
void intersect(const SkRect& clipBounds, bool is_aa);
};

enum class BoundsAccumulatorType {
kRect,
kRTree,
};

class BoundsAccumulator {
public:
/// function definition for modifying the bounds of a rectangle
Expand Down Expand Up @@ -393,6 +398,8 @@ class BoundsAccumulator {
virtual bool restore(
std::function<bool(const SkRect& original, SkRect& modified)> map,
const SkRect* clip = nullptr) = 0;

virtual BoundsAccumulatorType type() const = 0;
};

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

BoundsAccumulatorType type() const override {
return BoundsAccumulatorType::kRect;
}

private:
class AccumulationRect {
public:
Expand Down Expand Up @@ -455,6 +466,10 @@ class RTreeBoundsAccumulator final : public virtual BoundsAccumulator {

sk_sp<DlRTree> rtree() const;

BoundsAccumulatorType type() const override {
return BoundsAccumulatorType::kRTree;
}

private:
std::vector<SkRect> rects_;
std::vector<size_t> saved_offsets_;
Expand Down
2 changes: 0 additions & 2 deletions testing/scenario_app/run_ios_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,10 @@ echo "Running simulator tests with Impeller"
echo ""

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

0 comments on commit f8048be

Please sign in to comment.