Skip to content

GitLab

T
tangled
Commit 08f33075 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot
Browse files
Options

[LayoutNG] Fix NGInlineNode::ComputeMinMaxSize when hyphenated 

This patch changes NGInlineNode::ComputeMinMaxSize to use
NGLineInfo::Width() instead of accumulating the list of
NGInlineItemResult.

This fixes min-/max-content size to include the width of
hyphens.

During the fix, an uninitialized variable was found when
NGLineInfo was re-used across different NextLine(). Re-using
NGLineInfo isn't much gain, this was prohibited to avoid rare
code path. Also a few DCHECK were added to ensure accumulated
with is in sync.

A test for min-content of hyphened line was added. There are
two tests, but both use table to compute it, and that they
may fail to test if table sizing is broken. The new test uses
'min-content' directly.

Bug: 636993
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Ibc7a3a2dcbf6a70a06bc974eb69dd47b23ad72fe
Reviewed-on: https://chromium-review.googlesource.com/1111895
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569954}
parent 65c91617
Hide whitespace changes
Inline Side-by-side
Showing with 50 additions and 23 deletions
third_party/WebKit/LayoutTests/fast/text/hyphens/soft-hyphen-min-content-expected.html 0 → 100644
View file @ 08f33075
<!DOCTYPE html>
<style>
div {
width: min-content;
border: 5px blue solid;
}
</style>
<div>0123-<br>45</div>
third_party/WebKit/LayoutTests/fast/text/hyphens/soft-hyphen-min-content.html 0 → 100644
View file @ 08f33075
<!DOCTYPE html>
<style>
div {
width: min-content;
border: 5px blue solid;
}
</style>
<div>0123&shy;45</div>
third_party/blink/renderer/core/layout/ng/inline/ng_inline_item_result.cc
View file @ 08f33075
...@@ -61,6 +61,20 @@ void NGInlineItemResult::CheckConsistency(bool during_line_break) const { ...@@ -61,6 +61,20 @@ void NGInlineItemResult::CheckConsistency(bool during_line_break) const {
} }
#endif #endif
LayoutUnit NGLineInfo::ComputeWidth() const {
LayoutUnit inline_size = TextIndent();
for (const NGInlineItemResult& item_result : Results())
inline_size += item_result.inline_size;
if (UNLIKELY(line_end_fragment_)) {
inline_size += line_end_fragment_->Size()
.ConvertToLogical(LineStyle().GetWritingMode())
.inline_size;
}
return inline_size;
}
void NGLineInfo::SetLineBfcOffset(NGBfcOffset line_bfc_offset, void NGLineInfo::SetLineBfcOffset(NGBfcOffset line_bfc_offset,
LayoutUnit available_width, LayoutUnit available_width,
LayoutUnit width) { LayoutUnit width) {
......
third_party/blink/renderer/core/layout/ng/inline/ng_inline_item_result.h
View file @ 08f33075
...@@ -105,9 +105,6 @@ class CORE_EXPORT NGLineInfo { ...@@ -105,9 +105,6 @@ class CORE_EXPORT NGLineInfo {
DISALLOW_NEW_EXCEPT_PLACEMENT_NEW(); DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
public: public:
NGLineInfo() = default;
explicit NGLineInfo(size_t capacity) : results_(capacity) {}
const NGInlineItemsData& ItemsData() const { const NGInlineItemsData& ItemsData() const {
DCHECK(items_data_); DCHECK(items_data_);
return *items_data_; return *items_data_;
...@@ -152,6 +149,7 @@ class CORE_EXPORT NGLineInfo { ...@@ -152,6 +149,7 @@ class CORE_EXPORT NGLineInfo {
NGBfcOffset LineBfcOffset() const { return line_bfc_offset_; } NGBfcOffset LineBfcOffset() const { return line_bfc_offset_; }
LayoutUnit AvailableWidth() const { return available_width_; } LayoutUnit AvailableWidth() const { return available_width_; }
LayoutUnit Width() const { return width_; } LayoutUnit Width() const { return width_; }
LayoutUnit ComputeWidth() const;
void SetLineBfcOffset(NGBfcOffset line_bfc_offset, void SetLineBfcOffset(NGBfcOffset line_bfc_offset,
LayoutUnit available_width, LayoutUnit available_width,
LayoutUnit width); LayoutUnit width);
......
third_party/blink/renderer/core/layout/ng/inline/ng_inline_node.cc
View file @ 08f33075
...@@ -670,7 +670,6 @@ static LayoutUnit ComputeContentSize(NGInlineNode node, ...@@ -670,7 +670,6 @@ static LayoutUnit ComputeContentSize(NGInlineNode node,
Vector<scoped_refptr<NGUnpositionedFloat>> unpositioned_floats; Vector<scoped_refptr<NGUnpositionedFloat>> unpositioned_floats;
scoped_refptr<NGInlineBreakToken> break_token; scoped_refptr<NGInlineBreakToken> break_token;
NGLineInfo line_info;
NGExclusionSpace empty_exclusion_space; NGExclusionSpace empty_exclusion_space;
NGLineLayoutOpportunity line_opportunity(available_inline_size); NGLineLayoutOpportunity line_opportunity(available_inline_size);
LayoutUnit result; LayoutUnit result;
...@@ -683,13 +682,13 @@ static LayoutUnit ComputeContentSize(NGInlineNode node, ...@@ -683,13 +682,13 @@ static LayoutUnit ComputeContentSize(NGInlineNode node,
&unpositioned_floats, &unpositioned_floats,
nullptr /* container_builder */, nullptr /* container_builder */,
&empty_exclusion_space, 0u, break_token.get()); &empty_exclusion_space, 0u, break_token.get());
NGLineInfo line_info;
if (!line_breaker.NextLine(line_opportunity, &line_info)) if (!line_breaker.NextLine(line_opportunity, &line_info))
break; break;
break_token = line_breaker.CreateBreakToken(line_info, nullptr); break_token = line_breaker.CreateBreakToken(line_info, nullptr);
LayoutUnit inline_size = line_info.TextIndent(); LayoutUnit inline_size = line_info.Width();
for (const NGInlineItemResult& item_result : line_info.Results()) DCHECK_EQ(inline_size, line_info.ComputeWidth().ClampNegativeToZero());
inline_size += item_result.inline_size;
// There should be no positioned floats while determining the min/max sizes. // There should be no positioned floats while determining the min/max sizes.
DCHECK_EQ(positioned_floats.size(), 0u); DCHECK_EQ(positioned_floats.size(), 0u);
......
third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.cc
View file @ 08f33075
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.h" #include "third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.h"
#include "build/build_config.h"
#include "third_party/blink/renderer/core/layout/layout_list_marker.h" #include "third_party/blink/renderer/core/layout/layout_list_marker.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_bidi_paragraph.h" #include "third_party/blink/renderer/core/layout/ng/inline/ng_bidi_paragraph.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_break_token.h"
...@@ -161,8 +162,10 @@ void NGLineBreaker::ComputeBaseDirection(const NGLineInfo& line_info) { ...@@ -161,8 +162,10 @@ void NGLineBreaker::ComputeBaseDirection(const NGLineInfo& line_info) {
void NGLineBreaker::PrepareNextLine( void NGLineBreaker::PrepareNextLine(
const NGLineLayoutOpportunity& line_opportunity, const NGLineLayoutOpportunity& line_opportunity,
NGLineInfo* line_info) { NGLineInfo* line_info) {
NGInlineItemResults* item_results = &line_info->Results(); // NGLineInfo is not supposed to be re-used becase it's not much gain and to
item_results->clear(); // avoid rare code path.
DCHECK(line_info->Results().IsEmpty());
line_info->SetStartOffset(offset_); line_info->SetStartOffset(offset_);
line_info->SetLineStyle( line_info->SetLineStyle(
node_, items_data_, constraint_space_, line_.is_first_formatted_line, node_, items_data_, constraint_space_, line_.is_first_formatted_line,
...@@ -291,19 +294,20 @@ void NGLineBreaker::BreakLine(NGLineInfo* line_info) { ...@@ -291,19 +294,20 @@ void NGLineBreaker::BreakLine(NGLineInfo* line_info) {
} }
} }
// Re-compute the current position from NGInlineItemResults. // Re-compute the current position from NGLineInfo.
// The current position is usually updated as NGLineBreaker builds // The current position is usually updated as NGLineBreaker builds
// NGInlineItemResults. This function re-computes it when it was lost. // NGInlineItemResults. This function re-computes it when it was lost.
void NGLineBreaker::UpdatePosition(const NGInlineItemResults& results) { void NGLineBreaker::UpdatePosition(const NGLineInfo& line_info) {
LayoutUnit position; line_.position = line_info.ComputeWidth();
for (const NGInlineItemResult& item_result : results)
position += item_result.inline_size;
line_.position = position;
} }
void NGLineBreaker::ComputeLineLocation(NGLineInfo* line_info) const { void NGLineBreaker::ComputeLineLocation(NGLineInfo* line_info) const {
LayoutUnit bfc_line_offset = line_.line_opportunity.line_left_offset; LayoutUnit bfc_line_offset = line_.line_opportunity.line_left_offset;
LayoutUnit available_width = line_.AvailableWidth(); LayoutUnit available_width = line_.AvailableWidth();
// TODO(kojii): IsBeforeAfterNonCollapsedLineWrapSpace fails only on Mac
#if !defined(OS_MACOSX)
DCHECK_EQ(line_.position, line_info->ComputeWidth());
#endif
// Negative margins can make the position negative, but the inline size is // Negative margins can make the position negative, but the inline size is
// always positive or 0. // always positive or 0.
...@@ -971,11 +975,7 @@ NGLineBreaker::LineBreakState NGLineBreaker::HandleOverflow( ...@@ -971,11 +975,7 @@ NGLineBreaker::LineBreakState NGLineBreaker::HandleOverflow(
available_width + next_width_to_rewind + item_result->inline_size; available_width + next_width_to_rewind + item_result->inline_size;
if (line_info->LineEndFragment()) if (line_info->LineEndFragment())
SetLineEndFragment(nullptr, line_info); SetLineEndFragment(nullptr, line_info);
#if DCHECK_IS_ON() DCHECK_EQ(line_.position, line_info->ComputeWidth());
LayoutUnit position_fast = line_.position;
UpdatePosition(line_info->Results());
DCHECK_EQ(line_.position, position_fast);
#endif
item_index_ = item_result->item_index; item_index_ = item_result->item_index;
offset_ = item_result->end_offset; offset_ = item_result->end_offset;
items_data_.AssertOffset(item_index_, offset_); items_data_.AssertOffset(item_index_, offset_);
...@@ -1030,7 +1030,7 @@ void NGLineBreaker::Rewind(NGLineInfo* line_info, unsigned new_end) { ...@@ -1030,7 +1030,7 @@ void NGLineBreaker::Rewind(NGLineInfo* line_info, unsigned new_end) {
item_results->Shrink(new_end); item_results->Shrink(new_end);
SetLineEndFragment(nullptr, line_info); SetLineEndFragment(nullptr, line_info);
UpdatePosition(line_info->Results()); UpdatePosition(*line_info);
} }
// Returns the LayoutObject at the current index/offset. // Returns the LayoutObject at the current index/offset.
......
third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.h
View file @ 08f33075
...@@ -117,7 +117,7 @@ class CORE_EXPORT NGLineBreaker { ...@@ -117,7 +117,7 @@ class CORE_EXPORT NGLineBreaker {
void PrepareNextLine(const NGLineLayoutOpportunity&, NGLineInfo*); void PrepareNextLine(const NGLineLayoutOpportunity&, NGLineInfo*);
void UpdatePosition(const NGInlineItemResults&); void UpdatePosition(const NGLineInfo&);
void ComputeLineLocation(NGLineInfo*) const; void ComputeLineLocation(NGLineInfo*) const;
enum class LineBreakState { enum class LineBreakState {
......
third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker_test.cc
View file @ 08f33075
...@@ -48,12 +48,12 @@ class NGLineBreakerTest : public NGBaseLayoutAlgorithmTest { ...@@ -48,12 +48,12 @@ class NGLineBreakerTest : public NGBaseLayoutAlgorithmTest {
Vector<NGInlineItemResults> lines; Vector<NGInlineItemResults> lines;
NGExclusionSpace exclusion_space; NGExclusionSpace exclusion_space;
NGLineLayoutOpportunity line_opportunity(available_width); NGLineLayoutOpportunity line_opportunity(available_width);
NGLineInfo line_info;
while (!break_token || !break_token->IsFinished()) { while (!break_token || !break_token->IsFinished()) {
NGLineBreaker line_breaker(node, NGLineBreakerMode::kContent, *space, NGLineBreaker line_breaker(node, NGLineBreakerMode::kContent, *space,
&positioned_floats, &unpositioned_floats, &positioned_floats, &unpositioned_floats,
/* container_builder */ nullptr, /* container_builder */ nullptr,
&exclusion_space, 0u, break_token.get()); &exclusion_space, 0u, break_token.get());
NGLineInfo line_info;
if (!line_breaker.NextLine(line_opportunity, &line_info)) if (!line_breaker.NextLine(line_opportunity, &line_info))
break; break;
......
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