Commit 315f4981 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[FragmentItem] Avoid creating NGPhysicalTextFragment

This patch creates |NGFragmentItem| without creating
intermediate |NGPhysicalTextFragment|.

Before this patch:
  |NGInlineItemResult| -> |NGPhysicalTextFragment|
    -> |NGFragmentItem|
With this patch:
  |NGInlineItemResult| -> |NGFragmentItem|

This patch improves benchmarks:
* chapter-reflow by 4%
* hindi-line-layout by 12%
* line-layout by 4%
https://pinpoint-dot-chromeperf.appspot.com/job/1670dc91e20000

Currently, these tests are slower than |NGPaintFragment|.
https://pinpoint-dot-chromeperf.appspot.com/job/10ea2589e20000

This patch also improves the currently shipping code path
by up to 5.8%. This is an unexpected surprise, probably
because the change improves code locality.
https://pinpoint-dot-chromeperf.appspot.com/job/14943f93e20000

Bug: 982194
Change-Id: I6ac7e8a9105ea05d3edee3048b3c8715baa07c5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2152063
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760418}
parent 8c74db02
......@@ -10,6 +10,7 @@
#include "third_party/blink/renderer/core/layout/ng/inline/ng_fragment_items_builder.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_cursor.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_item.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_item_result.h"
#include "third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.h"
namespace blink {
......@@ -41,6 +42,29 @@ NGFragmentItem::NGFragmentItem(const NGPhysicalTextFragment& text)
DCHECK(!IsFormattingContextRoot());
}
NGFragmentItem::NGFragmentItem(NGInlineItemResult&& item_result,
const PhysicalSize& size)
: layout_object_(item_result.item->GetLayoutObject()),
text_({std::move(item_result.shape_result), item_result.TextOffset()}),
rect_({PhysicalOffset(), size}),
type_(kText),
sub_type_(static_cast<unsigned>(item_result.item->TextType())),
style_variant_(static_cast<unsigned>(item_result.item->StyleVariant())),
is_hidden_for_paint_(false), // TODO(kojii): not supported yet.
text_direction_(static_cast<unsigned>(item_result.item->Direction())),
ink_overflow_computed_(false),
is_first_for_node_(item_result.IsFirstForNode()) {
#if DCHECK_IS_ON()
if (text_.shape_result) {
DCHECK_EQ(text_.shape_result->StartIndex(), StartOffset());
DCHECK_EQ(text_.shape_result->EndIndex(), EndOffset());
}
#endif
// TODO(kojii): Generated text not supported yet.
DCHECK_NE(TextType(), NGTextType::kLayoutGenerated);
DCHECK(!IsFormattingContextRoot());
}
NGFragmentItem::NGFragmentItem(const NGPhysicalLineBoxFragment& line,
wtf_size_t item_count)
: layout_object_(line.ContainerLayoutObject()),
......@@ -87,6 +111,43 @@ NGFragmentItem::NGFragmentItem(const NGInlineItem& inline_item,
DCHECK(!IsFormattingContextRoot());
}
// static
void NGFragmentItem::Create(NGLineBoxFragmentBuilder::ChildList* child_list,
const String& text_content,
WritingMode writing_mode) {
for (auto& child : *child_list) {
DCHECK(!child.fragment_item);
if (child.fragment) {
child.fragment_item = base::AdoptRef(new NGFragmentItem(*child.fragment));
continue;
}
if (child.item_result) {
child.fragment_item = base::AdoptRef(new NGFragmentItem(
std::move(*child.item_result),
ToPhysicalSize({child.inline_size, child.rect.size.block_size},
writing_mode)));
continue;
}
if (child.layout_result) {
const NGPhysicalBoxFragment& fragment =
To<NGPhysicalBoxFragment>(child.layout_result->PhysicalFragment());
child.fragment_item = base::AdoptRef(
new NGFragmentItem(fragment, child.ResolvedDirection()));
continue;
}
if (child.inline_item) {
child.fragment_item = base::AdoptRef(new NGFragmentItem(
*child.inline_item,
ToPhysicalSize(child.rect.size,
child.inline_item->Style()->GetWritingMode())));
}
}
}
NGFragmentItem::~NGFragmentItem() {
switch (Type()) {
case kText:
......
......@@ -66,13 +66,22 @@ class CORE_EXPORT NGFragmentItem : public RefCounted<NGFragmentItem>,
enum ItemType { kText, kGeneratedText, kLine, kBox };
// Create a text item.
// TODO(kojii): Should be able to create without once creating fragments.
NGFragmentItem(const NGPhysicalTextFragment& text);
explicit NGFragmentItem(const NGPhysicalTextFragment& text);
// Create a box item.
NGFragmentItem(const NGPhysicalBoxFragment& box,
TextDirection resolved_direction);
// Create a culled box item.
NGFragmentItem(const NGInlineItem& inline_item, const PhysicalSize& size);
// Create a line item.
NGFragmentItem(const NGPhysicalLineBoxFragment& line, wtf_size_t item_count);
// Create |NGFragmentItem| for all items in |child_list|.
static void Create(NGLineBoxFragmentBuilder::ChildList* child_list,
const String& text_content,
WritingMode writing_mode);
~NGFragmentItem() final;
ItemType Type() const { return static_cast<ItemType>(type_); }
......@@ -339,6 +348,9 @@ class CORE_EXPORT NGFragmentItem : public RefCounted<NGFragmentItem>,
const NGFragmentItems& items) const;
private:
// Create a text item.
NGFragmentItem(NGInlineItemResult&& item_result, const PhysicalSize& size);
const LayoutBox* InkOverflowOwnerBox() const;
LayoutBox* MutableInkOverflowOwnerBox();
......
......@@ -79,36 +79,20 @@ void NGFragmentItemsBuilder::AddItems(Child* child_begin, Child* child_end) {
for (Child* child_iter = child_begin; child_iter != child_end;) {
Child& child = *child_iter;
if (const NGPhysicalTextFragment* text = child.fragment.get()) {
items_.emplace_back(base::MakeRefCounted<NGFragmentItem>(*text),
child.rect.offset);
// OOF children should have been added to their parent box fragments.
DCHECK(!child.out_of_flow_positioned_box);
if (!child.fragment_item) {
++child_iter;
continue;
}
if (child.layout_result || child.inline_item) {
// Create an item if this box has no inline children.
scoped_refptr<NGFragmentItem> item;
if (child.layout_result) {
const NGPhysicalBoxFragment& box =
To<NGPhysicalBoxFragment>(child.layout_result->PhysicalFragment());
item = base::MakeRefCounted<NGFragmentItem>(box,
child.ResolvedDirection());
} else {
DCHECK(child.inline_item);
item = base::MakeRefCounted<NGFragmentItem>(
*child.inline_item,
ToPhysicalSize(child.rect.size,
child.inline_item->Style()->GetWritingMode()));
}
// Take the fast path when we know |child| does not have child items.
if (child.children_count <= 1) {
items_.emplace_back(std::move(item), child.rect.offset);
items_.emplace_back(std::move(child.fragment_item), child.rect.offset);
++child_iter;
continue;
}
DCHECK(!item->IsFloating());
DCHECK(child.fragment_item->IsContainer());
DCHECK(!child.fragment_item->IsFloating());
// Children of inline boxes are flattened and added to |items_|, with the
// count of descendant items to preserve the tree structure.
......@@ -129,16 +113,9 @@ void NGFragmentItemsBuilder::AddItems(Child* child_begin, Child* child_end) {
wtf_size_t item_count = items_.size() - box_start_index;
// Create an item for the start of the box.
item->SetDescendantsCount(item_count);
child.fragment_item->SetDescendantsCount(item_count);
DCHECK(!items_[box_start_index].item);
items_[box_start_index].item = std::move(item);
continue;
}
// OOF children should have been added to their parent box fragments.
// TODO(kojii): Consider handling them in NGFragmentItem too.
DCHECK(!child.out_of_flow_positioned_box);
++child_iter;
items_[box_start_index].item = std::move(child.fragment_item);
}
}
......
......@@ -32,6 +32,7 @@ struct CORE_EXPORT NGInlineItemResult {
DISALLOW_NEW();
public:
NGTextOffset TextOffset() const { return {start_offset, end_offset}; }
unsigned Length() const {
DCHECK_GT(end_offset, start_offset);
return end_offset - start_offset;
......
......@@ -206,8 +206,6 @@ void NGInlineLayoutAlgorithm::CreateLine(
// of items, which are needed to compute inline static positions.
LayoutUnit line_offset_for_text_align = ApplyTextAlign(line_info);
NGTextFragmentBuilder text_builder(ConstraintSpace().GetWritingMode());
// Compute heights of all inline items by placing the dominant baseline at 0.
// The baseline is adjusted after the height of the line box is computed.
const ComputedStyle& line_style = line_info->LineStyle();
......@@ -229,6 +227,7 @@ void NGInlineLayoutAlgorithm::CreateLine(
if (quirks_mode_ && line_style.Display() == EDisplay::kListItem)
box->ComputeTextMetrics(line_style, baseline_type_);
bool has_logical_text_items = false;
for (NGInlineItemResult& item_result : *line_items) {
DCHECK(item_result.item);
const NGInlineItem& item = *item_result.item;
......@@ -238,8 +237,6 @@ void NGInlineLayoutAlgorithm::CreateLine(
item.GetLayoutObject()->IsLayoutNGListItem());
DCHECK(item_result.shape_result);
text_builder.SetIsFirstForNode(item_result.IsFirstForNode());
if (UNLIKELY(quirks_mode_))
box->EnsureTextMetrics(*item.Style(), baseline_type_);
......@@ -251,18 +248,18 @@ void NGInlineLayoutAlgorithm::CreateLine(
DCHECK(item.TextType() == NGTextType::kNormal ||
item.TextType() == NGTextType::kSymbolMarker);
text_builder.SetItem(line_info->ItemsData(), &item_result,
box->text_height);
if (UNLIKELY(item_result.hyphen_shape_result)) {
LayoutUnit hyphen_inline_size = item_result.HyphenInlineSize();
line_box_.AddChild(text_builder.ToTextFragment(), box->text_top,
line_box_.AddChild(&item_result, box->text_top,
item_result.inline_size - hyphen_inline_size,
item.BidiLevel());
box->text_height, item.BidiLevel());
PlaceHyphen(item_result, hyphen_inline_size, box);
} else {
line_box_.AddChild(text_builder.ToTextFragment(), box->text_top,
item_result.inline_size, item.BidiLevel());
line_box_.AddChild(&item_result, box->text_top, item_result.inline_size,
box->text_height, item.BidiLevel());
}
has_logical_text_items = true;
// Text boxes always need full paint invalidations.
item.GetLayoutObject()->ClearNeedsLayoutWithFullPaintInvalidation();
......@@ -323,6 +320,13 @@ void NGInlineLayoutAlgorithm::CreateLine(
line_info->AvailableWidth() - line_info->TextIndent() &&
node_.GetLayoutBlockFlow()->ShouldTruncateOverflowingText()) ||
ShouldTruncateForLineClamp(*line_info))) {
// TODO(kojii): |NGLineTruncator| does not support |Child|-based truncation
// yet, so create |NGPhysicalTextFragment| first.
if (has_logical_text_items) {
line_box_.CreateTextFragments(ConstraintSpace().GetWritingMode(),
line_info->ItemsData().text_content);
has_logical_text_items = false;
}
NGLineTruncator truncator(*line_info);
auto* input =
DynamicTo<HTMLInputElement>(node_.GetLayoutBlockFlow()->GetNode());
......@@ -335,6 +339,12 @@ void NGInlineLayoutAlgorithm::CreateLine(
}
}
if (has_logical_text_items &&
!RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled()) {
line_box_.CreateTextFragments(ConstraintSpace().GetWritingMode(),
line_info->ItemsData().text_content);
}
// Negative margins can make the position negative, but the inline size is
// always positive or 0.
inline_size = inline_size.ClampNegativeToZero();
......@@ -380,6 +390,11 @@ void NGInlineLayoutAlgorithm::CreateLine(
context_->SetItemIndex(line_info->ItemsData().items,
line_info->EndItemIndex());
if (UNLIKELY(RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled())) {
NGFragmentItem::Create(&line_box_, line_info->ItemsData().text_content,
ConstraintSpace().GetWritingMode());
}
// Even if we have something in-flow, it may just be empty items that
// shouldn't trigger creation of a line. Exit now if that's the case.
if (line_info->IsEmptyLine()) {
......@@ -444,7 +459,8 @@ void NGInlineLayoutAlgorithm::PlaceControlItem(const NGInlineItem& item,
box->EnsureTextMetrics(*item.Style(), baseline_type_);
NGTextFragmentBuilder text_builder(ConstraintSpace().GetWritingMode());
text_builder.SetItem(line_info.ItemsData(), item_result, box->text_height);
text_builder.SetItem(line_info.ItemsData().text_content, item_result,
box->text_height);
text_builder.SetIsFirstForNode(item_result->IsFirstForNode());
line_box_.AddChild(text_builder.ToTextFragment(), box->text_top,
item_result->inline_size, item.BidiLevel());
......@@ -1126,7 +1142,9 @@ void NGInlineLayoutAlgorithm::BidiReorder(TextDirection base_direction) {
visual_items.ReserveInitialCapacity(line_box_.size());
for (unsigned logical_index : indices_in_visual_order) {
visual_items.AddChild(std::move(line_box_[logical_index]));
DCHECK(!line_box_[logical_index].HasInFlowFragment());
DCHECK(!line_box_[logical_index].HasInFlowFragment() ||
// |item_result| will not be null by moving.
line_box_[logical_index].item_result);
}
DCHECK_EQ(line_box_.size(), visual_items.size());
line_box_ = std::move(visual_items);
......
......@@ -6,8 +6,10 @@
#include "third_party/blink/renderer/core/layout/ng/exclusions/ng_exclusion_space.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_break_token.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_item_result.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_node.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_physical_line_box_fragment.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_text_fragment_builder.h"
#include "third_party/blink/renderer/core/layout/ng/ng_fragment.h"
#include "third_party/blink/renderer/core/layout/ng/ng_layout_result.h"
#include "third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.h"
......@@ -38,6 +40,27 @@ void NGLineBoxFragmentBuilder::SetIsEmptyLineBox() {
line_box_type_ = NGPhysicalLineBoxFragment::kEmptyLineBox;
}
void NGLineBoxFragmentBuilder::ChildList::CreateTextFragments(
WritingMode writing_mode,
const String& text_content) {
NGTextFragmentBuilder text_builder(writing_mode);
for (auto& child : *this) {
if (NGInlineItemResult* item_result = child.item_result) {
DCHECK(item_result->item);
const NGInlineItem& item = *item_result->item;
DCHECK(item.Type() == NGInlineItem::kText ||
item.Type() == NGInlineItem::kControl);
DCHECK(item.TextType() == NGTextType::kNormal ||
item.TextType() == NGTextType::kSymbolMarker);
text_builder.SetIsFirstForNode(item_result->IsFirstForNode());
text_builder.SetItem(text_content, item_result,
child.rect.size.block_size);
DCHECK(!child.fragment);
child.fragment = text_builder.ToTextFragment();
}
}
}
NGLineBoxFragmentBuilder::Child*
NGLineBoxFragmentBuilder::ChildList::FirstInFlowChild() {
for (auto& child : *this) {
......
......@@ -21,6 +21,7 @@ namespace blink {
class ComputedStyle;
class NGInlineBreakToken;
struct NGInlineItemResult;
class CORE_EXPORT NGLineBoxFragmentBuilder final
: public NGContainerFragmentBuilder {
......@@ -75,9 +76,12 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final
struct Child {
DISALLOW_NEW();
scoped_refptr<NGFragmentItem> fragment_item;
scoped_refptr<const NGLayoutResult> layout_result;
scoped_refptr<const NGPhysicalTextFragment> fragment;
const NGInlineItem* inline_item = nullptr;
// |NGInlineItemResult| to create a text fragment from.
NGInlineItemResult* item_result = nullptr;
LayoutObject* out_of_flow_positioned_box = nullptr;
LayoutObject* unpositioned_float = nullptr;
// The offset of the border box, initially in this child coordinate system.
......@@ -134,7 +138,16 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final
inline_size(inline_size),
children_count(children_count),
bidi_level(bidi_level) {}
// Create an in-flow |NGPhysicalTextFragment|.
// Create an in-flow text fragment.
Child(NGInlineItemResult* item_result,
LayoutUnit block_offset,
LayoutUnit inline_size,
LayoutUnit text_height,
UBiDiLevel bidi_level)
: item_result(item_result),
rect(LayoutUnit(), block_offset, LayoutUnit(), text_height),
inline_size(inline_size),
bidi_level(bidi_level) {}
Child(scoped_refptr<const NGPhysicalTextFragment> fragment,
LogicalOffset offset,
LayoutUnit inline_size,
......@@ -170,9 +183,12 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final
bidi_level(bidi_level) {}
bool HasInFlowFragment() const {
if (fragment_item)
return true;
if (fragment)
return true;
if (item_result)
return true;
if (layout_result && !layout_result->PhysicalFragment().IsFloating())
return true;
......@@ -280,6 +296,10 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final
void MoveInBlockDirection(LayoutUnit);
void MoveInBlockDirection(LayoutUnit, unsigned start, unsigned end);
// Create |NGPhysicalTextFragment| for all text children.
void CreateTextFragments(WritingMode writing_mode,
const String& text_content);
private:
void WillInsertChild(unsigned index);
......
......@@ -22,7 +22,7 @@ NGTextFragmentBuilder::NGTextFragmentBuilder(
shape_result_(fragment.TextShapeResult()),
text_type_(fragment.TextType()) {}
void NGTextFragmentBuilder::SetItem(const NGInlineItemsData& items_data,
void NGTextFragmentBuilder::SetItem(const String& text_content,
NGInlineItemResult* item_result,
LayoutUnit line_height) {
DCHECK(item_result);
......@@ -33,7 +33,7 @@ void NGTextFragmentBuilder::SetItem(const NGInlineItemsData& items_data,
DCHECK(item->Style());
text_type_ = item->TextType();
text_ = items_data.text_content;
text_ = text_content;
start_offset_ = item_result->start_offset;
end_offset_ = item_result->end_offset;
resolved_direction_ = item->Direction();
......
......@@ -29,7 +29,7 @@ class CORE_EXPORT NGTextFragmentBuilder final : public NGFragmentBuilder {
TextDirection ResolvedDirection() const { return resolved_direction_; }
// NOTE: Takes ownership of the shape result within the item result.
void SetItem(const NGInlineItemsData&,
void SetItem(const String& text_content,
NGInlineItemResult*,
LayoutUnit line_height);
......
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