Skip to content

Commit

Permalink
[IFC][Geometry] Annotation BoxGeometry should be all logical
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=267500

Reviewed by Antti Koivisto.

Last step of converting BoxGeometry coords to all logical.

While annotation box geometry is logical, the associated display box is (always) visual.
Also overlap checking (as part of the overhanging process) needs to happen in virtual coords.

* Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp:
(WebCore::Layout::InlineDisplayContentBuilder::processRubyBase):
(WebCore::Layout::InlineDisplayContentBuilder::processRubyContent):
(WebCore::Layout::InlineDisplayContentBuilder::applyRubyOverhang):
* Source/WebCore/layout/formattingContexts/inline/ruby/RubyFormattingContext.cpp:
(WebCore::Layout::RubyFormattingContext::placeAnnotationBox):
(WebCore::Layout::RubyFormattingContext::sizeAnnotationBox):
* Source/WebCore/layout/formattingContexts/inline/ruby/RubyFormattingContext.h:
* Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp:
(WebCore::LayoutIntegration::LineLayout::updateRenderTreePositions):

Canonical link: https://commits.webkit.org/273027@main
  • Loading branch information
alanbaradlay committed Jan 14, 2024
1 parent 75f3768 commit f4658f2
Show file tree
Hide file tree
Showing 8 changed files with 174 additions and 192 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ layer at (0,0) size 800x600
RenderInline (generated) at (0,0) size 16x18
RenderText {#text} at (190,22) size 10x18
text run at (190,22) width 10: "T"
RenderBlock {RT} at (187,12) size 17x10
RenderBlock {RT} at (187,12) size 16x10
RenderText {#text} at (0,0) size 16x10
text run at (0,0) width 16: "Text"
RenderInline (generated) at (0,0) size 29x18
Expand Down
2 changes: 1 addition & 1 deletion LayoutTests/platform/mac/fast/ruby/ruby-runs-expected.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ layer at (0,0) size 800x600
RenderInline (generated) at (0,0) size 16x18
RenderText {#text} at (178,10) size 10x18
text run at (178,10) width 10: "T"
RenderBlock {RT} at (175,0) size 17x10
RenderBlock {RT} at (175,0) size 16x10
RenderText {#text} at (0,0) size 16x10
text run at (0,0) width 16: "Text"
RenderInline (generated) at (0,0) size 29x18
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,8 @@ static inline void shiftDisplayBox(InlineDisplay::Box& displayBox, InlineLayoutU
return;
auto isHorizontalWritingMode = inlineFormattingContext.root().style().isHorizontalWritingMode();
isHorizontalWritingMode ? displayBox.moveHorizontally(offset) : displayBox.moveVertically(offset);
if (!displayBox.isTextOrSoftLineBreak() && !displayBox.isRootInlineBox()) {
auto& boxGeometry = inlineFormattingContext.geometryForBox(displayBox.layoutBox());
isHorizontalWritingMode ? boxGeometry.moveHorizontally(LayoutUnit { offset }) : boxGeometry.moveVertically(LayoutUnit { offset });
}
if (!displayBox.isTextOrSoftLineBreak() && !displayBox.isRootInlineBox())
inlineFormattingContext.geometryForBox(displayBox.layoutBox()).moveHorizontally(LayoutUnit { offset });
}

static inline void expandInlineBox(InlineLayoutUnit expansion, InlineDisplay::Box& displayBox, InlineFormattingContext& inlineFormattingContext)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1044,8 +1044,7 @@ size_t InlineDisplayContentBuilder::processRubyBase(size_t rubyBaseStart, Inline
auto& rubyBaseDisplayBox = displayBoxes[rubyBaseStart];
auto& rubyBaseLayoutBox = rubyBaseDisplayBox.layoutBox();
ASSERT(rubyBaseDisplayBox.isInlineBox());
// FIXME: This is temporary and will be converted to logical.
auto baseBorderBoxVisualRect = Rect { rubyBaseDisplayBox.visualRectIgnoringBlockDirection() };
auto baseBorderBoxLogicalRect = BoxGeometry::borderBoxRect(formattingContext.geometryForBox(rubyBaseLayoutBox));

auto* annotationBox = rubyBaseLayoutBox.associatedRubyAnnotationBox();
if (annotationBox)
Expand All @@ -1069,7 +1068,7 @@ size_t InlineDisplayContentBuilder::processRubyBase(size_t rubyBaseStart, Inline
};
if (isNestedRubyBase()) {
auto nestedAnnotationMarginBoxRect = BoxGeometry::marginBoxRect(formattingContext.geometryForBox(interlinearAnnotationBox));
baseBorderBoxVisualRect.expandToContain(nestedAnnotationMarginBoxRect);
baseBorderBoxLogicalRect.expandToContain(nestedAnnotationMarginBoxRect);
}
}

Expand All @@ -1094,14 +1093,14 @@ size_t InlineDisplayContentBuilder::processRubyBase(size_t rubyBaseStart, Inline
if (RubyFormattingContext::hasInterCharacterAnnotation(rubyBaseLayoutBox)) {
auto letterSpacing = LayoutUnit { rubyBaseLayoutBox.style().letterSpacing() };
// FIXME: Consult the LineBox to see if letter spacing indeed applies.
baseBorderBoxVisualRect.setWidth(std::max(0_lu, baseBorderBoxVisualRect.width() - letterSpacing));
baseBorderBoxLogicalRect.setWidth(std::max(0_lu, baseBorderBoxLogicalRect.width() - letterSpacing));
}

auto visualBorderBoxTopLeft = RubyFormattingContext::placeAnnotationBox(rubyBaseLayoutBox, baseBorderBoxVisualRect, formattingContext);
auto visualContentBoxSize = RubyFormattingContext::sizeAnnotationBox(rubyBaseLayoutBox, baseBorderBoxVisualRect, formattingContext);
auto borderBoxLogicalTopLeft = RubyFormattingContext::placeAnnotationBox(rubyBaseLayoutBox, baseBorderBoxLogicalRect, formattingContext);
auto contentBoxLogicalSize = RubyFormattingContext::sizeAnnotationBox(rubyBaseLayoutBox, baseBorderBoxLogicalRect, formattingContext);
auto& annotationBoxGeometry = formattingContext.geometryForBox(*annotationBox);
annotationBoxGeometry.setTopLeft(toLayoutPoint(visualBorderBoxTopLeft));
annotationBoxGeometry.setContentBoxSize(toLayoutSize(visualContentBoxSize));
annotationBoxGeometry.setTopLeft(toLayoutPoint(borderBoxLogicalTopLeft));
annotationBoxGeometry.setContentBoxSize(toLayoutSize(contentBoxLogicalSize));
};
placeAndSizeAnnotationBox();
}
Expand Down Expand Up @@ -1140,10 +1139,19 @@ void InlineDisplayContentBuilder::processRubyContent(InlineDisplay::Boxes& displ
}
applyRubyOverhang(displayBoxes, interlinearRubyColumnRangeList);

auto lineBoxLogicalRect = lineBox().logicalRect();
auto writingMode = root().style().writingMode();
auto isHorizontalWritingMode = WebCore::isHorizontalWritingMode(writingMode);
for (auto baseIndex : makeReversedRange(rubyBaseStartIndexListWithAnnotation)) {
auto& annotationBox = *displayBoxes[baseIndex].layoutBox().associatedRubyAnnotationBox();
auto visualBorderBoxRect = BoxGeometry::borderBoxRect(formattingContext().geometryForBox(annotationBox));
insertRubyAnnotationBox(annotationBox, baseIndex + 1, { visualBorderBoxRect }, displayBoxes);
auto annotationBorderBoxVisualRect = [&] {
auto borderBoxLogicalRect = InlineRect { BoxGeometry::borderBoxRect(formattingContext().geometryForBox(annotationBox)) };
borderBoxLogicalRect.setTop(borderBoxLogicalRect.top() - lineBoxLogicalRect.top());
auto visualRect = flipLogicalRectToVisualForWritingModeWithinLine(borderBoxLogicalRect, lineBoxLogicalRect, writingMode);
isHorizontalWritingMode ? visualRect.moveVertically(lineBoxLogicalRect.top()) : visualRect.moveHorizontally(lineBoxLogicalRect.top());
return visualRect;
};
insertRubyAnnotationBox(annotationBox, baseIndex + 1, annotationBorderBoxVisualRect(), displayBoxes);
}
}

Expand Down Expand Up @@ -1173,18 +1181,14 @@ void InlineDisplayContentBuilder::applyRubyOverhang(InlineDisplay::Boxes& displa
auto moveBoxRangeToVisualLeft = [&](auto start, auto end, auto shiftValue) {
for (auto index = start; index <= end; ++index) {
auto& displayBox = displayBoxes[index];
auto& layoutBox = displayBox.layoutBox();
isHorizontalWritingMode ? displayBox.moveHorizontally(-shiftValue) : displayBox.moveVertically(-shiftValue);

auto& layoutBox = displayBox.layoutBox();
if (displayBox.isInlineLevelBox() && !displayBox.isRootInlineBox())
formattingContext.geometryForBox(layoutBox).moveHorizontally(LayoutUnit { -shiftValue });

auto updateAnnotationGeometryIfNeeded = [&] {
if (!layoutBox.isRubyBase() || !layoutBox.associatedRubyAnnotationBox())
return;
auto& annotationBoxGeometry = formattingContext.geometryForBox(*layoutBox.associatedRubyAnnotationBox());
isHorizontalWritingMode ? annotationBoxGeometry.moveHorizontally(LayoutUnit { -shiftValue }) : annotationBoxGeometry.moveVertically(LayoutUnit { -shiftValue });
};
updateAnnotationGeometryIfNeeded();
if (layoutBox.isRubyBase() && layoutBox.associatedRubyAnnotationBox())
formattingContext.geometryForBox(*layoutBox.associatedRubyAnnotationBox()).moveHorizontally(LayoutUnit { -shiftValue });
}
};
if (beforeOverhang)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ static InlineLayoutRect visualRectIncludingBlockDirection(const InlineLayoutRect
return flippedRect;
}

static inline InlineRect annotationMarginBoxVisualRect(const Box& annotationBox, const InlineFormattingContext& inlineFormattingContext)
{
auto marginBoxLogicalRect = InlineRect { BoxGeometry::marginBoxRect(inlineFormattingContext.geometryForBox(annotationBox)) };
if (inlineFormattingContext.root().style().isHorizontalWritingMode())
return marginBoxLogicalRect;
return InlineRect { marginBoxLogicalRect.topLeft().transposedPoint(), marginBoxLogicalRect.size().transposedSize() };
}

static InlineLayoutUnit baseLogicalWidthFromRubyBaseEnd(const Box& rubyBaseLayoutBox, const Line::RunList& lineRuns, const InlineContentBreaker::ContinuousContent::RunList& candidateRuns)
{
ASSERT(rubyBaseLayoutBox.isRubyBase());
Expand Down Expand Up @@ -107,8 +115,7 @@ static bool annotationOverlapCheck(const InlineDisplay::Box& adjacentDisplayBox,
auto& adjacentLayoutBox = adjacentDisplayBox.layoutBox();
// Adjacent ruby may have overlapping annotation.
if (adjacentLayoutBox.isRubyBase() && adjacentLayoutBox.associatedRubyAnnotationBox()) {
auto annotationMarginBoxVisualRect = InlineLayoutRect { BoxGeometry::marginBoxRect(inlineFormattingContext.geometryForBox(*adjacentLayoutBox.associatedRubyAnnotationBox())) };
if (visualRectIncludingBlockDirection(annotationMarginBoxVisualRect, rootStyle).intersects(visualRectIncludingBlockDirection(overhangingRect, rootStyle)))
if (visualRectIncludingBlockDirection(annotationMarginBoxVisualRect(*adjacentLayoutBox.associatedRubyAnnotationBox(), inlineFormattingContext), rootStyle).intersects(visualRectIncludingBlockDirection(overhangingRect, rootStyle)))
return true;
}
return false;
Expand Down Expand Up @@ -319,7 +326,7 @@ InlineLayoutUnit RubyFormattingContext::baseEndAdditionalLogicalWidth(const Box&
return annotationBoxLogicalWidth(rubyBaseLayoutBox, inlineFormattingContext);
}

InlineLayoutPoint RubyFormattingContext::placeAnnotationBox(const Box& rubyBaseLayoutBox, const Rect& rubyBaseMarginBoxVisualRect, const InlineFormattingContext& inlineFormattingContext)
InlineLayoutPoint RubyFormattingContext::placeAnnotationBox(const Box& rubyBaseLayoutBox, const Rect& rubyBaseMarginBoxLogicalRect, const InlineFormattingContext& inlineFormattingContext)
{
ASSERT(rubyBaseLayoutBox.isRubyBase());
auto* annotationBox = rubyBaseLayoutBox.associatedRubyAnnotationBox();
Expand All @@ -332,39 +339,19 @@ InlineLayoutPoint RubyFormattingContext::placeAnnotationBox(const Box& rubyBaseL
if (hasInterlinearAnnotation(rubyBaseLayoutBox)) {
// Move it over/under the base and make it border box positioned.
auto leftOffset = annotationBoxLogicalGeometry.marginStart();
auto position = rubyPosition(rubyBaseLayoutBox);
auto topOffset = position == RubyPosition::Before ? -annotationBoxLogicalGeometry.marginBoxHeight() : rubyBaseMarginBoxVisualRect.height();
switch (writingModeToBlockFlowDirection(rubyBaseLayoutBox.style().writingMode())) {
case BlockFlowDirection::TopToBottom:
break;
case BlockFlowDirection::BottomToTop:
topOffset = position == RubyPosition::Before ? rubyBaseMarginBoxVisualRect.height() : -annotationBoxLogicalGeometry.marginBoxHeight();
break;
case BlockFlowDirection::RightToLeft:
topOffset = leftOffset;
leftOffset = position == RubyPosition::Before ? -annotationBoxLogicalGeometry.marginBoxHeight() : rubyBaseMarginBoxVisualRect.width();
break;
case BlockFlowDirection::LeftToRight:
topOffset = leftOffset;
leftOffset = position == RubyPosition::Before ? rubyBaseMarginBoxVisualRect.width() : -annotationBoxLogicalGeometry.marginBoxHeight();
break;
default:
ASSERT_NOT_REACHED();
break;
}
auto visualTopLeft = rubyBaseMarginBoxVisualRect.topLeft();
visualTopLeft.move(LayoutSize { leftOffset, topOffset });
return visualTopLeft;
auto topOffset = rubyPosition(rubyBaseLayoutBox) == RubyPosition::Before ? -annotationBoxLogicalGeometry.marginBoxHeight() : rubyBaseMarginBoxLogicalRect.height();
auto logicalTopLeft = rubyBaseMarginBoxLogicalRect.topLeft();
logicalTopLeft.move(LayoutSize { leftOffset, topOffset });
return logicalTopLeft;
}
// Inter-character annotation box is stretched to the size of the base content box and vertically centered.
auto isHorizontalWritingMode = rubyBaseLayoutBox.style().isHorizontalWritingMode();
auto annotationContentBoxVisualHeight = isHorizontalWritingMode ? annotationBoxLogicalGeometry.contentBoxHeight() : annotationBoxLogicalGeometry.contentBoxWidth();
auto annotationBorderTop = isHorizontalWritingMode ? annotationBoxLogicalGeometry.borderBefore() : annotationBoxLogicalGeometry.borderStart();
auto borderBoxRight = rubyBaseMarginBoxVisualRect.right() - annotationBoxLogicalGeometry.marginBoxWidth() + annotationBoxLogicalGeometry.marginStart();
return { borderBoxRight, rubyBaseMarginBoxVisualRect.top() + ((rubyBaseMarginBoxVisualRect.height() - annotationContentBoxVisualHeight) / 2) - annotationBorderTop };
auto annotationContentBoxLogicalHeight = annotationBoxLogicalGeometry.contentBoxHeight();
auto annotationBorderTop = annotationBoxLogicalGeometry.borderBefore();
auto borderBoxRight = rubyBaseMarginBoxLogicalRect.right() - annotationBoxLogicalGeometry.marginBoxWidth() + annotationBoxLogicalGeometry.marginStart();
return { borderBoxRight, rubyBaseMarginBoxLogicalRect.top() + ((rubyBaseMarginBoxLogicalRect.height() - annotationContentBoxLogicalHeight) / 2) - annotationBorderTop };
}

InlineLayoutSize RubyFormattingContext::sizeAnnotationBox(const Box& rubyBaseLayoutBox, const Rect& rubyBaseMarginBoxVisualRect, const InlineFormattingContext& inlineFormattingContext)
InlineLayoutSize RubyFormattingContext::sizeAnnotationBox(const Box& rubyBaseLayoutBox, const Rect& rubyBaseMarginBoxLogicalRect, const InlineFormattingContext& inlineFormattingContext)
{
// FIXME: This is where we should take advantage of the ruby-column setup.
ASSERT(rubyBaseLayoutBox.isRubyBase());
Expand All @@ -373,15 +360,9 @@ InlineLayoutSize RubyFormattingContext::sizeAnnotationBox(const Box& rubyBaseLay
ASSERT_NOT_REACHED();
return { };
}
auto isHorizontalWritingMode = rubyBaseLayoutBox.style().isHorizontalWritingMode();
auto& annotationBoxLogicalGeometry = inlineFormattingContext.geometryForBox(*annotationBox);

if (hasInterlinearAnnotation(rubyBaseLayoutBox)) {
if (isHorizontalWritingMode)
return { std::max(rubyBaseMarginBoxVisualRect.width(), annotationBoxLogicalGeometry.marginBoxWidth()) - annotationBoxLogicalGeometry.horizontalMarginBorderAndPadding(), annotationBoxLogicalGeometry.contentBoxHeight() };
return { annotationBoxLogicalGeometry.contentBoxHeight(), std::max(annotationBoxLogicalGeometry.marginBoxWidth(), rubyBaseMarginBoxVisualRect.height()) - annotationBoxLogicalGeometry.horizontalMarginBorderAndPadding() };
}

if (hasInterlinearAnnotation(rubyBaseLayoutBox))
return { std::max(rubyBaseMarginBoxLogicalRect.width(), annotationBoxLogicalGeometry.marginBoxWidth()) - annotationBoxLogicalGeometry.horizontalMarginBorderAndPadding(), annotationBoxLogicalGeometry.contentBoxHeight() };
return annotationBoxLogicalGeometry.contentBoxSize();
}

Expand Down Expand Up @@ -464,8 +445,7 @@ InlineLayoutUnit RubyFormattingContext::overhangForAnnotationBefore(const Box& r
ASSERT_NOT_REACHED();
return { };
}

auto isHorizontalWritingMode = rubyBaseLayoutBox.style().isHorizontalWritingMode();
auto isHorizontalWritingMode = inlineFormattingContext.root().style().isHorizontalWritingMode();
auto baseContentStart = baseContentIndex(rubyBaseStart, boxes);
if (baseContentStart >= boxes.size()) {
ASSERT_NOT_REACHED();
Expand All @@ -482,16 +462,16 @@ InlineLayoutUnit RubyFormattingContext::overhangForAnnotationBefore(const Box& r
auto overhangValue = std::min(halfOfAFullWidthCharacter(*annotationBox), gapBetweenBaseAndContent());
auto wouldAnnotationOrBaseOverlapAdjacentContent = [&] {
// Check of adjacent (previous) content for overlapping.
auto overhangingAnnotationRect = InlineLayoutRect { BoxGeometry::marginBoxRect(inlineFormattingContext.geometryForBox(*annotationBox)) };
auto overhangingAnnotationVisualRect = annotationMarginBoxVisualRect(*annotationBox, inlineFormattingContext);
auto baseContentBoxRect = boxes[baseContentStart].inkOverflow();
// This is how much the annotation box/ base content would be closer to content outside of base.
auto offset = isHorizontalWritingMode ? LayoutSize(-overhangValue, 0.f) : LayoutSize(0.f, -overhangValue);
overhangingAnnotationRect.move(offset);
baseContentBoxRect.move(offset);
// This is how much the annotation box/base content would be closer to content outside of base.
auto offset = isHorizontalWritingMode ? InlineLayoutPoint(-overhangValue, 0.f) : InlineLayoutPoint(0.f, -overhangValue);
overhangingAnnotationVisualRect.moveBy(offset);
baseContentBoxRect.moveBy(offset);

for (size_t index = 1; index < rubyBaseStart - 1; ++index) {
auto& previousDisplayBox = boxes[index];
if (annotationOverlapCheck(previousDisplayBox, overhangingAnnotationRect, inlineFormattingContext))
if (annotationOverlapCheck(previousDisplayBox, overhangingAnnotationVisualRect, inlineFormattingContext))
return true;
if (annotationOverlapCheck(previousDisplayBox, baseContentBoxRect, inlineFormattingContext))
return true;
Expand All @@ -510,7 +490,7 @@ InlineLayoutUnit RubyFormattingContext::overhangForAnnotationAfter(const Box& ru
if (!rubyBaseRange || rubyBaseRange.distance() == 1 || rubyBaseRange.end() == boxes.size())
return { };

auto isHorizontalWritingMode = rubyBaseLayoutBox.style().isHorizontalWritingMode();
auto isHorizontalWritingMode = inlineFormattingContext.root().style().isHorizontalWritingMode();
// FIXME: Usually the last content box is visually the rightmost, but negative margin may override it.
// FIXME: Currently justified content always expands producing 0 value for gapBetweenBaseEndAndContent.
auto rubyBaseContentEnd = rubyBaseRange.end() - 1;
Expand All @@ -524,16 +504,16 @@ InlineLayoutUnit RubyFormattingContext::overhangForAnnotationAfter(const Box& ru
auto overhangValue = std::min(halfOfAFullWidthCharacter(*annotationBox), gapBetweenBaseEndAndContent());
auto wouldAnnotationOrBaseOverlapLineContent = [&] {
// Check of adjacent (next) content for overlapping.
auto overhangingAnnotationRect = InlineLayoutRect { BoxGeometry::marginBoxRect(inlineFormattingContext.geometryForBox(*annotationBox)) };
auto overhangingAnnotationVisualRect = annotationMarginBoxVisualRect(*annotationBox, inlineFormattingContext);
auto baseContentBoxRect = boxes[rubyBaseContentEnd].inkOverflow();
// This is how much the base content would be closer to content outside of base.
auto offset = isHorizontalWritingMode ? LayoutSize(overhangValue, 0.f) : LayoutSize(0.f, overhangValue);
overhangingAnnotationRect.move(offset);
baseContentBoxRect.move(offset);
auto offset = isHorizontalWritingMode ? InlineLayoutPoint(overhangValue, 0.f) : InlineLayoutPoint(0.f, overhangValue);
overhangingAnnotationVisualRect.moveBy(offset);
baseContentBoxRect.moveBy(offset);

for (size_t index = boxes.size() - 1; index >= rubyBaseRange.end(); --index) {
auto& previousDisplayBox = boxes[index];
if (annotationOverlapCheck(previousDisplayBox, overhangingAnnotationRect, inlineFormattingContext))
if (annotationOverlapCheck(previousDisplayBox, overhangingAnnotationVisualRect, inlineFormattingContext))
return true;
if (annotationOverlapCheck(previousDisplayBox, baseContentBoxRect, inlineFormattingContext))
return true;
Expand Down
Loading

0 comments on commit f4658f2

Please sign in to comment.