Commit d9f39476 authored by Kent Tamura's avatar Kent Tamura Committed by Commit Bot

RubyNG: Implement line space sharing for block-end ruby annotation

If a line has block-end annotation overflow, we add the overflow amount
to logical_block_offset as ever.
(See NGBlockLayoutAlgorithm::ComputeInflowPosition())

Then, we adjust logical_block_offset if
  - The next line has block-start annotation space.
    (See NGInlineLayoutAlgorithm::CreateLine())
  - The block container has block-end padding.
    (See NGBlockLayoutAlgorithm::Layout())

The block-end annotation overflow value is passed to the next line via
NGConstraintSpace::BlockStartAnnotationSpace().

Bug: 1069817
Change-Id: I019b982783232e744abb2e709399ded4b7e9a644
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2249238
Commit-Queue: Kent Tamura <tkent@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779649}
parent 6bdc968e
...@@ -434,10 +434,23 @@ void NGInlineLayoutAlgorithm::CreateLine( ...@@ -434,10 +434,23 @@ void NGInlineLayoutAlgorithm::CreateLine(
annotation_block_start = annotation_metrics.descent; annotation_block_start = annotation_metrics.descent;
annotation_block_end = annotation_metrics.ascent; annotation_block_end = annotation_metrics.ascent;
} }
// Borrow the bottom space of the previous line.
LayoutUnit block_offset_shift = annotation_block_start.ClampNegativeToZero();
// If the previous line has block-end annotation overflow and this line has
// block-start annotation space, shift up the block offset of this line.
if (ConstraintSpace().BlockStartAnnotationSpace() < LayoutUnit() &&
annotation_block_start < LayoutUnit()) {
const LayoutUnit overflow = -ConstraintSpace().BlockStartAnnotationSpace();
const LayoutUnit space = -annotation_block_start;
block_offset_shift = -std::min(space, overflow);
}
// If this line has block-start annotation overflow and the previous line has
// block-end annotation space, borrow the block-end space of the previous line
// and shift down the block offset by |overflow - space|.
if (annotation_block_start > LayoutUnit() && if (annotation_block_start > LayoutUnit() &&
ConstraintSpace().BlockStartAnnotationSpace() > LayoutUnit()) { ConstraintSpace().BlockStartAnnotationSpace() > LayoutUnit()) {
annotation_block_start = block_offset_shift =
(annotation_block_start - ConstraintSpace().BlockStartAnnotationSpace()) (annotation_block_start - ConstraintSpace().BlockStartAnnotationSpace())
.ClampNegativeToZero(); .ClampNegativeToZero();
} }
...@@ -447,9 +460,8 @@ void NGInlineLayoutAlgorithm::CreateLine( ...@@ -447,9 +460,8 @@ void NGInlineLayoutAlgorithm::CreateLine(
container_builder_.SetBaseDirection(line_info->BaseDirection()); container_builder_.SetBaseDirection(line_info->BaseDirection());
container_builder_.SetInlineSize(inline_size); container_builder_.SetInlineSize(inline_size);
container_builder_.SetMetrics(line_box_metrics); container_builder_.SetMetrics(line_box_metrics);
container_builder_.SetBfcBlockOffset( container_builder_.SetBfcBlockOffset(line_info->BfcOffset().block_offset +
line_info->BfcOffset().block_offset + block_offset_shift);
annotation_block_start.ClampNegativeToZero());
if (annotation_block_end > LayoutUnit()) if (annotation_block_end > LayoutUnit())
container_builder_.SetAnnotationOverflow(annotation_block_end); container_builder_.SetAnnotationOverflow(annotation_block_end);
else if (annotation_block_end < LayoutUnit()) else if (annotation_block_end < LayoutUnit())
......
...@@ -730,6 +730,19 @@ inline scoped_refptr<const NGLayoutResult> NGBlockLayoutAlgorithm::Layout( ...@@ -730,6 +730,19 @@ inline scoped_refptr<const NGLayoutResult> NGBlockLayoutAlgorithm::Layout(
Node().GetLayoutBox()->LogicalHeightForEmptyLine()); Node().GetLayoutBox()->LogicalHeightForEmptyLine());
} }
// Collapse annotation overflow and padding.
// logical_block_offset already contains block-end annotation overflow.
// However, if the container has non-zero block-end padding, the annotation
// can extend on the padding. So we decrease logical_block_offset by
// shareable part of the annotation overflow and the padding.
if (previous_inflow_position.block_end_annotation_space < LayoutUnit()) {
DCHECK(RuntimeEnabledFeatures::LayoutNGRubyEnabled());
const LayoutUnit annotation_overflow =
-previous_inflow_position.block_end_annotation_space;
previous_inflow_position.logical_block_offset -=
std::min(container_builder_.Padding().block_end, annotation_overflow);
}
// To save space of the stack when we recurse into children, the rest of this // To save space of the stack when we recurse into children, the rest of this
// function is continued within |FinishLayout|. However it should be read as // function is continued within |FinishLayout|. However it should be read as
// one function. // one function.
...@@ -1980,9 +1993,13 @@ NGPreviousInflowPosition NGBlockLayoutAlgorithm::ComputeInflowPosition( ...@@ -1980,9 +1993,13 @@ NGPreviousInflowPosition NGBlockLayoutAlgorithm::ComputeInflowPosition(
if (!container_builder_.BfcBlockOffset()) if (!container_builder_.BfcBlockOffset())
DCHECK_EQ(logical_block_offset, LayoutUnit()); DCHECK_EQ(logical_block_offset, LayoutUnit());
} else { } else {
// TODO(crbug.com/1069817): We should not add AnnotationOverflow // We add AnnotationOverflow unconditionally here. Then, we cancel it if
// unconditionally here. Instead, we should pass it to the next child // - The next line box has block-start annotation space, or
// layout. // - There are no following child boxes and this container has block-end
// padding.
//
// See NGInlineLayoutAlgorithm::CreateLine() and
// BlockLayoutAlgorithm::Layout().
logical_block_offset = logical_offset.block_offset + fragment.BlockSize() + logical_block_offset = logical_offset.block_offset + fragment.BlockSize() +
layout_result.AnnotationOverflow(); layout_result.AnnotationOverflow();
} }
...@@ -2014,8 +2031,13 @@ NGPreviousInflowPosition NGBlockLayoutAlgorithm::ComputeInflowPosition( ...@@ -2014,8 +2031,13 @@ NGPreviousInflowPosition NGBlockLayoutAlgorithm::ComputeInflowPosition(
(previous_inflow_position.self_collapsing_child_had_clearance && (previous_inflow_position.self_collapsing_child_had_clearance &&
is_self_collapsing); is_self_collapsing);
return {logical_block_offset, margin_strut, LayoutUnit annotation_space = layout_result.BlockEndAnnotationSpace();
layout_result.BlockEndAnnotationSpace(), if (layout_result.AnnotationOverflow() > LayoutUnit()) {
DCHECK(!annotation_space);
annotation_space = -layout_result.AnnotationOverflow();
}
return {logical_block_offset, margin_strut, annotation_space,
self_or_sibling_self_collapsing_child_had_clearance}; self_or_sibling_self_collapsing_child_had_clearance};
} }
......
...@@ -32,6 +32,8 @@ class NGFragment; ...@@ -32,6 +32,8 @@ class NGFragment;
struct NGPreviousInflowPosition { struct NGPreviousInflowPosition {
LayoutUnit logical_block_offset; LayoutUnit logical_block_offset;
NGMarginStrut margin_strut; NGMarginStrut margin_strut;
// > 0: Block-end annotation space of the previous line
// < 0: Block-end annotation overflow of the previous line
LayoutUnit block_end_annotation_space; LayoutUnit block_end_annotation_space;
bool self_collapsing_child_had_clearance; bool self_collapsing_child_had_clearance;
}; };
......
...@@ -463,6 +463,8 @@ class CORE_EXPORT NGConstraintSpace final { ...@@ -463,6 +463,8 @@ class CORE_EXPORT NGConstraintSpace final {
// The amount of available space for block-start side annotation. // The amount of available space for block-start side annotation.
// For the first box, this is the padding-block-start value of the container. // For the first box, this is the padding-block-start value of the container.
// Otherwise, this comes from NGLayoutResult::BlockEndAnnotationSpace(). // Otherwise, this comes from NGLayoutResult::BlockEndAnnotationSpace().
// If the value is negative, it's block-end annotation overflow of the
// previous box.
LayoutUnit BlockStartAnnotationSpace() const { LayoutUnit BlockStartAnnotationSpace() const {
return HasRareData() ? rare_data_->BlockStartAnnotationSpace() return HasRareData() ? rare_data_->BlockStartAnnotationSpace()
: LayoutUnit(); : LayoutUnit();
......
This is a testharness.js-based test.
PASS Over ruby doesn't overflow the block
PASS Over ruby + vertical-align doesn't overflow the block
PASS Under ruby doesn't overflow the block
PASS Under ruby + vertical-align doesn't overflow the block
PASS Under ruby doesn't overwrap with the next block
PASS Expand inter-lines spacing
PASS Consume half-leading of the previous line
PASS Don't Consume half-leading of the previous line with text-emphasis
FAIL Consume half-leading of the next line assert_approx_equals: expected 28 +/- 1 but got 24
PASS Don't Consume half-leading of the next line with text-emphasis
Harness: the test ran to completion.
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