Commit 94f516ad authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

Skip DisplayItemList conversion when possible

This makes pre-CompositeAfterPaint work like CompositeAfterPaint:
only convert DisplayItemList if needed.

This fixes a perforance regression caused by crrev.com/c/2437735
which moved DisplayItemList conversion into Paint document lifecycle
for ease of benchmarking and consistency with CompositeAfterPaint.

Compared to before crrev.com/c/2437735, this CL can also avoid
conversion when cc side needs update but blink side does not (so
the change in recording_source.cc).

Bug: 1134633
Change-Id: I27a899f787c91a809fef5ce8edd663ac2720e0ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2451271
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarvmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814949}
parent 0215ea17
...@@ -90,10 +90,15 @@ void RecordingSource::UpdateDisplayItemList( ...@@ -90,10 +90,15 @@ void RecordingSource::UpdateDisplayItemList(
float recording_scale_factor) { float recording_scale_factor) {
recording_scale_factor_ = recording_scale_factor; recording_scale_factor_ = recording_scale_factor;
display_list_ = display_list;
painter_reported_memory_usage_ = painter_reported_memory_usage; painter_reported_memory_usage_ = painter_reported_memory_usage;
FinishDisplayItemListUpdate(); if (display_list_ != display_list) {
display_list_ = display_list;
// Do the following only if the display list changes. Though we use
// recording_scale_factor in DetermineIfSolidColor(), change of it doesn't
// affect whether the same display list is solid or not.
FinishDisplayItemListUpdate();
}
} }
gfx::Size RecordingSource::GetSize() const { gfx::Size RecordingSource::GetSize() const {
......
...@@ -85,7 +85,6 @@ scoped_refptr<cc::PictureLayer> ContentLayerClientImpl::UpdateCcPictureLayer( ...@@ -85,7 +85,6 @@ scoped_refptr<cc::PictureLayer> ContentLayerClientImpl::UpdateCcPictureLayer(
if (layer_state != layer_state_) if (layer_state != layer_state_)
cc_picture_layer_->SetSubtreePropertyChanged(); cc_picture_layer_->SetSubtreePropertyChanged();
raster_invalidated_ = false;
gfx::Size old_layer_size = raster_invalidator_.LayerBounds().size(); gfx::Size old_layer_size = raster_invalidator_.LayerBounds().size();
DCHECK_EQ(old_layer_size, cc_picture_layer_->bounds()); DCHECK_EQ(old_layer_size, cc_picture_layer_->bounds());
raster_invalidator_.Generate(raster_invalidation_function_, paint_chunks, raster_invalidator_.Generate(raster_invalidation_function_, paint_chunks,
...@@ -111,9 +110,9 @@ scoped_refptr<cc::PictureLayer> ContentLayerClientImpl::UpdateCcPictureLayer( ...@@ -111,9 +110,9 @@ scoped_refptr<cc::PictureLayer> ContentLayerClientImpl::UpdateCcPictureLayer(
// If nothing changed in the layer, keep the original display item list. // If nothing changed in the layer, keep the original display item list.
// Here check layer_bounds because RasterInvalidator doesn't issue raster // Here check layer_bounds because RasterInvalidator doesn't issue raster
// invalidation when layer_bounds become empty or non-empty from empty. // invalidation when only layer_bounds changes.
if (layer_bounds.size() == old_layer_size && !raster_invalidated_ && if (cc_display_item_list_ && layer_bounds.size() == old_layer_size &&
!raster_under_invalidation_params && cc_display_item_list_) { !raster_under_invalidation_params) {
DCHECK_EQ(cc_picture_layer_->bounds(), layer_bounds.size()); DCHECK_EQ(cc_picture_layer_->bounds(), layer_bounds.size());
return cc_picture_layer_; return cc_picture_layer_;
} }
...@@ -137,4 +136,9 @@ scoped_refptr<cc::PictureLayer> ContentLayerClientImpl::UpdateCcPictureLayer( ...@@ -137,4 +136,9 @@ scoped_refptr<cc::PictureLayer> ContentLayerClientImpl::UpdateCcPictureLayer(
return cc_picture_layer_; return cc_picture_layer_;
} }
void ContentLayerClientImpl::InvalidateRect(const IntRect& rect) {
cc_display_item_list_ = nullptr;
cc_picture_layer_->SetNeedsDisplayRect(rect);
}
} // namespace blink } // namespace blink
...@@ -63,17 +63,13 @@ class PLATFORM_EXPORT ContentLayerClientImpl : public cc::ContentLayerClient, ...@@ -63,17 +63,13 @@ class PLATFORM_EXPORT ContentLayerClientImpl : public cc::ContentLayerClient,
private: private:
// Callback from raster_invalidator_. // Callback from raster_invalidator_.
void InvalidateRect(const IntRect& rect) { void InvalidateRect(const IntRect&);
raster_invalidated_ = true;
cc_picture_layer_->SetNeedsDisplayRect(rect);
}
base::Optional<PaintChunk::Id> id_; base::Optional<PaintChunk::Id> id_;
scoped_refptr<cc::PictureLayer> cc_picture_layer_; scoped_refptr<cc::PictureLayer> cc_picture_layer_;
scoped_refptr<cc::DisplayItemList> cc_display_item_list_; scoped_refptr<cc::DisplayItemList> cc_display_item_list_;
RasterInvalidator raster_invalidator_; RasterInvalidator raster_invalidator_;
RasterInvalidator::RasterInvalidationFunction raster_invalidation_function_; RasterInvalidator::RasterInvalidationFunction raster_invalidation_function_;
bool raster_invalidated_ = false;
PropertyTreeState layer_state_; PropertyTreeState layer_state_;
......
...@@ -72,6 +72,7 @@ GraphicsLayer::GraphicsLayer(GraphicsLayerClient& client) ...@@ -72,6 +72,7 @@ GraphicsLayer::GraphicsLayer(GraphicsLayerClient& client)
contents_visible_(true), contents_visible_(true),
hit_testable_(false), hit_testable_(false),
needs_check_raster_invalidation_(false), needs_check_raster_invalidation_(false),
raster_invalidated_(false),
should_create_layers_after_paint_(false), should_create_layers_after_paint_(false),
repainted_(false), repainted_(false),
painting_phase_(kGraphicsLayerPaintAllWithOverflowClip), painting_phase_(kGraphicsLayerPaintAllWithOverflowClip),
...@@ -318,6 +319,8 @@ void GraphicsLayer::Paint() { ...@@ -318,6 +319,8 @@ void GraphicsLayer::Paint() {
DCHECK(layer_state_) << "No layer state for GraphicsLayer: " << DebugName(); DCHECK(layer_state_) << "No layer state for GraphicsLayer: " << DebugName();
// Generate raster invalidations for SPv1. // Generate raster invalidations for SPv1.
if (!ShouldCreateLayersAfterPaint()) { if (!ShouldCreateLayersAfterPaint()) {
auto& raster_invalidator = EnsureRasterInvalidator();
IntSize old_layer_size(raster_invalidator.LayerBounds().size());
IntRect layer_bounds(layer_state_->offset, IntSize(Size())); IntRect layer_bounds(layer_state_->offset, IntSize(Size()));
PaintChunkSubset chunks(GetPaintController().GetPaintArtifactShared()); PaintChunkSubset chunks(GetPaintController().GetPaintArtifactShared());
EnsureRasterInvalidator().Generate(raster_invalidation_function_, chunks, EnsureRasterInvalidator().Generate(raster_invalidation_function_, chunks,
...@@ -333,11 +336,19 @@ void GraphicsLayer::Paint() { ...@@ -333,11 +336,19 @@ void GraphicsLayer::Paint() {
DebugName()); DebugName());
} }
cc_display_item_list_ = PaintChunksToCcLayer::Convert( // If nothing changed in the layer, keep the original display item list.
chunks, layer_state_->state.Unalias(), // Here check layer_bounds because RasterInvalidator doesn't issue raster
gfx::Vector2dF(layer_state_->offset.X(), layer_state_->offset.Y()), // invalidation when only layer_bounds change.
cc::DisplayItemList::kTopLevelDisplayItemList, if (raster_invalidated_ || !cc_display_item_list_ ||
base::OptionalOrNullptr(raster_under_invalidation_params)); old_layer_size != layer_bounds.Size() ||
raster_under_invalidation_params) {
cc_display_item_list_ = PaintChunksToCcLayer::Convert(
chunks, layer_state_->state.Unalias(),
gfx::Vector2dF(layer_state_->offset.X(), layer_state_->offset.Y()),
cc::DisplayItemList::kTopLevelDisplayItemList,
base::OptionalOrNullptr(raster_under_invalidation_params));
raster_invalidated_ = false;
}
} }
needs_check_raster_invalidation_ = false; needs_check_raster_invalidation_ = false;
...@@ -619,6 +630,7 @@ void GraphicsLayer::SetHitTestable(bool should_hit_test) { ...@@ -619,6 +630,7 @@ void GraphicsLayer::SetHitTestable(bool should_hit_test) {
void GraphicsLayer::SetContentsNeedsDisplay() { void GraphicsLayer::SetContentsNeedsDisplay() {
if (contents_layer_) { if (contents_layer_) {
raster_invalidated_ = true;
contents_layer_->SetNeedsDisplay(); contents_layer_->SetNeedsDisplay();
TrackRasterInvalidation(*this, contents_rect_, TrackRasterInvalidation(*this, contents_rect_,
PaintInvalidationReason::kFullLayer); PaintInvalidationReason::kFullLayer);
...@@ -629,6 +641,7 @@ void GraphicsLayer::SetNeedsDisplay() { ...@@ -629,6 +641,7 @@ void GraphicsLayer::SetNeedsDisplay() {
if (!PaintsContentOrHitTest()) if (!PaintsContentOrHitTest())
return; return;
raster_invalidated_ = true;
CcLayer().SetNeedsDisplay(); CcLayer().SetNeedsDisplay();
// Invalidate the paint controller if it exists, but don't bother creating one // Invalidate the paint controller if it exists, but don't bother creating one
...@@ -645,6 +658,7 @@ void GraphicsLayer::SetNeedsDisplay() { ...@@ -645,6 +658,7 @@ void GraphicsLayer::SetNeedsDisplay() {
void GraphicsLayer::SetNeedsDisplayInRect(const IntRect& rect) { void GraphicsLayer::SetNeedsDisplayInRect(const IntRect& rect) {
DCHECK(PaintsContentOrHitTest()); DCHECK(PaintsContentOrHitTest());
raster_invalidated_ = true;
CcLayer().SetNeedsDisplayRect(rect); CcLayer().SetNeedsDisplayRect(rect);
} }
......
...@@ -287,6 +287,7 @@ class PLATFORM_EXPORT GraphicsLayer : public DisplayItemClient, ...@@ -287,6 +287,7 @@ class PLATFORM_EXPORT GraphicsLayer : public DisplayItemClient,
bool contents_visible_ : 1; bool contents_visible_ : 1;
bool hit_testable_ : 1; bool hit_testable_ : 1;
bool needs_check_raster_invalidation_ : 1; bool needs_check_raster_invalidation_ : 1;
bool raster_invalidated_ : 1;
// True if the cc::Layers for this GraphicsLayer should be created after // True if the cc::Layers for this GraphicsLayer should be created after
// paint (in PaintArtifactCompositor). This depends on the display item list // paint (in PaintArtifactCompositor). This depends on the display item list
// and is updated after CommitNewDisplayItems. // and is updated after CommitNewDisplayItems.
......
...@@ -334,6 +334,9 @@ void RasterInvalidator::Generate( ...@@ -334,6 +334,9 @@ void RasterInvalidator::Generate(
if (tracking_info_ && layer_bounds_was_empty && !layer_bounds.IsEmpty() && if (tracking_info_ && layer_bounds_was_empty && !layer_bounds.IsEmpty() &&
!new_chunks.IsEmpty()) { !new_chunks.IsEmpty()) {
// This is useful to track the implicit raster invalidations caused by
// change of layerization which has performance impact, especially for
// tests under web_tests/paint/invalidation.
TrackImplicitFullLayerInvalidation( TrackImplicitFullLayerInvalidation(
layer_client ? *layer_client : new_chunks.begin()->id.client); layer_client ? *layer_client : new_chunks.begin()->id.client);
} }
...@@ -352,16 +355,9 @@ void RasterInvalidator::Generate( ...@@ -352,16 +355,9 @@ void RasterInvalidator::Generate(
void RasterInvalidator::TrackImplicitFullLayerInvalidation( void RasterInvalidator::TrackImplicitFullLayerInvalidation(
const DisplayItemClient& layer_client) { const DisplayItemClient& layer_client) {
DCHECK(tracking_info_); DCHECK(tracking_info_);
// Early return if we have already invalidated the whole layer.
IntRect full_layer_rect(0, 0, layer_bounds_.width(), layer_bounds_.height());
for (const auto& invalidation : tracking_info_->tracking.Invalidations()) {
if (invalidation.rect.Contains(full_layer_rect))
return;
}
tracking_info_->tracking.AddInvalidation( tracking_info_->tracking.AddInvalidation(
&layer_client, layer_client.DebugName(), full_layer_rect, &layer_client, layer_client.DebugName(),
IntRect(IntPoint(), IntSize(layer_bounds_.size())),
PaintInvalidationReason::kFullLayer); PaintInvalidationReason::kFullLayer);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment