From b6fcc7b768b4507171f2a3048d839b445a0e2f56 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 29 May 2023 15:18:51 +0200 Subject: [PATCH 01/21] Add DlRegion class --- BUILD.gn | 1 + ci/licenses_golden/licenses_flutter | 6 + display_list/BUILD.gn | 15 + .../benchmarking/dl_region_benchmarks.cc | 77 ++++++ display_list/geometry/dl_region.cc | 260 ++++++++++++++++++ display_list/geometry/dl_region.h | 54 ++++ display_list/geometry/dl_region_unittests.cc | 138 ++++++++++ 7 files changed, 551 insertions(+) create mode 100644 display_list/benchmarking/dl_region_benchmarks.cc create mode 100644 display_list/geometry/dl_region.cc create mode 100644 display_list/geometry/dl_region.h create mode 100644 display_list/geometry/dl_region_unittests.cc diff --git a/BUILD.gn b/BUILD.gn index 33053946e3f53..46a1e76c8fcea 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -116,6 +116,7 @@ group("flutter") { public_deps += [ "//flutter/display_list:display_list_benchmarks", "//flutter/display_list:display_list_builder_benchmarks", + "//flutter/display_list:display_list_region_benchmarks", "//flutter/fml:fml_benchmarks", "//flutter/impeller/geometry:geometry_benchmarks", "//flutter/lib/ui:ui_benchmarks", diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 69d87bce22845..a98cab24c4da3 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -714,6 +714,7 @@ ORIGIN: ../../../flutter/display_list/benchmarking/dl_complexity_gl.h + ../../.. ORIGIN: ../../../flutter/display_list/benchmarking/dl_complexity_helper.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/benchmarking/dl_complexity_metal.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/benchmarking/dl_complexity_metal.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/benchmarking/dl_region_benchmarks.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/dl_attributes.h + ../../../flutter/LICENSE @@ -748,6 +749,8 @@ ORIGIN: ../../../flutter/display_list/effects/dl_path_effect.cc + ../../../flutt ORIGIN: ../../../flutter/display_list/effects/dl_path_effect.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/effects/dl_runtime_effect.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/effects/dl_runtime_effect.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/geometry/dl_region.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/geometry/dl_region.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/geometry/dl_rtree.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/geometry/dl_rtree.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/image/dl_image.cc + ../../../flutter/LICENSE @@ -3380,6 +3383,7 @@ FILE: ../../../flutter/display_list/benchmarking/dl_complexity_gl.h FILE: ../../../flutter/display_list/benchmarking/dl_complexity_helper.h FILE: ../../../flutter/display_list/benchmarking/dl_complexity_metal.cc FILE: ../../../flutter/display_list/benchmarking/dl_complexity_metal.h +FILE: ../../../flutter/display_list/benchmarking/dl_region_benchmarks.cc FILE: ../../../flutter/display_list/display_list.cc FILE: ../../../flutter/display_list/display_list.h FILE: ../../../flutter/display_list/dl_attributes.h @@ -3414,6 +3418,8 @@ FILE: ../../../flutter/display_list/effects/dl_path_effect.cc FILE: ../../../flutter/display_list/effects/dl_path_effect.h FILE: ../../../flutter/display_list/effects/dl_runtime_effect.cc FILE: ../../../flutter/display_list/effects/dl_runtime_effect.h +FILE: ../../../flutter/display_list/geometry/dl_region.cc +FILE: ../../../flutter/display_list/geometry/dl_region.h FILE: ../../../flutter/display_list/geometry/dl_rtree.cc FILE: ../../../flutter/display_list/geometry/dl_rtree.h FILE: ../../../flutter/display_list/image/dl_image.cc diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn index 0830d3c5864fa..97c9683511b61 100644 --- a/display_list/BUILD.gn +++ b/display_list/BUILD.gn @@ -57,6 +57,8 @@ source_set("display_list") { "effects/dl_path_effect.h", "effects/dl_runtime_effect.cc", "effects/dl_runtime_effect.h", + "geometry/dl_region.cc", + "geometry/dl_region.h", "geometry/dl_rtree.cc", "geometry/dl_rtree.h", "image/dl_image.cc", @@ -112,6 +114,7 @@ if (enable_unittests) { "effects/dl_image_filter_unittests.cc", "effects/dl_mask_filter_unittests.cc", "effects/dl_path_effect_unittests.cc", + "geometry/dl_region_unittests.cc", "geometry/dl_rtree_unittests.cc", "skia/dl_sk_conversions_unittests.cc", "skia/dl_sk_paint_dispatcher_unittests.cc", @@ -182,6 +185,18 @@ if (enable_unittests) { "//flutter/testing:testing_lib", ] } + + executable("display_list_region_benchmarks") { + testonly = true + + sources = [ "benchmarking/dl_region_benchmarks.cc" ] + + deps = [ + ":display_list_fixtures", + "//flutter/benchmarking", + "//flutter/testing:testing_lib", + ] + } } fixtures_location("display_list_benchmarks_fixtures") { diff --git a/display_list/benchmarking/dl_region_benchmarks.cc b/display_list/benchmarking/dl_region_benchmarks.cc new file mode 100644 index 0000000000000..c1745ff514770 --- /dev/null +++ b/display_list/benchmarking/dl_region_benchmarks.cc @@ -0,0 +1,77 @@ +// 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/benchmarking/benchmarking.h" + +#include "flutter/display_list/geometry/dl_region.h" +#include "third_party/skia/include/core/SkRegion.h" + +#include + +class SkRegionAdapter { + public: + void addRect(const SkIRect& rect) { region_.op(rect, SkRegion::kUnion_Op); } + + std::vector getRects() { + std::vector rects; + SkRegion::Iterator it(region_); + while (!it.done()) { + rects.push_back(it.rect()); + it.next(); + } + return rects; + } + + private: + SkRegion region_; +}; + +template +void RunRegionBenchmark(benchmark::State& state, int maxSize) { + while (state.KeepRunning()) { + std::random_device d; + std::seed_seq seed{2, 1, 3}; + std::mt19937 rng(seed); + + std::uniform_int_distribution pos(0, 4000); + std::uniform_int_distribution size(1, maxSize); + + Region region; + + for (int i = 0; i < 2000; ++i) { + SkIRect rect = + SkIRect::MakeXYWH(pos(rng), pos(rng), size(rng), size(rng)); + region.addRect(rect); + } + + auto vec2 = region.getRects(); + } +} + +namespace flutter { + +static void BM_RegionBenchmarkSkRegion(benchmark::State& state, int maxSize) { + RunRegionBenchmark(state, maxSize); +} + +static void BM_RegionBenchmarkDlRegion(benchmark::State& state, int maxSize) { + RunRegionBenchmark(state, maxSize); +} + +BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion, Small, 100) + ->Unit(benchmark::kMicrosecond); +BENCHMARK_CAPTURE(BM_RegionBenchmarkSkRegion, Small, 100) + ->Unit(benchmark::kMicrosecond); + +BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion, Medium, 400) + ->Unit(benchmark::kMicrosecond); +BENCHMARK_CAPTURE(BM_RegionBenchmarkSkRegion, Medium, 400) + ->Unit(benchmark::kMicrosecond); + +BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion, Large, 1500) + ->Unit(benchmark::kMicrosecond); +BENCHMARK_CAPTURE(BM_RegionBenchmarkSkRegion, Large, 1500) + ->Unit(benchmark::kMicrosecond); + +} // namespace flutter \ No newline at end of file diff --git a/display_list/geometry/dl_region.cc b/display_list/geometry/dl_region.cc new file mode 100644 index 0000000000000..eb5f8fb72147b --- /dev/null +++ b/display_list/geometry/dl_region.cc @@ -0,0 +1,260 @@ +// 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/display_list/geometry/dl_region.h" + +#include + +namespace flutter { + +struct DlRegion::Span { + int32_t left; + int32_t right; +}; + +struct DlRegion::SpanLine { + int32_t top; + int32_t bottom; + // For performance reasons SpanLine must be trivially constructible. + // DlRegion is responsible for allocating and deallocating the + // memory for spans. + SpanVec* spans; + + /// Inserts span into this span line. If there are existing spans + /// that overlap with the new span, they will be merged into single + /// span. + void insertSpan(int32_t left, int32_t right) { + SpanVec& spans = *this->spans; + + auto size = spans.size(); + for (size_t i = 0; i < size; ++i) { + Span& span = spans[i]; + if (right < span.left) { + _insertSpanAt(i, left, right); + return; + } + if (left > span.right) { + continue; + } + size_t last_index = i; + while (last_index + 1 < size && right >= spans[last_index + 1].left) { + ++last_index; + } + span.left = std::min(span.left, left); + span.right = std::max(spans[last_index].right, right); + if (last_index > i) { + spans.erase(spans.begin() + i + 1, spans.begin() + last_index + 1); + } + return; + } + + spans.push_back({left, right}); + } + + bool operator==(const SpanLine& l2) const { + SpanVec& spans = *this->spans; + SpanVec& otherSpans = *l2.spans; + assert(this != &l2); + + if (spans.size() != otherSpans.size()) { + return false; + } + return memcmp(spans.data(), otherSpans.data(), + spans.size() * sizeof(Span)) == 0; + } + + private: + void _insertSpanAt(size_t index, int32_t x1, int32_t x2) { + spans->insert(spans->begin() + index, {x1, x2}); + } +}; + +DlRegion::DlRegion() { + // If we can't memmove SpanLines addRect would be signifantly slower. + static_assert(std::is_trivially_constructible::value, + "SpanLine must be trivially constructible."); +} + +DlRegion::~DlRegion() { + for (auto& spanvec : spanvec_pool_) { + delete spanvec; + } + for (auto& line : lines_) { + delete line.spans; + } +} + +void DlRegion::addRect(const SkIRect& rect) { + if (rect.isEmpty()) { + return; + } + + int32_t i1 = rect.fTop; + int32_t i2 = rect.fBottom; + + size_t dirty_start = -1; + size_t dirty_end = 1; + + // Marks line as dirty. Dirty lines will be checked for equality + // later and merged as needed. + auto mark_dirty = [&](size_t line) { + if (dirty_start == static_cast(-1)) { + dirty_start = line; + dirty_end = line; + } else { + dirty_start = std::min(dirty_start, line); + dirty_end = std::max(dirty_end, line); + } + }; + + auto upper_bound = std::upper_bound( + lines_.begin(), lines_.end(), i1, + [](int32_t i, const SpanLine& line) { return i < line.bottom; }); + + auto start_index = upper_bound - lines_.begin(); + + for (size_t i = start_index; i < lines_.size() && i1 < i2; ++i) { + SpanLine& line = lines_[i]; + + // If this is false the start index is wrong. + assert(i1 < line.bottom); + + if (i2 <= line.top) { + insertLine(i, makeLine(i1, i2, rect.fLeft, rect.fRight)); + mark_dirty(i); + i1 = i2; + break; + } + if (i1 < line.top) { + auto prevLineStart = line.top; + insertLine(i, makeLine(i1, prevLineStart, rect.fLeft, rect.fRight)); + mark_dirty(i); + i1 = prevLineStart; + continue; + } + if (i1 > line.top) { + // duplicate line + auto prevLineEnd = line.bottom; + line.bottom = i1; + mark_dirty(i); + insertLine(i + 1, makeLine(i1, prevLineEnd, *line.spans)); + continue; + } + assert(i1 == line.top); + if (i2 < line.bottom) { + // duplicate line + auto newLine = makeLine(i2, line.bottom, *line.spans); + line.bottom = i2; + line.insertSpan(rect.fLeft, rect.fRight); + insertLine(i + 1, newLine); + i1 = i2; + mark_dirty(i); + break; + } + assert(i2 >= line.bottom); + line.insertSpan(rect.fLeft, rect.fRight); + mark_dirty(i); + i1 = line.bottom; + } + + if (i1 < i2) { + lines_.push_back(makeLine(i1, i2, rect.fLeft, rect.fRight)); + mark_dirty(lines_.size() - 1); + } + + // Check for duplicate lines and merge them. + if (dirty_start != static_cast(-1)) { + // Expand the region by one if possible. + if (dirty_start > 0) { + --dirty_start; + } + if (dirty_end + 1 < lines_.size()) { + ++dirty_end; + } + for (auto i = lines_.begin() + dirty_start; + i < lines_.begin() + dirty_end;) { + auto& line = *i; + auto& next = *(i + 1); + if (line == next) { + --dirty_end; + next.top = line.top; + i = removeLine(i); + } else { + ++i; + } + } + } +} + +std::vector DlRegion::getRects(bool deband) const { + std::vector rects; + for (const auto& line : lines_) { + for (const Span& span : *line.spans) { + SkIRect rect{span.left, line.top, span.right, line.bottom}; + if (deband) { + auto iter = rects.end(); + // If there is recangle previously in rect on which this one is a + // vertical continuation, remove the previous rectangle and expand this + // one vertically to cover the area. + while (iter != rects.begin()) { + --iter; + if (iter->bottom() < rect.top()) { + // Went too far. + break; + } else if (iter->bottom() == rect.top() && + iter->left() == rect.left() && + iter->right() == rect.right()) { + rect.fTop = iter->fTop; + rects.erase(iter); + break; + } + } + } + rects.push_back(rect); + } + } + return rects; +} + +void DlRegion::insertLine(size_t position, SpanLine line) { + lines_.insert(lines_.begin() + position, line); +} + +DlRegion::LineVec::iterator DlRegion::removeLine( + DlRegion::LineVec::iterator line) { + spanvec_pool_.push_back(line->spans); + return lines_.erase(line); +} + +DlRegion::SpanLine DlRegion::makeLine(int32_t top, + int32_t bottom, + int32_t spanLeft, + int32_t spanRight) { + SpanVec* span_vec; + if (!spanvec_pool_.empty()) { + span_vec = spanvec_pool_.back(); + spanvec_pool_.pop_back(); + span_vec->clear(); + } else { + span_vec = new SpanVec(); + } + span_vec->push_back({spanLeft, spanRight}); + return {top, bottom, span_vec}; +} + +DlRegion::SpanLine DlRegion::makeLine(int32_t top, + int32_t bottom, + const SpanVec& spans) { + SpanVec* span_vec; + if (!spanvec_pool_.empty()) { + span_vec = spanvec_pool_.back(); + spanvec_pool_.pop_back(); + } else { + span_vec = new SpanVec(); + } + *span_vec = spans; + return {top, bottom, span_vec}; +} + +} // namespace flutter diff --git a/display_list/geometry/dl_region.h b/display_list/geometry/dl_region.h new file mode 100644 index 0000000000000..8577982adfa07 --- /dev/null +++ b/display_list/geometry/dl_region.h @@ -0,0 +1,54 @@ +// 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. + +#ifndef FLUTTER_DISPLAY_LIST_GEOMETRY_REGION_H_ +#define FLUTTER_DISPLAY_LIST_GEOMETRY_REGION_H_ + +#include "third_party/skia/include/core/SkRect.h" + +#include + +namespace flutter { + +/// Represents a region as a collection of non-overlapping rectangles. +/// Implements a subset of SkRegion functionality optimized for quickly +/// converting set of overlapping rectangles to non-overlapping rectangles. +class DlRegion { + public: + DlRegion(); + ~DlRegion(); + + /// Adds rectangle to current region. + /// Matches SkRegion::op(rect, SkRegion::kUnion_Op) behavior. + void addRect(const SkIRect& rect); + + /// Returns list of non-overlapping rectangles that cover current region. + /// If |deband| is false, each span line will result in separate rectangles, + /// closely matching SkRegion::Iterator behavior. + /// If |deband| is true, matching rectangles from adjacent span lines will be + /// merged into single rectange. + std::vector getRects(bool deband = true) const; + + private: + struct Span; + struct SpanLine; + typedef std::vector SpanVec; + typedef std::vector LineVec; + + void insertLine(size_t position, SpanLine line); + LineVec::iterator removeLine(LineVec::iterator position); + + SpanLine makeLine(int32_t top, + int32_t bottom, + int32_t spanLeft, + int32_t spanRight); + SpanLine makeLine(int32_t top, int32_t bottom, const SpanVec& spans); + + LineVec lines_; + std::vector spanvec_pool_; +}; + +} // namespace flutter + +#endif // FLUTTER_DISPLAY_LIST_GEOMETRY_REGION_H_ diff --git a/display_list/geometry/dl_region_unittests.cc b/display_list/geometry/dl_region_unittests.cc new file mode 100644 index 0000000000000..3dec05d96619d --- /dev/null +++ b/display_list/geometry/dl_region_unittests.cc @@ -0,0 +1,138 @@ +// 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/display_list/geometry/dl_region.h" +#include "gtest/gtest.h" + +#include "third_party/skia/include/core/SkRegion.h" + +#include + +namespace flutter { +namespace testing { + +TEST(DisplayListRegion, EmptyRegion) { + DlRegion region; + EXPECT_TRUE(region.getRects().empty()); +} + +TEST(DisplayListRegion, SingleRectangle) { + DlRegion region; + region.addRect(SkIRect::MakeLTRB(10, 10, 50, 50)); + auto rects = region.getRects(); + ASSERT_EQ(rects.size(), 1u); + EXPECT_EQ(rects.front(), SkIRect::MakeLTRB(10, 10, 50, 50)); +} + +TEST(DisplayListRegion, NonOverlappingRectangles) { + DlRegion region; + for (int i = 0; i < 10; ++i) { + SkIRect rect = SkIRect::MakeXYWH(50 * i, 50 * i, 50, 50); + region.addRect(rect); + } + auto rects = region.getRects(); + std::vector expected{ + {0, 0, 50, 50}, {50, 50, 100, 100}, {100, 100, 150, 150}, + {150, 150, 200, 200}, {200, 200, 250, 250}, {250, 250, 300, 300}, + {300, 300, 350, 350}, {350, 350, 400, 400}, {400, 400, 450, 450}, + {450, 450, 500, 500}, + }; + EXPECT_EQ(rects, expected); +} + +TEST(DisplayListRegion, OverlappingRectangles) { + DlRegion region; + for (int i = 0; i < 10; ++i) { + SkIRect rect = SkIRect::MakeXYWH(10 * i, 10 * i, 50, 50); + region.addRect(rect); + } + auto rects = region.getRects(); + std::vector expected{ + {0, 0, 50, 10}, {0, 10, 60, 20}, {0, 20, 70, 30}, + {0, 30, 80, 40}, {0, 40, 90, 50}, {10, 50, 100, 60}, + {20, 60, 110, 70}, {30, 70, 120, 80}, {40, 80, 130, 90}, + {50, 90, 140, 100}, {60, 100, 140, 110}, {70, 110, 140, 120}, + {80, 120, 140, 130}, {90, 130, 140, 140}, + }; + + EXPECT_EQ(rects, expected); +} + +TEST(DisplayListRegion, Deband) { + DlRegion region; + region.addRect(SkIRect::MakeXYWH(0, 0, 50, 50)); + region.addRect(SkIRect::MakeXYWH(60, 0, 20, 20)); + region.addRect(SkIRect::MakeXYWH(90, 0, 50, 50)); + + auto rects_with_deband = region.getRects(true); + std::vector expected{ + SkIRect::MakeXYWH(60, 0, 20, 20), + SkIRect::MakeXYWH(0, 0, 50, 50), + SkIRect::MakeXYWH(90, 0, 50, 50), + }; + EXPECT_EQ(rects_with_deband, expected); + + auto rects_without_deband = region.getRects(false); + std::vector expected_without_deband{ + SkIRect::MakeXYWH(0, 0, 50, 20), // + SkIRect::MakeXYWH(60, 0, 20, 20), // + SkIRect::MakeXYWH(90, 0, 50, 20), // + SkIRect::MakeXYWH(0, 20, 50, 30), // + SkIRect::MakeXYWH(90, 20, 50, 30), + }; + EXPECT_EQ(rects_without_deband, expected_without_deband); +} + +TEST(DisplayListRegion, TestAgainstSkRegion) { + struct Settings { + int max_size; + size_t iteration_count; + }; + std::vector all_settings{ + {100, 10}, // + {100, 100}, // + {100, 1000}, // + {400, 10}, // + {400, 100}, // + {400, 1000}, // + {800, 10}, // + {800, 100}, // + {800, 1000}, + }; + + for (const auto& settings : all_settings) { + std::random_device d; + std::seed_seq seed{::testing::UnitTest::GetInstance()->random_seed()}; + std::mt19937 rng(seed); + + DlRegion region; + SkRegion sk_region; + + std::uniform_int_distribution pos(0, 4000); + std::uniform_int_distribution size(1, settings.max_size); + + for (size_t i = 0; i < settings.iteration_count; ++i) { + SkIRect rect = + SkIRect::MakeXYWH(pos(rng), pos(rng), size(rng), size(rng)); + region.addRect(rect); + sk_region.op(rect, SkRegion::kUnion_Op); + } + + // Do not deband the rectangles - identical to SkRegion::Iterator + auto rects = region.getRects(false); + + std::vector skia_rects; + + auto iterator = SkRegion::Iterator(sk_region); + while (!iterator.done()) { + skia_rects.push_back(iterator.rect()); + iterator.next(); + } + + EXPECT_EQ(rects, skia_rects); + } +} + +} // namespace testing +} // namespace flutter From d32f808216d7a861dcb54d3935f0be404921fa35 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 29 May 2023 16:17:10 +0200 Subject: [PATCH 02/21] Use DlRegion in flow RTree --- flow/rtree.cc | 42 +++++++++++------------------------------ flow/rtree_unittests.cc | 29 +++++++++++++++------------- 2 files changed, 27 insertions(+), 44 deletions(-) diff --git a/flow/rtree.cc b/flow/rtree.cc index 70266346ee223..f0db9aaf84eec 100644 --- a/flow/rtree.cc +++ b/flow/rtree.cc @@ -6,6 +6,7 @@ #include +#include "flutter/display_list/geometry/dl_region.h" #include "flutter/fml/logging.h" #include "third_party/skia/include/core/SkBBHFactory.h" #include "third_party/skia/include/core/SkRect.h" @@ -41,43 +42,22 @@ std::list RTree::searchNonOverlappingDrawnRects( std::vector intermediary_results; search(query, &intermediary_results); - std::list final_results; + DlRegion region; for (int index : intermediary_results) { auto draw_op = draw_op_.find(index); // Ignore records that don't draw anything. if (draw_op == draw_op_.end()) { continue; } - auto current_record_rect = draw_op->second; - auto replaced_existing_rect = false; - // // If the current record rect intersects with any of the rects in the - // // result list, then join them, and update the rect in final_results. - std::list::iterator curr_rect_itr = final_results.begin(); - std::list::iterator first_intersecting_rect_itr; - while (!replaced_existing_rect && curr_rect_itr != final_results.end()) { - if (SkRect::Intersects(*curr_rect_itr, current_record_rect)) { - replaced_existing_rect = true; - first_intersecting_rect_itr = curr_rect_itr; - curr_rect_itr->join(current_record_rect); - } - curr_rect_itr++; - } - // It's possible that the result contains duplicated rects at this point. - // For example, consider a result list that contains rects A, B. If a - // new rect C is a superset of A and B, then A and B are the same set after - // the merge. As a result, find such cases and remove them from the result - // list. - while (replaced_existing_rect && curr_rect_itr != final_results.end()) { - if (SkRect::Intersects(*curr_rect_itr, *first_intersecting_rect_itr)) { - first_intersecting_rect_itr->join(*curr_rect_itr); - curr_rect_itr = final_results.erase(curr_rect_itr); - } else { - curr_rect_itr++; - } - } - if (!replaced_existing_rect) { - final_results.push_back(current_record_rect); - } + SkIRect current_record_rect; + draw_op->second.roundOut(¤t_record_rect); + region.addRect(current_record_rect); + } + + auto non_overlapping_rects = region.getRects(true); + std::list final_results; + for (const auto& rect : non_overlapping_rects) { + final_results.push_back(SkRect::Make(rect)); } return final_results; } diff --git a/flow/rtree_unittests.cc b/flow/rtree_unittests.cc index 1a1498efc8af3..873b5e398a49c 100644 --- a/flow/rtree_unittests.cc +++ b/flow/rtree_unittests.cc @@ -123,15 +123,15 @@ TEST(RTree, searchNonOverlappingDrawnRectsJoinRectsWhenIntersectedCase1) { rect_paint.setStyle(SkPaint::Style::kFill_Style); // Given the A, and B rects, which intersect with the query rect, - // the result list contains the rect resulting from the union of A and B. + // the result list contains the three rectangles covering same area. // - // +-----+ - // | A | - // | +-----+ - // | | C | - // | +-----+ - // | | - // +-----+ + // +-----+ +-----+ + // | A | | A | + // | +-----+ +---------+ + // | | | | B | + // +---| B | +---+-----+ + // | | | C | + // +-----+ +-----+ // A recording_canvas->drawRect(SkRect::MakeLTRB(100, 100, 150, 150), rect_paint); @@ -142,8 +142,10 @@ TEST(RTree, searchNonOverlappingDrawnRectsJoinRectsWhenIntersectedCase1) { auto hits = rtree_factory.getInstance()->searchNonOverlappingDrawnRects( SkRect::MakeXYWH(120, 120, 126, 126)); - ASSERT_EQ(1UL, hits.size()); - ASSERT_EQ(*hits.begin(), SkRect::MakeLTRB(100, 100, 175, 175)); + ASSERT_EQ(3UL, hits.size()); + ASSERT_EQ(*hits.begin(), SkRect::MakeLTRB(100, 100, 150, 125)); + ASSERT_EQ(*std::next(hits.begin(), 1), SkRect::MakeLTRB(100, 125, 175, 150)); + ASSERT_EQ(*std::next(hits.begin(), 2), SkRect::MakeLTRB(125, 150, 175, 175)); } TEST(RTree, searchNonOverlappingDrawnRectsJoinRectsWhenIntersectedCase2) { @@ -198,7 +200,7 @@ TEST(RTree, searchNonOverlappingDrawnRectsJoinRectsWhenIntersectedCase3) { rect_paint.setStyle(SkPaint::Style::kFill_Style); // Given the A, B, C and D rects that intersect with the query rect, - // the result list contains a single rect, which is the union of + // the result list contains two rects - D and remainder of C // these four rects. // // +------------------------------+ @@ -227,8 +229,9 @@ TEST(RTree, searchNonOverlappingDrawnRectsJoinRectsWhenIntersectedCase3) { auto hits = rtree_factory.getInstance()->searchNonOverlappingDrawnRects( SkRect::MakeLTRB(30, 30, 550, 270)); - ASSERT_EQ(1UL, hits.size()); - ASSERT_EQ(*hits.begin(), SkRect::MakeLTRB(50, 50, 620, 300)); + ASSERT_EQ(2UL, hits.size()); + ASSERT_EQ(*hits.begin(), SkRect::MakeLTRB(50, 50, 620, 250)); + ASSERT_EQ(*std::next(hits.begin(), 1), SkRect::MakeLTRB(500, 250, 600, 300)); } } // namespace testing From ed6d9f82459ca17df8a50c72bc39667c86d7f41d Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 29 May 2023 17:21:26 +0200 Subject: [PATCH 03/21] Add missing check when merging lines --- display_list/geometry/dl_region.cc | 4 ++-- display_list/geometry/dl_region_unittests.cc | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/display_list/geometry/dl_region.cc b/display_list/geometry/dl_region.cc index eb5f8fb72147b..2f0ed536154ef 100644 --- a/display_list/geometry/dl_region.cc +++ b/display_list/geometry/dl_region.cc @@ -52,7 +52,7 @@ struct DlRegion::SpanLine { spans.push_back({left, right}); } - bool operator==(const SpanLine& l2) const { + bool spansEqual(const SpanLine& l2) const { SpanVec& spans = *this->spans; SpanVec& otherSpans = *l2.spans; assert(this != &l2); @@ -176,7 +176,7 @@ void DlRegion::addRect(const SkIRect& rect) { i < lines_.begin() + dirty_end;) { auto& line = *i; auto& next = *(i + 1); - if (line == next) { + if (line.bottom == next.top && line.spansEqual(next)) { --dirty_end; next.top = line.top; i = removeLine(i); diff --git a/display_list/geometry/dl_region_unittests.cc b/display_list/geometry/dl_region_unittests.cc index 3dec05d96619d..c9dc2bfc9f9da 100644 --- a/display_list/geometry/dl_region_unittests.cc +++ b/display_list/geometry/dl_region_unittests.cc @@ -25,7 +25,7 @@ TEST(DisplayListRegion, SingleRectangle) { EXPECT_EQ(rects.front(), SkIRect::MakeLTRB(10, 10, 50, 50)); } -TEST(DisplayListRegion, NonOverlappingRectangles) { +TEST(DisplayListRegion, NonOverlappingRectangles1) { DlRegion region; for (int i = 0; i < 10; ++i) { SkIRect rect = SkIRect::MakeXYWH(50 * i, 50 * i, 50, 50); @@ -41,6 +41,22 @@ TEST(DisplayListRegion, NonOverlappingRectangles) { EXPECT_EQ(rects, expected); } +TEST(DisplayListRegion, NonOverlappingRectangles2) { + DlRegion region; + region.addRect(SkIRect::MakeXYWH(5, 5, 10, 10)); + region.addRect(SkIRect::MakeXYWH(25, 5, 10, 10)); + region.addRect(SkIRect::MakeXYWH(5, 25, 10, 10)); + region.addRect(SkIRect::MakeXYWH(25, 25, 10, 10)); + auto rects = region.getRects(); + std::vector expected{ + SkIRect::MakeXYWH(5, 5, 10, 10), + SkIRect::MakeXYWH(25, 5, 10, 10), + SkIRect::MakeXYWH(5, 25, 10, 10), + SkIRect::MakeXYWH(25, 25, 10, 10), + }; + EXPECT_EQ(rects, expected); +} + TEST(DisplayListRegion, OverlappingRectangles) { DlRegion region; for (int i = 0; i < 10; ++i) { From 03a5be585ebba876bad1559eee075f155169aace Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 29 May 2023 17:21:57 +0200 Subject: [PATCH 04/21] Use DlRegion in DlRTree --- display_list/geometry/dl_rtree.cc | 42 ++++++++----------------------- 1 file changed, 11 insertions(+), 31 deletions(-) diff --git a/display_list/geometry/dl_rtree.cc b/display_list/geometry/dl_rtree.cc index 509607e50d7ce..e6bcd97442764 100644 --- a/display_list/geometry/dl_rtree.cc +++ b/display_list/geometry/dl_rtree.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "flutter/display_list/geometry/dl_rtree.h" +#include "flutter/display_list/geometry/dl_region.h" #include "flutter/fml/logging.h" @@ -165,38 +166,17 @@ std::list DlRTree::searchAndConsolidateRects( std::vector intermediary_results; search(query, &intermediary_results); - std::list final_results; + DlRegion region; for (int index : intermediary_results) { - auto current_record_rect = bounds(index); - auto replaced_existing_rect = false; - // // If the current record rect intersects with any of the rects in the - // // result list, then join them, and update the rect in final_results. - std::list::iterator curr_rect_itr = final_results.begin(); - std::list::iterator first_intersecting_rect_itr; - while (!replaced_existing_rect && curr_rect_itr != final_results.end()) { - if (SkRect::Intersects(*curr_rect_itr, current_record_rect)) { - replaced_existing_rect = true; - first_intersecting_rect_itr = curr_rect_itr; - curr_rect_itr->join(current_record_rect); - } - curr_rect_itr++; - } - // It's possible that the result contains duplicated rects at this point. - // For example, consider a result list that contains rects A, B. If a - // new rect C is a superset of A and B, then A and B are the same set after - // the merge. As a result, find such cases and remove them from the result - // list. - while (replaced_existing_rect && curr_rect_itr != final_results.end()) { - if (SkRect::Intersects(*curr_rect_itr, *first_intersecting_rect_itr)) { - first_intersecting_rect_itr->join(*curr_rect_itr); - curr_rect_itr = final_results.erase(curr_rect_itr); - } else { - curr_rect_itr++; - } - } - if (!replaced_existing_rect) { - final_results.push_back(current_record_rect); - } + SkIRect current_record_rect; + bounds(index).roundOut(¤t_record_rect); + region.addRect(current_record_rect); + } + + auto non_overlapping_rects = region.getRects(true); + std::list final_results; + for (const auto& rect : non_overlapping_rects) { + final_results.push_back(SkRect::Make(rect)); } return final_results; } From df1bcb5406e2d68190900b7410bec396340e46a9 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 29 May 2023 17:37:10 +0200 Subject: [PATCH 05/21] Add excluded file to licenses_golden --- 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 5d8ae172abdeb..78947e7716127 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -39,6 +39,7 @@ ../../../flutter/display_list/effects/dl_image_filter_unittests.cc ../../../flutter/display_list/effects/dl_mask_filter_unittests.cc ../../../flutter/display_list/effects/dl_path_effect_unittests.cc +../../../flutter/display_list/geometry/dl_region_unittests.cc ../../../flutter/display_list/geometry/dl_rtree_unittests.cc ../../../flutter/display_list/skia/dl_sk_conversions_unittests.cc ../../../flutter/display_list/skia/dl_sk_paint_dispatcher_unittests.cc From eec95994e5a2e9472c85678723edabf22979d081 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 29 May 2023 18:52:48 +0200 Subject: [PATCH 06/21] Fix iOS integration test --- .../UnobstructedPlatformViewTests.m | 24 +++++-------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/testing/scenario_app/ios/Scenarios/ScenariosUITests/UnobstructedPlatformViewTests.m b/testing/scenario_app/ios/Scenarios/ScenariosUITests/UnobstructedPlatformViewTests.m index 57d627e2b2145..3397e4b1effa2 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosUITests/UnobstructedPlatformViewTests.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosUITests/UnobstructedPlatformViewTests.m @@ -158,17 +158,14 @@ - (void)testOneOverlayAndTwoIntersectingOverlays { XCUIElement* overlay1 = app.otherElements[@"platform_view[0].overlay[0]"]; XCTAssertTrue(overlay1.exists); - XCTAssertEqual(overlay1.frame.origin.x, 150); + XCTAssertEqual(overlay1.frame.origin.x, 75); XCTAssertEqual(overlay1.frame.origin.y, 150); - XCTAssertEqual(overlay1.frame.size.width, 75); - XCTAssertEqual(overlay1.frame.size.height, 75); + XCTAssertEqual(overlay1.frame.size.width, 150); + XCTAssertEqual(overlay1.frame.size.height, 100); - XCUIElement* overlay2 = app.otherElements[@"platform_view[0].overlay[1]"]; - XCTAssertTrue(overlay2.exists); - XCTAssertEqual(overlay2.frame.origin.x, 75); - XCTAssertEqual(overlay2.frame.origin.y, 225); - XCTAssertEqual(overlay2.frame.size.width, 50); - XCTAssertEqual(overlay2.frame.size.height, 25); + // There are three non overlapping rects above platform view, which + // FlutterPlatformViewsController merges into one. + XCTAssertFalse(app.otherElements[@"platform_view[0].overlay[1]"].exists); XCUIElement* overlayView0 = app.otherElements[@"platform_view[0].overlay_view[0]"]; XCTAssertTrue(overlayView0.exists); @@ -178,15 +175,6 @@ - (void)testOneOverlayAndTwoIntersectingOverlays { XCTAssertEqualWithAccuracy(overlayView0.frame.size.width, app.frame.size.width, kCompareAccuracy); XCTAssertEqualWithAccuracy(overlayView0.frame.size.height, app.frame.size.height, kCompareAccuracy); - - XCUIElement* overlayView1 = app.otherElements[@"platform_view[0].overlay_view[1]"]; - XCTAssertTrue(overlayView1.exists); - // Overlay should always be the same frame as the app. - XCTAssertEqualWithAccuracy(overlayView1.frame.origin.x, app.frame.origin.x, kCompareAccuracy); - XCTAssertEqualWithAccuracy(overlayView1.frame.origin.y, app.frame.origin.x, kCompareAccuracy); - XCTAssertEqualWithAccuracy(overlayView1.frame.size.width, app.frame.size.width, kCompareAccuracy); - XCTAssertEqualWithAccuracy(overlayView1.frame.size.height, app.frame.size.height, - kCompareAccuracy); } // A is the layer, which z index is higher than the platform view. From 6b90874cfc89e40421efcc51158103b5ac07ba41 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 29 May 2023 22:52:36 +0200 Subject: [PATCH 07/21] Add more tests --- display_list/geometry/dl_region_unittests.cc | 45 ++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/display_list/geometry/dl_region_unittests.cc b/display_list/geometry/dl_region_unittests.cc index c9dc2bfc9f9da..73a62e9cf7005 100644 --- a/display_list/geometry/dl_region_unittests.cc +++ b/display_list/geometry/dl_region_unittests.cc @@ -57,6 +57,51 @@ TEST(DisplayListRegion, NonOverlappingRectangles2) { EXPECT_EQ(rects, expected); } +TEST(DisplayListRegion, NonOverlappingRectangles3) { + DlRegion region; + region.addRect(SkIRect::MakeXYWH(0, 0, 10, 10)); + region.addRect(SkIRect::MakeXYWH(-11, -11, 10, 10)); + region.addRect(SkIRect::MakeXYWH(11, 11, 10, 10)); + region.addRect(SkIRect::MakeXYWH(-11, 0, 10, 10)); + region.addRect(SkIRect::MakeXYWH(0, 11, 10, 10)); + region.addRect(SkIRect::MakeXYWH(0, -11, 10, 10)); + region.addRect(SkIRect::MakeXYWH(11, 0, 10, 10)); + region.addRect(SkIRect::MakeXYWH(11, -11, 10, 10)); + region.addRect(SkIRect::MakeXYWH(-11, 11, 10, 10)); + auto rects = region.getRects(); + std::vector expected{ + SkIRect::MakeXYWH(-11, -11, 10, 10), // + SkIRect::MakeXYWH(0, -11, 10, 10), // + SkIRect::MakeXYWH(11, -11, 10, 10), // + SkIRect::MakeXYWH(-11, 0, 10, 10), // + SkIRect::MakeXYWH(0, 0, 10, 10), // + SkIRect::MakeXYWH(11, 0, 10, 10), // + SkIRect::MakeXYWH(-11, 11, 10, 10), // + SkIRect::MakeXYWH(0, 11, 10, 10), // + SkIRect::MakeXYWH(11, 11, 10, 10), + }; + EXPECT_EQ(rects, expected); +} + +TEST(DisplayListRegion, MergeTouchingRectangles) { + DlRegion region; + region.addRect(SkIRect::MakeXYWH(0, 0, 10, 10)); + region.addRect(SkIRect::MakeXYWH(-10, -10, 10, 10)); + region.addRect(SkIRect::MakeXYWH(10, 10, 10, 10)); + region.addRect(SkIRect::MakeXYWH(-10, 0, 10, 10)); + region.addRect(SkIRect::MakeXYWH(0, 10, 10, 10)); + region.addRect(SkIRect::MakeXYWH(0, -10, 10, 10)); + region.addRect(SkIRect::MakeXYWH(10, 0, 10, 10)); + region.addRect(SkIRect::MakeXYWH(10, -10, 10, 10)); + region.addRect(SkIRect::MakeXYWH(-10, 10, 10, 10)); + + auto rects = region.getRects(); + std::vector expected{ + SkIRect::MakeXYWH(-10, -10, 30, 30), + }; + EXPECT_EQ(rects, expected); +} + TEST(DisplayListRegion, OverlappingRectangles) { DlRegion region; for (int i = 0; i < 10; ++i) { From d2afa5362f2cf823d33ab8dfd940fd9c5ccc0158 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 29 May 2023 23:03:37 +0200 Subject: [PATCH 08/21] Update documentation --- display_list/geometry/dl_rtree.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/display_list/geometry/dl_rtree.h b/display_list/geometry/dl_rtree.h index 9a3ab407d62c3..296512161a0aa 100644 --- a/display_list/geometry/dl_rtree.h +++ b/display_list/geometry/dl_rtree.h @@ -109,8 +109,7 @@ class DlRTree : public SkRefCnt { /// Finds the rects in the tree that intersect with the query rect. /// - /// When two matching query results intersect with each other, they are - /// joined into a single rect which also intersects with the query rect. + /// The returned list of rectangles will be non-overlapping. /// In other words, the bounds of each rect in the result list are mutually /// exclusive. std::list searchAndConsolidateRects(const SkRect& query) const; From 2233837319bc769d5bf602f37781ff88b57fcd05 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 30 May 2023 18:32:43 +0200 Subject: [PATCH 09/21] Rename i1, i2 to y1, y2 --- display_list/geometry/dl_region.cc | 46 +++++++++++++++--------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/display_list/geometry/dl_region.cc b/display_list/geometry/dl_region.cc index 2f0ed536154ef..f4d8235ec146e 100644 --- a/display_list/geometry/dl_region.cc +++ b/display_list/geometry/dl_region.cc @@ -90,8 +90,8 @@ void DlRegion::addRect(const SkIRect& rect) { return; } - int32_t i1 = rect.fTop; - int32_t i2 = rect.fBottom; + int32_t y1 = rect.fTop; + int32_t y2 = rect.fBottom; size_t dirty_start = -1; size_t dirty_end = 1; @@ -109,57 +109,57 @@ void DlRegion::addRect(const SkIRect& rect) { }; auto upper_bound = std::upper_bound( - lines_.begin(), lines_.end(), i1, + lines_.begin(), lines_.end(), y1, [](int32_t i, const SpanLine& line) { return i < line.bottom; }); auto start_index = upper_bound - lines_.begin(); - for (size_t i = start_index; i < lines_.size() && i1 < i2; ++i) { + for (size_t i = start_index; i < lines_.size() && y1 < y2; ++i) { SpanLine& line = lines_[i]; // If this is false the start index is wrong. - assert(i1 < line.bottom); + assert(y1 < line.bottom); - if (i2 <= line.top) { - insertLine(i, makeLine(i1, i2, rect.fLeft, rect.fRight)); + if (y2 <= line.top) { + insertLine(i, makeLine(y1, y2, rect.fLeft, rect.fRight)); mark_dirty(i); - i1 = i2; + y1 = y2; break; } - if (i1 < line.top) { + if (y1 < line.top) { auto prevLineStart = line.top; - insertLine(i, makeLine(i1, prevLineStart, rect.fLeft, rect.fRight)); + insertLine(i, makeLine(y1, prevLineStart, rect.fLeft, rect.fRight)); mark_dirty(i); - i1 = prevLineStart; + y1 = prevLineStart; continue; } - if (i1 > line.top) { + if (y1 > line.top) { // duplicate line auto prevLineEnd = line.bottom; - line.bottom = i1; + line.bottom = y1; mark_dirty(i); - insertLine(i + 1, makeLine(i1, prevLineEnd, *line.spans)); + insertLine(i + 1, makeLine(y1, prevLineEnd, *line.spans)); continue; } - assert(i1 == line.top); - if (i2 < line.bottom) { + assert(y1 == line.top); + if (y2 < line.bottom) { // duplicate line - auto newLine = makeLine(i2, line.bottom, *line.spans); - line.bottom = i2; + auto newLine = makeLine(y2, line.bottom, *line.spans); + line.bottom = y2; line.insertSpan(rect.fLeft, rect.fRight); insertLine(i + 1, newLine); - i1 = i2; + y1 = y2; mark_dirty(i); break; } - assert(i2 >= line.bottom); + assert(y2 >= line.bottom); line.insertSpan(rect.fLeft, rect.fRight); mark_dirty(i); - i1 = line.bottom; + y1 = line.bottom; } - if (i1 < i2) { - lines_.push_back(makeLine(i1, i2, rect.fLeft, rect.fRight)); + if (y1 < y2) { + lines_.push_back(makeLine(y1, y2, rect.fLeft, rect.fRight)); mark_dirty(lines_.size() - 1); } From 4e6deb5a5c6d33a59cfcae481f5933acab5d5918 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 30 May 2023 20:13:20 +0200 Subject: [PATCH 10/21] Fixup comments --- display_list/geometry/dl_region.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/display_list/geometry/dl_region.cc b/display_list/geometry/dl_region.cc index f4d8235ec146e..788f461991683 100644 --- a/display_list/geometry/dl_region.cc +++ b/display_list/geometry/dl_region.cc @@ -71,7 +71,8 @@ struct DlRegion::SpanLine { }; DlRegion::DlRegion() { - // If we can't memmove SpanLines addRect would be signifantly slower. + // If SpanLines can not be memmoved `addRect` would be signifantly slower + // due to cost of inserting and removing elements from the `lines_` vector. static_assert(std::is_trivially_constructible::value, "SpanLine must be trivially constructible."); } @@ -194,7 +195,7 @@ std::vector DlRegion::getRects(bool deband) const { SkIRect rect{span.left, line.top, span.right, line.bottom}; if (deband) { auto iter = rects.end(); - // If there is recangle previously in rect on which this one is a + // If there is recangle previously in rects on which this one is a // vertical continuation, remove the previous rectangle and expand this // one vertically to cover the area. while (iter != rects.begin()) { From 2a5f14d10fbe3669374f8f04e9cf7918c19dacd1 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 30 May 2023 21:30:03 +0200 Subject: [PATCH 11/21] Skip rectangles from current span line when debanding --- display_list/geometry/dl_region.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/display_list/geometry/dl_region.cc b/display_list/geometry/dl_region.cc index 788f461991683..2c703cc76a0e5 100644 --- a/display_list/geometry/dl_region.cc +++ b/display_list/geometry/dl_region.cc @@ -190,18 +190,19 @@ void DlRegion::addRect(const SkIRect& rect) { std::vector DlRegion::getRects(bool deband) const { std::vector rects; + size_t previous_span_end = 0; for (const auto& line : lines_) { for (const Span& span : *line.spans) { SkIRect rect{span.left, line.top, span.right, line.bottom}; if (deband) { - auto iter = rects.end(); - // If there is recangle previously in rects on which this one is a + auto iter = rects.begin() + previous_span_end; + // If there is rectangle previously in rects on which this one is a // vertical continuation, remove the previous rectangle and expand this // one vertically to cover the area. while (iter != rects.begin()) { --iter; if (iter->bottom() < rect.top()) { - // Went too far. + // Went all the way to previous span line. break; } else if (iter->bottom() == rect.top() && iter->left() == rect.left() && @@ -214,6 +215,7 @@ std::vector DlRegion::getRects(bool deband) const { } rects.push_back(rect); } + previous_span_end = rects.size(); } return rects; } From 80e722b080fd45e3143f944d44cd51b7ca771053 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 30 May 2023 21:49:49 +0200 Subject: [PATCH 12/21] Opt out debanding in DisplayListBuilder::DrawDisplayList --- display_list/dl_builder.cc | 3 ++- display_list/geometry/dl_rtree.cc | 6 +++--- display_list/geometry/dl_rtree.h | 7 ++++++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/display_list/dl_builder.cc b/display_list/dl_builder.cc index 6aa6c94ab868c..f224c444c42bf 100644 --- a/display_list/dl_builder.cc +++ b/display_list/dl_builder.cc @@ -1106,7 +1106,8 @@ void DisplayListBuilder::DrawDisplayList(const sk_sp display_list, case BoundsAccumulatorType::kRTree: auto rtree = display_list->rtree(); if (rtree) { - std::list rects = rtree->searchAndConsolidateRects(bounds); + std::list rects = + rtree->searchAndConsolidateRects(bounds, false); for (const SkRect& rect : rects) { // TODO (https://github.com/flutter/flutter/issues/114919): Attributes // are not necessarily `kDrawDisplayListFlags`. diff --git a/display_list/geometry/dl_rtree.cc b/display_list/geometry/dl_rtree.cc index e6bcd97442764..e667d74d1c8ec 100644 --- a/display_list/geometry/dl_rtree.cc +++ b/display_list/geometry/dl_rtree.cc @@ -160,8 +160,8 @@ void DlRTree::search(const SkRect& query, std::vector* results) const { } } -std::list DlRTree::searchAndConsolidateRects( - const SkRect& query) const { +std::list DlRTree::searchAndConsolidateRects(const SkRect& query, + bool deband) const { // Get the indexes for the operations that intersect with the query rect. std::vector intermediary_results; search(query, &intermediary_results); @@ -173,7 +173,7 @@ std::list DlRTree::searchAndConsolidateRects( region.addRect(current_record_rect); } - auto non_overlapping_rects = region.getRects(true); + auto non_overlapping_rects = region.getRects(deband); std::list final_results; for (const auto& rect : non_overlapping_rects) { final_results.push_back(SkRect::Make(rect)); diff --git a/display_list/geometry/dl_rtree.h b/display_list/geometry/dl_rtree.h index 296512161a0aa..0c2f25abc766a 100644 --- a/display_list/geometry/dl_rtree.h +++ b/display_list/geometry/dl_rtree.h @@ -112,7 +112,12 @@ class DlRTree : public SkRefCnt { /// The returned list of rectangles will be non-overlapping. /// In other words, the bounds of each rect in the result list are mutually /// exclusive. - std::list searchAndConsolidateRects(const SkRect& query) const; + /// + /// If |deband| is true, then matching rectangles from adjacent DlRegion + /// spanlines will be joined together. This reduces the number of + /// rectangles returned, but requires some extra computation. + std::list searchAndConsolidateRects(const SkRect& query, + bool deband = true) const; private: static constexpr SkRect empty_ = SkRect::MakeEmpty(); From 36892e219d51b7bedf85d0569a216e7afce26b8e Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sat, 3 Jun 2023 00:22:38 +0200 Subject: [PATCH 13/21] Benchmarks with different implementation --- .../benchmarking/dl_region_benchmarks.cc | 58 +++ display_list/geometry/dl_region.cc | 335 ++++++++++++++++++ display_list/geometry/dl_region.h | 67 ++++ 3 files changed, 460 insertions(+) diff --git a/display_list/benchmarking/dl_region_benchmarks.cc b/display_list/benchmarking/dl_region_benchmarks.cc index c1745ff514770..c7ce576410931 100644 --- a/display_list/benchmarking/dl_region_benchmarks.cc +++ b/display_list/benchmarking/dl_region_benchmarks.cc @@ -27,6 +27,34 @@ class SkRegionAdapter { SkRegion region_; }; +class DlRegion2Adapter { + public: + void addRect(const SkIRect& rect) { rects_.push_back(rect); } + + std::vector getRects() { + flutter::DlRegion2 region; + region.addRects(std::move(rects_)); + return region.getRects(false); + } + + private: + std::vector rects_; +}; + +class DlRegionSortedAdapter { + public: + void addRect(const SkIRect& rect) { rects_.push_back(rect); } + + std::vector getRects() { + flutter::DlRegionSorted region; + region.addRects(std::move(rects_)); + return region.getRects(false); + } + + private: + std::vector rects_; +}; + template void RunRegionBenchmark(benchmark::State& state, int maxSize) { while (state.KeepRunning()) { @@ -59,18 +87,48 @@ static void BM_RegionBenchmarkDlRegion(benchmark::State& state, int maxSize) { RunRegionBenchmark(state, maxSize); } +static void BM_RegionBenchmarkDlRegionSorted(benchmark::State& state, + int maxSize) { + RunRegionBenchmark(state, maxSize); +} + +static void BM_RegionBenchmarkDlRegion2(benchmark::State& state, int maxSize) { + RunRegionBenchmark(state, maxSize); +} + +BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion, Tiny, 30) + ->Unit(benchmark::kMicrosecond); +BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion2, Tiny, 30) + ->Unit(benchmark::kMicrosecond); +BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegionSorted, Tiny, 30) + ->Unit(benchmark::kMicrosecond); +BENCHMARK_CAPTURE(BM_RegionBenchmarkSkRegion, Tiny, 30) + ->Unit(benchmark::kMicrosecond); + BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion, Small, 100) ->Unit(benchmark::kMicrosecond); +BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion2, Small, 100) + ->Unit(benchmark::kMicrosecond); +BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegionSorted, Small, 100) + ->Unit(benchmark::kMicrosecond); BENCHMARK_CAPTURE(BM_RegionBenchmarkSkRegion, Small, 100) ->Unit(benchmark::kMicrosecond); BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion, Medium, 400) ->Unit(benchmark::kMicrosecond); +BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion2, Medium, 400) + ->Unit(benchmark::kMicrosecond); +BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegionSorted, Medium, 400) + ->Unit(benchmark::kMicrosecond); BENCHMARK_CAPTURE(BM_RegionBenchmarkSkRegion, Medium, 400) ->Unit(benchmark::kMicrosecond); BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion, Large, 1500) ->Unit(benchmark::kMicrosecond); +BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion2, Large, 1500) + ->Unit(benchmark::kMicrosecond); +BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegionSorted, Large, 1500) + ->Unit(benchmark::kMicrosecond); BENCHMARK_CAPTURE(BM_RegionBenchmarkSkRegion, Large, 1500) ->Unit(benchmark::kMicrosecond); diff --git a/display_list/geometry/dl_region.cc b/display_list/geometry/dl_region.cc index 2c703cc76a0e5..4c173b3d96f34 100644 --- a/display_list/geometry/dl_region.cc +++ b/display_list/geometry/dl_region.cc @@ -260,4 +260,339 @@ DlRegion::SpanLine DlRegion::makeLine(int32_t top, return {top, bottom, span_vec}; } +void DlRegion2::SpanLine::insertSpan(int32_t left, int32_t right) { + auto size = spans.size(); + for (size_t i = 0; i < size; ++i) { + Span& span = spans[i]; + if (right < span.left) { + spans.insert(spans.begin() + i, {left, right}); + return; + } + if (left > span.right) { + continue; + } + size_t last_index = i; + while (last_index + 1 < size && right >= spans[last_index + 1].left) { + ++last_index; + } + span.left = std::min(span.left, left); + span.right = std::max(spans[last_index].right, right); + if (last_index > i) { + spans.erase(spans.begin() + i + 1, spans.begin() + last_index + 1); + } + return; + } + + spans.push_back({left, right}); +} + +void DlRegion2::addRects(std::vector&& rects) { + std::sort(rects.begin(), rects.end(), [](const SkIRect& a, const SkIRect& b) { + if (a.top() == b.top()) { + return a.left() < b.left(); + } else { + return a.top() < b.top(); + } + }); + + size_t span_start = 0; + while (span_start < rects.size()) { + size_t span_end = span_start; + int32_t span_top = rects[span_start].fTop; + int32_t span_bottom = rects[span_start].fBottom; + while (true) { + ++span_end; + if (span_end == rects.size()) { + break; + } + if (rects[span_end].fTop != span_top) { + span_bottom = std::min(span_bottom, rects[span_end].fTop); + break; + } + span_bottom = std::min(span_bottom, rects[span_end].fBottom); + } + SpanLine line; + line.top = span_top; + line.bottom = span_bottom; + + for (size_t i = span_start; i < span_end; ++i) { + line.insertSpan(rects[i].fLeft, rects[i].fRight); + rects[i].fTop = span_bottom; + if (rects[i].fBottom == span_bottom) { + if (i > span_start) { + std::memmove(&rects[span_start + 1], &rects[span_start], + (i - span_start) * sizeof(SkIRect)); + } + ++span_start; + } + } + + if (!lines_.empty() && lines_.back().bottom == line.top && + lines_.back().spans.size() == line.spans.size() && + memcmp(lines_.back().spans.data(), line.spans.data(), + line.spans.size() * sizeof(Span)) == 0) { + lines_.back().bottom = line.bottom; + } else { + lines_.push_back(std::move(line)); + } + } +} + +std::vector DlRegion2::getRects(bool deband) const { + std::vector rects; + size_t previous_span_end = 0; + for (const auto& line : lines_) { + for (const Span& span : line.spans) { + SkIRect rect{span.left, line.top, span.right, line.bottom}; + if (deband) { + auto iter = rects.begin() + previous_span_end; + // If there is rectangle previously in rects on which this one is a + // vertical continuation, remove the previous rectangle and expand + // this one vertically to cover the area. + while (iter != rects.begin()) { + --iter; + if (iter->bottom() < rect.top()) { + // Went all the way to previous span line. + break; + } else if (iter->bottom() == rect.top() && + iter->left() == rect.left() && + iter->right() == rect.right()) { + rect.fTop = iter->fTop; + rects.erase(iter); + break; + } + } + } + rects.push_back(rect); + } + previous_span_end = rects.size(); + } + return rects; +} + +std::vector DlRegionSorted::getRects(bool deband) const { + std::vector rects; + size_t previous_span_end = 0; + for (const auto& line : lines_) { + for (const Span& span : *line.spans) { + SkIRect rect{span.left, line.top, span.right, line.bottom}; + if (deband) { + auto iter = rects.begin() + previous_span_end; + // If there is rectangle previously in rects on which this one is a + // vertical continuation, remove the previous rectangle and expand + // this one vertically to cover the area. + while (iter != rects.begin()) { + --iter; + if (iter->bottom() < rect.top()) { + // Went all the way to previous span line. + break; + } else if (iter->bottom() == rect.top() && + iter->left() == rect.left() && + iter->right() == rect.right()) { + rect.fTop = iter->fTop; + rects.erase(iter); + break; + } + } + } + rects.push_back(rect); + } + previous_span_end = rects.size(); + } + return rects; +} + +void DlRegionSorted::SpanLine::insertSpan(int32_t left, int32_t right) { + auto& spans = *this->spans; + auto size = spans.size(); + for (size_t i = 0; i < size; ++i) { + Span& span = spans[i]; + if (right < span.left) { + spans.insert(spans.begin() + i, {left, right}); + return; + } + if (left > span.right) { + continue; + } + size_t last_index = i; + while (last_index + 1 < size && right >= spans[last_index + 1].left) { + ++last_index; + } + span.left = std::min(span.left, left); + span.right = std::max(spans[last_index].right, right); + if (last_index > i) { + spans.erase(spans.begin() + i + 1, spans.begin() + last_index + 1); + } + return; + } + + spans.push_back({left, right}); +} + +bool DlRegionSorted::SpanLine::spansEqual(const SpanLine& l2) const { + SpanVec& spans = *this->spans; + SpanVec& otherSpans = *l2.spans; + assert(this != &l2); + + if (spans.size() != otherSpans.size()) { + return false; + } + return memcmp(spans.data(), otherSpans.data(), spans.size() * sizeof(Span)) == + 0; +} + +void DlRegionSorted::insertLine(size_t position, SpanLine line) { + lines_.insert(lines_.begin() + position, line); +} + +DlRegionSorted::LineVec::iterator DlRegionSorted::removeLine( + DlRegionSorted::LineVec::iterator line) { + spanvec_pool_.push_back(line->spans); + return lines_.erase(line); +} + +DlRegionSorted::SpanLine DlRegionSorted::makeLine(int32_t top, + int32_t bottom, + int32_t spanLeft, + int32_t spanRight) { + SpanVec* span_vec; + if (!spanvec_pool_.empty()) { + span_vec = spanvec_pool_.back(); + spanvec_pool_.pop_back(); + span_vec->clear(); + } else { + span_vec = new SpanVec(); + } + span_vec->push_back({spanLeft, spanRight}); + return {top, bottom, span_vec}; +} + +DlRegionSorted::SpanLine DlRegionSorted::makeLine(int32_t top, + int32_t bottom, + const SpanVec& spans) { + SpanVec* span_vec; + if (!spanvec_pool_.empty()) { + span_vec = spanvec_pool_.back(); + spanvec_pool_.pop_back(); + } else { + span_vec = new SpanVec(); + } + *span_vec = spans; + return {top, bottom, span_vec}; +} + +void DlRegionSorted::addRects(std::vector&& rects) { + std::sort(rects.begin(), rects.end(), [](const SkIRect& a, const SkIRect& b) { + // Sort the rectangles by Y axis. Because the rectangles have varying + // height, they are added to span lines in non-deterministic order and thus + // it makes no difference if they are also sorted by the X axis. + return a.top() < b.top(); + }); + + size_t start_index = 0; + + size_t dirty_start = -1; + size_t dirty_end = 1; + + // Marks line as dirty. Dirty lines will be checked for equality + // later and merged as needed. + auto mark_dirty = [&](size_t line) { + if (dirty_start == static_cast(-1)) { + dirty_start = line; + dirty_end = line; + } else { + dirty_start = std::min(dirty_start, line); + dirty_end = std::max(dirty_end, line); + } + }; + + size_t processed_count = 0; + + for (const SkIRect& rect : rects) { + ++processed_count; + if (rect.isEmpty()) { + continue; + } + + int32_t y1 = rect.fTop; + int32_t y2 = rect.fBottom; + + for (size_t i = start_index; i < lines_.size() && y1 < y2; ++i) { + SpanLine& line = lines_[i]; + + if (y1 >= line.bottom) { + start_index = i + 1; + continue; + } + + if (y2 <= line.top) { + insertLine(i, makeLine(y1, y2, rect.fLeft, rect.fRight)); + mark_dirty(i); + y1 = y2; + break; + } + if (y1 < line.top) { + auto prevLineStart = line.top; + insertLine(i, makeLine(y1, prevLineStart, rect.fLeft, rect.fRight)); + mark_dirty(i); + y1 = prevLineStart; + continue; + } + if (y1 > line.top) { + // duplicate line + auto prevLineEnd = line.bottom; + line.bottom = y1; + mark_dirty(i); + insertLine(i + 1, makeLine(y1, prevLineEnd, *line.spans)); + continue; + } + assert(y1 == line.top); + if (y2 < line.bottom) { + // duplicate line + auto newLine = makeLine(y2, line.bottom, *line.spans); + line.bottom = y2; + line.insertSpan(rect.fLeft, rect.fRight); + insertLine(i + 1, newLine); + y1 = y2; + mark_dirty(i); + break; + } + assert(y2 >= line.bottom); + line.insertSpan(rect.fLeft, rect.fRight); + mark_dirty(i); + y1 = line.bottom; + } + + if (y1 < y2) { + lines_.push_back(makeLine(y1, y2, rect.fLeft, rect.fRight)); + mark_dirty(lines_.size() - 1); + } + + // Check for duplicate lines and merge them. + if (dirty_start != static_cast(-1)) { + // Expand the region by one if possible. + if (dirty_start > 0) { + --dirty_start; + } + if (dirty_end + 1 < lines_.size()) { + ++dirty_end; + } + for (auto i = lines_.begin() + dirty_start; + i < lines_.begin() + dirty_end;) { + auto& line = *i; + auto& next = *(i + 1); + if (line.bottom == next.top && line.spansEqual(next)) { + --dirty_end; + next.top = line.top; + i = removeLine(i); + } else { + ++i; + } + } + } + dirty_start = -1; + dirty_end = -1; + } +} + } // namespace flutter diff --git a/display_list/geometry/dl_region.h b/display_list/geometry/dl_region.h index 8577982adfa07..0a4ab550d1486 100644 --- a/display_list/geometry/dl_region.h +++ b/display_list/geometry/dl_region.h @@ -49,6 +49,73 @@ class DlRegion { std::vector spanvec_pool_; }; +class DlRegion2 { + public: + void addRects(std::vector&& rects); + std::vector getRects(bool deband = true) const; + + private: + struct Span { + int32_t left; + int32_t right; + }; + struct SpanLine { + int32_t top; + int32_t bottom; + std::vector spans; + + void insertSpan(int32_t left, int32_t right); + }; + + std::vector lines_; +}; + +/// Represents a region as a collection of non-overlapping rectangles. +/// Implements a subset of SkRegion functionality optimized for quickly +/// converting set of overlapping rectangles to non-overlapping rectangles. +class DlRegionSorted { + public: + /// Bulks adds rectangles to current region. + /// Matches SkRegion::op(rect, SkRegion::kUnion_Op) behavior. + void addRects(std::vector&& rects); + + /// Returns list of non-overlapping rectangles that cover current region. + /// If |deband| is false, each span line will result in separate rectangles, + /// closely matching SkRegion::Iterator behavior. + /// If |deband| is true, matching rectangles from adjacent span lines will be + /// merged into single rectange. + std::vector getRects(bool deband = true) const; + + private: + struct Span { + int32_t left; + int32_t right; + }; + typedef std::vector SpanVec; + struct SpanLine { + int32_t top; + int32_t bottom; + SpanVec* spans; + + void insertSpan(int32_t left, int32_t right); + bool spansEqual(const SpanLine& l2) const; + }; + + typedef std::vector LineVec; + + std::vector lines_; + std::vector spanvec_pool_; + + void insertLine(size_t position, SpanLine line); + LineVec::iterator removeLine(LineVec::iterator position); + + SpanLine makeLine(int32_t top, + int32_t bottom, + int32_t spanLeft, + int32_t spanRight); + SpanLine makeLine(int32_t top, int32_t bottom, const SpanVec& spans); +}; + } // namespace flutter #endif // FLUTTER_DISPLAY_LIST_GEOMETRY_REGION_H_ From 856bc353857289cb06df6ecec8b649a75caf2921 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sat, 3 Jun 2023 01:17:45 +0200 Subject: [PATCH 14/21] Use DlRegionSorted as the algorithm --- .../benchmarking/dl_region_benchmarks.cc | 48 +-- display_list/geometry/dl_region.cc | 395 +----------------- display_list/geometry/dl_region.h | 59 --- display_list/geometry/dl_region_unittests.cc | 83 ++-- display_list/geometry/dl_rtree.cc | 7 +- flow/rtree.cc | 6 +- 6 files changed, 77 insertions(+), 521 deletions(-) diff --git a/display_list/benchmarking/dl_region_benchmarks.cc b/display_list/benchmarking/dl_region_benchmarks.cc index c7ce576410931..bc5ef50c9d367 100644 --- a/display_list/benchmarking/dl_region_benchmarks.cc +++ b/display_list/benchmarking/dl_region_benchmarks.cc @@ -27,26 +27,12 @@ class SkRegionAdapter { SkRegion region_; }; -class DlRegion2Adapter { +class DlRegionAdapter { public: void addRect(const SkIRect& rect) { rects_.push_back(rect); } std::vector getRects() { - flutter::DlRegion2 region; - region.addRects(std::move(rects_)); - return region.getRects(false); - } - - private: - std::vector rects_; -}; - -class DlRegionSortedAdapter { - public: - void addRect(const SkIRect& rect) { rects_.push_back(rect); } - - std::vector getRects() { - flutter::DlRegionSorted region; + flutter::DlRegion region; region.addRects(std::move(rects_)); return region.getRects(false); } @@ -84,51 +70,23 @@ static void BM_RegionBenchmarkSkRegion(benchmark::State& state, int maxSize) { } static void BM_RegionBenchmarkDlRegion(benchmark::State& state, int maxSize) { - RunRegionBenchmark(state, maxSize); -} - -static void BM_RegionBenchmarkDlRegionSorted(benchmark::State& state, - int maxSize) { - RunRegionBenchmark(state, maxSize); -} - -static void BM_RegionBenchmarkDlRegion2(benchmark::State& state, int maxSize) { - RunRegionBenchmark(state, maxSize); + RunRegionBenchmark(state, maxSize); } BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion, Tiny, 30) ->Unit(benchmark::kMicrosecond); -BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion2, Tiny, 30) - ->Unit(benchmark::kMicrosecond); -BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegionSorted, Tiny, 30) - ->Unit(benchmark::kMicrosecond); BENCHMARK_CAPTURE(BM_RegionBenchmarkSkRegion, Tiny, 30) ->Unit(benchmark::kMicrosecond); - BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion, Small, 100) ->Unit(benchmark::kMicrosecond); -BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion2, Small, 100) - ->Unit(benchmark::kMicrosecond); -BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegionSorted, Small, 100) - ->Unit(benchmark::kMicrosecond); BENCHMARK_CAPTURE(BM_RegionBenchmarkSkRegion, Small, 100) ->Unit(benchmark::kMicrosecond); - BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion, Medium, 400) ->Unit(benchmark::kMicrosecond); -BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion2, Medium, 400) - ->Unit(benchmark::kMicrosecond); -BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegionSorted, Medium, 400) - ->Unit(benchmark::kMicrosecond); BENCHMARK_CAPTURE(BM_RegionBenchmarkSkRegion, Medium, 400) ->Unit(benchmark::kMicrosecond); - BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion, Large, 1500) ->Unit(benchmark::kMicrosecond); -BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegion2, Large, 1500) - ->Unit(benchmark::kMicrosecond); -BENCHMARK_CAPTURE(BM_RegionBenchmarkDlRegionSorted, Large, 1500) - ->Unit(benchmark::kMicrosecond); BENCHMARK_CAPTURE(BM_RegionBenchmarkSkRegion, Large, 1500) ->Unit(benchmark::kMicrosecond); diff --git a/display_list/geometry/dl_region.cc b/display_list/geometry/dl_region.cc index 4c173b3d96f34..6f4ab24e54904 100644 --- a/display_list/geometry/dl_region.cc +++ b/display_list/geometry/dl_region.cc @@ -8,369 +8,7 @@ namespace flutter { -struct DlRegion::Span { - int32_t left; - int32_t right; -}; - -struct DlRegion::SpanLine { - int32_t top; - int32_t bottom; - // For performance reasons SpanLine must be trivially constructible. - // DlRegion is responsible for allocating and deallocating the - // memory for spans. - SpanVec* spans; - - /// Inserts span into this span line. If there are existing spans - /// that overlap with the new span, they will be merged into single - /// span. - void insertSpan(int32_t left, int32_t right) { - SpanVec& spans = *this->spans; - - auto size = spans.size(); - for (size_t i = 0; i < size; ++i) { - Span& span = spans[i]; - if (right < span.left) { - _insertSpanAt(i, left, right); - return; - } - if (left > span.right) { - continue; - } - size_t last_index = i; - while (last_index + 1 < size && right >= spans[last_index + 1].left) { - ++last_index; - } - span.left = std::min(span.left, left); - span.right = std::max(spans[last_index].right, right); - if (last_index > i) { - spans.erase(spans.begin() + i + 1, spans.begin() + last_index + 1); - } - return; - } - - spans.push_back({left, right}); - } - - bool spansEqual(const SpanLine& l2) const { - SpanVec& spans = *this->spans; - SpanVec& otherSpans = *l2.spans; - assert(this != &l2); - - if (spans.size() != otherSpans.size()) { - return false; - } - return memcmp(spans.data(), otherSpans.data(), - spans.size() * sizeof(Span)) == 0; - } - - private: - void _insertSpanAt(size_t index, int32_t x1, int32_t x2) { - spans->insert(spans->begin() + index, {x1, x2}); - } -}; - -DlRegion::DlRegion() { - // If SpanLines can not be memmoved `addRect` would be signifantly slower - // due to cost of inserting and removing elements from the `lines_` vector. - static_assert(std::is_trivially_constructible::value, - "SpanLine must be trivially constructible."); -} - -DlRegion::~DlRegion() { - for (auto& spanvec : spanvec_pool_) { - delete spanvec; - } - for (auto& line : lines_) { - delete line.spans; - } -} - -void DlRegion::addRect(const SkIRect& rect) { - if (rect.isEmpty()) { - return; - } - - int32_t y1 = rect.fTop; - int32_t y2 = rect.fBottom; - - size_t dirty_start = -1; - size_t dirty_end = 1; - - // Marks line as dirty. Dirty lines will be checked for equality - // later and merged as needed. - auto mark_dirty = [&](size_t line) { - if (dirty_start == static_cast(-1)) { - dirty_start = line; - dirty_end = line; - } else { - dirty_start = std::min(dirty_start, line); - dirty_end = std::max(dirty_end, line); - } - }; - - auto upper_bound = std::upper_bound( - lines_.begin(), lines_.end(), y1, - [](int32_t i, const SpanLine& line) { return i < line.bottom; }); - - auto start_index = upper_bound - lines_.begin(); - - for (size_t i = start_index; i < lines_.size() && y1 < y2; ++i) { - SpanLine& line = lines_[i]; - - // If this is false the start index is wrong. - assert(y1 < line.bottom); - - if (y2 <= line.top) { - insertLine(i, makeLine(y1, y2, rect.fLeft, rect.fRight)); - mark_dirty(i); - y1 = y2; - break; - } - if (y1 < line.top) { - auto prevLineStart = line.top; - insertLine(i, makeLine(y1, prevLineStart, rect.fLeft, rect.fRight)); - mark_dirty(i); - y1 = prevLineStart; - continue; - } - if (y1 > line.top) { - // duplicate line - auto prevLineEnd = line.bottom; - line.bottom = y1; - mark_dirty(i); - insertLine(i + 1, makeLine(y1, prevLineEnd, *line.spans)); - continue; - } - assert(y1 == line.top); - if (y2 < line.bottom) { - // duplicate line - auto newLine = makeLine(y2, line.bottom, *line.spans); - line.bottom = y2; - line.insertSpan(rect.fLeft, rect.fRight); - insertLine(i + 1, newLine); - y1 = y2; - mark_dirty(i); - break; - } - assert(y2 >= line.bottom); - line.insertSpan(rect.fLeft, rect.fRight); - mark_dirty(i); - y1 = line.bottom; - } - - if (y1 < y2) { - lines_.push_back(makeLine(y1, y2, rect.fLeft, rect.fRight)); - mark_dirty(lines_.size() - 1); - } - - // Check for duplicate lines and merge them. - if (dirty_start != static_cast(-1)) { - // Expand the region by one if possible. - if (dirty_start > 0) { - --dirty_start; - } - if (dirty_end + 1 < lines_.size()) { - ++dirty_end; - } - for (auto i = lines_.begin() + dirty_start; - i < lines_.begin() + dirty_end;) { - auto& line = *i; - auto& next = *(i + 1); - if (line.bottom == next.top && line.spansEqual(next)) { - --dirty_end; - next.top = line.top; - i = removeLine(i); - } else { - ++i; - } - } - } -} - std::vector DlRegion::getRects(bool deband) const { - std::vector rects; - size_t previous_span_end = 0; - for (const auto& line : lines_) { - for (const Span& span : *line.spans) { - SkIRect rect{span.left, line.top, span.right, line.bottom}; - if (deband) { - auto iter = rects.begin() + previous_span_end; - // If there is rectangle previously in rects on which this one is a - // vertical continuation, remove the previous rectangle and expand this - // one vertically to cover the area. - while (iter != rects.begin()) { - --iter; - if (iter->bottom() < rect.top()) { - // Went all the way to previous span line. - break; - } else if (iter->bottom() == rect.top() && - iter->left() == rect.left() && - iter->right() == rect.right()) { - rect.fTop = iter->fTop; - rects.erase(iter); - break; - } - } - } - rects.push_back(rect); - } - previous_span_end = rects.size(); - } - return rects; -} - -void DlRegion::insertLine(size_t position, SpanLine line) { - lines_.insert(lines_.begin() + position, line); -} - -DlRegion::LineVec::iterator DlRegion::removeLine( - DlRegion::LineVec::iterator line) { - spanvec_pool_.push_back(line->spans); - return lines_.erase(line); -} - -DlRegion::SpanLine DlRegion::makeLine(int32_t top, - int32_t bottom, - int32_t spanLeft, - int32_t spanRight) { - SpanVec* span_vec; - if (!spanvec_pool_.empty()) { - span_vec = spanvec_pool_.back(); - spanvec_pool_.pop_back(); - span_vec->clear(); - } else { - span_vec = new SpanVec(); - } - span_vec->push_back({spanLeft, spanRight}); - return {top, bottom, span_vec}; -} - -DlRegion::SpanLine DlRegion::makeLine(int32_t top, - int32_t bottom, - const SpanVec& spans) { - SpanVec* span_vec; - if (!spanvec_pool_.empty()) { - span_vec = spanvec_pool_.back(); - spanvec_pool_.pop_back(); - } else { - span_vec = new SpanVec(); - } - *span_vec = spans; - return {top, bottom, span_vec}; -} - -void DlRegion2::SpanLine::insertSpan(int32_t left, int32_t right) { - auto size = spans.size(); - for (size_t i = 0; i < size; ++i) { - Span& span = spans[i]; - if (right < span.left) { - spans.insert(spans.begin() + i, {left, right}); - return; - } - if (left > span.right) { - continue; - } - size_t last_index = i; - while (last_index + 1 < size && right >= spans[last_index + 1].left) { - ++last_index; - } - span.left = std::min(span.left, left); - span.right = std::max(spans[last_index].right, right); - if (last_index > i) { - spans.erase(spans.begin() + i + 1, spans.begin() + last_index + 1); - } - return; - } - - spans.push_back({left, right}); -} - -void DlRegion2::addRects(std::vector&& rects) { - std::sort(rects.begin(), rects.end(), [](const SkIRect& a, const SkIRect& b) { - if (a.top() == b.top()) { - return a.left() < b.left(); - } else { - return a.top() < b.top(); - } - }); - - size_t span_start = 0; - while (span_start < rects.size()) { - size_t span_end = span_start; - int32_t span_top = rects[span_start].fTop; - int32_t span_bottom = rects[span_start].fBottom; - while (true) { - ++span_end; - if (span_end == rects.size()) { - break; - } - if (rects[span_end].fTop != span_top) { - span_bottom = std::min(span_bottom, rects[span_end].fTop); - break; - } - span_bottom = std::min(span_bottom, rects[span_end].fBottom); - } - SpanLine line; - line.top = span_top; - line.bottom = span_bottom; - - for (size_t i = span_start; i < span_end; ++i) { - line.insertSpan(rects[i].fLeft, rects[i].fRight); - rects[i].fTop = span_bottom; - if (rects[i].fBottom == span_bottom) { - if (i > span_start) { - std::memmove(&rects[span_start + 1], &rects[span_start], - (i - span_start) * sizeof(SkIRect)); - } - ++span_start; - } - } - - if (!lines_.empty() && lines_.back().bottom == line.top && - lines_.back().spans.size() == line.spans.size() && - memcmp(lines_.back().spans.data(), line.spans.data(), - line.spans.size() * sizeof(Span)) == 0) { - lines_.back().bottom = line.bottom; - } else { - lines_.push_back(std::move(line)); - } - } -} - -std::vector DlRegion2::getRects(bool deband) const { - std::vector rects; - size_t previous_span_end = 0; - for (const auto& line : lines_) { - for (const Span& span : line.spans) { - SkIRect rect{span.left, line.top, span.right, line.bottom}; - if (deband) { - auto iter = rects.begin() + previous_span_end; - // If there is rectangle previously in rects on which this one is a - // vertical continuation, remove the previous rectangle and expand - // this one vertically to cover the area. - while (iter != rects.begin()) { - --iter; - if (iter->bottom() < rect.top()) { - // Went all the way to previous span line. - break; - } else if (iter->bottom() == rect.top() && - iter->left() == rect.left() && - iter->right() == rect.right()) { - rect.fTop = iter->fTop; - rects.erase(iter); - break; - } - } - } - rects.push_back(rect); - } - previous_span_end = rects.size(); - } - return rects; -} - -std::vector DlRegionSorted::getRects(bool deband) const { std::vector rects; size_t previous_span_end = 0; for (const auto& line : lines_) { @@ -402,7 +40,7 @@ std::vector DlRegionSorted::getRects(bool deband) const { return rects; } -void DlRegionSorted::SpanLine::insertSpan(int32_t left, int32_t right) { +void DlRegion::SpanLine::insertSpan(int32_t left, int32_t right) { auto& spans = *this->spans; auto size = spans.size(); for (size_t i = 0; i < size; ++i) { @@ -429,7 +67,7 @@ void DlRegionSorted::SpanLine::insertSpan(int32_t left, int32_t right) { spans.push_back({left, right}); } -bool DlRegionSorted::SpanLine::spansEqual(const SpanLine& l2) const { +bool DlRegion::SpanLine::spansEqual(const SpanLine& l2) const { SpanVec& spans = *this->spans; SpanVec& otherSpans = *l2.spans; assert(this != &l2); @@ -441,20 +79,20 @@ bool DlRegionSorted::SpanLine::spansEqual(const SpanLine& l2) const { 0; } -void DlRegionSorted::insertLine(size_t position, SpanLine line) { +void DlRegion::insertLine(size_t position, SpanLine line) { lines_.insert(lines_.begin() + position, line); } -DlRegionSorted::LineVec::iterator DlRegionSorted::removeLine( - DlRegionSorted::LineVec::iterator line) { +DlRegion::LineVec::iterator DlRegion::removeLine( + DlRegion::LineVec::iterator line) { spanvec_pool_.push_back(line->spans); return lines_.erase(line); } -DlRegionSorted::SpanLine DlRegionSorted::makeLine(int32_t top, - int32_t bottom, - int32_t spanLeft, - int32_t spanRight) { +DlRegion::SpanLine DlRegion::makeLine(int32_t top, + int32_t bottom, + int32_t spanLeft, + int32_t spanRight) { SpanVec* span_vec; if (!spanvec_pool_.empty()) { span_vec = spanvec_pool_.back(); @@ -467,9 +105,9 @@ DlRegionSorted::SpanLine DlRegionSorted::makeLine(int32_t top, return {top, bottom, span_vec}; } -DlRegionSorted::SpanLine DlRegionSorted::makeLine(int32_t top, - int32_t bottom, - const SpanVec& spans) { +DlRegion::SpanLine DlRegion::makeLine(int32_t top, + int32_t bottom, + const SpanVec& spans) { SpanVec* span_vec; if (!spanvec_pool_.empty()) { span_vec = spanvec_pool_.back(); @@ -481,7 +119,7 @@ DlRegionSorted::SpanLine DlRegionSorted::makeLine(int32_t top, return {top, bottom, span_vec}; } -void DlRegionSorted::addRects(std::vector&& rects) { +void DlRegion::addRects(std::vector&& rects) { std::sort(rects.begin(), rects.end(), [](const SkIRect& a, const SkIRect& b) { // Sort the rectangles by Y axis. Because the rectangles have varying // height, they are added to span lines in non-deterministic order and thus @@ -506,10 +144,7 @@ void DlRegionSorted::addRects(std::vector&& rects) { } }; - size_t processed_count = 0; - for (const SkIRect& rect : rects) { - ++processed_count; if (rect.isEmpty()) { continue; } @@ -520,8 +155,8 @@ void DlRegionSorted::addRects(std::vector&& rects) { for (size_t i = start_index; i < lines_.size() && y1 < y2; ++i) { SpanLine& line = lines_[i]; - if (y1 >= line.bottom) { - start_index = i + 1; + if (rect.fTop >= line.bottom) { + start_index = i; continue; } diff --git a/display_list/geometry/dl_region.h b/display_list/geometry/dl_region.h index 0a4ab550d1486..e10efed5199c6 100644 --- a/display_list/geometry/dl_region.h +++ b/display_list/geometry/dl_region.h @@ -15,65 +15,6 @@ namespace flutter { /// Implements a subset of SkRegion functionality optimized for quickly /// converting set of overlapping rectangles to non-overlapping rectangles. class DlRegion { - public: - DlRegion(); - ~DlRegion(); - - /// Adds rectangle to current region. - /// Matches SkRegion::op(rect, SkRegion::kUnion_Op) behavior. - void addRect(const SkIRect& rect); - - /// Returns list of non-overlapping rectangles that cover current region. - /// If |deband| is false, each span line will result in separate rectangles, - /// closely matching SkRegion::Iterator behavior. - /// If |deband| is true, matching rectangles from adjacent span lines will be - /// merged into single rectange. - std::vector getRects(bool deband = true) const; - - private: - struct Span; - struct SpanLine; - typedef std::vector SpanVec; - typedef std::vector LineVec; - - void insertLine(size_t position, SpanLine line); - LineVec::iterator removeLine(LineVec::iterator position); - - SpanLine makeLine(int32_t top, - int32_t bottom, - int32_t spanLeft, - int32_t spanRight); - SpanLine makeLine(int32_t top, int32_t bottom, const SpanVec& spans); - - LineVec lines_; - std::vector spanvec_pool_; -}; - -class DlRegion2 { - public: - void addRects(std::vector&& rects); - std::vector getRects(bool deband = true) const; - - private: - struct Span { - int32_t left; - int32_t right; - }; - struct SpanLine { - int32_t top; - int32_t bottom; - std::vector spans; - - void insertSpan(int32_t left, int32_t right); - }; - - std::vector lines_; -}; - -/// Represents a region as a collection of non-overlapping rectangles. -/// Implements a subset of SkRegion functionality optimized for quickly -/// converting set of overlapping rectangles to non-overlapping rectangles. -class DlRegionSorted { public: /// Bulks adds rectangles to current region. /// Matches SkRegion::op(rect, SkRegion::kUnion_Op) behavior. diff --git a/display_list/geometry/dl_region_unittests.cc b/display_list/geometry/dl_region_unittests.cc index 73a62e9cf7005..611c97a50bb2f 100644 --- a/display_list/geometry/dl_region_unittests.cc +++ b/display_list/geometry/dl_region_unittests.cc @@ -19,18 +19,20 @@ TEST(DisplayListRegion, EmptyRegion) { TEST(DisplayListRegion, SingleRectangle) { DlRegion region; - region.addRect(SkIRect::MakeLTRB(10, 10, 50, 50)); + region.addRects({SkIRect::MakeLTRB(10, 10, 50, 50)}); auto rects = region.getRects(); ASSERT_EQ(rects.size(), 1u); EXPECT_EQ(rects.front(), SkIRect::MakeLTRB(10, 10, 50, 50)); } TEST(DisplayListRegion, NonOverlappingRectangles1) { - DlRegion region; + std::vector rects_in; for (int i = 0; i < 10; ++i) { SkIRect rect = SkIRect::MakeXYWH(50 * i, 50 * i, 50, 50); - region.addRect(rect); + rects_in.push_back(rect); } + DlRegion region; + region.addRects(std::move(rects_in)); auto rects = region.getRects(); std::vector expected{ {0, 0, 50, 50}, {50, 50, 100, 100}, {100, 100, 150, 150}, @@ -43,10 +45,12 @@ TEST(DisplayListRegion, NonOverlappingRectangles1) { TEST(DisplayListRegion, NonOverlappingRectangles2) { DlRegion region; - region.addRect(SkIRect::MakeXYWH(5, 5, 10, 10)); - region.addRect(SkIRect::MakeXYWH(25, 5, 10, 10)); - region.addRect(SkIRect::MakeXYWH(5, 25, 10, 10)); - region.addRect(SkIRect::MakeXYWH(25, 25, 10, 10)); + region.addRects({ + SkIRect::MakeXYWH(5, 5, 10, 10), + SkIRect::MakeXYWH(25, 5, 10, 10), + SkIRect::MakeXYWH(5, 25, 10, 10), + SkIRect::MakeXYWH(25, 25, 10, 10), + }); auto rects = region.getRects(); std::vector expected{ SkIRect::MakeXYWH(5, 5, 10, 10), @@ -59,15 +63,17 @@ TEST(DisplayListRegion, NonOverlappingRectangles2) { TEST(DisplayListRegion, NonOverlappingRectangles3) { DlRegion region; - region.addRect(SkIRect::MakeXYWH(0, 0, 10, 10)); - region.addRect(SkIRect::MakeXYWH(-11, -11, 10, 10)); - region.addRect(SkIRect::MakeXYWH(11, 11, 10, 10)); - region.addRect(SkIRect::MakeXYWH(-11, 0, 10, 10)); - region.addRect(SkIRect::MakeXYWH(0, 11, 10, 10)); - region.addRect(SkIRect::MakeXYWH(0, -11, 10, 10)); - region.addRect(SkIRect::MakeXYWH(11, 0, 10, 10)); - region.addRect(SkIRect::MakeXYWH(11, -11, 10, 10)); - region.addRect(SkIRect::MakeXYWH(-11, 11, 10, 10)); + region.addRects({ + SkIRect::MakeXYWH(0, 0, 10, 10), + SkIRect::MakeXYWH(-11, -11, 10, 10), + SkIRect::MakeXYWH(11, 11, 10, 10), + SkIRect::MakeXYWH(-11, 0, 10, 10), + SkIRect::MakeXYWH(0, 11, 10, 10), + SkIRect::MakeXYWH(0, -11, 10, 10), + SkIRect::MakeXYWH(11, 0, 10, 10), + SkIRect::MakeXYWH(11, -11, 10, 10), + SkIRect::MakeXYWH(-11, 11, 10, 10), + }); auto rects = region.getRects(); std::vector expected{ SkIRect::MakeXYWH(-11, -11, 10, 10), // @@ -85,15 +91,17 @@ TEST(DisplayListRegion, NonOverlappingRectangles3) { TEST(DisplayListRegion, MergeTouchingRectangles) { DlRegion region; - region.addRect(SkIRect::MakeXYWH(0, 0, 10, 10)); - region.addRect(SkIRect::MakeXYWH(-10, -10, 10, 10)); - region.addRect(SkIRect::MakeXYWH(10, 10, 10, 10)); - region.addRect(SkIRect::MakeXYWH(-10, 0, 10, 10)); - region.addRect(SkIRect::MakeXYWH(0, 10, 10, 10)); - region.addRect(SkIRect::MakeXYWH(0, -10, 10, 10)); - region.addRect(SkIRect::MakeXYWH(10, 0, 10, 10)); - region.addRect(SkIRect::MakeXYWH(10, -10, 10, 10)); - region.addRect(SkIRect::MakeXYWH(-10, 10, 10, 10)); + region.addRects({ + SkIRect::MakeXYWH(0, 0, 10, 10), + SkIRect::MakeXYWH(-10, -10, 10, 10), + SkIRect::MakeXYWH(10, 10, 10, 10), + SkIRect::MakeXYWH(-10, 0, 10, 10), + SkIRect::MakeXYWH(0, 10, 10, 10), + SkIRect::MakeXYWH(0, -10, 10, 10), + SkIRect::MakeXYWH(10, 0, 10, 10), + SkIRect::MakeXYWH(10, -10, 10, 10), + SkIRect::MakeXYWH(-10, 10, 10, 10), + }); auto rects = region.getRects(); std::vector expected{ @@ -103,11 +111,13 @@ TEST(DisplayListRegion, MergeTouchingRectangles) { } TEST(DisplayListRegion, OverlappingRectangles) { - DlRegion region; + std::vector rects_in; for (int i = 0; i < 10; ++i) { SkIRect rect = SkIRect::MakeXYWH(10 * i, 10 * i, 50, 50); - region.addRect(rect); + rects_in.push_back(rect); } + DlRegion region; + region.addRects(std::move(rects_in)); auto rects = region.getRects(); std::vector expected{ {0, 0, 50, 10}, {0, 10, 60, 20}, {0, 20, 70, 30}, @@ -122,9 +132,11 @@ TEST(DisplayListRegion, OverlappingRectangles) { TEST(DisplayListRegion, Deband) { DlRegion region; - region.addRect(SkIRect::MakeXYWH(0, 0, 50, 50)); - region.addRect(SkIRect::MakeXYWH(60, 0, 20, 20)); - region.addRect(SkIRect::MakeXYWH(90, 0, 50, 50)); + region.addRects({ + SkIRect::MakeXYWH(0, 0, 50, 50), + SkIRect::MakeXYWH(60, 0, 20, 20), + SkIRect::MakeXYWH(90, 0, 50, 50), + }); auto rects_with_deband = region.getRects(true); std::vector expected{ @@ -164,22 +176,27 @@ TEST(DisplayListRegion, TestAgainstSkRegion) { for (const auto& settings : all_settings) { std::random_device d; - std::seed_seq seed{::testing::UnitTest::GetInstance()->random_seed()}; + // std::seed_seq seed{::testing::UnitTest::GetInstance()->random_seed()}; + std::seed_seq seed{4}; std::mt19937 rng(seed); - DlRegion region; SkRegion sk_region; std::uniform_int_distribution pos(0, 4000); std::uniform_int_distribution size(1, settings.max_size); + std::vector rects_in; + for (size_t i = 0; i < settings.iteration_count; ++i) { SkIRect rect = SkIRect::MakeXYWH(pos(rng), pos(rng), size(rng), size(rng)); - region.addRect(rect); + rects_in.push_back(rect); sk_region.op(rect, SkRegion::kUnion_Op); } + DlRegion region; + region.addRects(std::move(rects_in)); + // Do not deband the rectangles - identical to SkRegion::Iterator auto rects = region.getRects(false); diff --git a/display_list/geometry/dl_rtree.cc b/display_list/geometry/dl_rtree.cc index e667d74d1c8ec..302fa7578f4dd 100644 --- a/display_list/geometry/dl_rtree.cc +++ b/display_list/geometry/dl_rtree.cc @@ -166,12 +166,15 @@ std::list DlRTree::searchAndConsolidateRects(const SkRect& query, std::vector intermediary_results; search(query, &intermediary_results); - DlRegion region; + std::vector rects; + rects.reserve(intermediary_results.size()); for (int index : intermediary_results) { SkIRect current_record_rect; bounds(index).roundOut(¤t_record_rect); - region.addRect(current_record_rect); + rects.push_back(current_record_rect); } + DlRegion region; + region.addRects(std::move(rects)); auto non_overlapping_rects = region.getRects(deband); std::list final_results; diff --git a/flow/rtree.cc b/flow/rtree.cc index f0db9aaf84eec..063cea40d041e 100644 --- a/flow/rtree.cc +++ b/flow/rtree.cc @@ -42,7 +42,7 @@ std::list RTree::searchNonOverlappingDrawnRects( std::vector intermediary_results; search(query, &intermediary_results); - DlRegion region; + std::vector rects; for (int index : intermediary_results) { auto draw_op = draw_op_.find(index); // Ignore records that don't draw anything. @@ -51,9 +51,11 @@ std::list RTree::searchNonOverlappingDrawnRects( } SkIRect current_record_rect; draw_op->second.roundOut(¤t_record_rect); - region.addRect(current_record_rect); + rects.push_back(current_record_rect); } + DlRegion region; + region.addRects(std::move(rects)); auto non_overlapping_rects = region.getRects(true); std::list final_results; for (const auto& rect : non_overlapping_rects) { From 324e040590b9b5bd77553666b9d2dfa7638ec4fa Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sat, 3 Jun 2023 10:45:50 +0200 Subject: [PATCH 15/21] Restore missing constructor and destructor --- display_list/geometry/dl_region.cc | 16 ++++++++++++++++ display_list/geometry/dl_region.h | 3 +++ 2 files changed, 19 insertions(+) diff --git a/display_list/geometry/dl_region.cc b/display_list/geometry/dl_region.cc index 6f4ab24e54904..3cc4bd43b05cb 100644 --- a/display_list/geometry/dl_region.cc +++ b/display_list/geometry/dl_region.cc @@ -8,6 +8,22 @@ namespace flutter { +DlRegion::DlRegion() { + // If SpanLines can not be memmoved `addRect` would be signifantly slower + // due to cost of inserting and removing elements from the `lines_` vector. + static_assert(std::is_trivially_constructible::value, + "SpanLine must be trivially constructible."); +} + +DlRegion::~DlRegion() { + for (auto& spanvec : spanvec_pool_) { + delete spanvec; + } + for (auto& line : lines_) { + delete line.spans; + } +} + std::vector DlRegion::getRects(bool deband) const { std::vector rects; size_t previous_span_end = 0; diff --git a/display_list/geometry/dl_region.h b/display_list/geometry/dl_region.h index e10efed5199c6..d727e4c8aa420 100644 --- a/display_list/geometry/dl_region.h +++ b/display_list/geometry/dl_region.h @@ -16,6 +16,9 @@ namespace flutter { /// converting set of overlapping rectangles to non-overlapping rectangles. class DlRegion { public: + DlRegion(); + ~DlRegion(); + /// Bulks adds rectangles to current region. /// Matches SkRegion::op(rect, SkRegion::kUnion_Op) behavior. void addRects(std::vector&& rects); From 46da2d151a8a1fc6eb48f819412d166a00e5553c Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sat, 3 Jun 2023 10:52:20 +0200 Subject: [PATCH 16/21] Change DlRegion API usage to closer resemble DlRTree --- .../benchmarking/dl_region_benchmarks.cc | 3 +-- display_list/geometry/dl_region.cc | 3 ++- display_list/geometry/dl_region.h | 10 +++---- display_list/geometry/dl_region_unittests.cc | 26 +++++++------------ display_list/geometry/dl_rtree.cc | 3 +-- flow/rtree.cc | 3 +-- 6 files changed, 19 insertions(+), 29 deletions(-) diff --git a/display_list/benchmarking/dl_region_benchmarks.cc b/display_list/benchmarking/dl_region_benchmarks.cc index bc5ef50c9d367..a07faeba88e8e 100644 --- a/display_list/benchmarking/dl_region_benchmarks.cc +++ b/display_list/benchmarking/dl_region_benchmarks.cc @@ -32,8 +32,7 @@ class DlRegionAdapter { void addRect(const SkIRect& rect) { rects_.push_back(rect); } std::vector getRects() { - flutter::DlRegion region; - region.addRects(std::move(rects_)); + flutter::DlRegion region(std::move(rects_)); return region.getRects(false); } diff --git a/display_list/geometry/dl_region.cc b/display_list/geometry/dl_region.cc index 3cc4bd43b05cb..33d24e1cc7e28 100644 --- a/display_list/geometry/dl_region.cc +++ b/display_list/geometry/dl_region.cc @@ -8,11 +8,12 @@ namespace flutter { -DlRegion::DlRegion() { +DlRegion::DlRegion(std::vector&& rects) { // If SpanLines can not be memmoved `addRect` would be signifantly slower // due to cost of inserting and removing elements from the `lines_` vector. static_assert(std::is_trivially_constructible::value, "SpanLine must be trivially constructible."); + addRects(std::move(rects)); } DlRegion::~DlRegion() { diff --git a/display_list/geometry/dl_region.h b/display_list/geometry/dl_region.h index d727e4c8aa420..159b32c37d7dd 100644 --- a/display_list/geometry/dl_region.h +++ b/display_list/geometry/dl_region.h @@ -16,13 +16,11 @@ namespace flutter { /// converting set of overlapping rectangles to non-overlapping rectangles. class DlRegion { public: - DlRegion(); + /// Creates region by bulk adding the rectangles./// Matches + /// SkRegion::op(rect, SkRegion::kUnion_Op) behavior. + explicit DlRegion(std::vector&& rects); ~DlRegion(); - /// Bulks adds rectangles to current region. - /// Matches SkRegion::op(rect, SkRegion::kUnion_Op) behavior. - void addRects(std::vector&& rects); - /// Returns list of non-overlapping rectangles that cover current region. /// If |deband| is false, each span line will result in separate rectangles, /// closely matching SkRegion::Iterator behavior. @@ -31,6 +29,8 @@ class DlRegion { std::vector getRects(bool deband = true) const; private: + void addRects(std::vector&& rects); + struct Span { int32_t left; int32_t right; diff --git a/display_list/geometry/dl_region_unittests.cc b/display_list/geometry/dl_region_unittests.cc index 611c97a50bb2f..05ea6f8046a3f 100644 --- a/display_list/geometry/dl_region_unittests.cc +++ b/display_list/geometry/dl_region_unittests.cc @@ -13,13 +13,12 @@ namespace flutter { namespace testing { TEST(DisplayListRegion, EmptyRegion) { - DlRegion region; + DlRegion region({}); EXPECT_TRUE(region.getRects().empty()); } TEST(DisplayListRegion, SingleRectangle) { - DlRegion region; - region.addRects({SkIRect::MakeLTRB(10, 10, 50, 50)}); + DlRegion region({SkIRect::MakeLTRB(10, 10, 50, 50)}); auto rects = region.getRects(); ASSERT_EQ(rects.size(), 1u); EXPECT_EQ(rects.front(), SkIRect::MakeLTRB(10, 10, 50, 50)); @@ -31,8 +30,7 @@ TEST(DisplayListRegion, NonOverlappingRectangles1) { SkIRect rect = SkIRect::MakeXYWH(50 * i, 50 * i, 50, 50); rects_in.push_back(rect); } - DlRegion region; - region.addRects(std::move(rects_in)); + DlRegion region(std::move(rects_in)); auto rects = region.getRects(); std::vector expected{ {0, 0, 50, 50}, {50, 50, 100, 100}, {100, 100, 150, 150}, @@ -44,8 +42,7 @@ TEST(DisplayListRegion, NonOverlappingRectangles1) { } TEST(DisplayListRegion, NonOverlappingRectangles2) { - DlRegion region; - region.addRects({ + DlRegion region({ SkIRect::MakeXYWH(5, 5, 10, 10), SkIRect::MakeXYWH(25, 5, 10, 10), SkIRect::MakeXYWH(5, 25, 10, 10), @@ -62,8 +59,7 @@ TEST(DisplayListRegion, NonOverlappingRectangles2) { } TEST(DisplayListRegion, NonOverlappingRectangles3) { - DlRegion region; - region.addRects({ + DlRegion region({ SkIRect::MakeXYWH(0, 0, 10, 10), SkIRect::MakeXYWH(-11, -11, 10, 10), SkIRect::MakeXYWH(11, 11, 10, 10), @@ -90,8 +86,7 @@ TEST(DisplayListRegion, NonOverlappingRectangles3) { } TEST(DisplayListRegion, MergeTouchingRectangles) { - DlRegion region; - region.addRects({ + DlRegion region({ SkIRect::MakeXYWH(0, 0, 10, 10), SkIRect::MakeXYWH(-10, -10, 10, 10), SkIRect::MakeXYWH(10, 10, 10, 10), @@ -116,8 +111,7 @@ TEST(DisplayListRegion, OverlappingRectangles) { SkIRect rect = SkIRect::MakeXYWH(10 * i, 10 * i, 50, 50); rects_in.push_back(rect); } - DlRegion region; - region.addRects(std::move(rects_in)); + DlRegion region(std::move(rects_in)); auto rects = region.getRects(); std::vector expected{ {0, 0, 50, 10}, {0, 10, 60, 20}, {0, 20, 70, 30}, @@ -131,8 +125,7 @@ TEST(DisplayListRegion, OverlappingRectangles) { } TEST(DisplayListRegion, Deband) { - DlRegion region; - region.addRects({ + DlRegion region({ SkIRect::MakeXYWH(0, 0, 50, 50), SkIRect::MakeXYWH(60, 0, 20, 20), SkIRect::MakeXYWH(90, 0, 50, 50), @@ -194,8 +187,7 @@ TEST(DisplayListRegion, TestAgainstSkRegion) { sk_region.op(rect, SkRegion::kUnion_Op); } - DlRegion region; - region.addRects(std::move(rects_in)); + DlRegion region(std::move(rects_in)); // Do not deband the rectangles - identical to SkRegion::Iterator auto rects = region.getRects(false); diff --git a/display_list/geometry/dl_rtree.cc b/display_list/geometry/dl_rtree.cc index 302fa7578f4dd..548ff505071a4 100644 --- a/display_list/geometry/dl_rtree.cc +++ b/display_list/geometry/dl_rtree.cc @@ -173,8 +173,7 @@ std::list DlRTree::searchAndConsolidateRects(const SkRect& query, bounds(index).roundOut(¤t_record_rect); rects.push_back(current_record_rect); } - DlRegion region; - region.addRects(std::move(rects)); + DlRegion region(std::move(rects)); auto non_overlapping_rects = region.getRects(deband); std::list final_results; diff --git a/flow/rtree.cc b/flow/rtree.cc index 063cea40d041e..5e2630f867960 100644 --- a/flow/rtree.cc +++ b/flow/rtree.cc @@ -54,8 +54,7 @@ std::list RTree::searchNonOverlappingDrawnRects( rects.push_back(current_record_rect); } - DlRegion region; - region.addRects(std::move(rects)); + DlRegion region(std::move(rects)); auto non_overlapping_rects = region.getRects(true); std::list final_results; for (const auto& rect : non_overlapping_rects) { From e2dd0215428d387448363b3e04bcef178559dc5a Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sat, 3 Jun 2023 11:05:37 +0200 Subject: [PATCH 17/21] Restore random seed --- display_list/geometry/dl_region_unittests.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/display_list/geometry/dl_region_unittests.cc b/display_list/geometry/dl_region_unittests.cc index 05ea6f8046a3f..f9610ff3c5561 100644 --- a/display_list/geometry/dl_region_unittests.cc +++ b/display_list/geometry/dl_region_unittests.cc @@ -169,8 +169,7 @@ TEST(DisplayListRegion, TestAgainstSkRegion) { for (const auto& settings : all_settings) { std::random_device d; - // std::seed_seq seed{::testing::UnitTest::GetInstance()->random_seed()}; - std::seed_seq seed{4}; + std::seed_seq seed{::testing::UnitTest::GetInstance()->random_seed()}; std::mt19937 rng(seed); SkRegion sk_region; From 199f43b6495a73e5cef5aeb2f4ff1ad718cb0828 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sat, 3 Jun 2023 15:39:52 +0200 Subject: [PATCH 18/21] Free the spanvec pool as soon as we're done with it. --- display_list/geometry/dl_region.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/display_list/geometry/dl_region.cc b/display_list/geometry/dl_region.cc index 33d24e1cc7e28..d2354197d7bea 100644 --- a/display_list/geometry/dl_region.cc +++ b/display_list/geometry/dl_region.cc @@ -14,12 +14,14 @@ DlRegion::DlRegion(std::vector&& rects) { static_assert(std::is_trivially_constructible::value, "SpanLine must be trivially constructible."); addRects(std::move(rects)); -} -DlRegion::~DlRegion() { for (auto& spanvec : spanvec_pool_) { delete spanvec; } + spanvec_pool_.clear(); +} + +DlRegion::~DlRegion() { for (auto& line : lines_) { delete line.spans; } From 7a852a44f8ac234ec84b81e3423a4277059d93f9 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sat, 3 Jun 2023 15:45:07 +0200 Subject: [PATCH 19/21] Simplify dirty_start/dirty_end checks --- display_list/geometry/dl_region.cc | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/display_list/geometry/dl_region.cc b/display_list/geometry/dl_region.cc index d2354197d7bea..a08bcb3f25f6d 100644 --- a/display_list/geometry/dl_region.cc +++ b/display_list/geometry/dl_region.cc @@ -148,19 +148,14 @@ void DlRegion::addRects(std::vector&& rects) { size_t start_index = 0; - size_t dirty_start = -1; - size_t dirty_end = 1; + size_t dirty_start = std::numeric_limits::max(); + size_t dirty_end = 0; // Marks line as dirty. Dirty lines will be checked for equality // later and merged as needed. auto mark_dirty = [&](size_t line) { - if (dirty_start == static_cast(-1)) { - dirty_start = line; - dirty_end = line; - } else { - dirty_start = std::min(dirty_start, line); - dirty_end = std::max(dirty_end, line); - } + dirty_start = std::min(dirty_start, line); + dirty_end = std::max(dirty_end, line); }; for (const SkIRect& rect : rects) { @@ -223,7 +218,7 @@ void DlRegion::addRects(std::vector&& rects) { } // Check for duplicate lines and merge them. - if (dirty_start != static_cast(-1)) { + if (dirty_start <= dirty_end) { // Expand the region by one if possible. if (dirty_start > 0) { --dirty_start; @@ -244,8 +239,8 @@ void DlRegion::addRects(std::vector&& rects) { } } } - dirty_start = -1; - dirty_end = -1; + dirty_start = std::numeric_limits::max(); + dirty_end = 0; } } From 9dba789c8963d485fab110c31205327ae0c4c910 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sat, 3 Jun 2023 16:01:47 +0200 Subject: [PATCH 20/21] Replace condition with DCHECK, replace asserts with DCHECK --- display_list/geometry/dl_region.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/display_list/geometry/dl_region.cc b/display_list/geometry/dl_region.cc index a08bcb3f25f6d..e1e00f2edb6b7 100644 --- a/display_list/geometry/dl_region.cc +++ b/display_list/geometry/dl_region.cc @@ -4,7 +4,7 @@ #include "flutter/display_list/geometry/dl_region.h" -#include +#include "flutter/fml/logging.h" namespace flutter { @@ -43,9 +43,9 @@ std::vector DlRegion::getRects(bool deband) const { if (iter->bottom() < rect.top()) { // Went all the way to previous span line. break; - } else if (iter->bottom() == rect.top() && - iter->left() == rect.left() && + } else if (iter->left() == rect.left() && iter->right() == rect.right()) { + FML_DCHECK(iter->bottom() == rect.top()); rect.fTop = iter->fTop; rects.erase(iter); break; @@ -89,7 +89,7 @@ void DlRegion::SpanLine::insertSpan(int32_t left, int32_t right) { bool DlRegion::SpanLine::spansEqual(const SpanLine& l2) const { SpanVec& spans = *this->spans; SpanVec& otherSpans = *l2.spans; - assert(this != &l2); + FML_DCHECK(this != &l2); if (spans.size() != otherSpans.size()) { return false; @@ -195,7 +195,7 @@ void DlRegion::addRects(std::vector&& rects) { insertLine(i + 1, makeLine(y1, prevLineEnd, *line.spans)); continue; } - assert(y1 == line.top); + FML_DCHECK(y1 == line.top); if (y2 < line.bottom) { // duplicate line auto newLine = makeLine(y2, line.bottom, *line.spans); @@ -206,7 +206,7 @@ void DlRegion::addRects(std::vector&& rects) { mark_dirty(i); break; } - assert(y2 >= line.bottom); + FML_DCHECK(y2 >= line.bottom); line.insertSpan(rect.fLeft, rect.fRight); mark_dirty(i); y1 = line.bottom; From 794ce5c57e44c9abd4e0f90dd8db9e6533820952 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sat, 3 Jun 2023 16:11:18 +0200 Subject: [PATCH 21/21] Adjust previous_span_end when erasing rectangle from previous span --- display_list/geometry/dl_region.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/display_list/geometry/dl_region.cc b/display_list/geometry/dl_region.cc index e1e00f2edb6b7..281fd142b393f 100644 --- a/display_list/geometry/dl_region.cc +++ b/display_list/geometry/dl_region.cc @@ -48,6 +48,7 @@ std::vector DlRegion::getRects(bool deband) const { FML_DCHECK(iter->bottom() == rect.top()); rect.fTop = iter->fTop; rects.erase(iter); + --previous_span_end; break; } }