From 67d9077e20d3af27812c2ca99cb806b0137ff427 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Sun, 4 Jun 2023 17:58:15 +0200 Subject: [PATCH 1/8] Raster cache should preserve RTree for overlay layers --- flow/layers/display_list_raster_cache_item.cc | 3 +- flow/layers/layer.h | 5 ++ flow/layers/layer_raster_cache_item.cc | 5 +- flow/layers/platform_view_layer.cc | 1 + flow/raster_cache.cc | 51 +++++++++++++++---- flow/raster_cache.h | 13 +++-- flow/raster_cache_unittests.cc | 50 ++++++++++++++++++ flow/testing/mock_raster_cache.h | 4 +- 8 files changed, 114 insertions(+), 18 deletions(-) diff --git a/flow/layers/display_list_raster_cache_item.cc b/flow/layers/display_list_raster_cache_item.cc index 089fb6c0c338b..56bfb4b6ad5a6 100644 --- a/flow/layers/display_list_raster_cache_item.cc +++ b/flow/layers/display_list_raster_cache_item.cc @@ -133,7 +133,8 @@ bool DisplayListRasterCacheItem::Draw(const PaintContext& context, return false; } if (cache_state_ == CacheState::kCurrent) { - return context.raster_cache->Draw(key_id_, *canvas, paint); + return context.raster_cache->Draw(key_id_, *canvas, paint, + context.is_root_canvas); } return false; } diff --git a/flow/layers/layer.h b/flow/layers/layer.h index f1f1056a5ea9f..0a7f41637e5b9 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -106,6 +106,11 @@ struct PaintContext { LayerStateStack& state_stack; DlCanvas* canvas; + // Whether current canvas is root canvas. Used to determine if the raster + // cache is painting to bottom-most surface, in which case it will not attempt + // to preserve the RTree. + bool is_root_canvas = true; + GrDirectContext* gr_context; SkColorSpace* dst_color_space; ExternalViewEmbedder* view_embedder; diff --git a/flow/layers/layer_raster_cache_item.cc b/flow/layers/layer_raster_cache_item.cc index b6597414958b3..8bce37e900d13 100644 --- a/flow/layers/layer_raster_cache_item.cc +++ b/flow/layers/layer_raster_cache_item.cc @@ -182,14 +182,15 @@ bool LayerRasterCacheItem::Draw(const PaintContext& context, case RasterCacheItem::kNone: return false; case RasterCacheItem::kCurrent: { - return context.raster_cache->Draw(key_id_, *canvas, paint); + return context.raster_cache->Draw(key_id_, *canvas, paint, + context.is_root_canvas); } case RasterCacheItem::kChildren: { if (!layer_children_id_.has_value()) { return false; } return context.raster_cache->Draw(layer_children_id_.value(), *canvas, - paint); + paint, context.is_root_canvas); } } } diff --git a/flow/layers/platform_view_layer.cc b/flow/layers/platform_view_layer.cc index 487bbc1e710e2..284a9fc04708f 100644 --- a/flow/layers/platform_view_layer.cc +++ b/flow/layers/platform_view_layer.cc @@ -44,6 +44,7 @@ void PlatformViewLayer::Paint(PaintContext& context) const { DlCanvas* canvas = context.view_embedder->CompositeEmbeddedView(view_id_); context.canvas = canvas; context.state_stack.set_delegate(canvas); + context.is_root_canvas = false; } } // namespace flutter diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 40c87e7331ab7..128240eb3a66e 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -8,6 +8,7 @@ #include #include "flutter/common/constants.h" +#include "flutter/display_list/skia/dl_sk_dispatcher.h" #include "flutter/flow/layers/container_layer.h" #include "flutter/flow/layers/layer.h" #include "flutter/flow/paint_utils.h" @@ -26,10 +27,16 @@ namespace flutter { RasterCacheResult::RasterCacheResult(sk_sp image, const SkRect& logical_rect, - const char* type) - : image_(std::move(image)), logical_rect_(logical_rect), flow_(type) {} - -void RasterCacheResult::draw(DlCanvas& canvas, const DlPaint* paint) const { + const char* type, + sk_sp rtree) + : image_(std::move(image)), + logical_rect_(logical_rect), + flow_(type), + rtree_(std::move(rtree)) {} + +void RasterCacheResult::draw(DlCanvas& canvas, + const DlPaint* paint, + bool is_root_canvas) const { DlAutoCanvasRestore auto_restore(&canvas, true); auto matrix = RasterCacheUtil::GetIntegralTransCTM(canvas.GetTransform()); @@ -39,8 +46,21 @@ void RasterCacheResult::draw(DlCanvas& canvas, const DlPaint* paint) const { std::abs(bounds.height() - image_->dimensions().height()) <= 1); canvas.TransformReset(); flow_.Step(); - canvas.DrawImage(image_, {bounds.fLeft, bounds.fTop}, - DlImageSampling::kNearestNeighbor, paint); + if (is_root_canvas || !rtree_) { + canvas.DrawImage(image_, {bounds.fLeft, bounds.fTop}, + DlImageSampling::kNearestNeighbor, paint); + } else { + // On some platforms RTree from overlay layers is used for unobstructed + // platform views and hit testing. To preserve the RTree raster cache must + // paint individual rects instead of the whole image. + auto rects = rtree_->searchAndConsolidateRects(kGiantRect); + for (auto rect : rects) { + SkRect dst(rect); + dst.offset(bounds.fLeft, bounds.fTop); + canvas.DrawImageRect(image_, rect, dst, DlImageSampling::kNearestNeighbor, + paint); + } + } } RasterCache::RasterCache(size_t access_threshold, @@ -73,8 +93,12 @@ std::unique_ptr RasterCache::Rasterize( return nullptr; } - DlSkCanvasAdapter canvas(surface->getCanvas()); - canvas.Clear(DlColor::kTransparent()); + // Clear the surface canvas directly to avoid RTree storing a giant clear + // rect. + auto surfaceCanvas = surface->getCanvas(); + surfaceCanvas->clear(SK_ColorTRANSPARENT); + + DisplayListBuilder canvas(true); canvas.Translate(-dest_rect.left(), -dest_rect.top()); canvas.Transform(matrix); draw_function(&canvas); @@ -83,9 +107,13 @@ std::unique_ptr RasterCache::Rasterize( draw_checkerboard(&canvas, context.logical_rect); } + auto display_list = canvas.Build(); + DlSkCanvasDispatcher dispatcher(surfaceCanvas); + display_list->Dispatch(dispatcher); + auto rtree = display_list->rtree(); auto image = DlImage::Make(surface->makeImageSnapshot()); return std::make_unique(image, context.logical_rect, - context.flow_type); + context.flow_type, rtree); } bool RasterCache::UpdateCacheEntry( @@ -146,7 +174,8 @@ bool RasterCache::HasEntry(const RasterCacheKeyID& id, bool RasterCache::Draw(const RasterCacheKeyID& id, DlCanvas& canvas, - const DlPaint* paint) const { + const DlPaint* paint, + bool is_root_canvas) const { auto it = cache_.find(RasterCacheKey(id, canvas.GetTransform())); if (it == cache_.end()) { return false; @@ -155,7 +184,7 @@ bool RasterCache::Draw(const RasterCacheKeyID& id, Entry& entry = it->second; if (entry.image) { - entry.image->draw(canvas, paint); + entry.image->draw(canvas, paint, is_root_canvas); return true; } diff --git a/flow/raster_cache.h b/flow/raster_cache.h index ea2f5b1829e66..9cbfcf89530c5 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -30,11 +30,14 @@ class RasterCacheResult { public: RasterCacheResult(sk_sp image, const SkRect& logical_rect, - const char* type); + const char* type, + sk_sp rtree = nullptr); virtual ~RasterCacheResult() = default; - virtual void draw(DlCanvas& canvas, const DlPaint* paint) const; + virtual void draw(DlCanvas& canvas, + const DlPaint* paint, + bool is_root_canvas) const; virtual SkISize image_dimensions() const { return image_ ? image_->dimensions() : SkISize::Make(0, 0); @@ -48,6 +51,7 @@ class RasterCacheResult { sk_sp image_; SkRect logical_rect_; fml::tracing::TraceFlow flow_; + sk_sp rtree_; }; class Layer; @@ -143,9 +147,12 @@ class RasterCache { // if the item was disabled due to conditions discovered during |Preroll| // or if the attempt to populate the entry failed due to bounds overflow // conditions. + // If |is_root_canvas| is false, the raster cache will preserve the original + // RTree of cached content. bool Draw(const RasterCacheKeyID& id, DlCanvas& canvas, - const DlPaint* paint) const; + const DlPaint* paint, + bool is_root_canvas = true) const; bool HasEntry(const RasterCacheKeyID& id, const SkMatrix&) const; diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index 43c314c7fff33..6329e1d19d931 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -886,6 +886,56 @@ TEST_F(RasterCacheTest, RasterCacheKeyIDLayerChildrenIds) { ASSERT_EQ(ids, expected_ids); } +TEST(RasterCache, PreservesRTree) { + flutter::RasterCache cache(1); + LayerStateStack preroll_state_stack; + SkMatrix matrix = SkMatrix::I(); + preroll_state_stack.set_preroll_delegate(kGiantRect, matrix); + + FixedRefreshRateStopwatch raster_time; + FixedRefreshRateStopwatch ui_time; + PaintContextHolder paint_context_holder = GetSamplePaintContextHolder( + preroll_state_stack, &cache, &raster_time, &ui_time); + auto& paint_context = paint_context_holder.paint_context; + RasterCache::Context r_context = { + // clang-format off + .gr_context = paint_context.gr_context, + .dst_color_space = paint_context.dst_color_space, + .matrix = matrix, + .logical_rect = SkRect::MakeWH(100, 100), + .flow_type = "RasterCacheFlow::DisplayList", + // clang-format on + }; + + RasterCacheKeyID id(10, RasterCacheKeyType::kLayer); + cache.UpdateCacheEntry(id, r_context, [&](DlCanvas* canvas) { + canvas->DrawRect(SkRect::MakeXYWH(0, 0, 100, 10), DlPaint()); + canvas->DrawRect(SkRect::MakeXYWH(0, 40, 100, 10), DlPaint()); + }); + + { + DisplayListBuilder canvas(true); + cache.Draw(id, canvas, nullptr, true); + auto display_list = canvas.Build(); + auto rtree = display_list->rtree(); + auto rects = rtree->searchAndConsolidateRects(kGiantRect); + // For root canvas the display may clober the RTree. + ASSERT_EQ(rects.size(), 1u); + ASSERT_EQ(rects.front(), SkRect::MakeWH(100, 100)); + } + { + DisplayListBuilder canvas(true); + cache.Draw(id, canvas, nullptr, false); + auto display_list = canvas.Build(); + auto rtree = display_list->rtree(); + auto rects = rtree->searchAndConsolidateRects(kGiantRect); + // For non-root canvas the RTree must be preserved + ASSERT_EQ(rects.size(), 2u); + ASSERT_EQ(rects.front(), SkRect::MakeXYWH(0, 0, 100, 10)); + ASSERT_EQ(*std::next(rects.begin(), 1), SkRect::MakeXYWH(0, 40, 100, 10)); + } +} + } // namespace testing } // namespace flutter diff --git a/flow/testing/mock_raster_cache.h b/flow/testing/mock_raster_cache.h index 4f625ad6df938..71ceefe23c4a1 100644 --- a/flow/testing/mock_raster_cache.h +++ b/flow/testing/mock_raster_cache.h @@ -29,7 +29,9 @@ class MockRasterCacheResult : public RasterCacheResult { public: explicit MockRasterCacheResult(SkRect device_rect); - void draw(DlCanvas& canvas, const DlPaint* paint = nullptr) const override{}; + void draw(DlCanvas& canvas, + const DlPaint* paint = nullptr, + bool is_root_canvas = true) const override{}; SkISize image_dimensions() const override { return SkSize::Make(device_rect_.width(), device_rect_.height()).toCeil(); From 78218b455a443c826439c0084ae6241e8295a9c9 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Jun 2023 17:57:08 +0200 Subject: [PATCH 2/8] Reuse existing RTree from DisplayListLayer --- flow/layers/display_list_layer_unittests.cc | 50 +++++++++++++++++ flow/layers/display_list_raster_cache_item.cc | 6 ++- flow/raster_cache.cc | 34 ++++++------ flow/raster_cache.h | 9 ++-- flow/raster_cache_unittests.cc | 54 +------------------ 5 files changed, 80 insertions(+), 73 deletions(-) diff --git a/flow/layers/display_list_layer_unittests.cc b/flow/layers/display_list_layer_unittests.cc index dee93d2a673e0..935ce4e7f4cb5 100644 --- a/flow/layers/display_list_layer_unittests.cc +++ b/flow/layers/display_list_layer_unittests.cc @@ -294,6 +294,56 @@ TEST_F(DisplayListLayerTest, CachedIncompatibleDisplayListOpacityInheritance) { EXPECT_TRUE(DisplayListsEQ_Verbose(expected.Build(), this->display_list())); } +TEST_F(DisplayListLayerTest, RasterCachePreservesRTree) { + const SkRect picture1_bounds = SkRect::MakeXYWH(10, 10, 10, 10); + const SkRect picture2_bounds = SkRect::MakeXYWH(15, 15, 10, 10); + DisplayListBuilder builder(true); + builder.DrawRect(picture1_bounds, DlPaint()); + builder.DrawRect(picture2_bounds, DlPaint()); + auto display_list = builder.Build(); + auto display_list_layer = std::make_shared( + SkPoint::Make(3, 3), display_list, true, false); + + use_skia_raster_cache(); + + auto context = preroll_context(); + display_list_layer->Preroll(preroll_context()); + EXPECT_EQ(context->renderable_state_flags, 0); + + // Pump the DisplayListLayer until it is ready to cache its DL + display_list_layer->Preroll(preroll_context()); + display_list_layer->Preroll(preroll_context()); + display_list_layer->Preroll(preroll_context()); + LayerTree::TryToRasterCache(*preroll_context()->raster_cached_entries, + &paint_context(), false); + + DisplayListBuilder expected_root_canvas(true); + context->raster_cache->Draw(display_list_layer->caching_key_id(), + expected_root_canvas, nullptr, true); + auto root_canvas_dl = expected_root_canvas.Build(); + const auto root_canvas_rects = + root_canvas_dl->rtree()->searchAndConsolidateRects(kGiantRect, true); + std::list root_canvas_rects_expected = { + SkRect::MakeLTRB(13, 13, 28, 28), + }; + EXPECT_EQ(root_canvas_rects_expected, root_canvas_rects); + + DisplayListBuilder expected_overlay_canvas(true); + context->raster_cache->Draw(display_list_layer->caching_key_id(), + expected_overlay_canvas, nullptr, false); + auto overlay_canvas_dl = expected_overlay_canvas.Build(); + const auto overlay_canvas_rects = + overlay_canvas_dl->rtree()->searchAndConsolidateRects(kGiantRect, true); + + // Same bounds as root canvas, but preserves individual rects. + std::list overlay_canvas_rects_expected = { + SkRect::MakeLTRB(13, 13, 23, 18), + SkRect::MakeLTRB(13, 18, 28, 23), + SkRect::MakeLTRB(18, 23, 28, 28), + }; + EXPECT_EQ(overlay_canvas_rects_expected, overlay_canvas_rects); +}; + using DisplayListLayerDiffTest = DiffContextTest; TEST_F(DisplayListLayerDiffTest, SimpleDisplayList) { diff --git a/flow/layers/display_list_raster_cache_item.cc b/flow/layers/display_list_raster_cache_item.cc index 56bfb4b6ad5a6..5baaa9c7dfabc 100644 --- a/flow/layers/display_list_raster_cache_item.cc +++ b/flow/layers/display_list_raster_cache_item.cc @@ -167,8 +167,10 @@ bool DisplayListRasterCacheItem::TryToPrepareRasterCache( // clang-format on }; return context.raster_cache->UpdateCacheEntry( - id.value(), r_context, [display_list = display_list_](DlCanvas* canvas) { + id.value(), r_context, + [display_list = display_list_](DlCanvas* canvas) { canvas->DrawDisplayList(display_list); - }); + }, + display_list_->rtree()); } } // namespace flutter diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 128240eb3a66e..b67c97e8caa52 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -54,10 +54,18 @@ void RasterCacheResult::draw(DlCanvas& canvas, // platform views and hit testing. To preserve the RTree raster cache must // paint individual rects instead of the whole image. auto rects = rtree_->searchAndConsolidateRects(kGiantRect); + + // Bounds of original display list. + SkRect rtree_bounds = SkRect::MakeEmpty(); + for (auto rect : rects) { + rtree_bounds.join(rect); + } for (auto rect : rects) { - SkRect dst(rect); + SkRect src(rect); + src.offset(-rtree_bounds.fLeft, -rtree_bounds.fTop); + SkRect dst(src); dst.offset(bounds.fLeft, bounds.fTop); - canvas.DrawImageRect(image_, rect, dst, DlImageSampling::kNearestNeighbor, + canvas.DrawImageRect(image_, src, dst, DlImageSampling::kNearestNeighbor, paint); } } @@ -72,6 +80,7 @@ RasterCache::RasterCache(size_t access_threshold, /// @note Procedure doesn't copy all closures. std::unique_ptr RasterCache::Rasterize( const RasterCache::Context& context, + sk_sp rtree, const std::function& draw_function, const std::function& draw_checkerboard) const { @@ -93,12 +102,9 @@ std::unique_ptr RasterCache::Rasterize( return nullptr; } - // Clear the surface canvas directly to avoid RTree storing a giant clear - // rect. - auto surfaceCanvas = surface->getCanvas(); - surfaceCanvas->clear(SK_ColorTRANSPARENT); + DlSkCanvasAdapter canvas(surface->getCanvas()); + canvas.Clear(DlColor::kTransparent()); - DisplayListBuilder canvas(true); canvas.Translate(-dest_rect.left(), -dest_rect.top()); canvas.Transform(matrix); draw_function(&canvas); @@ -107,24 +113,22 @@ std::unique_ptr RasterCache::Rasterize( draw_checkerboard(&canvas, context.logical_rect); } - auto display_list = canvas.Build(); - DlSkCanvasDispatcher dispatcher(surfaceCanvas); - display_list->Dispatch(dispatcher); - auto rtree = display_list->rtree(); auto image = DlImage::Make(surface->makeImageSnapshot()); - return std::make_unique(image, context.logical_rect, - context.flow_type, rtree); + return std::make_unique( + image, context.logical_rect, context.flow_type, std::move(rtree)); } bool RasterCache::UpdateCacheEntry( const RasterCacheKeyID& id, const Context& raster_cache_context, - const std::function& render_function) const { + const std::function& render_function, + sk_sp rtree) const { RasterCacheKey key = RasterCacheKey(id, raster_cache_context.matrix); Entry& entry = cache_[key]; if (!entry.image) { void (*func)(DlCanvas*, const SkRect& rect) = DrawCheckerboard; - entry.image = Rasterize(raster_cache_context, render_function, func); + entry.image = Rasterize(raster_cache_context, std::move(rtree), + render_function, func); if (entry.image != nullptr) { switch (id.type()) { case RasterCacheKeyType::kDisplayList: { diff --git a/flow/raster_cache.h b/flow/raster_cache.h index 9cbfcf89530c5..71401112d14ad 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -131,6 +131,7 @@ class RasterCache { std::unique_ptr Rasterize( const RasterCache::Context& context, + sk_sp rtree, const std::function& draw_function, const std::function& draw_checkerboard) const; @@ -241,10 +242,10 @@ class RasterCache { */ int GetAccessCount(const RasterCacheKeyID& id, const SkMatrix& matrix) const; - bool UpdateCacheEntry( - const RasterCacheKeyID& id, - const Context& raster_cache_context, - const std::function& render_function) const; + bool UpdateCacheEntry(const RasterCacheKeyID& id, + const Context& raster_cache_context, + const std::function& render_function, + sk_sp rtree = nullptr) const; private: struct Entry { diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index 6329e1d19d931..daa0146d14d95 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -178,11 +178,11 @@ TEST(RasterCache, SetCheckboardCacheImages) { }; cache.SetCheckboardCacheImages(false); - cache.Rasterize(r_context, dummy_draw_function, draw_checkerboard); + cache.Rasterize(r_context, nullptr, dummy_draw_function, draw_checkerboard); ASSERT_FALSE(did_draw_checkerboard); cache.SetCheckboardCacheImages(true); - cache.Rasterize(r_context, dummy_draw_function, draw_checkerboard); + cache.Rasterize(r_context, nullptr, dummy_draw_function, draw_checkerboard); ASSERT_TRUE(did_draw_checkerboard); } @@ -886,56 +886,6 @@ TEST_F(RasterCacheTest, RasterCacheKeyIDLayerChildrenIds) { ASSERT_EQ(ids, expected_ids); } -TEST(RasterCache, PreservesRTree) { - flutter::RasterCache cache(1); - LayerStateStack preroll_state_stack; - SkMatrix matrix = SkMatrix::I(); - preroll_state_stack.set_preroll_delegate(kGiantRect, matrix); - - FixedRefreshRateStopwatch raster_time; - FixedRefreshRateStopwatch ui_time; - PaintContextHolder paint_context_holder = GetSamplePaintContextHolder( - preroll_state_stack, &cache, &raster_time, &ui_time); - auto& paint_context = paint_context_holder.paint_context; - RasterCache::Context r_context = { - // clang-format off - .gr_context = paint_context.gr_context, - .dst_color_space = paint_context.dst_color_space, - .matrix = matrix, - .logical_rect = SkRect::MakeWH(100, 100), - .flow_type = "RasterCacheFlow::DisplayList", - // clang-format on - }; - - RasterCacheKeyID id(10, RasterCacheKeyType::kLayer); - cache.UpdateCacheEntry(id, r_context, [&](DlCanvas* canvas) { - canvas->DrawRect(SkRect::MakeXYWH(0, 0, 100, 10), DlPaint()); - canvas->DrawRect(SkRect::MakeXYWH(0, 40, 100, 10), DlPaint()); - }); - - { - DisplayListBuilder canvas(true); - cache.Draw(id, canvas, nullptr, true); - auto display_list = canvas.Build(); - auto rtree = display_list->rtree(); - auto rects = rtree->searchAndConsolidateRects(kGiantRect); - // For root canvas the display may clober the RTree. - ASSERT_EQ(rects.size(), 1u); - ASSERT_EQ(rects.front(), SkRect::MakeWH(100, 100)); - } - { - DisplayListBuilder canvas(true); - cache.Draw(id, canvas, nullptr, false); - auto display_list = canvas.Build(); - auto rtree = display_list->rtree(); - auto rects = rtree->searchAndConsolidateRects(kGiantRect); - // For non-root canvas the RTree must be preserved - ASSERT_EQ(rects.size(), 2u); - ASSERT_EQ(rects.front(), SkRect::MakeXYWH(0, 0, 100, 10)); - ASSERT_EQ(*std::next(rects.begin(), 1), SkRect::MakeXYWH(0, 40, 100, 10)); - } -} - } // namespace testing } // namespace flutter From 3814a2120257850706c8425244be33bba58bf592 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Jun 2023 22:09:51 +0200 Subject: [PATCH 3/8] Add DlRTree::bounds() --- display_list/geometry/dl_rtree.cc | 8 ++++++++ display_list/geometry/dl_rtree.h | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/display_list/geometry/dl_rtree.cc b/display_list/geometry/dl_rtree.cc index 548ff505071a4..d5fb8ad64d1ff 100644 --- a/display_list/geometry/dl_rtree.cc +++ b/display_list/geometry/dl_rtree.cc @@ -201,4 +201,12 @@ void DlRTree::search(const Node& parent, } } +const SkRect& DlRTree::bounds() const { + if (!nodes_.empty()) { + return nodes_.back().bounds; + } else { + return empty_; + } +} + } // namespace flutter diff --git a/display_list/geometry/dl_rtree.h b/display_list/geometry/dl_rtree.h index 0c2f25abc766a..0de686abc52d5 100644 --- a/display_list/geometry/dl_rtree.h +++ b/display_list/geometry/dl_rtree.h @@ -85,6 +85,10 @@ class DlRTree : public SkRefCnt { : invalid_id_; } + /// Returns maximum and minimum axis values of rectangles in this R-Tree. + /// If R-Tree is empty returns an empty SkRect. + const SkRect& bounds() const; + /// Return the rectangle bounds for the indicated result of a query /// or an empty rect if the index is not a valid leaf node index. const SkRect& bounds(int result_index) const { From def7e399c35558755b2871aeb4a7ec06a391320a Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Jun 2023 22:10:24 +0200 Subject: [PATCH 4/8] Use DlRTree::bounds() and translate the canvas instead of individual rects --- flow/raster_cache.cc | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index b67c97e8caa52..99b341fc9f6d9 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -55,18 +55,13 @@ void RasterCacheResult::draw(DlCanvas& canvas, // paint individual rects instead of the whole image. auto rects = rtree_->searchAndConsolidateRects(kGiantRect); - // Bounds of original display list. - SkRect rtree_bounds = SkRect::MakeEmpty(); - for (auto rect : rects) { - rtree_bounds.join(rect); - } + canvas.Translate(bounds.fLeft, bounds.fTop); + + SkRect rtree_bounds = rtree_->bounds(); for (auto rect : rects) { - SkRect src(rect); - src.offset(-rtree_bounds.fLeft, -rtree_bounds.fTop); - SkRect dst(src); - dst.offset(bounds.fLeft, bounds.fTop); - canvas.DrawImageRect(image_, src, dst, DlImageSampling::kNearestNeighbor, - paint); + rect.offset(-rtree_bounds.fLeft, -rtree_bounds.fTop); + canvas.DrawImageRect(image_, rect, rect, + DlImageSampling::kNearestNeighbor, paint); } } } From 1b9952a65c20484cfa1ff77039ff30d57e8a83cb Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Jun 2023 22:10:32 +0200 Subject: [PATCH 5/8] Missing asserts in unit test --- flow/layers/display_list_layer_unittests.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/flow/layers/display_list_layer_unittests.cc b/flow/layers/display_list_layer_unittests.cc index 935ce4e7f4cb5..f85fc7be28509 100644 --- a/flow/layers/display_list_layer_unittests.cc +++ b/flow/layers/display_list_layer_unittests.cc @@ -318,8 +318,8 @@ TEST_F(DisplayListLayerTest, RasterCachePreservesRTree) { &paint_context(), false); DisplayListBuilder expected_root_canvas(true); - context->raster_cache->Draw(display_list_layer->caching_key_id(), - expected_root_canvas, nullptr, true); + ASSERT_TRUE(context->raster_cache->Draw(display_list_layer->caching_key_id(), + expected_root_canvas, nullptr, true)); auto root_canvas_dl = expected_root_canvas.Build(); const auto root_canvas_rects = root_canvas_dl->rtree()->searchAndConsolidateRects(kGiantRect, true); @@ -329,8 +329,9 @@ TEST_F(DisplayListLayerTest, RasterCachePreservesRTree) { EXPECT_EQ(root_canvas_rects_expected, root_canvas_rects); DisplayListBuilder expected_overlay_canvas(true); - context->raster_cache->Draw(display_list_layer->caching_key_id(), - expected_overlay_canvas, nullptr, false); + ASSERT_TRUE(context->raster_cache->Draw(display_list_layer->caching_key_id(), + expected_overlay_canvas, nullptr, + false)); auto overlay_canvas_dl = expected_overlay_canvas.Build(); const auto overlay_canvas_rects = overlay_canvas_dl->rtree()->searchAndConsolidateRects(kGiantRect, true); From 1b7813ca783b7bf2ab0c6a872cc2bdeb38579746 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 13 Jun 2023 22:48:07 +0200 Subject: [PATCH 6/8] Rename argument name and paint context flag --- flow/layers/display_list_layer_unittests.cc | 5 +++-- flow/layers/display_list_raster_cache_item.cc | 2 +- flow/layers/layer.h | 8 ++++---- flow/layers/layer_raster_cache_item.cc | 5 +++-- flow/layers/platform_view_layer.cc | 2 +- flow/raster_cache.cc | 8 ++++---- flow/raster_cache.h | 11 +++++++---- flow/testing/mock_raster_cache.h | 2 +- 8 files changed, 24 insertions(+), 19 deletions(-) diff --git a/flow/layers/display_list_layer_unittests.cc b/flow/layers/display_list_layer_unittests.cc index f85fc7be28509..0780a12dda77f 100644 --- a/flow/layers/display_list_layer_unittests.cc +++ b/flow/layers/display_list_layer_unittests.cc @@ -319,7 +319,8 @@ TEST_F(DisplayListLayerTest, RasterCachePreservesRTree) { DisplayListBuilder expected_root_canvas(true); ASSERT_TRUE(context->raster_cache->Draw(display_list_layer->caching_key_id(), - expected_root_canvas, nullptr, true)); + expected_root_canvas, nullptr, + false)); auto root_canvas_dl = expected_root_canvas.Build(); const auto root_canvas_rects = root_canvas_dl->rtree()->searchAndConsolidateRects(kGiantRect, true); @@ -331,7 +332,7 @@ TEST_F(DisplayListLayerTest, RasterCachePreservesRTree) { DisplayListBuilder expected_overlay_canvas(true); ASSERT_TRUE(context->raster_cache->Draw(display_list_layer->caching_key_id(), expected_overlay_canvas, nullptr, - false)); + true)); auto overlay_canvas_dl = expected_overlay_canvas.Build(); const auto overlay_canvas_rects = overlay_canvas_dl->rtree()->searchAndConsolidateRects(kGiantRect, true); diff --git a/flow/layers/display_list_raster_cache_item.cc b/flow/layers/display_list_raster_cache_item.cc index 5baaa9c7dfabc..c0675785f724b 100644 --- a/flow/layers/display_list_raster_cache_item.cc +++ b/flow/layers/display_list_raster_cache_item.cc @@ -134,7 +134,7 @@ bool DisplayListRasterCacheItem::Draw(const PaintContext& context, } if (cache_state_ == CacheState::kCurrent) { return context.raster_cache->Draw(key_id_, *canvas, paint, - context.is_root_canvas); + context.rendering_above_platform_view); } return false; } diff --git a/flow/layers/layer.h b/flow/layers/layer.h index 0a7f41637e5b9..cbaa6d1833fe6 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -106,10 +106,10 @@ struct PaintContext { LayerStateStack& state_stack; DlCanvas* canvas; - // Whether current canvas is root canvas. Used to determine if the raster - // cache is painting to bottom-most surface, in which case it will not attempt - // to preserve the RTree. - bool is_root_canvas = true; + // Whether current canvas is an overlay canvas. Used to determine if the + // raster cache is painting to a surface that will be displayed above a + // platform view, in which case it will attempt to preserve the R-Tree. + bool rendering_above_platform_view = false; GrDirectContext* gr_context; SkColorSpace* dst_color_space; diff --git a/flow/layers/layer_raster_cache_item.cc b/flow/layers/layer_raster_cache_item.cc index 8bce37e900d13..61c0f8596e303 100644 --- a/flow/layers/layer_raster_cache_item.cc +++ b/flow/layers/layer_raster_cache_item.cc @@ -183,14 +183,15 @@ bool LayerRasterCacheItem::Draw(const PaintContext& context, return false; case RasterCacheItem::kCurrent: { return context.raster_cache->Draw(key_id_, *canvas, paint, - context.is_root_canvas); + context.rendering_above_platform_view); } case RasterCacheItem::kChildren: { if (!layer_children_id_.has_value()) { return false; } return context.raster_cache->Draw(layer_children_id_.value(), *canvas, - paint, context.is_root_canvas); + paint, + context.rendering_above_platform_view); } } } diff --git a/flow/layers/platform_view_layer.cc b/flow/layers/platform_view_layer.cc index 284a9fc04708f..0f3fc7fc3deec 100644 --- a/flow/layers/platform_view_layer.cc +++ b/flow/layers/platform_view_layer.cc @@ -44,7 +44,7 @@ void PlatformViewLayer::Paint(PaintContext& context) const { DlCanvas* canvas = context.view_embedder->CompositeEmbeddedView(view_id_); context.canvas = canvas; context.state_stack.set_delegate(canvas); - context.is_root_canvas = false; + context.rendering_above_platform_view = true; } } // namespace flutter diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 99b341fc9f6d9..fee052ee27b20 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -36,7 +36,7 @@ RasterCacheResult::RasterCacheResult(sk_sp image, void RasterCacheResult::draw(DlCanvas& canvas, const DlPaint* paint, - bool is_root_canvas) const { + bool preserve_rtree) const { DlAutoCanvasRestore auto_restore(&canvas, true); auto matrix = RasterCacheUtil::GetIntegralTransCTM(canvas.GetTransform()); @@ -46,7 +46,7 @@ void RasterCacheResult::draw(DlCanvas& canvas, std::abs(bounds.height() - image_->dimensions().height()) <= 1); canvas.TransformReset(); flow_.Step(); - if (is_root_canvas || !rtree_) { + if (!preserve_rtree || !rtree_) { canvas.DrawImage(image_, {bounds.fLeft, bounds.fTop}, DlImageSampling::kNearestNeighbor, paint); } else { @@ -174,7 +174,7 @@ bool RasterCache::HasEntry(const RasterCacheKeyID& id, bool RasterCache::Draw(const RasterCacheKeyID& id, DlCanvas& canvas, const DlPaint* paint, - bool is_root_canvas) const { + bool preserve_rtree) const { auto it = cache_.find(RasterCacheKey(id, canvas.GetTransform())); if (it == cache_.end()) { return false; @@ -183,7 +183,7 @@ bool RasterCache::Draw(const RasterCacheKeyID& id, Entry& entry = it->second; if (entry.image) { - entry.image->draw(canvas, paint, is_root_canvas); + entry.image->draw(canvas, paint, preserve_rtree); return true; } diff --git a/flow/raster_cache.h b/flow/raster_cache.h index 71401112d14ad..cbb9be4ba7b37 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -37,7 +37,7 @@ class RasterCacheResult { virtual void draw(DlCanvas& canvas, const DlPaint* paint, - bool is_root_canvas) const; + bool preserve_rtree) const; virtual SkISize image_dimensions() const { return image_ ? image_->dimensions() : SkISize::Make(0, 0); @@ -148,12 +148,15 @@ class RasterCache { // if the item was disabled due to conditions discovered during |Preroll| // or if the attempt to populate the entry failed due to bounds overflow // conditions. - // If |is_root_canvas| is false, the raster cache will preserve the original - // RTree of cached content. + // If |preserve_rtree| is true, the raster cache will preserve the original + // RTree of cached content by blitting individual rectangles from the cached + // image of the canvas according to the original layer R-Tree (if present). + // This is to ensure that the target surface R-Tree with be cloberred with + // one large blit as it can affect platform view overlays and hit testing. bool Draw(const RasterCacheKeyID& id, DlCanvas& canvas, const DlPaint* paint, - bool is_root_canvas = true) const; + bool preserve_rtree = false) const; bool HasEntry(const RasterCacheKeyID& id, const SkMatrix&) const; diff --git a/flow/testing/mock_raster_cache.h b/flow/testing/mock_raster_cache.h index 71ceefe23c4a1..e3478b38c3942 100644 --- a/flow/testing/mock_raster_cache.h +++ b/flow/testing/mock_raster_cache.h @@ -31,7 +31,7 @@ class MockRasterCacheResult : public RasterCacheResult { void draw(DlCanvas& canvas, const DlPaint* paint = nullptr, - bool is_root_canvas = true) const override{}; + bool preserve_rtree = false) const override{}; SkISize image_dimensions() const override { return SkSize::Make(device_rect_.width(), device_rect_.height()).toCeil(); From f3af18a280e6a517c7cca71f5da8493446dbf409 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Wed, 14 Jun 2023 21:58:55 +0200 Subject: [PATCH 7/8] Properly transform local R-Tree coordiates to screen space --- flow/layers/display_list_layer_unittests.cc | 32 ++++++++++++--------- flow/raster_cache.cc | 4 ++- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/flow/layers/display_list_layer_unittests.cc b/flow/layers/display_list_layer_unittests.cc index 0780a12dda77f..2791577666b8d 100644 --- a/flow/layers/display_list_layer_unittests.cc +++ b/flow/layers/display_list_layer_unittests.cc @@ -307,17 +307,22 @@ TEST_F(DisplayListLayerTest, RasterCachePreservesRTree) { use_skia_raster_cache(); auto context = preroll_context(); - display_list_layer->Preroll(preroll_context()); - EXPECT_EQ(context->renderable_state_flags, 0); - - // Pump the DisplayListLayer until it is ready to cache its DL - display_list_layer->Preroll(preroll_context()); - display_list_layer->Preroll(preroll_context()); - display_list_layer->Preroll(preroll_context()); - LayerTree::TryToRasterCache(*preroll_context()->raster_cached_entries, - &paint_context(), false); + { + auto mutator = context->state_stack.save(); + mutator.transform(SkMatrix::Scale(2.0, 2.0)); + display_list_layer->Preroll(preroll_context()); + EXPECT_EQ(context->renderable_state_flags, 0); + + // Pump the DisplayListLayer until it is ready to cache its DL + display_list_layer->Preroll(preroll_context()); + display_list_layer->Preroll(preroll_context()); + display_list_layer->Preroll(preroll_context()); + LayerTree::TryToRasterCache(*preroll_context()->raster_cached_entries, + &paint_context(), false); + } DisplayListBuilder expected_root_canvas(true); + expected_root_canvas.Scale(2.0, 2.0); ASSERT_TRUE(context->raster_cache->Draw(display_list_layer->caching_key_id(), expected_root_canvas, nullptr, false)); @@ -325,11 +330,12 @@ TEST_F(DisplayListLayerTest, RasterCachePreservesRTree) { const auto root_canvas_rects = root_canvas_dl->rtree()->searchAndConsolidateRects(kGiantRect, true); std::list root_canvas_rects_expected = { - SkRect::MakeLTRB(13, 13, 28, 28), + SkRect::MakeLTRB(26, 26, 56, 56), }; EXPECT_EQ(root_canvas_rects_expected, root_canvas_rects); DisplayListBuilder expected_overlay_canvas(true); + expected_overlay_canvas.Scale(2.0, 2.0); ASSERT_TRUE(context->raster_cache->Draw(display_list_layer->caching_key_id(), expected_overlay_canvas, nullptr, true)); @@ -339,9 +345,9 @@ TEST_F(DisplayListLayerTest, RasterCachePreservesRTree) { // Same bounds as root canvas, but preserves individual rects. std::list overlay_canvas_rects_expected = { - SkRect::MakeLTRB(13, 13, 23, 18), - SkRect::MakeLTRB(13, 18, 28, 23), - SkRect::MakeLTRB(18, 23, 28, 28), + SkRect::MakeLTRB(26, 26, 46, 36), + SkRect::MakeLTRB(26, 36, 56, 46), + SkRect::MakeLTRB(36, 46, 56, 56), }; EXPECT_EQ(overlay_canvas_rects_expected, overlay_canvas_rects); }; diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index fee052ee27b20..90f49d0371c14 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -57,8 +57,10 @@ void RasterCacheResult::draw(DlCanvas& canvas, canvas.Translate(bounds.fLeft, bounds.fTop); - SkRect rtree_bounds = rtree_->bounds(); + SkRect rtree_bounds = + RasterCacheUtil::GetRoundedOutDeviceBounds(rtree_->bounds(), matrix); for (auto rect : rects) { + rect = RasterCacheUtil::GetRoundedOutDeviceBounds(rect, matrix); rect.offset(-rtree_bounds.fLeft, -rtree_bounds.fTop); canvas.DrawImageRect(image_, rect, rect, DlImageSampling::kNearestNeighbor, paint); From dc7e36d5e3325a3a9a738f527be65b48021244c6 Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Thu, 15 Jun 2023 10:01:19 +0200 Subject: [PATCH 8/8] Fix comment --- flow/raster_cache.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flow/raster_cache.h b/flow/raster_cache.h index cbb9be4ba7b37..a683350f792dc 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -150,8 +150,8 @@ class RasterCache { // conditions. // If |preserve_rtree| is true, the raster cache will preserve the original // RTree of cached content by blitting individual rectangles from the cached - // image of the canvas according to the original layer R-Tree (if present). - // This is to ensure that the target surface R-Tree with be cloberred with + // image to the canvas according to the original layer R-Tree (if present). + // This is to ensure that the target surface R-Tree will not be clobbered with // one large blit as it can affect platform view overlays and hit testing. bool Draw(const RasterCacheKeyID& id, DlCanvas& canvas,