Skip to content

Commit

Permalink
[IFC][Cleanup] Variable names should reflect whether they are logical…
Browse files Browse the repository at this point in the history
… or visual

https://bugs.webkit.org/show_bug.cgi?id=267018

Reviewed by Antti Koivisto.

We normally deal with logical coords in layout and mix them with visual coords only at display content construction.
This patch makes it clear whether a local variable holds logical or visual coords.

* Source/WebCore/layout/formattingContexts/inline/InlineFormattingContext.cpp: Let's just pass in LineBox at construction time so that we don't have to pass it around in InlineDisplayContentBuilder.
(WebCore::Layout::InlineFormattingContext::createDisplayContentForInlineContent):
* Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp:
(WebCore::Layout::InlineDisplayContentBuilder::InlineDisplayContentBuilder):
(WebCore::Layout::InlineDisplayContentBuilder::build):
(WebCore::Layout::InlineDisplayContentBuilder::appendTextDisplayBox):
(WebCore::Layout::InlineDisplayContentBuilder::appendSoftLineBreakDisplayBox):
(WebCore::Layout::InlineDisplayContentBuilder::appendHardLineBreakDisplayBox):
(WebCore::Layout::InlineDisplayContentBuilder::appendAtomicInlineLevelDisplayBox):
(WebCore::Layout::InlineDisplayContentBuilder::appendRootInlineBoxDisplayBox):
(WebCore::Layout::InlineDisplayContentBuilder::appendInlineBoxDisplayBox):
(WebCore::Layout::InlineDisplayContentBuilder::appendInlineDisplayBoxAtBidiBoundary):
(WebCore::Layout::InlineDisplayContentBuilder::insertRubyAnnotationBox):
(WebCore::Layout::InlineDisplayContentBuilder::processNonBidiContent):
(WebCore::Layout::InlineDisplayContentBuilder::adjustVisualGeometryForDisplayBox):
(WebCore::Layout::InlineDisplayContentBuilder::processBidiContent):
(WebCore::Layout::InlineDisplayContentBuilder::setGeometryForBlockLevelOutOfFlowBoxes):
* Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.h:
(WebCore::Layout::InlineDisplayContentBuilder::setGeometryForBlockLevelOutOfFlowBoxes):
(WebCore::Layout::InlineDisplayContentBuilder::lineBox const):
(WebCore::Layout::InlineDisplayContentBuilder::lineIndex const):
(WebCore::Layout::InlineDisplayContentBuilder::rootStyle const):
* Source/WebCore/layout/formattingContexts/inline/ruby/RubyFormattingContext.cpp:
(WebCore::Layout::RubyFormattingContext::baseEndAdditionalLogicalWidth):
(WebCore::Layout::RubyFormattingContext::placeAnnotationBox):
(WebCore::Layout::RubyFormattingContext::sizeAnnotationBox):
(WebCore::Layout::RubyFormattingContext::baseEndAdditionalVisualWidth): Deleted.
* Source/WebCore/layout/formattingContexts/inline/ruby/RubyFormattingContext.h:

Canonical link: https://commits.webkit.org/272609@main
  • Loading branch information
