Commit c7a4e70d authored by kojii's avatar kojii Committed by Commit bot

[LayoutNG] Fix empty inlines to influence the used line height

This patch matches NGInlineLayoutAlgorithm to CSS2 10.8 Line height
calculations: the 'line-height' and 'vertical-align' properties[1]
defining empty inline elements should influence line height.

Also, since the introduction of NGInlineBoxState, we had two places to
compute the union of boxes; NGInlineBoxState.stack_[0] and
NGLineBoxFragmentBuilder. Some code were uniting boxes to wrong one.
This patch unifies them to NGInlineBoxState.

[1] https://drafts.csswg.org/css2/visudet.html#line-height

BUG=636993

Review-Url: https://codereview.chromium.org/2845493002
Cr-Commit-Position: refs/heads/master@{#467278}
parent b58fb795
...@@ -826,6 +826,7 @@ crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-repl ...@@ -826,6 +826,7 @@ crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-repl
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-006.xht [ Skip ] crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-006.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-008.xht [ Skip ] crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-008.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-009.xht [ Skip ] crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-009.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-011.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-012.xht [ Skip ] crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-012.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-013.xht [ Skip ] crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-013.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-014.xht [ Skip ] crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-014.xht [ Skip ]
...@@ -1287,7 +1288,6 @@ crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/border-padding- ...@@ -1287,7 +1288,6 @@ crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/border-padding-
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/border-padding-bleed-002.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/border-padding-bleed-002.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/border-padding-bleed-003.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/border-padding-bleed-003.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/empty-inline-002.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/empty-inline-002.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/empty-inline-003.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-001.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-001.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-002.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-002.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-003.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-003.xht [ Skip ]
...@@ -1305,7 +1305,6 @@ crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatti ...@@ -1305,7 +1305,6 @@ crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatti
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-002.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-002.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-004.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-004.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-005.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-005.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-applies-to-012.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-013.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-013.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-015.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-015.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-016.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-016.xht [ Skip ]
...@@ -1335,7 +1334,6 @@ crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-103 ...@@ -1335,7 +1334,6 @@ crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-103
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-104.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-104.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-125.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-125.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-126.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-126.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-127.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-128.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-128.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-129.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-129.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-bleed-001.xht [ Skip ] crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-bleed-001.xht [ Skip ]
......
...@@ -11,9 +11,8 @@ ...@@ -11,9 +11,8 @@
namespace blink { namespace blink {
void NGInlineBoxState::ComputeTextMetrics(const NGInlineItem& item, void NGInlineBoxState::ComputeTextMetrics(const ComputedStyle& style,
FontBaseline baseline_type) { FontBaseline baseline_type) {
const ComputedStyle& style = *item.Style();
text_metrics = NGLineHeightMetrics(style, baseline_type); text_metrics = NGLineHeightMetrics(style, baseline_type);
text_top = -text_metrics.ascent; text_top = -text_metrics.ascent;
text_metrics.AddLeading(style.ComputedLineHeightAsFixed()); text_metrics.AddLeading(style.ComputedLineHeightAsFixed());
...@@ -22,23 +21,47 @@ void NGInlineBoxState::ComputeTextMetrics(const NGInlineItem& item, ...@@ -22,23 +21,47 @@ void NGInlineBoxState::ComputeTextMetrics(const NGInlineItem& item,
include_used_fonts = style.LineHeight().IsNegative(); include_used_fonts = style.LineHeight().IsNegative();
} }
void NGInlineBoxState::AccumulateUsedFonts(const NGInlineItem& item,
unsigned start,
unsigned end,
FontBaseline baseline_type) {
HashSet<const SimpleFontData*> fallback_fonts;
item.GetFallbackFonts(&fallback_fonts, start, end);
for (const auto& fallback_font : fallback_fonts) {
NGLineHeightMetrics fallback_metrics(fallback_font->GetFontMetrics(),
baseline_type);
fallback_metrics.AddLeading(
fallback_font->GetFontMetrics().FixedLineSpacing());
metrics.Unite(fallback_metrics);
}
}
NGInlineBoxState* NGInlineLayoutStateStack::OnBeginPlaceItems( NGInlineBoxState* NGInlineLayoutStateStack::OnBeginPlaceItems(
const ComputedStyle* line_style) { const ComputedStyle* line_style,
FontBaseline baseline_type) {
if (stack_.IsEmpty()) { if (stack_.IsEmpty()) {
// For the first line, push a box state for the line itself. // For the first line, push a box state for the line itself.
stack_.Resize(1); stack_.Resize(1);
NGInlineBoxState* box = &stack_.back(); NGInlineBoxState* box = &stack_.back();
box->fragment_start = 0; box->fragment_start = 0;
box->style = line_style; } else {
return box; // For the following lines, clear states that are not shared across lines.
for (auto& box : stack_) {
box.fragment_start = 0;
box.metrics = NGLineHeightMetrics();
DCHECK(box.pending_descendants.IsEmpty());
}
} }
// For the following lines, clear states that are not shared across lines. // Initialize the box state for the line box.
for (auto& box : stack_) { NGInlineBoxState& line_box = LineBoxState();
box.fragment_start = 0; line_box.style = line_style;
box.metrics = NGLineHeightMetrics();
DCHECK(box.pending_descendants.IsEmpty()); // Use a "strut" (a zero-width inline box with the element's font and
} // line height properties) as the initial metrics for the line box.
// https://drafts.csswg.org/css2/visudet.html#strut
line_box.ComputeTextMetrics(*line_style, baseline_type);
return &stack_.back(); return &stack_.back();
} }
...@@ -72,7 +95,9 @@ void NGInlineLayoutStateStack::OnEndPlaceItems( ...@@ -72,7 +95,9 @@ void NGInlineLayoutStateStack::OnEndPlaceItems(
NGInlineBoxState* box = &(*it); NGInlineBoxState* box = &(*it);
EndBoxState(box, line_box); EndBoxState(box, line_box);
} }
line_box->UniteMetrics(stack_.front().metrics);
DCHECK(!LineBoxState().metrics.IsEmpty());
line_box->SetMetrics(LineBoxState().metrics);
} }
void NGInlineLayoutStateStack::EndBoxState(NGInlineBoxState* box, void NGInlineLayoutStateStack::EndBoxState(NGInlineBoxState* box,
......
...@@ -40,7 +40,11 @@ struct NGInlineBoxState { ...@@ -40,7 +40,11 @@ struct NGInlineBoxState {
bool include_used_fonts = false; bool include_used_fonts = false;
// Compute text metrics for a box. All text in a box share the same metrics. // Compute text metrics for a box. All text in a box share the same metrics.
void ComputeTextMetrics(const NGInlineItem&, FontBaseline); void ComputeTextMetrics(const ComputedStyle& style, FontBaseline);
void AccumulateUsedFonts(const NGInlineItem&,
unsigned start,
unsigned end,
FontBaseline);
}; };
// Represents the inline tree structure. This class provides: // Represents the inline tree structure. This class provides:
...@@ -49,9 +53,12 @@ struct NGInlineBoxState { ...@@ -49,9 +53,12 @@ struct NGInlineBoxState {
// 3) Cache common values for a box. // 3) Cache common values for a box.
class NGInlineLayoutStateStack { class NGInlineLayoutStateStack {
public: public:
// The box state for the line box.
NGInlineBoxState& LineBoxState() { return stack_.front(); }
// Initialize the box state stack for a new line. // Initialize the box state stack for a new line.
// @return The initial box state for the line. // @return The initial box state for the line.
NGInlineBoxState* OnBeginPlaceItems(const ComputedStyle*); NGInlineBoxState* OnBeginPlaceItems(const ComputedStyle*, FontBaseline);
// Push a box state stack. // Push a box state stack.
NGInlineBoxState* OnOpenTag(const NGInlineItem&, NGInlineBoxState* OnOpenTag(const NGInlineItem&,
......
...@@ -394,35 +394,40 @@ bool NGInlineLayoutAlgorithm::PlaceItems( ...@@ -394,35 +394,40 @@ bool NGInlineLayoutAlgorithm::PlaceItems(
const Vector<LineItemChunk, 32>& line_item_chunks) { const Vector<LineItemChunk, 32>& line_item_chunks) {
const Vector<NGInlineItem>& items = Node()->Items(); const Vector<NGInlineItem>& items = Node()->Items();
// Use a "strut" (a zero-width inline box with the element's font and
// line height properties) as the initial metrics for the line box.
// https://drafts.csswg.org/css2/visudet.html#strut
const ComputedStyle& line_style = LineStyle(); const ComputedStyle& line_style = LineStyle();
NGLineHeightMetrics line_metrics(line_style, baseline_type_); NGLineHeightMetrics line_metrics(line_style, baseline_type_);
NGLineHeightMetrics line_metrics_with_leading = line_metrics; NGLineHeightMetrics line_metrics_with_leading = line_metrics;
line_metrics_with_leading.AddLeading(line_style.ComputedLineHeightAsFixed()); line_metrics_with_leading.AddLeading(line_style.ComputedLineHeightAsFixed());
NGLineBoxFragmentBuilder line_box(Node(), line_metrics_with_leading); NGLineBoxFragmentBuilder line_box(Node());
// Compute heights of all inline items by placing the dominant baseline at 0. // 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. // The baseline is adjusted after the height of the line box is computed.
NGTextFragmentBuilder text_builder(Node()); NGTextFragmentBuilder text_builder(Node());
NGInlineBoxState* box = box_states_.OnBeginPlaceItems(&LineStyle()); NGInlineBoxState* box =
box_states_.OnBeginPlaceItems(&LineStyle(), baseline_type_);
LayoutUnit inline_size; LayoutUnit inline_size;
for (const auto& line_item_chunk : line_item_chunks) { for (const auto& line_item_chunk : line_item_chunks) {
const NGInlineItem& item = items[line_item_chunk.index]; const NGInlineItem& item = items[line_item_chunk.index];
LayoutUnit line_top; LayoutUnit line_top;
if (item.Type() == NGInlineItem::kText) { if (item.Type() == NGInlineItem::kText) {
DCHECK(item.GetLayoutObject()->IsText()); DCHECK(item.GetLayoutObject()->IsText());
if (box->text_metrics.IsEmpty()) DCHECK(!box->text_metrics.IsEmpty());
box->ComputeTextMetrics(item, baseline_type_);
line_top = box->text_top; line_top = box->text_top;
text_builder.SetSize( text_builder.SetSize(
{line_item_chunk.inline_size, box->text_metrics.LineHeight()}); {line_item_chunk.inline_size, box->text_metrics.LineHeight()});
// Take all used fonts into account if 'line-height: normal'. // Take all used fonts into account if 'line-height: normal'.
if (box->include_used_fonts) if (box->include_used_fonts) {
AccumulateUsedFonts(item, line_item_chunk, &line_box); box->AccumulateUsedFonts(item, line_item_chunk.start_offset,
line_item_chunk.end_offset, baseline_type_);
}
} else if (item.Type() == NGInlineItem::kOpenTag) { } else if (item.Type() == NGInlineItem::kOpenTag) {
box = box_states_.OnOpenTag(item, &line_box, &text_builder); box = box_states_.OnOpenTag(item, &line_box, &text_builder);
// Compute text metrics for all inline boxes since even empty inlines
// influence the line height.
// https://drafts.csswg.org/css2/visudet.html#line-height
// TODO(kojii): Review if atomic inline level should have open/close.
if (!item.GetLayoutObject()->IsAtomicInlineLevel())
box->ComputeTextMetrics(*item.Style(), baseline_type_);
continue; continue;
} else if (item.Type() == NGInlineItem::kCloseTag) { } else if (item.Type() == NGInlineItem::kCloseTag) {
box = box_states_.OnCloseTag(item, &line_box, box); box = box_states_.OnCloseTag(item, &line_box, box);
...@@ -491,29 +496,15 @@ bool NGInlineLayoutAlgorithm::PlaceItems( ...@@ -491,29 +496,15 @@ bool NGInlineLayoutAlgorithm::PlaceItems(
// the line box to the line top. // the line box to the line top.
line_box.MoveChildrenInBlockDirection(baseline); line_box.MoveChildrenInBlockDirection(baseline);
line_box.SetInlineSize(inline_size); line_box.SetInlineSize(inline_size);
container_builder_.AddChild(line_box.ToLineBoxFragment(), container_builder_.AddChild(
{LayoutUnit(), baseline - line_metrics.ascent}); line_box.ToLineBoxFragment(),
{LayoutUnit(), baseline + box_states_.LineBoxState().text_top});
max_inline_size_ = std::max(max_inline_size_, inline_size); max_inline_size_ = std::max(max_inline_size_, inline_size);
content_size_ = line_bottom; content_size_ = line_bottom;
return true; return true;
} }
void NGInlineLayoutAlgorithm::AccumulateUsedFonts(
const NGInlineItem& item,
const LineItemChunk& line_item_chunk,
NGLineBoxFragmentBuilder* line_box) {
HashSet<const SimpleFontData*> fallback_fonts;
item.GetFallbackFonts(&fallback_fonts, line_item_chunk.start_offset,
line_item_chunk.end_offset);
for (const auto& fallback_font : fallback_fonts) {
NGLineHeightMetrics metrics(fallback_font->GetFontMetrics(),
baseline_type_);
metrics.AddLeading(fallback_font->GetFontMetrics().FixedLineSpacing());
line_box->UniteMetrics(metrics);
}
}
LayoutUnit NGInlineLayoutAlgorithm::PlaceAtomicInline( LayoutUnit NGInlineLayoutAlgorithm::PlaceAtomicInline(
const NGInlineItem& item, const NGInlineItem& item,
NGLineBoxFragmentBuilder* line_box, NGLineBoxFragmentBuilder* line_box,
......
...@@ -13,10 +13,8 @@ ...@@ -13,10 +13,8 @@
namespace blink { namespace blink {
NGLineBoxFragmentBuilder::NGLineBoxFragmentBuilder( NGLineBoxFragmentBuilder::NGLineBoxFragmentBuilder(NGInlineNode* node)
NGInlineNode* node, : direction_(TextDirection::kLtr), node_(node) {}
const NGLineHeightMetrics& metrics)
: direction_(TextDirection::kLtr), node_(node), metrics_(metrics) {}
NGLineBoxFragmentBuilder& NGLineBoxFragmentBuilder::SetDirection( NGLineBoxFragmentBuilder& NGLineBoxFragmentBuilder::SetDirection(
TextDirection direction) { TextDirection direction) {
...@@ -51,9 +49,8 @@ void NGLineBoxFragmentBuilder::MoveChildrenInBlockDirection(LayoutUnit delta, ...@@ -51,9 +49,8 @@ void NGLineBoxFragmentBuilder::MoveChildrenInBlockDirection(LayoutUnit delta,
offsets_[index].block_offset += delta; offsets_[index].block_offset += delta;
} }
void NGLineBoxFragmentBuilder::UniteMetrics( void NGLineBoxFragmentBuilder::SetMetrics(const NGLineHeightMetrics& metrics) {
const NGLineHeightMetrics& metrics) { metrics_ = metrics;
metrics_.Unite(metrics);
} }
void NGLineBoxFragmentBuilder::SetBreakToken( void NGLineBoxFragmentBuilder::SetBreakToken(
......
...@@ -20,7 +20,7 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final { ...@@ -20,7 +20,7 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final {
STACK_ALLOCATED(); STACK_ALLOCATED();
public: public:
NGLineBoxFragmentBuilder(NGInlineNode*, const NGLineHeightMetrics&); explicit NGLineBoxFragmentBuilder(NGInlineNode*);
NGLineBoxFragmentBuilder& SetDirection(TextDirection); NGLineBoxFragmentBuilder& SetDirection(TextDirection);
...@@ -35,7 +35,7 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final { ...@@ -35,7 +35,7 @@ class CORE_EXPORT NGLineBoxFragmentBuilder final {
return children_; return children_;
} }
void UniteMetrics(const NGLineHeightMetrics&); void SetMetrics(const NGLineHeightMetrics&);
const NGLineHeightMetrics& Metrics() const { return metrics_; } const NGLineHeightMetrics& Metrics() const { return metrics_; }
// Set the break token for the fragment to build. // Set the break token for the fragment to build.
......
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