Commit e94d2968 authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

[CI] Allow all display items to be cacheable

We treated only DrawingDisplayItem as cacheable before SlimmingPaintV175
because other display items were all about paint properties which are
not feasible to be cached (mostly because they are paired and sometimes
don't have unique id).

Now we have removed paired display items, thus the remaining types of
display items are all eligible to be cached.

This prepares for making HitTestDisplayItem not DrawingDisplayItem to
reduce its size, etc.

Change-Id: I28bd5533d3b85a67fc16e90aea059115b51188e7
Reviewed-on: https://chromium-review.googlesource.com/c/1329805Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607497}
parent b00f729f
...@@ -36,13 +36,11 @@ void EllipsisBoxPainter::PaintEllipsis(const PaintInfo& paint_info, ...@@ -36,13 +36,11 @@ void EllipsisBoxPainter::PaintEllipsis(const PaintInfo& paint_info,
box_origin.MoveBy(paint_offset); box_origin.MoveBy(paint_offset);
GraphicsContext& context = paint_info.context; GraphicsContext& context = paint_info.context;
DisplayItem::Type display_item_type =
DisplayItem::PaintPhaseToDrawingType(paint_info.phase);
if (DrawingRecorder::UseCachedDrawingIfPossible(context, ellipsis_box_, if (DrawingRecorder::UseCachedDrawingIfPossible(context, ellipsis_box_,
display_item_type)) paint_info.phase))
return; return;
DrawingRecorder recorder(context, ellipsis_box_, display_item_type); DrawingRecorder recorder(context, ellipsis_box_, paint_info.phase);
LayoutRect box_rect(box_origin, LayoutRect box_rect(box_origin,
LayoutSize(ellipsis_box_.LogicalWidth(), LayoutSize(ellipsis_box_.LogicalWidth(),
......
...@@ -240,12 +240,10 @@ void InlineFlowBoxPainter::PaintMask(const PaintInfo& paint_info, ...@@ -240,12 +240,10 @@ void InlineFlowBoxPainter::PaintMask(const PaintInfo& paint_info,
return; return;
if (DrawingRecorder::UseCachedDrawingIfPossible( if (DrawingRecorder::UseCachedDrawingIfPossible(
paint_info.context, inline_flow_box_, paint_info.context, inline_flow_box_, paint_info.phase))
DisplayItem::PaintPhaseToDrawingType(paint_info.phase)))
return; return;
DrawingRecorder recorder( DrawingRecorder recorder(paint_info.context, inline_flow_box_,
paint_info.context, inline_flow_box_, paint_info.phase);
DisplayItem::PaintPhaseToDrawingType(paint_info.phase));
LayoutRect paint_rect = AdjustedPaintRect(paint_offset); LayoutRect paint_rect = AdjustedPaintRect(paint_offset);
......
...@@ -147,11 +147,9 @@ void InlineTextBoxPainter::Paint(const PaintInfo& paint_info, ...@@ -147,11 +147,9 @@ void InlineTextBoxPainter::Paint(const PaintInfo& paint_info,
base::Optional<DrawingRecorder> recorder; base::Optional<DrawingRecorder> recorder;
if (paint_info.phase != PaintPhase::kTextClip) { if (paint_info.phase != PaintPhase::kTextClip) {
if (DrawingRecorder::UseCachedDrawingIfPossible( if (DrawingRecorder::UseCachedDrawingIfPossible(
paint_info.context, inline_text_box_, paint_info.context, inline_text_box_, paint_info.phase))
DisplayItem::PaintPhaseToDrawingType(paint_info.phase)))
return; return;
recorder.emplace(paint_info.context, inline_text_box_, recorder.emplace(paint_info.context, inline_text_box_, paint_info.phase);
DisplayItem::PaintPhaseToDrawingType(paint_info.phase));
} }
GraphicsContext& context = paint_info.context; GraphicsContext& context = paint_info.context;
......
...@@ -325,12 +325,10 @@ void NGBoxFragmentPainter::PaintBlockFlowContents( ...@@ -325,12 +325,10 @@ void NGBoxFragmentPainter::PaintBlockFlowContents(
if (paint_info.phase == PaintPhase::kMask) { if (paint_info.phase == PaintPhase::kMask) {
if (DrawingRecorder::UseCachedDrawingIfPossible( if (DrawingRecorder::UseCachedDrawingIfPossible(
paint_info.context, box_fragment_, paint_info.context, box_fragment_, paint_info.phase))
DisplayItem::PaintPhaseToDrawingType(paint_info.phase)))
return; return;
DrawingRecorder recorder( DrawingRecorder recorder(paint_info.context, box_fragment_,
paint_info.context, box_fragment_, paint_info.phase);
DisplayItem::PaintPhaseToDrawingType(paint_info.phase));
PaintMask(paint_info, paint_offset); PaintMask(paint_info, paint_offset);
return; return;
} }
...@@ -803,11 +801,9 @@ void NGBoxFragmentPainter::PaintTextChild(const NGPaintFragment& text_fragment, ...@@ -803,11 +801,9 @@ void NGBoxFragmentPainter::PaintTextChild(const NGPaintFragment& text_fragment,
base::Optional<DrawingRecorder> recorder; base::Optional<DrawingRecorder> recorder;
if (paint_info.phase != PaintPhase::kTextClip) { if (paint_info.phase != PaintPhase::kTextClip) {
if (DrawingRecorder::UseCachedDrawingIfPossible( if (DrawingRecorder::UseCachedDrawingIfPossible(
paint_info.context, text_fragment, paint_info.context, text_fragment, paint_info.phase))
DisplayItem::PaintPhaseToDrawingType(paint_info.phase)))
return; return;
recorder.emplace(paint_info.context, text_fragment, recorder.emplace(paint_info.context, text_fragment, paint_info.phase);
DisplayItem::PaintPhaseToDrawingType(paint_info.phase));
} }
const NGPhysicalTextFragment& physical_text_fragment = const NGPhysicalTextFragment& physical_text_fragment =
......
...@@ -93,15 +93,13 @@ void SVGInlineTextBoxPainter::Paint(const PaintInfo& paint_info, ...@@ -93,15 +93,13 @@ void SVGInlineTextBoxPainter::Paint(const PaintInfo& paint_info,
if (!TextShouldBePainted(text_layout_object)) if (!TextShouldBePainted(text_layout_object))
return; return;
DisplayItem::Type display_item_type =
DisplayItem::PaintPhaseToDrawingType(paint_info.phase);
if (!DrawingRecorder::UseCachedDrawingIfPossible( if (!DrawingRecorder::UseCachedDrawingIfPossible(
paint_info.context, svg_inline_text_box_, display_item_type)) { paint_info.context, svg_inline_text_box_, paint_info.phase)) {
LayoutObject& parent_layout_object = ParentInlineLayoutObject(); LayoutObject& parent_layout_object = ParentInlineLayoutObject();
const ComputedStyle& style = parent_layout_object.StyleRef(); const ComputedStyle& style = parent_layout_object.StyleRef();
DrawingRecorder recorder(paint_info.context, svg_inline_text_box_, DrawingRecorder recorder(paint_info.context, svg_inline_text_box_,
display_item_type); paint_info.phase);
InlineTextBoxPainter text_painter(svg_inline_text_box_); InlineTextBoxPainter text_painter(svg_inline_text_box_);
const DocumentMarkerVector& markers_to_paint = const DocumentMarkerVector& markers_to_paint =
text_painter.ComputeMarkersToPaint(); text_painter.ComputeMarkersToPaint();
......
...@@ -201,8 +201,7 @@ scoped_refptr<cc::PictureLayer> ContentLayerClientImpl::UpdateCcPictureLayer( ...@@ -201,8 +201,7 @@ scoped_refptr<cc::PictureLayer> ContentLayerClientImpl::UpdateCcPictureLayer(
json->SetArray("displayItems", json->SetArray("displayItems",
paint_artifact->GetDisplayItemList().SubsequenceAsJSON( paint_artifact->GetDisplayItemList().SubsequenceAsJSON(
chunk.begin_index, chunk.end_index, chunk.begin_index, chunk.end_index,
DisplayItemList::kSkipNonDrawings | DisplayItemList::kShownOnlyDisplayItemTypes));
DisplayItemList::kShownOnlyDisplayItemTypes));
paint_chunk_debug_data_->PushObject(std::move(json)); paint_chunk_debug_data_->PushObject(std::move(json));
} }
#endif #endif
......
...@@ -140,8 +140,8 @@ but can be other Blink objects which get painted, such as inline boxes and drag ...@@ -140,8 +140,8 @@ but can be other Blink objects which get painted, such as inline boxes and drag
images. images.
*** note *** note
It is illegal for there to be two drawings with the same ID in a display item It is illegal for there to be two display items with the same ID in a display
list, except for drawings that are marked uncacheable item list, except for display items that are marked uncacheable
(see [DisplayItemCacheSkipper](DisplayItemCacheSkipper.h)). (see [DisplayItemCacheSkipper](DisplayItemCacheSkipper.h)).
*** ***
...@@ -179,10 +179,10 @@ a `PaintController`. ...@@ -179,10 +179,10 @@ a `PaintController`.
the *current* paint artifact, and *new* display items and paint chunks, which the *current* paint artifact, and *new* display items and paint chunks, which
are added as content is painted. are added as content is painted.
Painters should call `PaintController::useCachedDrawingIfPossible()` or Painters should call `PaintController::UseCachedItemIfPossible()` or
`PaintController::useCachedSubsequenceIfPossible()` and if the function returns `PaintController::UseCachedSubsequenceIfPossible()` and if the function returns
`true`, existing display items that are still valid in the *current* paint artifact `true`, existing display items that are still valid in the *current* paint artifact
will be reused and the painter should skip real painting of the drawing or subsequence. will be reused and the painter should skip real painting of the item or subsequence.
When the new display items have been populated, clients call When the new display items have been populated, clients call
`commitNewDisplayItems`, which replaces the previous artifact with the new data, `commitNewDisplayItems`, which replaces the previous artifact with the new data,
......
...@@ -153,8 +153,7 @@ class PLATFORM_EXPORT DisplayItem { ...@@ -153,8 +153,7 @@ class PLATFORM_EXPORT DisplayItem {
type_(type), type_(type),
derived_size_(derived_size), derived_size_(derived_size),
fragment_(0), fragment_(0),
// TODO(pdr): Should this return true for IsScrollHitTestType too? is_cacheable_(client.IsCacheable()),
is_cacheable_(client.IsCacheable() && IsDrawingType(type)),
is_tombstone_(false) { is_tombstone_(false) {
// |derived_size| must fit in |derived_size_|. // |derived_size| must fit in |derived_size_|.
// If it doesn't, enlarge |derived_size_| and fix this assert. // If it doesn't, enlarge |derived_size_| and fix this assert.
...@@ -196,7 +195,7 @@ class PLATFORM_EXPORT DisplayItem { ...@@ -196,7 +195,7 @@ class PLATFORM_EXPORT DisplayItem {
// Visual rect can change without needing invalidation of the client, e.g. // Visual rect can change without needing invalidation of the client, e.g.
// when ancestor clip changes. This is called from PaintController:: // when ancestor clip changes. This is called from PaintController::
// UseCachedDrawingIfPossible() to update the visual rect of a cached display // UseCachedItemIfPossible() to update the visual rect of a cached display
// item. // item.
void UpdateVisualRect() { visual_rect_ = FloatRect(client_->VisualRect()); } void UpdateVisualRect() { visual_rect_ = FloatRect(client_->VisualRect()); }
......
...@@ -41,9 +41,6 @@ void DisplayItemList::AppendSubsequenceAsJSON(size_t begin_index, ...@@ -41,9 +41,6 @@ void DisplayItemList::AppendSubsequenceAsJSON(size_t begin_index,
std::unique_ptr<JSONObject> json = JSONObject::Create(); std::unique_ptr<JSONObject> json = JSONObject::Create();
const auto& item = (*this)[i]; const auto& item = (*this)[i];
if ((flags & kSkipNonDrawings) && !item.IsDrawing())
continue;
json->SetInteger("index", i); json->SetInteger("index", i);
if (flags & kShownOnlyDisplayItemTypes) { if (flags & kShownOnlyDisplayItemTypes) {
......
...@@ -74,10 +74,9 @@ class PLATFORM_EXPORT DisplayItemList ...@@ -74,10 +74,9 @@ class PLATFORM_EXPORT DisplayItemList
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
enum JsonOptions { enum JsonOptions {
kDefault = 0, kDefault = 0,
kShowPaintRecords = 1, kShowPaintRecords = 1 << 0,
kSkipNonDrawings = 1 << 1, kClientKnownToBeAlive = 1 << 1,
kClientKnownToBeAlive = 1 << 2, kShownOnlyDisplayItemTypes = 1 << 2
kShownOnlyDisplayItemTypes = 1 << 3
}; };
typedef unsigned JsonFlags; typedef unsigned JsonFlags;
......
...@@ -34,7 +34,7 @@ DrawingRecorder::DrawingRecorder(GraphicsContext& context, ...@@ -34,7 +34,7 @@ DrawingRecorder::DrawingRecorder(GraphicsContext& context,
if (context.GetPaintController().DisplayItemConstructionIsDisabled()) if (context.GetPaintController().DisplayItemConstructionIsDisabled())
return; return;
// Must check DrawingRecorder::useCachedDrawingIfPossible before creating the // Must check DrawingRecorder::UseCachedDrawingIfPossible before creating the
// DrawingRecorder. // DrawingRecorder.
DCHECK(RuntimeEnabledFeatures::PaintUnderInvalidationCheckingEnabled() || DCHECK(RuntimeEnabledFeatures::PaintUnderInvalidationCheckingEnabled() ||
!UseCachedDrawingIfPossible(context_, client_, type_)); !UseCachedDrawingIfPossible(context_, client_, type_));
......
...@@ -27,8 +27,7 @@ class PLATFORM_EXPORT DrawingRecorder final { ...@@ -27,8 +27,7 @@ class PLATFORM_EXPORT DrawingRecorder final {
static bool UseCachedDrawingIfPossible(GraphicsContext& context, static bool UseCachedDrawingIfPossible(GraphicsContext& context,
const DisplayItemClient& client, const DisplayItemClient& client,
DisplayItem::Type type) { DisplayItem::Type type) {
return context.GetPaintController().UseCachedDrawingIfPossible(client, return context.GetPaintController().UseCachedItemIfPossible(client, type);
type);
} }
static bool UseCachedDrawingIfPossible(GraphicsContext& context, static bool UseCachedDrawingIfPossible(GraphicsContext& context,
......
...@@ -18,8 +18,7 @@ void HitTestDisplayItem::Record(GraphicsContext& context, ...@@ -18,8 +18,7 @@ void HitTestDisplayItem::Record(GraphicsContext& context,
if (paint_controller.DisplayItemConstructionIsDisabled()) if (paint_controller.DisplayItemConstructionIsDisabled())
return; return;
if (paint_controller.UseCachedDrawingIfPossible(client, if (paint_controller.UseCachedItemIfPossible(client, DisplayItem::kHitTest))
DisplayItem::kHitTest))
return; return;
paint_controller.CreateAndAppend<HitTestDisplayItem>(client, hit_test_rect); paint_controller.CreateAndAppend<HitTestDisplayItem>(client, hit_test_rect);
......
...@@ -29,11 +29,8 @@ PaintController::~PaintController() { ...@@ -29,11 +29,8 @@ PaintController::~PaintController() {
DCHECK(new_display_item_list_.IsEmpty()); DCHECK(new_display_item_list_.IsEmpty());
} }
bool PaintController::UseCachedDrawingIfPossible( bool PaintController::UseCachedItemIfPossible(const DisplayItemClient& client,
const DisplayItemClient& client, DisplayItem::Type type) {
DisplayItem::Type type) {
DCHECK(DisplayItem::IsDrawingType(type));
if (usage_ == kTransient) if (usage_ == kTransient)
return false; return false;
......
...@@ -129,10 +129,10 @@ class PLATFORM_EXPORT PaintController { ...@@ -129,10 +129,10 @@ class PLATFORM_EXPORT PaintController {
ProcessNewItem(display_item); ProcessNewItem(display_item);
} }
// Tries to find the cached drawing display item corresponding to the given // Tries to find the cached display item corresponding to the given
// parameters. If found, appends the cached display item to the new display // parameters. If found, appends the cached display item to the new display
// list and returns true. Otherwise returns false. // list and returns true. Otherwise returns false.
bool UseCachedDrawingIfPossible(const DisplayItemClient&, DisplayItem::Type); bool UseCachedItemIfPossible(const DisplayItemClient&, DisplayItem::Type);
// Tries to find the cached subsequence corresponding to the given parameters. // Tries to find the cached subsequence corresponding to the given parameters.
// If found, copies the cache subsequence to the new display list and returns // If found, copies the cache subsequence to the new display list and returns
...@@ -255,7 +255,7 @@ class PLATFORM_EXPORT PaintController { ...@@ -255,7 +255,7 @@ class PLATFORM_EXPORT PaintController {
// However, the current algorithm allows the following situations even if // However, the current algorithm allows the following situations even if
// ClientCacheIsValid() is true for a client during painting: // ClientCacheIsValid() is true for a client during painting:
// 1. The client paints a new display item that is not cached: // 1. The client paints a new display item that is not cached:
// UseCachedDrawingIfPossible() returns false for the display item and the // UseCachedItemIfPossible() returns false for the display item and the
// newly painted display item will be added into the cache. This situation // newly painted display item will be added into the cache. This situation
// has slight performance hit (see FindOutOfOrderCachedItemForward()) so we // has slight performance hit (see FindOutOfOrderCachedItemForward()) so we
// print a warning in the situation and should keep it rare. // print a warning in the situation and should keep it rare.
...@@ -367,7 +367,7 @@ class PLATFORM_EXPORT PaintController { ...@@ -367,7 +367,7 @@ class PLATFORM_EXPORT PaintController {
// Stores indices to valid cacheable display items in // Stores indices to valid cacheable display items in
// current_paint_artifact_.GetDisplayItemList() that have not been matched by // current_paint_artifact_.GetDisplayItemList() that have not been matched by
// requests of cached display items (using UseCachedDrawingIfPossible() and // requests of cached display items (using UseCachedItemIfPossible() and
// UseCachedSubsequenceIfPossible()) during sequential matching. The indexed // UseCachedSubsequenceIfPossible()) during sequential matching. The indexed
// items will be matched by later out-of-order requests of cached display // items will be matched by later out-of-order requests of cached display
// items. This ensures that when out-of-order cached display items are // items. This ensures that when out-of-order cached display items are
...@@ -394,7 +394,7 @@ class PLATFORM_EXPORT PaintController { ...@@ -394,7 +394,7 @@ class PLATFORM_EXPORT PaintController {
IndicesByClientMap new_paint_chunk_indices_by_client_; IndicesByClientMap new_paint_chunk_indices_by_client_;
#endif #endif
// These are set in UseCachedDrawingIfPossible() and // These are set in UseCachedItemIfPossible() and
// UseCachedSubsequenceIfPossible() when we could use cached drawing or // UseCachedSubsequenceIfPossible() when we could use cached drawing or
// subsequence and under-invalidation checking is on, indicating the begin and // subsequence and under-invalidation checking is on, indicating the begin and
// end of the cached drawing or subsequence in the current list. The functions // end of the cached drawing or subsequence in the current list. The functions
......
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