0

[cc-slimming] Adds DontAlwaysPushPictureLayerImpls feature

The idea is as follows:
 1) PictureLayerImpls on the Pending tree that pushed any properties
    from Main will also be marked to push on Activation.
 2) PictureLayerImpls on the Pending tree that create new Tiles will
    also be pushed, so they can transfer their Tiles. Marking is a 1
    way street, so once a Tile has been created on a Tiling, we will
    push props even if that tile happens to be removed later.

Since PictureLayerImpls have pushed props by default for, over 10 years,
this change could reveal other bugs that were masked. All tests passed
for now.

Bug: 335450599, 40335690
Change-Id: I9dff5030426536ec3f5bc3a1a59a631244885bfb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5529643
Commit-Queue: Victor Miura <vmiura@chromium.org>
Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1299464}
This commit is contained in:
Victor Miura
2024-05-10 20:04:23 +00:00
committed by Chromium LUCI CQ
parent c36e2c6030
commit e2d5340d7d
14 changed files with 71 additions and 8 deletions

@ -207,4 +207,8 @@ BASE_FEATURE(kNonBatchedCopySharedImage,
"NonBatchedCopySharedImage",
base::FEATURE_ENABLED_BY_DEFAULT);
BASE_FEATURE(kDontAlwaysPushPictureLayerImpls,
"DontAlwaysPushPictureLayerImpls",
base::FEATURE_DISABLED_BY_DEFAULT);
} // namespace features

@ -196,6 +196,10 @@ CC_BASE_EXPORT BASE_DECLARE_FEATURE(kWaitForLateScrollEvents);
CC_BASE_EXPORT extern const base::FeatureParam<double>
kWaitForLateScrollEventsDeadlineRatio;
// When enabled we stop always pushing PictureLayerImpl properties on
// tree Activation. See crbug.com/40335690.
CC_BASE_EXPORT BASE_DECLARE_FEATURE(kDontAlwaysPushPictureLayerImpls);
} // namespace features
#endif // CC_BASE_FEATURES_H_

@ -133,6 +133,8 @@ class FixedInvalidationPictureLayerTilingClient
return base_client_->CurrentScrollCheckerboardsDueToNoRecording();
}
void OnTilesAdded() override { return base_client_->OnTilesAdded(); }
private:
raw_ptr<PictureLayerTilingClient> base_client_;
Region invalidation_;

@ -99,7 +99,9 @@ gfx::Rect SafeIntersectRects(const gfx::Rect& one, const gfx::Rect& two) {
} // namespace
PictureLayerImpl::PictureLayerImpl(LayerTreeImpl* tree_impl, int id)
: LayerImpl(tree_impl, id, /*will_always_push_properties=*/true) {
: LayerImpl(tree_impl,
id,
tree_impl->always_push_properties_on_picture_layers()) {
layer_tree_impl()->RegisterPictureLayerImpl(this);
}
@ -1010,6 +1012,10 @@ bool PictureLayerImpl::CurrentScrollCheckerboardsDueToNoRecording() const {
return layer_tree_impl()->CurrentScrollCheckerboardsDueToNoRecording();
}
void PictureLayerImpl::OnTilesAdded() {
SetNeedsPushProperties();
}
gfx::Rect PictureLayerImpl::GetEnclosingVisibleRectInTargetSpace() const {
return GetScaledEnclosingVisibleRectInTargetSpace(
MaximumTilingContentsScale());

@ -83,6 +83,7 @@ class CC_EXPORT PictureLayerImpl
const PaintWorkletRecordMap& GetPaintWorkletRecords() const override;
bool ScrollInteractionInProgress() const override;
bool CurrentScrollCheckerboardsDueToNoRecording() const override;
void OnTilesAdded() override;
// ImageAnimationController::AnimationDriver overrides.
bool ShouldAnimate(PaintImage::Id paint_image_id) const override;

@ -92,4 +92,6 @@ bool FakePictureLayerTilingClient::CurrentScrollCheckerboardsDueToNoRecording()
return false;
}
void FakePictureLayerTilingClient::OnTilesAdded() {}
} // namespace cc

@ -45,6 +45,7 @@ class FakePictureLayerTilingClient : public PictureLayerTilingClient {
const PaintWorkletRecordMap& GetPaintWorkletRecords() const override;
bool ScrollInteractionInProgress() const override;
bool CurrentScrollCheckerboardsDueToNoRecording() const override;
void OnTilesAdded() override;
void set_twin_tiling_set(PictureLayerTilingSet* set) {
twin_set_ = set;

@ -149,6 +149,7 @@ void TestLayerTreeHostBase::SetupPendingTree(
pending_layer_->SetBounds(raster_source->size());
pending_layer_->SetRasterSource(raster_source, invalidation);
}
pending_layer_->SetNeedsPushProperties();
host_impl()->pending_tree()->set_needs_update_draw_properties();
UpdateDrawProperties(host_impl()->pending_tree());

@ -87,6 +87,8 @@ Tile* PictureLayerTiling::CreateTile(const Tile::CreateInfo& info) {
}
all_tiles_done_ = false;
client_->OnTilesAdded();
std::unique_ptr<Tile> tile = client_->CreateTile(info);
Tile* tile_ptr = tile.get();
tiles_[key] = std::move(tile);

@ -55,6 +55,7 @@ class CC_EXPORT PictureLayerTilingClient {
virtual const PaintWorkletRecordMap& GetPaintWorkletRecords() const = 0;
virtual bool ScrollInteractionInProgress() const = 0;
virtual bool CurrentScrollCheckerboardsDueToNoRecording() const = 0;
virtual void OnTilesAdded() = 0;
protected:
virtual ~PictureLayerTilingClient() {}

@ -162,6 +162,8 @@ LayerTreeImpl::LayerTreeImpl(
external_page_scale_factor_(1.f),
device_scale_factor_(1.f),
painted_device_scale_factor_(1.f),
always_push_properties_on_picture_layers_(!base::FeatureList::IsEnabled(
features::kDontAlwaysPushPictureLayerImpls)),
elastic_overscroll_(elastic_overscroll),
event_listener_properties_(),
top_controls_shown_ratio_(std::move(top_controls_shown_ratio)),
@ -1792,9 +1794,10 @@ void LayerTreeImpl::ClearSurfaceRanges() {
void LayerTreeImpl::AddLayerShouldPushProperties(LayerImpl* layer) {
DCHECK(!IsActiveTree()) << "The active tree does not push layer properties";
// TODO(crbug.com/40335690): PictureLayerImpls always push properties so
// should not go into this set or we'd push them twice.
DCHECK(!base::Contains(picture_layers_, layer));
// PictureLayerImpls should only go into this when
// always_push_properties_on_picture_layers() is disabled.
DCHECK(!always_push_properties_on_picture_layers() ||
!base::Contains(picture_layers_, layer));
layers_that_should_push_properties_.insert(layer);
}

@ -517,6 +517,10 @@ class CC_EXPORT LayerTreeImpl {
needs_surface_ranges_sync_ = needs_surface_ranges_sync;
}
bool always_push_properties_on_picture_layers() const {
return always_push_properties_on_picture_layers_;
}
void ForceRedrawNextActivation() { next_activation_forces_redraw_ = true; }
void set_ui_resource_request_queue(UIResourceRequestQueue queue);
@ -887,6 +891,8 @@ class CC_EXPORT LayerTreeImpl {
// frame.
bool force_send_metadata_request_ : 1 = false;
bool always_push_properties_on_picture_layers_ : 1 = false;
gfx::Rect device_viewport_rect_;
scoped_refptr<SyncedElasticOverscroll> elastic_overscroll_;

@ -195,13 +195,20 @@ static void PushLayerPropertiesInternal(Iterator source_layers_begin,
void TreeSynchronizer::PushLayerProperties(LayerTreeImpl* pending_tree,
LayerTreeImpl* active_tree) {
const auto& layers = pending_tree->LayersThatShouldPushProperties();
// TODO(crbug.com/40335690): Stop always pushing PictureLayerImpl properties.
const auto& picture_layers = pending_tree->picture_layers();
const size_t push_count =
layers.size() + (pending_tree->always_push_properties_on_picture_layers()
? picture_layers.size()
: 0);
TRACE_EVENT1("cc", "TreeSynchronizer::PushLayerPropertiesTo.Impl",
"layer_count", layers.size() + picture_layers.size());
"layer_count", push_count);
PushLayerPropertiesInternal(layers.begin(), layers.end(), active_tree);
PushLayerPropertiesInternal(picture_layers.begin(), picture_layers.end(),
active_tree);
if (pending_tree->always_push_properties_on_picture_layers()) {
// TODO(crbug.com/40335690): Stop always pushing PictureLayerImpl
// properties.
PushLayerPropertiesInternal(picture_layers.begin(), picture_layers.end(),
active_tree);
}
pending_tree->ClearLayersThatShouldPushProperties();
}

@ -6736,6 +6736,29 @@
]
}
],
"DontAlwaysPushPictureLayerImpls": [
{
"platforms": [
"android",
"android_webview",
"chromeos",
"chromeos_lacros",
"fuchsia",
"ios",
"linux",
"mac",
"windows"
],
"experiments": [
{
"name": "Enabled",
"enable_features": [
"DontAlwaysPushPictureLayerImpls"
]
}
]
}
],
"DownloadLater": [
{
"platforms": [