-
Notifications
You must be signed in to change notification settings - Fork 6k
share platform view slicing logic across iOS and Android. #54010
Changes from all commits
d2eed60
df98325
87e37ea
0a681cc
8c73382
08f691b
4a47fae
e211be2
b0bfd8b
c16de49
3dcefa2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,116 @@ | ||
// 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/flow/view_slicer.h" | ||
|
||
#include <unordered_map> | ||
#include "flow/embedded_views.h" | ||
#include "fml/logging.h" | ||
|
||
namespace flutter { | ||
|
||
std::unordered_map<int64_t, SkRect> SliceViews( | ||
DlCanvas* background_canvas, | ||
const std::vector<int64_t>& composition_order, | ||
const std::unordered_map<int64_t, std::unique_ptr<EmbedderViewSlice>>& | ||
slices, | ||
const std::unordered_map<int64_t, SkRect>& view_rects) { | ||
std::unordered_map<int64_t, SkRect> overlay_layers; | ||
|
||
auto current_frame_view_count = composition_order.size(); | ||
|
||
// Restore the clip context after exiting this method since it's changed | ||
// below. | ||
DlAutoCanvasRestore save(background_canvas, /*do_save=*/true); | ||
|
||
for (size_t i = 0; i < current_frame_view_count; i++) { | ||
int64_t view_id = composition_order[i]; | ||
EmbedderViewSlice* slice = slices.at(view_id).get(); | ||
if (slice->canvas() == nullptr) { | ||
continue; | ||
} | ||
|
||
slice->end_recording(); | ||
|
||
SkRect full_joined_rect = SkRect::MakeEmpty(); | ||
|
||
// Determinate if Flutter UI intersects with any of the previous | ||
// platform views stacked by z position. | ||
// | ||
// This is done by querying the r-tree that holds the records for the | ||
// picture recorder corresponding to the flow layers added after a platform | ||
// view layer. | ||
for (int j = i; j >= 0; j--) { | ||
int64_t current_view_id = composition_order[j]; | ||
auto maybe_rect = view_rects.find(current_view_id); | ||
FML_DCHECK(maybe_rect != view_rects.end()); | ||
if (maybe_rect == view_rects.end()) { | ||
continue; | ||
} | ||
|
||
SkRect current_view_rect = maybe_rect->second; | ||
const SkIRect rounded_in_platform_view_rect = current_view_rect.roundIn(); | ||
|
||
// Each rect corresponds to a native view that renders Flutter UI. | ||
std::vector<SkIRect> intersection_rects = | ||
slice->region(current_view_rect).getRects(); | ||
|
||
// Ignore intersections of single width/height on the edge of the platform | ||
// view. | ||
// This is to address the following performance issue when interleaving | ||
// adjacent platform views and layers: Since we `roundOut` both platform | ||
// view rects and the layer rects, as long as the coordinate is | ||
// fractional, there will be an intersection of a single pixel width (or | ||
// height) after rounding out, even if they do not intersect before | ||
// rounding out. We have to round out both platform view rect and the | ||
// layer rect. Rounding in platform view rect will result in missing pixel | ||
// on the intersection edge. Rounding in layer rect will result in missing | ||
// pixel on the edge of the layer on top of the platform view. | ||
for (auto it = intersection_rects.begin(); it != intersection_rects.end(); | ||
/*no-op*/) { | ||
// If intersection_rect does not intersect with the *rounded in* | ||
// platform view rect, then the intersection must be a single pixel | ||
// width (or height) on edge. | ||
if (!SkIRect::Intersects(*it, rounded_in_platform_view_rect)) { | ||
it = intersection_rects.erase(it); | ||
} else { | ||
++it; | ||
} | ||
} | ||
|
||
// Limit the number of native views, so it doesn't grow forever. | ||
// | ||
// In this case, the rects are merged into a single one that is the union | ||
// of all the rects. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. iOS used to have 2 overlays per platform view as the limit. Do you have any context why we picked 2? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as far as I can tell it was aribtrarily chosen There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wondering if there are any benefit of having multiple overlays despite of the performance overhead. Otherwise it's odd that we picked 2 before. @cbracken do you recall? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. its possible that at one point the overlay surfaces weren't screen sized, so shrinking the overlays as small as possible improved performance. overlay sizes aren;t stable however, which is what lead to using the frame size in all cases - which defeats the purpose of minimizing overlay size. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Talked to @jonahwilliams about this this morning but no, I don't recall this splitting logic at all and it's surprising to me that we're doing it -- adding layers should harm performance if anything. Could be something like what Jonah is suggesting. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Do you mean this? flutter/flutter#143420 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gotcha. This PR should move the metrics on both iOS (by reducing the limit from 2 to 1) and Android (by fixing the 1 px overlay). Do you think I should make our ads benchmark work on Android (should be a quick change? CC @jmagman correct me if it's not), so that we can measure both? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Almost all Android PVs will use TLHC by default, which does not use platform view slicing. There is work this year to make android PVs work more like iOS by default, but without any of the terrible downsides of the current approach. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This means we don't need the Android benchmark to run for this PR (since it's not the implementation used in Android yet), but we may need it later down the road, right? (flutter/flutter#152200) |
||
SkRect partial_joined_rect = SkRect::MakeEmpty(); | ||
for (const SkIRect& rect : intersection_rects) { | ||
partial_joined_rect.join(SkRect::Make(rect)); | ||
} | ||
|
||
// Get the intersection rect with the `current_view_rect`, | ||
partial_joined_rect.intersect(SkRect::Make(current_view_rect.roundOut())); | ||
|
||
// Join the `partial_joined_rect` into `full_joined_rect` to get the rect | ||
// above the current `slice` | ||
full_joined_rect.join(partial_joined_rect); | ||
} | ||
|
||
if (!full_joined_rect.isEmpty()) { | ||
overlay_layers.insert({view_id, full_joined_rect}); | ||
|
||
// Clip the background canvas, so it doesn't contain any of the pixels | ||
// drawn on the overlay layer. | ||
background_canvas->ClipRect(full_joined_rect, | ||
DlCanvas::ClipOp::kDifference); | ||
} | ||
slice->render_into(background_canvas); | ||
} | ||
|
||
// Manually trigger the DlAutoCanvasRestore before we submit the frame | ||
save.Restore(); | ||
|
||
return overlay_layers; | ||
} | ||
|
||
} // namespace flutter |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
// 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_FLOW_VIEW_SLICER_H_ | ||
#define FLUTTER_FLOW_VIEW_SLICER_H_ | ||
|
||
#include <unordered_map> | ||
#include "display_list/dl_canvas.h" | ||
#include "flow/embedded_views.h" | ||
|
||
namespace flutter { | ||
|
||
/// @brief Compute the required overlay layers and clip the view slices | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this documentation be elaborated? What are the integer handles? How are they created. What is the rect that is returned? The embedder view slice is also not documented so there are no breadcrumbs there either. Can these go in a struct with a custom hasher and equality checker? If this is just moving code around and you want to limit scope, then we could probably do it in a followup. Just that from the perspective of someone not super familiar with slicing, this reads a bit dense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sgtm |
||
/// according to the size and position of the platform views. | ||
std::unordered_map<int64_t, SkRect> SliceViews( | ||
DlCanvas* background_canvas, | ||
const std::vector<int64_t>& composition_order, | ||
const std::unordered_map<int64_t, std::unique_ptr<EmbedderViewSlice>>& | ||
slices, | ||
const std::unordered_map<int64_t, SkRect>& view_rects); | ||
|
||
} // namespace flutter | ||
|
||
#endif // FLUTTER_FLOW_VIEW_SLICER_H_ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 <unordered_map> | ||
#include "display_list/dl_builder.h" | ||
#include "flow/embedded_views.h" | ||
#include "flutter/flow/view_slicer.h" | ||
#include "gtest/gtest.h" | ||
|
||
namespace flutter { | ||
namespace testing { | ||
|
||
namespace { | ||
void AddSliceOfSize( | ||
std::unordered_map<int64_t, std::unique_ptr<EmbedderViewSlice>>& slices, | ||
int64_t id, | ||
SkRect rect) { | ||
slices[id] = std::make_unique<DisplayListEmbedderViewSlice>(rect); | ||
DlPaint paint; | ||
paint.setColor(DlColor::kBlack()); | ||
slices[id]->canvas()->DrawRect(rect, paint); | ||
} | ||
} // namespace | ||
|
||
TEST(ViewSlicerTest, CanSlicerNonOverlappingViews) { | ||
DisplayListBuilder builder(SkRect::MakeLTRB(0, 0, 100, 100)); | ||
|
||
std::vector<int64_t> composition_order = {1}; | ||
std::unordered_map<int64_t, std::unique_ptr<EmbedderViewSlice>> slices; | ||
AddSliceOfSize(slices, 1, SkRect::MakeLTRB(99, 99, 100, 100)); | ||
|
||
std::unordered_map<int64_t, SkRect> view_rects = { | ||
{1, SkRect::MakeLTRB(50, 50, 60, 60)}}; | ||
|
||
auto computed_overlays = | ||
SliceViews(&builder, composition_order, slices, view_rects); | ||
|
||
EXPECT_TRUE(computed_overlays.empty()); | ||
} | ||
|
||
TEST(ViewSlicerTest, IgnoresFractionalOverlaps) { | ||
DisplayListBuilder builder(SkRect::MakeLTRB(0, 0, 100, 100)); | ||
|
||
std::vector<int64_t> composition_order = {1}; | ||
std::unordered_map<int64_t, std::unique_ptr<EmbedderViewSlice>> slices; | ||
AddSliceOfSize(slices, 1, SkRect::MakeLTRB(0, 0, 50.49, 50.49)); | ||
|
||
std::unordered_map<int64_t, SkRect> view_rects = { | ||
{1, SkRect::MakeLTRB(50.5, 50.5, 100, 100)}}; | ||
|
||
auto computed_overlays = | ||
SliceViews(&builder, composition_order, slices, view_rects); | ||
|
||
EXPECT_TRUE(computed_overlays.empty()); | ||
} | ||
|
||
TEST(ViewSlicerTest, ComputesOverlapWith1PV) { | ||
DisplayListBuilder builder(SkRect::MakeLTRB(0, 0, 100, 100)); | ||
|
||
std::vector<int64_t> composition_order = {1}; | ||
std::unordered_map<int64_t, std::unique_ptr<EmbedderViewSlice>> slices; | ||
AddSliceOfSize(slices, 1, SkRect::MakeLTRB(0, 0, 50, 50)); | ||
|
||
std::unordered_map<int64_t, SkRect> view_rects = { | ||
{1, SkRect::MakeLTRB(0, 0, 100, 100)}}; | ||
|
||
auto computed_overlays = | ||
SliceViews(&builder, composition_order, slices, view_rects); | ||
|
||
EXPECT_EQ(computed_overlays.size(), 1u); | ||
auto overlay = computed_overlays.find(1); | ||
ASSERT_NE(overlay, computed_overlays.end()); | ||
|
||
EXPECT_EQ(overlay->second, SkRect::MakeLTRB(0, 0, 50, 50)); | ||
} | ||
|
||
TEST(ViewSlicerTest, ComputesOverlapWith2PV) { | ||
DisplayListBuilder builder(SkRect::MakeLTRB(0, 0, 100, 100)); | ||
|
||
std::vector<int64_t> composition_order = {1, 2}; | ||
std::unordered_map<int64_t, std::unique_ptr<EmbedderViewSlice>> slices; | ||
AddSliceOfSize(slices, 1, SkRect::MakeLTRB(0, 0, 50, 50)); | ||
AddSliceOfSize(slices, 2, SkRect::MakeLTRB(50, 50, 100, 100)); | ||
|
||
std::unordered_map<int64_t, SkRect> view_rects = { | ||
{1, SkRect::MakeLTRB(0, 0, 50, 50)}, // | ||
{2, SkRect::MakeLTRB(50, 50, 100, 100)}, // | ||
}; | ||
|
||
auto computed_overlays = | ||
SliceViews(&builder, composition_order, slices, view_rects); | ||
|
||
EXPECT_EQ(computed_overlays.size(), 2u); | ||
|
||
auto overlay = computed_overlays.find(1); | ||
ASSERT_NE(overlay, computed_overlays.end()); | ||
|
||
EXPECT_EQ(overlay->second, SkRect::MakeLTRB(0, 0, 50, 50)); | ||
|
||
overlay = computed_overlays.find(2); | ||
ASSERT_NE(overlay, computed_overlays.end()); | ||
EXPECT_EQ(overlay->second, SkRect::MakeLTRB(50, 50, 100, 100)); | ||
} | ||
|
||
TEST(ViewSlicerTest, OverlappingTwoPVs) { | ||
DisplayListBuilder builder(SkRect::MakeLTRB(0, 0, 100, 100)); | ||
|
||
std::vector<int64_t> composition_order = {1, 2}; | ||
std::unordered_map<int64_t, std::unique_ptr<EmbedderViewSlice>> slices; | ||
// This embeded view overlaps both platform views: | ||
// | ||
// [ A [ ]] | ||
// [_____[ C ]] | ||
// [ B [ ]] | ||
// [ ] | ||
AddSliceOfSize(slices, 1, SkRect::MakeLTRB(0, 0, 0, 0)); | ||
AddSliceOfSize(slices, 2, SkRect::MakeLTRB(0, 0, 100, 100)); | ||
|
||
std::unordered_map<int64_t, SkRect> view_rects = { | ||
{1, SkRect::MakeLTRB(0, 0, 50, 50)}, // | ||
{2, SkRect::MakeLTRB(50, 50, 100, 100)}, // | ||
}; | ||
|
||
auto computed_overlays = | ||
SliceViews(&builder, composition_order, slices, view_rects); | ||
|
||
EXPECT_EQ(computed_overlays.size(), 1u); | ||
|
||
auto overlay = computed_overlays.find(2); | ||
ASSERT_NE(overlay, computed_overlays.end()); | ||
|
||
// We create a single overlay for both overlapping sections. | ||
EXPECT_EQ(overlay->second, SkRect::MakeLTRB(0, 0, 100, 100)); | ||
} | ||
|
||
} // namespace testing | ||
} // namespace flutter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just so that i understand - this count is the count of platform views right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct