Skip to content

Commit

Permalink
[IFC][Ruby] Annotation overhanging fails in vertical-lr mode
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=267821

Reviewed by Antti Koivisto.

1. Input rectangles to overlap checking should share the same coordinate system.
2. However it does not have to be the final visual rect.

Let's use the display box's native coordinate system (before flipping) to check whether annotation can overlap adjacent content.

* LayoutTests/fast/ruby/simple-overhang-vertical-lr-expected.html: Added.
* LayoutTests/fast/ruby/simple-overhang-vertical-lr.html: Added.
* Source/WebCore/layout/formattingContexts/inline/InlineRect.h:
(WebCore::Layout::InlineRect::intersects):
* Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp:
(WebCore::Layout::InlineDisplayContentBuilder::applyRubyOverhang):
* Source/WebCore/layout/formattingContexts/inline/ruby/RubyFormattingContext.cpp:
(WebCore::Layout::annotationMarginBoxVisualRect):
(WebCore::Layout::annotationOverlapCheck):
(WebCore::Layout::RubyFormattingContext::overhangForAnnotationBefore):
(WebCore::Layout::RubyFormattingContext::overhangForAnnotationAfter):
(WebCore::Layout::visualRectIncludingBlockDirection): Deleted.
* Source/WebCore/layout/formattingContexts/inline/ruby/RubyFormattingContext.h:

Canonical link: https://commits.webkit.org/273280@main
  • Loading branch information
alanbaradlay committed Jan 22, 2024
1 parent f6f45d4 commit 177e6f3
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 32 deletions.
13 changes: 13 additions & 0 deletions LayoutTests/fast/ruby/simple-overhang-vertical-lr-expected.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<style>
rt {
font-size: 20px;
color: transparent;
}

.container {
font-family: Ahem;
font-size: 20px;
writing-mode: vertical-rl;
}
</style>
<div class=container>X<ruby>base<rt>annotation</rt></ruby>X</div>
13 changes: 13 additions & 0 deletions LayoutTests/fast/ruby/simple-overhang-vertical-lr.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<style>
rt {
font-size: 20px;
color: transparent;
}

