0

Remove LayerTreeHostImpl::CurrentScrollCheckerboardsDueToNoRecording()

Before CompositeAfterPaint, it was set to true when the current frame
shows incompletedly recorded area, and we would
1. Disable smooth scrolling;
2. Disable process_for_images_only for PrioritizedTiles.

The logic has been inactive since we launched CompositeAfterPaint 2.5
years ago, and has been explicitly disabled for
UseRecordedBoundsForTiling because the logic would do unexpected
things as checkerboarded_no_recording_content_area doesn't have its
original meaning as before CompositeAfterPaint.

I think the no-recording situation should be improved in blink by
adjusting the cull rect in the scroll direction. Prioritizing raster
in cc won't help because that would just raster incompletely recorded
tiles.

Bug: 41490692, 40680264
Change-Id: I41929f45cbcedaa4fb09ae3f9e408774b4df0264
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5565398
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1305735}
This commit is contained in:
Xianzhu Wang
2024-05-24 15:41:41 +00:00
committed by Chromium LUCI CQ
parent e11a173398
commit 355e7d8845
12 changed files with 16 additions and 150 deletions

@ -125,14 +125,6 @@ class FixedInvalidationPictureLayerTilingClient
return base_client_->GetPaintWorkletRecords();
}
bool ScrollInteractionInProgress() const override {
return base_client_->ScrollInteractionInProgress();
}
bool CurrentScrollCheckerboardsDueToNoRecording() const override {
return base_client_->CurrentScrollCheckerboardsDueToNoRecording();
}
void OnTilesAdded() override { return base_client_->OnTilesAdded(); }
ScrollOffsetMap GetRasterInducingScrollOffsets() const override {

@ -1003,15 +1003,6 @@ bool PictureLayerImpl::IsDirectlyCompositedImage() const {
return directly_composited_image_default_raster_scale_ > 0.f;
}
bool PictureLayerImpl::ScrollInteractionInProgress() const {
return layer_tree_impl()->GetActivelyScrollingType() !=
ActivelyScrollingType::kNone;
}
bool PictureLayerImpl::CurrentScrollCheckerboardsDueToNoRecording() const {
return layer_tree_impl()->CurrentScrollCheckerboardsDueToNoRecording();
}
void PictureLayerImpl::OnTilesAdded() {
SetNeedsPushProperties();
}
@ -1275,8 +1266,10 @@ bool PictureLayerImpl::CanRecreateHighResTilingForLCDTextAndRasterTransform(
if (layer_tree_impl()->IsSyncTree() && layer_tree_impl()->IsReadyToActivate())
return false;
// To reduce memory usage, don't recreate highres tiling during scroll
if (ScrollInteractionInProgress())
if (layer_tree_impl()->GetActivelyScrollingType() !=
ActivelyScrollingType::kNone) {
return false;
}
return true;
}

@ -81,8 +81,6 @@ class CC_EXPORT PictureLayerImpl
bool HasValidTilePriorities() const override;
bool RequiresHighResToDraw() const override;
const PaintWorkletRecordMap& GetPaintWorkletRecords() const override;
bool ScrollInteractionInProgress() const override;
bool CurrentScrollCheckerboardsDueToNoRecording() const override;
void OnTilesAdded() override;
ScrollOffsetMap GetRasterInducingScrollOffsets() const override;

@ -84,13 +84,6 @@ FakePictureLayerTilingClient::GetPaintWorkletRecords() const {
return paint_worklet_records_;
}
bool FakePictureLayerTilingClient::ScrollInteractionInProgress() const {
return false;
}
bool FakePictureLayerTilingClient::CurrentScrollCheckerboardsDueToNoRecording()
const {
return false;
}
ScrollOffsetMap FakePictureLayerTilingClient::GetRasterInducingScrollOffsets()
const {
return ScrollOffsetMap();

@ -43,8 +43,6 @@ class FakePictureLayerTilingClient : public PictureLayerTilingClient {
const PictureLayerTiling* tiling) const override;
bool RequiresHighResToDraw() const override;
const PaintWorkletRecordMap& GetPaintWorkletRecords() const override;
bool ScrollInteractionInProgress() const override;
bool CurrentScrollCheckerboardsDueToNoRecording() const override;
void OnTilesAdded() override;
ScrollOffsetMap GetRasterInducingScrollOffsets() const override;

@ -830,14 +830,6 @@ PrioritizedTile PictureLayerTiling::MakePrioritizedTile(
tile_priority.distance_to_visible >
0.5f * max_skewport_extent_in_screen_space_);
// If the tile is within max_skewport_extent and a scroll interaction
// experiencing checkerboarding is currently taking place, then
// continue to rasterize the tile right now rather than for images only.
if (tile_priority.distance_to_visible <
max_skewport_extent_in_screen_space_ &&
client_->ScrollInteractionInProgress() &&
client_->CurrentScrollCheckerboardsDueToNoRecording())
process_for_images_only = false;
return PrioritizedTile(tile, this, tile_priority, is_tile_occluded,
process_for_images_only,
ShouldDecodeCheckeredImagesForTile(tile));

@ -54,8 +54,6 @@ class CC_EXPORT PictureLayerTilingClient {
virtual bool HasValidTilePriorities() const = 0;
virtual bool RequiresHighResToDraw() const = 0;
virtual const PaintWorkletRecordMap& GetPaintWorkletRecords() const = 0;
virtual bool ScrollInteractionInProgress() const = 0;
virtual bool CurrentScrollCheckerboardsDueToNoRecording() const = 0;
virtual void OnTilesAdded() = 0;
virtual ScrollOffsetMap GetRasterInducingScrollOffsets() const = 0;

@ -320,7 +320,6 @@ void LayerTreeHostImpl::DidStartScroll() {
void LayerTreeHostImpl::DidEndScroll() {
scroll_affects_scroll_handler_ = false;
current_scroll_did_checkerboard_large_area_ = false;
if (!settings().single_thread_proxy_scheduler) {
client_->SetHasActiveThreadedScroll(false);
@ -1333,7 +1332,6 @@ DrawResult LayerTreeHostImpl::CalculateRenderPasses(FrameData* frame) {
int num_missing_tiles = 0;
int num_incomplete_tiles = 0;
int64_t checkerboarded_no_recording_content_area = 0;
bool have_copy_request =
active_tree()->property_trees()->effect_tree().HasCopyRequests();
@ -1399,8 +1397,6 @@ DrawResult LayerTreeHostImpl::CalculateRenderPasses(FrameData* frame) {
num_missing_tiles += append_quads_data.num_missing_tiles;
num_incomplete_tiles += append_quads_data.num_incomplete_tiles;
checkerboarded_no_recording_content_area +=
append_quads_data.checkerboarded_no_recording_content_area;
if (append_quads_data.num_missing_tiles > 0) {
have_missing_animated_tiles |=
@ -1425,19 +1421,6 @@ DrawResult LayerTreeHostImpl::CalculateRenderPasses(FrameData* frame) {
append_quads_data.has_shared_element_resources;
}
if (GetActivelyScrollingType() != ActivelyScrollingType::kNone &&
checkerboarded_no_recording_content_area > 0) {
CHECK(base::FeatureList::IsEnabled(features::kUseRecordedBoundsForTiling));
// TODO(crbug.com/41490692): With UseRecordedBoundsForTiling enabled,
// checkerboarded_no_recording_content_area means differently from its
// original meaning (which was some tiles out of blink interest rect has
// become visible) before CompositeAfterPaint. Calling
// SetCurrentScrollCheckerboardsDueToNoRecording() may cause unexpected
// consequences. Disable it for now, and will clean up after checking
// finch data of UseRecordedBoundsForTiling.
// SetCurrentScrollCheckerboardsDueToNoRecording();
}
// If CommitToActiveTree() is true, then we wait to draw until
// NotifyReadyToDraw. That means we're in as good shape as is possible now,
// so there's no reason to stop the draw now (and this is not supported by

@ -686,12 +686,6 @@ class CC_EXPORT LayerTreeHostImpl : public TileManagerClient,
ActivelyScrollingType GetActivelyScrollingType() const;
bool IsCurrentScrollMainRepainted() const;
bool ScrollAffectsScrollHandler() const;
bool CurrentScrollCheckerboardsDueToNoRecording() const {
return current_scroll_did_checkerboard_large_area_;
}
void SetCurrentScrollCheckerboardsDueToNoRecording() {
current_scroll_did_checkerboard_large_area_ = true;
}
void SetExternalPinchGestureActive(bool active);
void set_force_smooth_wheel_scrolling_for_testing(bool enabled) {
GetInputHandler().set_force_smooth_wheel_scrolling_for_testing(enabled);
@ -1285,11 +1279,6 @@ class CC_EXPORT LayerTreeHostImpl : public TileManagerClient,
// sophisticated since so it's not clear how much value it's still providing.
bool scroll_affects_scroll_handler_ = false;
// Whether at least 30% of the viewport at the time of draw was
// checkerboarded during a scroll. This bit can get set during a scroll and
// is sticky for the duration of the scroll.
bool current_scroll_did_checkerboard_large_area_ = false;
// Provides support for PaintWorklets which depend on input properties that
// are being animated by the compositor (aka 'animated' PaintWorklets).
// Responsible for storing animated custom property values and for

@ -3730,74 +3730,6 @@ class MissingTilesLayer : public LayerImpl {
}
};
TEST_F(LayerTreeHostImplTest, CurrentScrollCheckerboardsDueToNoRecording) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(
features::kUseRecordedBoundsForTiling);
LayerTreeSettings settings = DefaultSettings();
CreateHostImpl(settings, CreateLayerTreeFrameSink());
host_impl_->active_tree()->PushPageScaleFromMainThread(1, 0.25f, 4);
const gfx::Size content_size(1000, 1000);
const gfx::Size viewport_size(500, 500);
SetupViewportLayersOuterScrolls(viewport_size, content_size);
LayerImpl* outer_scroll_layer = OuterViewportScrollLayer();
outer_scroll_layer->SetDrawsContent(true);
LayerImpl* inner_scroll_layer = InnerViewportScrollLayer();
inner_scroll_layer->SetDrawsContent(true);
// Add layer that draws content and has checkerboarded areas.
auto* scroll_layer = AddLayer<MissingTilesLayer>(host_impl_->active_tree());
CopyProperties(inner_scroll_layer, scroll_layer);
scroll_layer->SetBounds(gfx::Size(500, 500));
scroll_layer->SetDrawsContent(true);
scroll_layer->SetHitTestOpaqueness(HitTestOpaqueness::kTransparent);
host_impl_->active_tree()->SetElementIdsForTesting();
UpdateDrawProperties(host_impl_->active_tree());
DrawFrame();
// No scroll has taken place so this should be false.
EXPECT_FALSE(host_impl_->CurrentScrollCheckerboardsDueToNoRecording());
// Send scroll begin.
GetInputHandler().ScrollBegin(
BeginState(gfx::Point(250, 250), gfx::Vector2dF(),
ui::ScrollInputType::kTouchscreen)
.get(),
ui::ScrollInputType::kTouchscreen);
DrawFrame();
// Even though a ScrollBegin has been processed, we still don't consider the
// interaction to be "actively scrolling". Expect this to be false.
EXPECT_FALSE(host_impl_->CurrentScrollCheckerboardsDueToNoRecording());
gfx::Vector2dF scroll_delta(0, 10);
// Send scroll update.
GetInputHandler().ScrollUpdate(
UpdateState(gfx::Point(10, 10), scroll_delta, ui::ScrollInputType::kWheel)
.get());
host_impl_->SetFullViewportDamage();
DrawFrame();
// Now that a scroll update has been processed and the latest
// CalculateRenderPasses run has computed significant visible checkerboarding,
// expect this flag to be true.
// TODO(crbug.com/41490692): For now this is disabled.
EXPECT_FALSE(host_impl_->CurrentScrollCheckerboardsDueToNoRecording());
GetInputHandler().ScrollEnd();
// Expect state to be reset after a scroll end.
EXPECT_FALSE(host_impl_->CurrentScrollCheckerboardsDueToNoRecording());
}
TEST_F(LayerTreeHostImplTest, ImplPinchZoom) {
SetupViewportLayersInnerScrolls(gfx::Size(50, 50), gfx::Size(100, 100));
DrawFrame();
@ -18930,10 +18862,10 @@ TEST_F(LayerTreeHostImplTest, RecomputeRasterCapsOnLayerTreeFrameSinkUpdate) {
EXPECT_FALSE(host_impl_->can_use_msaa());
}
// Check if picturelayer's ScrollInteractionInProgress() return true even when
// BrowserControl is consuming ScrollUpdate.
// Check if LayerTreeImpl::GetActivelyScrollingType() returns kPrecise even
// when BrowserControl is consuming ScrollUpdate.
TEST_F(LayerTreeHostImplBrowserControlsTest,
BrowserControlsScrollInteractionInProgress) {
BrowserControlsActivelyScrollingType) {
gfx::Size inner_size = gfx::Size(100, 100);
gfx::Size outer_size = gfx::Size(100, 100);
gfx::Size content_size = gfx::Size(100, 200);
@ -18964,7 +18896,8 @@ TEST_F(LayerTreeHostImplBrowserControlsTest,
.thread);
// shownratio == 1
EXPECT_EQ(1, host_impl_->active_tree()->CurrentTopControlsShownRatio());
EXPECT_EQ(picture_layer->ScrollInteractionInProgress(), false);
EXPECT_EQ(host_impl_->active_tree()->GetActivelyScrollingType(),
ActivelyScrollingType::kNone);
// 0 < shownratio <1
GetInputHandler().ScrollUpdate(UpdateState(gfx::Point(),
@ -18973,7 +18906,8 @@ TEST_F(LayerTreeHostImplBrowserControlsTest,
.get());
EXPECT_GT(host_impl_->active_tree()->CurrentTopControlsShownRatio(), 0);
EXPECT_LT(host_impl_->active_tree()->CurrentTopControlsShownRatio(), 1);
EXPECT_EQ(picture_layer->ScrollInteractionInProgress(), true);
EXPECT_EQ(host_impl_->active_tree()->GetActivelyScrollingType(),
ActivelyScrollingType::kPrecise);
GetInputHandler().ScrollUpdate(UpdateState(gfx::Point(),
gfx::Vector2dF(0, 30),
@ -18981,12 +18915,14 @@ TEST_F(LayerTreeHostImplBrowserControlsTest,
.get());
// now shownratio == 0
EXPECT_EQ(0, host_impl_->active_tree()->CurrentTopControlsShownRatio());
EXPECT_EQ(picture_layer->ScrollInteractionInProgress(), true);
EXPECT_EQ(host_impl_->active_tree()->GetActivelyScrollingType(),
ActivelyScrollingType::kPrecise);
GetInputHandler().ScrollEnd();
// scroll end, shownratio == 0
EXPECT_EQ(0, host_impl_->active_tree()->CurrentTopControlsShownRatio());
EXPECT_EQ(picture_layer->ScrollInteractionInProgress(), false);
EXPECT_EQ(host_impl_->active_tree()->GetActivelyScrollingType(),
ActivelyScrollingType::kNone);
}
TEST_F(LayerTreeHostImplTest, AnimatedScrollSnapStrategyCurrentOffset) {

@ -768,10 +768,6 @@ class CC_EXPORT LayerTreeImpl {
return host_impl_->GetActivelyScrollingType();
}
bool CurrentScrollCheckerboardsDueToNoRecording() {
return host_impl_->CurrentScrollCheckerboardsDueToNoRecording();
}
// These functions are used for plumbing DelegatedInkMetadata from blink
// through the compositor and into viz via a compositor frame. They should
// only be called after the JS API |updateInkTrailStartPoint| has been

@ -503,10 +503,8 @@ void ProxyImpl::RenewTreePriority() {
bool precise_scrolling_in_progress =
host_impl_->GetActivelyScrollingType() == ActivelyScrollingType::kPrecise;
bool avoid_entering_smoothness =
host_impl_->CurrentScrollCheckerboardsDueToNoRecording() ||
(precise_scrolling_in_progress &&
host_impl_->IsCurrentScrollMainRepainted());
bool avoid_entering_smoothness = precise_scrolling_in_progress &&
host_impl_->IsCurrentScrollMainRepainted();
bool non_scroll_interaction_in_progress =
host_impl_->IsPinchGestureActive() ||