alanbaradlay committed Jan 3, 2024
1 parent aec9d2a commit 74ebfc4
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ InlineRect InlineFormattingContext::createDisplayContentForInlineContent(const L
auto numberOfLinesWithInlineContent = numberOfPreviousLinesWithInlineContent + (!lineLayoutResult.inlineContent.isEmpty() ? 1 : 0);
auto lineIsFullyTruncatedInBlockDirection = numberOfVisibleLinesAllowed && numberOfLinesWithInlineContent > *numberOfVisibleLinesAllowed;
auto displayLine = InlineDisplayLineBuilder { *this, constraints }.build(lineLayoutResult, lineBox, lineIsFullyTruncatedInBlockDirection);
auto boxes = InlineDisplayContentBuilder { *this, constraints, displayLine, lineBox.lineIndex() }.build(lineLayoutResult, lineBox);
auto boxes = InlineDisplayContentBuilder { *this, constraints, lineBox, displayLine }.build(lineLayoutResult);

auto ellipsisPolicy = InlineFormattingUtils::lineEndingEllipsisPolicy(root().style(), numberOfLinesWithInlineContent, numberOfVisibleLinesAllowed);
if (auto ellipsisRect = InlineDisplayLineBuilder::trailingEllipsisVisualRectAfterTruncation(ellipsisPolicy, displayLine, boxes, lineLayoutResult.isFirstLast.isLastLineWithInlineContent)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,26 +166,26 @@ static inline OptionSet<InlineDisplay::Box::PositionWithinInlineLevelBox> isFirs
return positionWithinInlineLevelBox;
}

InlineDisplayContentBuilder::InlineDisplayContentBuilder(InlineFormattingContext& formattingContext, const ConstraintsForInlineContent& constraints, const InlineDisplay::Line& displayLine, size_t lineIndex)
InlineDisplayContentBuilder::InlineDisplayContentBuilder(InlineFormattingContext& formattingContext, const ConstraintsForInlineContent& constraints, const LineBox& lineBox, const InlineDisplay::Line& displayLine)
: m_formattingContext(formattingContext)
, m_constraints(constraints)
, m_lineBox(lineBox)
, m_displayLine(displayLine)
, m_lineIndex(lineIndex)
{
auto& initialContainingBlockGeometry = m_formattingContext.geometryForBox(FormattingContext::initialContainingBlock(root()), InlineFormattingContext::EscapeReason::InkOverflowNeedsInitialContiningBlockForStrokeWidth);
m_initialContaingBlockSize = ceiledIntSize(LayoutSize { initialContainingBlockGeometry.contentBoxWidth(), initialContainingBlockGeometry.contentBoxHeight() });
}

InlineDisplay::Boxes InlineDisplayContentBuilder::build(const LineLayoutResult& lineLayoutResult, const LineBox& lineBox)
InlineDisplay::Boxes InlineDisplayContentBuilder::build(const LineLayoutResult& lineLayoutResult)
{
auto boxes = InlineDisplay::Boxes { };
boxes.reserveInitialCapacity(lineLayoutResult.inlineContent.size() + lineBox.nonRootInlineLevelBoxes().size() + 1);
boxes.reserveInitialCapacity(lineLayoutResult.inlineContent.size() + lineBox().nonRootInlineLevelBoxes().size() + 1);

auto contentNeedsBidiReordering = !lineLayoutResult.directionality.visualOrderList.isEmpty();
if (contentNeedsBidiReordering)
processBidiContent(lineLayoutResult, lineBox, boxes);
processBidiContent(lineLayoutResult, boxes);
else
processNonBidiContent(lineLayoutResult, lineBox, boxes);
processNonBidiContent(lineLayoutResult, boxes);
processFloatBoxes(lineLayoutResult);
processRubyContent(boxes, lineLayoutResult);

Expand Down Expand Up @@ -245,7 +245,7 @@ void InlineDisplayContentBuilder::appendTextDisplayBox(const Line::Run& lineRun,
ASSERT(lineRun.textContent() && is<InlineTextBox>(lineRun.layoutBox()));

auto& inlineTextBox = downcast<InlineTextBox>(lineRun.layoutBox());
auto& style = !m_lineIndex ? inlineTextBox.firstLineStyle() : inlineTextBox.style();
auto& style = !lineIndex() ? inlineTextBox.firstLineStyle() : inlineTextBox.style();
auto& content = inlineTextBox.content();
auto& text = lineRun.textContent();
auto isContentful = true;
Expand Down Expand Up @@ -300,7 +300,7 @@ void InlineDisplayContentBuilder::appendTextDisplayBox(const Line::Run& lineRun,
if (inlineTextBox.isCombined()) {
static auto objectReplacementCharacterString = NeverDestroyed<String> { &objectReplacementCharacter, 1 };
// The rendered text is the actual combined content, while the "original" one is blank.
boxes.append({ m_lineIndex
boxes.append({ lineIndex()
, InlineDisplay::Box::Type::Text
, inlineTextBox
, lineRun.bidiLevel()
Expand All @@ -317,7 +317,7 @@ void InlineDisplayContentBuilder::appendTextDisplayBox(const Line::Run& lineRun,
auto adjustedContentToRender = [&] {
return text->needsHyphen ? makeString(StringView(content).substring(text->start, text->length), style.hyphenString()) : String();
};
boxes.append({ m_lineIndex
boxes.append({ lineIndex()
, lineRun.isWordSeparator() ? InlineDisplay::Box::Type::WordSeparator : InlineDisplay::Box::Type::Text
, inlineTextBox
, lineRun.bidiLevel()
Expand All @@ -338,7 +338,7 @@ void InlineDisplayContentBuilder::appendSoftLineBreakDisplayBox(const Line::Run&
auto& text = lineRun.textContent();
auto isContentful = true;

boxes.append({ m_lineIndex
boxes.append({ lineIndex()
, InlineDisplay::Box::Type::SoftLineBreak
, layoutBox
, lineRun.bidiLevel()
Expand All @@ -354,7 +354,7 @@ void InlineDisplayContentBuilder::appendSoftLineBreakDisplayBox(const Line::Run&
void InlineDisplayContentBuilder::appendHardLineBreakDisplayBox(const Line::Run& lineRun, const InlineRect& lineBreakBoxRect, InlineDisplay::Boxes& boxes)
{
auto isContentful = true;
boxes.append({ m_lineIndex
boxes.append({ lineIndex()
, InlineDisplay::Box::Type::LineBreakBox
, lineRun.layoutBox()
, lineRun.bidiLevel()
Expand All @@ -375,14 +375,14 @@ void InlineDisplayContentBuilder::appendAtomicInlineLevelDisplayBox(const Line::
auto isContentful = true;
auto inkOverflow = [&] {
auto inkOverflow = FloatRect { borderBoxRect };
auto& style = !m_lineIndex ? layoutBox.firstLineStyle() : layoutBox.style();
auto& style = !lineIndex() ? layoutBox.firstLineStyle() : layoutBox.style();
computeInkOverflowForInlineLevelBox(style, inkOverflow);
// Atomic inline box contribute to their inline box parents ink overflow at all times (e.g. <span><img></span>).
m_contentHasInkOverflow = m_contentHasInkOverflow || &layoutBox.parent() != &root();
return inkOverflow;
};

boxes.append({ m_lineIndex
boxes.append({ lineIndex()
, InlineDisplay::Box::Type::AtomicInlineLevelBox
, layoutBox
, lineRun.bidiLevel()
Expand All @@ -397,7 +397,7 @@ void InlineDisplayContentBuilder::appendAtomicInlineLevelDisplayBox(const Line::

void InlineDisplayContentBuilder::appendRootInlineBoxDisplayBox(const InlineRect& rootInlineBoxVisualRect, bool lineHasContent, InlineDisplay::Boxes& boxes)
{
boxes.append({ m_lineIndex
boxes.append({ lineIndex()
, InlineDisplay::Box::Type::RootInlineBox
, root()
, UBIDI_DEFAULT_LTR
Expand All @@ -420,13 +420,13 @@ void InlineDisplayContentBuilder::appendInlineBoxDisplayBox(const Line::Run& lin
m_hasSeenRubyBase = m_hasSeenRubyBase || layoutBox.isRubyBase();

auto inkOverflow = [&] {
auto& style = !m_lineIndex ? layoutBox.firstLineStyle() : layoutBox.style();
auto& style = !lineIndex() ? layoutBox.firstLineStyle() : layoutBox.style();
auto inkOverflow = FloatRect { inlineBoxBorderBox };
m_contentHasInkOverflow = computeInkOverflowForInlineBox(inlineBox, style, inkOverflow) || m_contentHasInkOverflow;
return inkOverflow;
};

boxes.append({ m_lineIndex
boxes.append({ lineIndex()
, InlineDisplay::Box::Type::NonRootInlineBox
, layoutBox
, lineRun.bidiLevel()
Expand All @@ -445,7 +445,7 @@ void InlineDisplayContentBuilder::appendInlineDisplayBoxAtBidiBoundary(const Box
// Geometries for inline boxes at bidi boundaries are computed at a post-process step.
auto isContentful = true;
m_hasSeenRubyBase = m_hasSeenRubyBase || layoutBox.isRubyBase();
boxes.append({ m_lineIndex
boxes.append({ lineIndex()
, InlineDisplay::Box::Type::NonRootInlineBox
, layoutBox
, UBIDI_DEFAULT_LTR
Expand All @@ -460,7 +460,7 @@ void InlineDisplayContentBuilder::appendInlineDisplayBoxAtBidiBoundary(const Box

void InlineDisplayContentBuilder::insertRubyAnnotationBox(const Box& annotationBox, size_t insertionPosition, const InlineRect& borderBoxRect, InlineDisplay::Boxes& boxes)
{
boxes.insert(insertionPosition, { m_lineIndex
boxes.insert(insertionPosition, { lineIndex()
, InlineDisplay::Box::Type::AtomicInlineLevelBox
, annotationBox
, UBIDI_DEFAULT_LTR
Expand All @@ -473,14 +473,15 @@ void InlineDisplayContentBuilder::insertRubyAnnotationBox(const Box& annotationB
});
}

void InlineDisplayContentBuilder::processNonBidiContent(const LineLayoutResult& lineLayoutResult, const LineBox& lineBox, InlineDisplay::Boxes& boxes)
void InlineDisplayContentBuilder::processNonBidiContent(const LineLayoutResult& lineLayoutResult, InlineDisplay::Boxes& boxes)
{
#ifndef NDEBUG
auto hasContent = false;
for (auto& lineRun : lineLayoutResult.inlineContent)
hasContent = hasContent || lineRun.isContentful();
ASSERT(lineLayoutResult.directionality.inlineBaseDirection == TextDirection::LTR || !hasContent);
#endif
auto& lineBox = this->lineBox();
auto writingMode = root().style().writingMode();
auto isHorizontalWritingMode = WebCore::isHorizontalWritingMode(writingMode);
auto isFlippedBlocksWritingMode = WebCore::isFlippedWritingMode(writingMode);
Expand Down Expand Up @@ -567,7 +568,7 @@ void InlineDisplayContentBuilder::processNonBidiContent(const LineLayoutResult&
};
updateAssociatedBoxGeometry();
}
setGeometryForBlockLevelOutOfFlowBoxes(blockLevelOutOfFlowBoxList, lineBox, lineLayoutResult.inlineContent);
setGeometryForBlockLevelOutOfFlowBoxes(blockLevelOutOfFlowBoxList, lineLayoutResult.inlineContent);
}

struct DisplayBoxTree {
Expand Down Expand Up @@ -651,7 +652,7 @@ struct IsFirstLastIndex {
std::optional<size_t> last;
};
using IsFirstLastIndexesMap = HashMap<const Box*, IsFirstLastIndex>;
void InlineDisplayContentBuilder::adjustVisualGeometryForDisplayBox(size_t displayBoxNodeIndex, InlineLayoutUnit& contentRightInInlineDirectionVisualOrder, InlineLayoutUnit lineBoxLogicalTop, const DisplayBoxTree& displayBoxTree, InlineDisplay::Boxes& boxes, const LineBox& lineBox, const IsFirstLastIndexesMap& isFirstLastIndexesMap)
void InlineDisplayContentBuilder::adjustVisualGeometryForDisplayBox(size_t displayBoxNodeIndex, InlineLayoutUnit& contentRightInInlineDirectionVisualOrder, InlineLayoutUnit lineBoxLogicalTop, const DisplayBoxTree& displayBoxTree, InlineDisplay::Boxes& boxes, const IsFirstLastIndexesMap& isFirstLastIndexesMap)
{
auto writingMode = root().style().writingMode();
auto isHorizontalWritingMode = WebCore::isHorizontalWritingMode(writingMode);
Expand Down Expand Up @@ -687,8 +688,8 @@ void InlineDisplayContentBuilder::adjustVisualGeometryForDisplayBox(size_t displ
auto isFirstBox = isFirstLastIndexes.first && *isFirstLastIndexes.first == displayBoxNodeIndex;
auto isLastBox = isFirstLastIndexes.last && *isFirstLastIndexes.last == displayBoxNodeIndex;
auto beforeInlineBoxContent = [&] {
auto logicalRect = lineBox.logicalBorderBoxForInlineBox(layoutBox, boxGeometry);
auto visualRect = flipLogicalRectToVisualForWritingModeWithinLine({ logicalRect.top(), contentRightInInlineDirectionVisualOrder, { }, logicalRect.height() }, lineBox.logicalRect(), writingMode);
auto logicalRect = lineBox().logicalBorderBoxForInlineBox(layoutBox, boxGeometry);
auto visualRect = flipLogicalRectToVisualForWritingModeWithinLine({ logicalRect.top(), contentRightInInlineDirectionVisualOrder, { }, logicalRect.height() }, lineBox().logicalRect(), writingMode);
isHorizontalWritingMode ? visualRect.moveVertically(m_displayLine.top()) : visualRect.moveHorizontally(m_displayLine.left());
displayBox.setRect(visualRect, visualRect);

Expand All @@ -703,25 +704,25 @@ void InlineDisplayContentBuilder::adjustVisualGeometryForDisplayBox(size_t displ
beforeInlineBoxContent();

for (auto childDisplayBoxNodeIndex : displayBoxTree.at(displayBoxNodeIndex).children)
adjustVisualGeometryForDisplayBox(childDisplayBoxNodeIndex, contentRightInInlineDirectionVisualOrder, lineBoxLogicalTop, displayBoxTree, boxes, lineBox, isFirstLastIndexesMap);
adjustVisualGeometryForDisplayBox(childDisplayBoxNodeIndex, contentRightInInlineDirectionVisualOrder, lineBoxLogicalTop, displayBoxTree, boxes, isFirstLastIndexesMap);

auto afterInlineBoxContent = [&] {
auto shouldApplyRightSide = (isLeftToRightDirection && isLastBox) || (!isLeftToRightDirection && isFirstBox);
if (!shouldApplyRightSide)
return setRightForWritingMode(displayBox, contentRightInInlineDirectionVisualOrder, writingMode);

contentRightInInlineDirectionVisualOrder += borderRightInInlineDirection(boxGeometry, isLeftToRightDirection) + paddingRightInInlineDirection(boxGeometry, isLeftToRightDirection);
contentRightInInlineDirectionVisualOrder += layoutBox.isRubyBase() ? RubyFormattingContext::baseEndAdditionalVisualWidth(layoutBox, displayBox, contentRightInInlineDirectionVisualOrder - displayBox.left(), formattingContext()) : 0.f;
contentRightInInlineDirectionVisualOrder += layoutBox.isRubyBase() ? RubyFormattingContext::baseEndAdditionalLogicalWidth(layoutBox, displayBox, contentRightInInlineDirectionVisualOrder - displayBox.left(), formattingContext()) : 0.f;
setRightForWritingMode(displayBox, contentRightInInlineDirectionVisualOrder, writingMode);
contentRightInInlineDirectionVisualOrder += marginRightInInlineDirection(boxGeometry, isLeftToRightDirection);
};
afterInlineBoxContent();

auto* inlineBox = lineBox.inlineLevelBoxFor(layoutBox);
auto* inlineBox = lineBox().inlineLevelBoxFor(layoutBox);
ASSERT(inlineBox);
auto computeInkOverflow = [&] {
auto inkOverflow = FloatRect { displayBox.visualRectIgnoringBlockDirection() };
m_contentHasInkOverflow = computeInkOverflowForInlineBox(*inlineBox, !m_lineIndex ? layoutBox.firstLineStyle() : layoutBox.style(), inkOverflow) || m_contentHasInkOverflow;
m_contentHasInkOverflow = computeInkOverflowForInlineBox(*inlineBox, !lineIndex() ? layoutBox.firstLineStyle() : layoutBox.style(), inkOverflow) || m_contentHasInkOverflow;
displayBox.adjustInkOverflow(inkOverflow);
};
computeInkOverflow();
Expand All @@ -731,14 +732,15 @@ void InlineDisplayContentBuilder::adjustVisualGeometryForDisplayBox(size_t displ
displayBox.setHasContent();
}

void InlineDisplayContentBuilder::processBidiContent(const LineLayoutResult& lineLayoutResult, const LineBox& lineBox, InlineDisplay::Boxes& boxes)
void InlineDisplayContentBuilder::processBidiContent(const LineLayoutResult& lineLayoutResult, InlineDisplay::Boxes& boxes)
{
ASSERT(lineLayoutResult.directionality.visualOrderList.size() <= lineLayoutResult.inlineContent.size());

AncestorStack ancestorStack;
auto displayBoxTree = DisplayBoxTree { };
ancestorStack.push({ }, root());

auto& lineBox = this->lineBox();
auto writingMode = root().style().writingMode();
auto isHorizontalWritingMode = WebCore::isHorizontalWritingMode(writingMode);

Expand Down Expand Up @@ -882,7 +884,7 @@ void InlineDisplayContentBuilder::processBidiContent(const LineLayoutResult& lin
}
ASSERT_NOT_REACHED();
}
setGeometryForBlockLevelOutOfFlowBoxes(blockLevelOutOfFlowBoxList, lineBox, inlineContent, lineLayoutResult.directionality.visualOrderList);
setGeometryForBlockLevelOutOfFlowBoxes(blockLevelOutOfFlowBoxList, inlineContent, lineLayoutResult.directionality.visualOrderList);
};
createDisplayBoxesInVisualOrder();

Expand Down Expand Up @@ -922,7 +924,7 @@ void InlineDisplayContentBuilder::processBidiContent(const LineLayoutResult& lin
auto contentRightInInlineDirectionVisualOrder = contentStartInInlineDirectionVisualOrder;

for (auto childDisplayBoxNodeIndex : displayBoxTree.root().children)
adjustVisualGeometryForDisplayBox(childDisplayBoxNodeIndex, contentRightInInlineDirectionVisualOrder, lineLogicalTop, displayBoxTree, boxes, lineBox, isFirstLastIndexesMap);
adjustVisualGeometryForDisplayBox(childDisplayBoxNodeIndex, contentRightInInlineDirectionVisualOrder, lineLogicalTop, displayBoxTree, boxes, isFirstLastIndexesMap);
};
adjustVisualGeometryWithInlineBoxes();
};
Expand Down Expand Up @@ -1014,7 +1016,7 @@ static inline void setGeometryForOutOfFlowBoxes(const Vector<size_t>& indexListO
formattingContext.geometryForBox(outOfFlowBox(i)).setTopLeft(visualTopLeft);
}

void InlineDisplayContentBuilder::setGeometryForBlockLevelOutOfFlowBoxes(const Vector<size_t>& indexListOfOutOfFlowBoxes, const LineBox& lineBox, const Line::RunList& lineRuns, const Vector<int32_t>& visualOrderList)
void InlineDisplayContentBuilder::setGeometryForBlockLevelOutOfFlowBoxes(const Vector<size_t>& indexListOfOutOfFlowBoxes, const Line::RunList& lineRuns, const Vector<int32_t>& visualOrderList)
{
if (indexListOfOutOfFlowBoxes.isEmpty())
return;
Expand All @@ -1041,7 +1043,7 @@ void InlineDisplayContentBuilder::setGeometryForBlockLevelOutOfFlowBoxes(const V
}

if (!firstContentfulInFlowRunIndex) {
setGeometryForOutOfFlowBoxes(indexListOfOutOfFlowBoxes, { }, lineRuns, visualOrderList, formattingContext, lineBox, m_displayLine, constraints());
setGeometryForOutOfFlowBoxes(indexListOfOutOfFlowBoxes, { }, lineRuns, visualOrderList, formattingContext, lineBox(), m_displayLine, constraints());
return;
}

Expand All @@ -1055,7 +1057,7 @@ void InlineDisplayContentBuilder::setGeometryForBlockLevelOutOfFlowBoxes(const V
break;
}
}
setGeometryForOutOfFlowBoxes(indexListOfOutOfFlowBoxes, firstOutOfFlowIndexWithPreviousInflowSibling, lineRuns, visualOrderList, formattingContext, lineBox, m_displayLine, constraints());
setGeometryForOutOfFlowBoxes(indexListOfOutOfFlowBoxes, firstOutOfFlowIndexWithPreviousInflowSibling, lineRuns, visualOrderList, formattingContext, lineBox(), m_displayLine, constraints());
}

static float logicalBottomForTextDecorationContent(const InlineDisplay::Boxes& boxes, bool isHorizontalWritingMode)
Expand Down
Loading

0 comments on commit 74ebfc4

Please sign in to comment.