.container {
font-family: Ahem;
font-size: 20px;
writing-mode: vertical-lr;
}
</style>
<div class=container>X<ruby>base<rt>annotation</rt></ruby>X</div>
2 changes: 2 additions & 0 deletions Source/WebCore/layout/formattingContexts/inline/InlineRect.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ class InlineRect {
void inflate(InlineLayoutUnit);
void inflate(InlineLayoutUnit top, InlineLayoutUnit right, InlineLayoutUnit bottom, InlineLayoutUnit left);

bool intersects(const InlineRect other) { return m_rect.intersects(other); }

bool isEmpty() const;

operator InlineLayoutRect() const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1162,6 +1162,7 @@ void InlineDisplayContentBuilder::applyRubyOverhang(InlineDisplay::Boxes& displa
return;

auto isHorizontalWritingMode = root().style().isHorizontalWritingMode();
auto lineLogicalHeight = lineBox().logicalRect().height();
auto& formattingContext = this->formattingContext();
for (auto startEndPair : interlinearRubyColumnRangeList) {
ASSERT(startEndPair);
Expand All @@ -1173,8 +1174,8 @@ void InlineDisplayContentBuilder::applyRubyOverhang(InlineDisplay::Boxes& displa
ASSERT(rubyBaseLayoutBox.isRubyBase());
ASSERT(RubyFormattingContext::hasInterlinearAnnotation(rubyBaseLayoutBox));

auto beforeOverhang = RubyFormattingContext::overhangForAnnotationBefore(rubyBaseLayoutBox, rubyBaseStart, displayBoxes, formattingContext);
auto afterOverhang = RubyFormattingContext::overhangForAnnotationAfter(rubyBaseLayoutBox, { rubyBaseStart, startEndPair.end() }, displayBoxes, formattingContext);
auto beforeOverhang = RubyFormattingContext::overhangForAnnotationBefore(rubyBaseLayoutBox, rubyBaseStart, displayBoxes, lineLogicalHeight, formattingContext);
auto afterOverhang = RubyFormattingContext::overhangForAnnotationAfter(rubyBaseLayoutBox, { rubyBaseStart, startEndPair.end() }, displayBoxes, lineLogicalHeight, formattingContext);

// FIXME: If this turns out to be a pref bottleneck, make sure we pass in the accumulated shift to overhangForAnnotationBefore/after and
// offset all box geometry as we check for overlap.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,17 @@ static RubyPosition rubyPosition(const Box& rubyBaseLayoutBox)
return computedRubyPosition == RubyPosition::InterCharacter ? RubyPosition::Before : computedRubyPosition;
}

static InlineLayoutRect visualRectIncludingBlockDirection(const InlineLayoutRect& visualRectIgnoringBlockDirection, const RenderStyle& rootStyle)
{
if (!rootStyle.isFlippedLinesWritingMode())
return visualRectIgnoringBlockDirection;

auto flippedRect = visualRectIgnoringBlockDirection;
rootStyle.isVerticalWritingMode() ? flippedRect.setX(flippedRect.x() - flippedRect.width()) : flippedRect.setY(flippedRect.y() - flippedRect.height());
return flippedRect;
}

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

static InlineLayoutUnit baseLogicalWidthFromRubyBaseEnd(const Box& rubyBaseLayoutBox, const Line::RunList& lineRuns, const InlineContentBreaker::ContinuousContent::RunList& candidateRuns)
Expand Down Expand Up @@ -101,23 +96,20 @@ static InlineLayoutUnit baseLogicalWidthFromRubyBaseEnd(const Box& rubyBaseLayou
return baseLogicalWidth;
}

static bool annotationOverlapCheck(const InlineDisplay::Box& adjacentDisplayBox, const InlineLayoutRect& overhangingRect, const InlineFormattingContext& inlineFormattingContext)
static bool annotationOverlapCheck(const InlineDisplay::Box& adjacentDisplayBox, const InlineLayoutRect& overhangingRect, InlineLayoutUnit lineLogicalHeight, const InlineFormattingContext& inlineFormattingContext)
{
// We are in the middle of a line, should not see any line breaks or ellipsis boxes here.
ASSERT(!adjacentDisplayBox.isEllipsis() && !adjacentDisplayBox.isRootInlineBox());
// Skip empty content like <span></span>
if (adjacentDisplayBox.visualRectIgnoringBlockDirection().isEmpty())
return false;

auto& rootStyle = inlineFormattingContext.root().style();
if (visualRectIncludingBlockDirection(adjacentDisplayBox.inkOverflow(), rootStyle).intersects(visualRectIncludingBlockDirection(overhangingRect, rootStyle)))
if (adjacentDisplayBox.inkOverflow().intersects(overhangingRect))
return true;
auto& adjacentLayoutBox = adjacentDisplayBox.layoutBox();
// Adjacent ruby may have overlapping annotation.
if (adjacentLayoutBox.isRubyBase() && adjacentLayoutBox.associatedRubyAnnotationBox()) {
if (visualRectIncludingBlockDirection(annotationMarginBoxVisualRect(*adjacentLayoutBox.associatedRubyAnnotationBox(), inlineFormattingContext), rootStyle).intersects(visualRectIncludingBlockDirection(overhangingRect, rootStyle)))
return true;
}
if (adjacentLayoutBox.isRubyBase() && adjacentLayoutBox.associatedRubyAnnotationBox())
return annotationMarginBoxVisualRect(*adjacentLayoutBox.associatedRubyAnnotationBox(), lineLogicalHeight, inlineFormattingContext).intersects(overhangingRect);
return false;
}

Expand Down Expand Up @@ -452,7 +444,7 @@ void RubyFormattingContext::applyAnnotationContributionToLayoutBounds(LineBox& l
}
}

InlineLayoutUnit RubyFormattingContext::overhangForAnnotationBefore(const Box& rubyBaseLayoutBox, size_t rubyBaseStart, const InlineDisplay::Boxes& boxes, const InlineFormattingContext& inlineFormattingContext)
InlineLayoutUnit RubyFormattingContext::overhangForAnnotationBefore(const Box& rubyBaseLayoutBox, size_t rubyBaseStart, const InlineDisplay::Boxes& boxes, InlineLayoutUnit lineLogicalHeight, const InlineFormattingContext& inlineFormattingContext)
{
// [root inline box][ruby container][ruby base][ruby annotation]
ASSERT(rubyBaseStart >= 2);
Expand Down Expand Up @@ -481,7 +473,7 @@ InlineLayoutUnit RubyFormattingContext::overhangForAnnotationBefore(const Box& r
auto overhangValue = std::min(halfOfAFullWidthCharacter(*annotationBox), gapBetweenBaseAndContent());
auto wouldAnnotationOrBaseOverlapAdjacentContent = [&] {
// Check of adjacent (previous) content for overlapping.
auto overhangingAnnotationVisualRect = annotationMarginBoxVisualRect(*annotationBox, inlineFormattingContext);
auto overhangingAnnotationVisualRect = annotationMarginBoxVisualRect(*annotationBox, lineLogicalHeight, 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 ? InlineLayoutPoint(-overhangValue, 0.f) : InlineLayoutPoint(0.f, -overhangValue);
Expand All @@ -490,17 +482,17 @@ InlineLayoutUnit RubyFormattingContext::overhangForAnnotationBefore(const Box& r

for (size_t index = 1; index < rubyBaseStart - 1; ++index) {
auto& previousDisplayBox = boxes[index];
if (annotationOverlapCheck(previousDisplayBox, overhangingAnnotationVisualRect, inlineFormattingContext))
if (annotationOverlapCheck(previousDisplayBox, overhangingAnnotationVisualRect, lineLogicalHeight, inlineFormattingContext))
return true;
if (annotationOverlapCheck(previousDisplayBox, baseContentBoxRect, inlineFormattingContext))
if (annotationOverlapCheck(previousDisplayBox, baseContentBoxRect, lineLogicalHeight, inlineFormattingContext))
return true;
}
return false;
};
return wouldAnnotationOrBaseOverlapAdjacentContent() ? 0.f : overhangValue;
}

InlineLayoutUnit RubyFormattingContext::overhangForAnnotationAfter(const Box& rubyBaseLayoutBox, WTF::Range<size_t> rubyBaseRange, const InlineDisplay::Boxes& boxes, const InlineFormattingContext& inlineFormattingContext)
InlineLayoutUnit RubyFormattingContext::overhangForAnnotationAfter(const Box& rubyBaseLayoutBox, WTF::Range<size_t> rubyBaseRange, const InlineDisplay::Boxes& boxes, InlineLayoutUnit lineLogicalHeight, const InlineFormattingContext& inlineFormattingContext)
{
auto* annotationBox = rubyBaseLayoutBox.associatedRubyAnnotationBox();
if (!annotationBox || !hasInterlinearAnnotation(rubyBaseLayoutBox))
Expand All @@ -523,7 +515,7 @@ InlineLayoutUnit RubyFormattingContext::overhangForAnnotationAfter(const Box& ru
auto overhangValue = std::min(halfOfAFullWidthCharacter(*annotationBox), gapBetweenBaseEndAndContent());
auto wouldAnnotationOrBaseOverlapLineContent = [&] {
// Check of adjacent (next) content for overlapping.
auto overhangingAnnotationVisualRect = annotationMarginBoxVisualRect(*annotationBox, inlineFormattingContext);
auto overhangingAnnotationVisualRect = annotationMarginBoxVisualRect(*annotationBox, lineLogicalHeight, inlineFormattingContext);
auto baseContentBoxRect = boxes[rubyBaseContentEnd].inkOverflow();
// This is how much the base content would be closer to content outside of base.
auto offset = isHorizontalWritingMode ? InlineLayoutPoint(overhangValue, 0.f) : InlineLayoutPoint(0.f, overhangValue);
Expand All @@ -532,9 +524,9 @@ InlineLayoutUnit RubyFormattingContext::overhangForAnnotationAfter(const Box& ru

for (size_t index = boxes.size() - 1; index >= rubyBaseRange.end(); --index) {
auto& previousDisplayBox = boxes[index];
if (annotationOverlapCheck(previousDisplayBox, overhangingAnnotationVisualRect, inlineFormattingContext))
if (annotationOverlapCheck(previousDisplayBox, overhangingAnnotationVisualRect, lineLogicalHeight, inlineFormattingContext))
return true;
if (annotationOverlapCheck(previousDisplayBox, baseContentBoxRect, inlineFormattingContext))
if (annotationOverlapCheck(previousDisplayBox, baseContentBoxRect, lineLogicalHeight, inlineFormattingContext))
return true;
}
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ class RubyFormattingContext {
static InlineLayoutPoint placeAnnotationBox(const Box& rubyBaseLayoutBox, const Rect& rubyBaseMarginBoxLogicalRect, const InlineFormattingContext&);
static InlineLayoutSize sizeAnnotationBox(const Box& rubyBaseLayoutBox, const Rect& rubyBaseMarginBoxLogicalRect, const InlineFormattingContext&);

static InlineLayoutUnit overhangForAnnotationBefore(const Box& rubyBaseLayoutBox, size_t rubyBaseStart, const InlineDisplay::Boxes&, const InlineFormattingContext&);
static InlineLayoutUnit overhangForAnnotationAfter(const Box& rubyBaseLayoutBox, WTF::Range<size_t> rubyBaseRange, const InlineDisplay::Boxes&, const InlineFormattingContext&);
static InlineLayoutUnit overhangForAnnotationBefore(const Box& rubyBaseLayoutBox, size_t rubyBaseStart, const InlineDisplay::Boxes&, InlineLayoutUnit lineLogicalHeight, const InlineFormattingContext&);
static InlineLayoutUnit overhangForAnnotationAfter(const Box& rubyBaseLayoutBox, WTF::Range<size_t> rubyBaseRange, const InlineDisplay::Boxes&, InlineLayoutUnit lineLogicalHeight, const InlineFormattingContext&);

enum class RubyBasesMayNeedResizing : bool { No, Yes };
static void applyAlignmentOffsetList(InlineDisplay::Boxes&, const HashMap<const Box*, InlineLayoutUnit>& alignmentOffsetList, RubyBasesMayNeedResizing, InlineFormattingContext&);
Expand Down

0 comments on commit 177e6f3

Please sign in to comment.