Title: [286801] trunk/Source/WebCore
- Revision
- 286801
- Author
- [email protected]
- Date
- 2021-12-09 13:28:22 -0800 (Thu, 09 Dec 2021)
Log Message
[LFC][IFC] Try not to include non-content type of inline boxes in the visual reordering
https://bugs.webkit.org/show_bug.cgi?id=234033
Reviewed by Antti Koivisto.
In InlineItemsBuilder::setBidiLevelForOpaqueInlineItems we try to figure out the bidi
level for inline box markers (<span> and </span>) mostly to be able to position
empty inline boxes (specifically with decorations). In some cases though the guessed
bidi level breaks the continuation between the content before and after the marker (e.g. before</span>after)
In this patch we try to limit the number of items associated with this guessed bidi level.
* layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp:
(WebCore::Layout::InlineDisplayContentBuilder::processBidiContent):
* layout/formattingContexts/inline/InlineItem.h:
* layout/formattingContexts/inline/InlineItemsBuilder.cpp:
(WebCore::Layout::InlineItemsBuilder::breakAndComputeBidiLevels):
* layout/formattingContexts/inline/InlineLineBuilder.cpp:
(WebCore::Layout::LineBuilder::layoutInlineContent): we may only submit a subset of runs to the reordering
algorithm. It also means we may end up with gaps between the visual index values and the real run item indexes.
The runIndexOffsetMap ensures that the indexes are always consistent with the content in the lineRuns vector.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (286800 => 286801)
--- trunk/Source/WebCore/ChangeLog 2021-12-09 21:14:04 UTC (rev 286800)
+++ trunk/Source/WebCore/ChangeLog 2021-12-09 21:28:22 UTC (rev 286801)
@@ -1,3 +1,26 @@
+2021-12-09 Alan Bujtas <[email protected]>
+
+ [LFC][IFC] Try not to include non-content type of inline boxes in the visual reordering
+ https://bugs.webkit.org/show_bug.cgi?id=234033
+
+ Reviewed by Antti Koivisto.
+
+ In InlineItemsBuilder::setBidiLevelForOpaqueInlineItems we try to figure out the bidi
+ level for inline box markers (<span> and </span>) mostly to be able to position
+ empty inline boxes (specifically with decorations). In some cases though the guessed
+ bidi level breaks the continuation between the content before and after the marker (e.g. before</span>after)
+ In this patch we try to limit the number of items associated with this guessed bidi level.
+
+ * layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp:
+ (WebCore::Layout::InlineDisplayContentBuilder::processBidiContent):
+ * layout/formattingContexts/inline/InlineItem.h:
+ * layout/formattingContexts/inline/InlineItemsBuilder.cpp:
+ (WebCore::Layout::InlineItemsBuilder::breakAndComputeBidiLevels):
+ * layout/formattingContexts/inline/InlineLineBuilder.cpp:
+ (WebCore::Layout::LineBuilder::layoutInlineContent): we may only submit a subset of runs to the reordering
+ algorithm. It also means we may end up with gaps between the visual index values and the real run item indexes.
+ The runIndexOffsetMap ensures that the indexes are always consistent with the content in the lineRuns vector.
+
2021-12-08 BJ Burg <[email protected]>
[Cocoa] Web Inspector: provide a way for _WKInspectorExtension clients to be to notified when an extension tab navigates
Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp (286800 => 286801)
--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp 2021-12-09 21:14:04 UTC (rev 286800)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp 2021-12-09 21:28:22 UTC (rev 286801)
@@ -460,7 +460,7 @@
void InlineDisplayContentBuilder::processBidiContent(const LineBuilder::LineContent& lineContent, const LineBox& lineBox, const InlineLayoutPoint& lineBoxLogicalTopLeft, DisplayBoxes& boxes)
{
- ASSERT(lineContent.visualOrderList.size() == lineContent.runs.size());
+ ASSERT(lineContent.visualOrderList.size() <= lineContent.runs.size());
AncestorStack ancestorStack;
DisplayBoxNode rootDisplayBoxNode = { };
@@ -479,12 +479,13 @@
auto contentRightInVisualOrder = contentStartInVisualOrder;
auto& runs = lineContent.runs;
- for (size_t i = 0; i < runs.size(); ++i) {
- auto visualIndex = lineContent.visualOrderList[i];
- auto& lineRun = runs[visualIndex];
+ for (auto visualOrder : lineContent.visualOrderList) {
+ ASSERT(runs[visualOrder].bidiLevel() != InlineItem::opaqueBidiLevel);
+
+ auto& lineRun = runs[visualOrder];
auto& layoutBox = lineRun.layoutBox();
- auto needsDisplayBox = !lineRun.isInlineBoxEnd() && !lineRun.isWordBreakOpportunity();
+ auto needsDisplayBox = !lineRun.isWordBreakOpportunity();
if (!needsDisplayBox)
continue;
Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineItem.h (286800 => 286801)
--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineItem.h 2021-12-09 21:14:04 UTC (rev 286800)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineItem.h 2021-12-09 21:28:22 UTC (rev 286801)
@@ -51,6 +51,7 @@
InlineItem(const Box& layoutBox, Type, UBiDiLevel = UBIDI_DEFAULT_LTR);
Type type() const { return m_type; }
+ static constexpr UBiDiLevel opaqueBidiLevel = 0xff;
UBiDiLevel bidiLevel() const { return m_bidiLevel; }
const Box& layoutBox() const { return *m_layoutBox; }
const RenderStyle& style() const { return layoutBox().style(); }
Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp (286800 => 286801)
--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp 2021-12-09 21:14:04 UTC (rev 286800)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp 2021-12-09 21:28:22 UTC (rev 286801)
@@ -333,15 +333,8 @@
continue;
}
if (inlineItems[index].isInlineBoxEnd()) {
- // Inline box end (e.g. </span>) also uses the content bidi level, but in this case it's the previous content.
- auto previousBidiLevel = [&]() -> std::optional<UBiDiLevel> {
- for (auto i = index; i--;) {
- if (inlineItemOffsets[i])
- return inlineItems[i].bidiLevel();
- }
- return { };
- }();
- inlineItems[index].setBidiLevel(previousBidiLevel.value_or(rootBidiLevel));
+ // Let's not confuse ubidi with non-content entries. Opaque runs are excluded from the visual list.
+ inlineItems[index].setBidiLevel(InlineItem::opaqueBidiLevel);
continue;
}
ASSERT_NOT_REACHED();
Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp (286800 => 286801)
--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp 2021-12-09 21:14:04 UTC (rev 286800)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp 2021-12-09 21:28:22 UTC (rev 286801)
@@ -300,13 +300,29 @@
if (!m_line.contentNeedsBidiReordering())
return { };
- Vector<UBiDiLevel> runLevels(lineRuns.size());
- // FIXME: We may cache these values in Line, if it turns out to be a perf hit.
- for (size_t i = 0; i < lineRuns.size(); ++i)
- runLevels[i] = lineRuns[i].bidiLevel();
+ Vector<UBiDiLevel> runLevels;
+ runLevels.reserveInitialCapacity(lineRuns.size());
- Vector<int32_t> visualOrderList(lineRuns.size());
+ Vector<size_t> runIndexOffsetMap;
+ runIndexOffsetMap.reserveInitialCapacity(lineRuns.size());
+ auto hasOpaqueRun = false;
+ for (size_t i = 0, accumulatedOffset = 0; i < lineRuns.size(); ++i) {
+ if (lineRuns[i].bidiLevel() == InlineItem::opaqueBidiLevel) {
+ ++accumulatedOffset;
+ hasOpaqueRun = true;
+ continue;
+ }
+ runLevels.append(lineRuns[i].bidiLevel());
+ runIndexOffsetMap.append(accumulatedOffset);
+ }
+
+ Vector<int32_t> visualOrderList(runLevels.size());
ubidi_reorderVisual(runLevels.data(), runLevels.size(), visualOrderList.data());
+ if (hasOpaqueRun) {
+ ASSERT(visualOrderList.size() == runIndexOffsetMap.size());
+ for (size_t i = 0; i < runIndexOffsetMap.size(); ++i)
+ visualOrderList[i] += runIndexOffsetMap[visualOrderList[i]];
+ }
return visualOrderList;
};
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
