Commit 11753154 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[FragmentItem] Make NGFragmentItem not a DisplayItemClient

Making |NGFragmentItem| not a |DisplayItemClient| has two
benefits:
1. Give a persistent |DisplayItemClientId| across multiple
   layout cycles.
2. Allow making |Vector<NGFragmentItem>| instead of
   |Vector<scoped_refptr<NGFragmentItem>>|, because a
   |Vector| of |DisplayItemClient| is not allowed.
   This change is expected to reduce the memory allocation
   cost, which is high for text-heavy pages.

This patch is only for 1. The work for 2 will be in following
patches. Still, this patch improves `blink_perf.paint/
paint-offset-changes` microbenchmark by ~10%:
https://pinpoint-dot-chromeperf.appspot.com/job/15b8f60a120000
https://pinpoint-dot-chromeperf.appspot.com/job/14e61bb6120000
and slight positive changes to blink_perf.layout, probably
due to simpler paint invalidations:
https://pinpoint-dot-chromeperf.appspot.com/job/17d3126c120000

This patch is on top of following foudnation patches:
* r770260 <crrev.com/c/2207948> supported |wtf_size_t| for
  fragment id.
* r770637 <crrev.com/c/2209774> added |NGFragment::
  FragmentId()|.

Bug: 982194
Change-Id: I1cb5e2633bd591723632f1acb0cbb09ab4d77f83
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2208586Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771601}
parent 074bd1be
......@@ -1789,12 +1789,22 @@ void LayoutInline::InvalidateDisplayItemClients(
if (IsInLayoutNGInlineFormattingContext()) {
if (!ShouldCreateBoxFragment())
return;
NGInlineCursor cursor;
for (cursor.MoveTo(*this); cursor; cursor.MoveToNextForSameLayoutObject()) {
DCHECK_EQ(cursor.Current().GetLayoutObject(), this);
paint_invalidator.InvalidateDisplayItemClient(
*cursor.Current().GetDisplayItemClient(), invalidation_reason);
if (!RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled()) {
NGInlineCursor cursor;
for (cursor.MoveTo(*this); cursor;
cursor.MoveToNextForSameLayoutObject()) {
DCHECK_EQ(cursor.Current().GetLayoutObject(), this);
paint_invalidator.InvalidateDisplayItemClient(
*cursor.Current().GetDisplayItemClient(), invalidation_reason);
}
return;
}
#if DCHECK_IS_ON()
NGInlineCursor cursor;
for (cursor.MoveTo(*this); cursor; cursor.MoveToNextForSameLayoutObject())
DCHECK_EQ(cursor.Current().GetDisplayItemClient(), this);
#endif
paint_invalidator.InvalidateDisplayItemClient(*this, invalidation_reason);
return;
}
......
......@@ -2493,11 +2493,21 @@ void LayoutText::InvalidateDisplayItemClients(
ObjectPaintInvalidator paint_invalidator(*this);
if (IsInLayoutNGInlineFormattingContext()) {
NGInlineCursor cursor;
for (cursor.MoveTo(*this); cursor; cursor.MoveToNextForSameLayoutObject()) {
paint_invalidator.InvalidateDisplayItemClient(
*cursor.Current().GetDisplayItemClient(), invalidation_reason);
if (!RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled()) {
NGInlineCursor cursor;
for (cursor.MoveTo(*this); cursor;
cursor.MoveToNextForSameLayoutObject()) {
paint_invalidator.InvalidateDisplayItemClient(
*cursor.Current().GetDisplayItemClient(), invalidation_reason);
}
return;
}
#if DCHECK_IS_ON()
NGInlineCursor cursor;
for (cursor.MoveTo(*this); cursor; cursor.MoveToNextForSameLayoutObject())
DCHECK_EQ(cursor.Current().GetDisplayItemClient(), this);
#endif
paint_invalidator.InvalidateDisplayItemClient(*this, invalidation_reason);
return;
}
......
......@@ -17,8 +17,7 @@ namespace blink {
namespace {
struct SameSizeAsNGFragmentItem : RefCounted<NGFragmentItem>,
public DisplayItemClient {
struct SameSizeAsNGFragmentItem : RefCounted<NGFragmentItem> {
struct {
void* pointer;
NGTextOffset text_offset;
......@@ -325,7 +324,7 @@ TextDirection NGFragmentItem::ResolvedDirection() const {
return static_cast<TextDirection>(text_direction_);
}
String NGFragmentItem::DebugName() const {
String NGFragmentItem::ToString() const {
// TODO(yosin): Once |NGPaintFragment| is removed, we should get rid of
// following if-statements.
// For ease of rebasing, we use same |DebugName()| as |NGPaintFrgment|.
......@@ -357,20 +356,6 @@ String NGFragmentItem::DebugName() const {
return "NGFragmentItem";
}
IntRect NGFragmentItem::VisualRect() const {
// TODO(kojii): Need to reconsider the storage of |VisualRect|, to integrate
// better with |FragmentData| and to avoid dependency to |LayoutObject|.
DCHECK(GetLayoutObject());
return GetLayoutObject()->VisualRectForInlineBox();
}
IntRect NGFragmentItem::PartialInvalidationVisualRect() const {
// TODO(yosin): Need to reconsider the storage of |VisualRect|, to integrate
// better with |FragmentData| and to avoid dependency to |LayoutObject|.
DCHECK(GetLayoutObject());
return GetLayoutObject()->PartialInvalidationVisualRectForInlineBox();
}
PhysicalRect NGFragmentItem::LocalVisualRectFor(
const LayoutObject& layout_object) {
DCHECK(RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled());
......
......@@ -29,8 +29,7 @@ struct NGTextFragmentPaintInfo;
//
// This class consumes less memory than a full fragment, and can be stored in a
// flat list (NGFragmentItems) for easier and faster traversal.
class CORE_EXPORT NGFragmentItem : public RefCounted<NGFragmentItem>,
public DisplayItemClient {
class CORE_EXPORT NGFragmentItem : public RefCounted<NGFragmentItem> {
public:
// Represents regular text that exists in the DOM.
struct TextItem {
......@@ -80,7 +79,7 @@ class CORE_EXPORT NGFragmentItem : public RefCounted<NGFragmentItem>,
const String& text_content,
WritingMode writing_mode);
~NGFragmentItem() final;
~NGFragmentItem();
ItemType Type() const { return static_cast<ItemType>(type_); }
......@@ -94,6 +93,7 @@ class CORE_EXPORT NGFragmentItem : public RefCounted<NGFragmentItem>,
bool IsListMarker() const;
// A sequence number of fragments generated from a |LayoutObject|.
// For line boxes, please see |kInitialLineFragmentId|.
wtf_size_t FragmentId() const {
DCHECK_NE(Type(), kLine);
return fragment_id_;
......@@ -102,6 +102,14 @@ class CORE_EXPORT NGFragmentItem : public RefCounted<NGFragmentItem>,
DCHECK_NE(Type(), kLine);
fragment_id_ = id;
}
// The initial framgent_id for line boxes.
// TODO(kojii): This is to avoid conflict with multicol because line boxes use
// its |LayoutBlockFlow| as their |DisplayItemClient|, but multicol also uses
// fragment id for |LayoutBlockFlow| today. The plan is to make |FragmentData|
// a |DisplayItemClient| instead.
// TODO(kojii): The fragment id for line boxes must be unique across NG block
// fragmentation. This is not implemented yet.
static constexpr wtf_size_t kInitialLineFragmentId = 0x80000000;
// Return true if this is the first fragment generated from a node.
bool IsFirstForNode() const { return !FragmentId(); }
......@@ -137,6 +145,11 @@ class CORE_EXPORT NGFragmentItem : public RefCounted<NGFragmentItem>,
Node* GetNode() const { return layout_object_->GetNode(); }
Node* NodeForHitTest() const { return layout_object_->NodeForHitTest(); }
// Use |LayoutObject|+|FragmentId()| for |DisplayItem::Id|.
const DisplayItemClient* GetDisplayItemClient() const {
return GetLayoutObject();
}
wtf_size_t DeltaToNextForSameLayoutObject() const {
return delta_to_next_for_same_layout_object_;
}
......@@ -205,11 +218,6 @@ class CORE_EXPORT NGFragmentItem : public RefCounted<NGFragmentItem>,
return NGLineBoxType::kNormalLineBox;
}
// DisplayItemClient overrides
String DebugName() const override;
IntRect VisualRect() const override;
IntRect PartialInvalidationVisualRect() const override;
static PhysicalRect LocalVisualRectFor(const LayoutObject& layout_object);
// Re-compute the ink overflow for the |cursor| until its end.
......@@ -350,6 +358,9 @@ class CORE_EXPORT NGFragmentItem : public RefCounted<NGFragmentItem>,
// Returns true if this item is reusable.
bool CanReuse() const;
// Get a description of |this| for the debug purposes.
String ToString() const;
private:
// Create a text item.
NGFragmentItem(const NGInlineItem& inline_item,
......
......@@ -457,7 +457,7 @@ const DisplayItemClient* NGInlineCursorPosition::GetDisplayItemClient() const {
if (paint_fragment_)
return paint_fragment_;
if (item_)
return item_;
return item_->GetDisplayItemClient();
NOTREACHED();
return nullptr;
}
......
......@@ -1252,19 +1252,27 @@ inline void NGBoxFragmentPainter::PaintLineBox(
const DisplayItemClient& display_item_client,
const NGPaintFragment* line_box_paint_fragment,
const NGFragmentItem* line_box_item,
wtf_size_t line_fragment_id,
const PaintInfo& paint_info,
const PhysicalOffset& child_offset) {
if (paint_info.phase != PaintPhase::kForeground)
return;
base::Optional<ScopedDisplayItemFragment> display_item_fragment;
if (ShouldRecordHitTestData(paint_info)) {
if (line_box_item)
display_item_fragment.emplace(paint_info.context, line_fragment_id);
PhysicalRect border_box = line_box_fragment.LocalRect();
border_box.offset += child_offset;
paint_info.context.GetPaintController().RecordHitTestData(
display_item_client, PixelSnappedIntRect(border_box),
PhysicalFragment().EffectiveAllowedTouchAction());
}
// Paint the background of the `::first-line` line box.
if (NGLineBoxFragmentPainter::NeedsPaint(line_box_fragment)) {
if (!display_item_fragment && line_box_item)
display_item_fragment.emplace(paint_info.context, line_fragment_id);
NGLineBoxFragmentPainter line_box_painter(
line_box_fragment, line_box_paint_fragment, line_box_item,
PhysicalFragment(), paint_fragment_);
......@@ -1343,8 +1351,10 @@ void NGBoxFragmentPainter::PaintLineBoxChildren(
continue;
}
DCHECK(child_fragment.IsLineBox());
PaintLineBox(child_fragment, *line, line, /* line_box_item */ nullptr,
paint_info, child_offset);
PaintLineBox(
child_fragment, *line, line, /* line_box_item */ nullptr,
// |line_fragment_id| is used only when |NGFragmentItem| is enabled.
/* line_fragment_id */ 0, paint_info, child_offset);
PaintInlineChildren(line->Children(), paint_info, child_offset);
}
}
......@@ -1354,6 +1364,7 @@ void NGBoxFragmentPainter::PaintLineBoxChildItems(
const PaintInfo& paint_info,
const PhysicalOffset& paint_offset) {
const bool is_horizontal = box_fragment_.Style().IsHorizontalWritingMode();
wtf_size_t line_fragment_id = NGFragmentItem::kInitialLineFragmentId;
for (; *children; children->MoveToNextSkippingChildren()) {
const NGFragmentItem* child_item = children->CurrentItem();
DCHECK(child_item);
......@@ -1380,9 +1391,9 @@ void NGBoxFragmentPainter::PaintLineBoxChildItems(
const NGPhysicalLineBoxFragment* line_box_fragment =
child_item->LineBoxFragment();
DCHECK(line_box_fragment);
PaintLineBox(*line_box_fragment, *child_item,
PaintLineBox(*line_box_fragment, *child_item->GetDisplayItemClient(),
/* line_box_paint_fragment */ nullptr, child_item,
paint_info, child_offset);
line_fragment_id++, paint_info, child_offset);
NGInlineCursor line_box_cursor = children->CursorForDescendants();
PaintInlineItems(paint_info, paint_offset,
child_item->OffsetInContainerBlock(), &line_box_cursor);
......@@ -1524,6 +1535,8 @@ void NGBoxFragmentPainter::PaintTextItem(const NGInlineCursor& cursor,
!(item.IsLineBreak() && HasSelection(item.GetLayoutObject())))
return;
ScopedDisplayItemFragment display_item_fragment(paint_info.context,
item.FragmentId());
NGTextFragmentPainter<NGInlineCursor> text_painter(cursor, parent_offset);
text_painter.Paint(paint_info, paint_offset);
}
......
......@@ -134,6 +134,7 @@ class NGBoxFragmentPainter : public BoxPainterBase {
const DisplayItemClient& display_item_client,
const NGPaintFragment* line_box_paint_fragment,
const NGFragmentItem* line_box_item,
wtf_size_t line_fragment_id,
const PaintInfo&,
const PhysicalOffset& paint_offset);
void PaintBackplate(NGInlineCursor* descendants,
......@@ -386,7 +387,7 @@ inline NGBoxFragmentPainter::NGBoxFragmentPainter(
const NGFragmentItem& item,
const NGPhysicalBoxFragment& fragment)
: NGBoxFragmentPainter(fragment,
item,
*item.GetDisplayItemClient(),
/* paint_fragment */ nullptr,
&inline_box_cursor,
&item) {
......
......@@ -17,6 +17,7 @@
#include "third_party/blink/renderer/core/style/nine_piece_image.h"
#include "third_party/blink/renderer/platform/graphics/graphics_context_state_saver.h"
#include "third_party/blink/renderer/platform/graphics/paint/drawing_recorder.h"
#include "third_party/blink/renderer/platform/graphics/paint/scoped_display_item_fragment.h"
namespace blink {
......@@ -57,6 +58,12 @@ const NGBorderEdges NGInlineBoxFragmentPainter::BorderEdges() const {
void NGInlineBoxFragmentPainter::Paint(const PaintInfo& paint_info,
const PhysicalOffset& paint_offset) {
base::Optional<ScopedDisplayItemFragment> display_item_fragment;
if (inline_box_item_) {
display_item_fragment.emplace(paint_info.context,
inline_box_item_->FragmentId());
}
const PhysicalOffset adjusted_paint_offset =
paint_offset + (inline_box_paint_fragment_
? inline_box_paint_fragment_->Offset()
......@@ -150,6 +157,14 @@ void NGLineBoxFragmentPainter::PaintBackgroundBorderShadow(
DCHECK_EQ(paint_info.phase, PaintPhase::kForeground);
DCHECK_EQ(inline_box_fragment_.Type(), NGPhysicalFragment::kFragmentLineBox);
DCHECK(NeedsPaint(inline_box_fragment_));
#if DCHECK_IS_ON()
if (RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled()) {
DCHECK(inline_box_item_);
// |NGFragmentItem| uses the fragment id when painting the background of
// line boxes. Please see |NGFragmentItem::kInitialLineFragmentId|.
DCHECK_NE(paint_info.context.GetPaintController().CurrentFragment(), 0u);
}
#endif
if (line_style_ == style_ ||
line_style_.Visibility() != EVisibility::kVisible)
......
......@@ -99,7 +99,7 @@ class NGInlineBoxFragmentPainterBase : public InlineBoxPainterBase {
if (inline_box_paint_fragment_)
return *inline_box_paint_fragment_;
DCHECK(inline_box_item_);
return *inline_box_item_;
return *inline_box_item_->GetDisplayItemClient();
}
const virtual NGBorderEdges BorderEdges() const = 0;
......
{
"layers": [
{
"name": "Scrolling Contents Layer",
"bounds": [800, 600],
"contentsOpaque": true,
"backgroundColor": "#FFFFFF",
"invalidations": [
[193, 8, 189, 22],
[12, 11, 53, 16]
]
}
]
}
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