Modified: trunk/Source/WebCore/ChangeLog (287207 => 287208)
--- trunk/Source/WebCore/ChangeLog 2021-12-17 22:25:08 UTC (rev 287207)
+++ trunk/Source/WebCore/ChangeLog 2021-12-17 22:41:39 UTC (rev 287208)
@@ -1,3 +1,27 @@
+2021-12-17 Myles C. Maxfield <[email protected]>
+
+ Refactor WidthIterator::applyCSSVisibilityRules() to be a little more elegant
+ https://bugs.webkit.org/show_bug.cgi?id=234428
+
+ Reviewed by Alan Bujtas.
+
+ This adds a few lambda operations in this function:
+ - adjustForSyntheticBold()
+ - clobberGlyph()
+ - clobberAdvance()
+ - deleteGlyph()
+ And then updates the rest of the function to use those operations. I think this makes
+ the body of the function a little easier to understand, because the code uses a higher
+ level of abstraction. It shoudln't be any slower, though, because the lambdas can all
+ be inlined to the same code that was there before.
+
+ No new tests because there is no behavior change.
+
+ * platform/graphics/WidthIterator.cpp:
+ (WebCore::WidthIterator::applyCSSVisibilityRules):
+ (WebCore::WidthIterator::adjustForSyntheticBold): Deleted.
+ * platform/graphics/WidthIterator.h:
+
2021-12-17 Sihui Liu <[email protected]>
Add custom copy() method for Ref<T> to CrossThreadCopier
Modified: trunk/Source/WebCore/platform/graphics/WidthIterator.cpp (287207 => 287208)
--- trunk/Source/WebCore/platform/graphics/WidthIterator.cpp 2021-12-17 22:25:08 UTC (rev 287207)
+++ trunk/Source/WebCore/platform/graphics/WidthIterator.cpp 2021-12-17 22:41:39 UTC (rev 287208)
@@ -554,16 +554,6 @@
return true;
}
-void WidthIterator::adjustForSyntheticBold(GlyphBuffer& glyphBuffer, unsigned index)
-{
- auto glyph = glyphBuffer.glyphAt(index);
- static constexpr const GlyphBufferGlyph deletedGlyph = 0xFFFF;
- auto syntheticBoldOffset = glyph == deletedGlyph ? 0 : glyphBuffer.fontAt(index).syntheticBoldOffset();
- m_runWidthSoFar += syntheticBoldOffset;
- auto& advance = glyphBuffer.advances(index)[0];
- setWidth(advance, width(advance) + syntheticBoldOffset);
-}
-
void WidthIterator::applyCSSVisibilityRules(GlyphBuffer& glyphBuffer, unsigned glyphBufferStartIndex)
{
// This function needs to be kept in sync with characterCanUseSimplifiedTextMeasuring().
@@ -571,6 +561,32 @@
Vector<unsigned> glyphsIndicesToBeDeleted;
float yPosition = height(glyphBuffer.initialAdvance());
+
+ auto adjustForSyntheticBold = [&] (auto index) {
+ auto glyph = glyphBuffer.glyphAt(index);
+ static constexpr const GlyphBufferGlyph deletedGlyph = 0xFFFF;
+ auto syntheticBoldOffset = glyph == deletedGlyph ? 0 : glyphBuffer.fontAt(index).syntheticBoldOffset();
+ m_runWidthSoFar += syntheticBoldOffset;
+ auto& advance = glyphBuffer.advances(index)[0];
+ setWidth(advance, width(advance) + syntheticBoldOffset);
+ };
+
+ auto clobberGlyph = [&] (auto index, auto newGlyph) {
+ glyphBuffer.glyphs(index)[0] = newGlyph;
+ };
+
+ auto clobberAdvance = [&] (auto index, auto newAdvance) {
+ auto advanceBeforeClobbering = glyphBuffer.advanceAt(index);
+ glyphBuffer.advances(index)[0] = makeGlyphBufferAdvance(newAdvance, height(advanceBeforeClobbering));
+ m_runWidthSoFar += width(glyphBuffer.advanceAt(index)) - width(advanceBeforeClobbering);
+ glyphBuffer.origins(index)[0] = makeGlyphBufferOrigin(0, -yPosition);
+ };
+
+ auto deleteGlyph = [&] (auto index) {
+ m_runWidthSoFar -= width(glyphBuffer.advanceAt(index));
+ glyphBuffer.deleteGlyphWithoutAffectingSize(index);
+ };
+
for (unsigned i = glyphBufferStartIndex; i < glyphBuffer.size(); yPosition += height(glyphBuffer.advanceAt(i)), ++i) {
auto stringOffset = glyphBuffer.checkedStringOffsetAt(i, m_run.length());
if (!stringOffset)
@@ -580,12 +596,12 @@
switch (characterResponsibleForThisGlyph) {
case newlineCharacter:
case carriageReturn:
+ case noBreakSpace:
case tabCharacter:
- case noBreakSpace:
ASSERT(glyphBuffer.fonts(i)[0]);
// FIXME: Is this actually necessary? If the font specifically has a glyph for NBSP, I don't see a reason not to use it.
- glyphBuffer.glyphs(i)[0] = glyphBuffer.fonts(i)[0]->spaceGlyph();
- adjustForSyntheticBold(glyphBuffer, i);
+ clobberGlyph(i, glyphBuffer.fontAt(i).spaceGlyph());
+ adjustForSyntheticBold(i);
continue;
}
@@ -593,22 +609,17 @@
// "Control characters (Unicode category Cc)—other than tabs (U+0009), line feeds (U+000A), carriage returns (U+000D) and sequences that form a segment break—must be rendered as a visible glyph"
if (u_charType(characterResponsibleForThisGlyph) == U_CONTROL_CHAR) {
// Let's assume that .notdef is visible.
- auto previousAdvance = glyphBuffer.advanceAt(i);
GlyphBufferGlyph visibleGlyph = 0;
- glyphBuffer.glyphs(i)[0] = visibleGlyph;
- ASSERT(glyphBuffer.fonts(i)[0]);
- glyphBuffer.advances(i)[0] = makeGlyphBufferAdvance(glyphBuffer.fonts(i)[0]->widthForGlyph(visibleGlyph), height(previousAdvance));
- m_runWidthSoFar += width(glyphBuffer.advanceAt(i)) - width(previousAdvance);
- glyphBuffer.origins(i)[0] = makeGlyphBufferOrigin(0, -yPosition);
+ clobberGlyph(i, visibleGlyph);
+ clobberAdvance(i, glyphBuffer.fontAt(i).widthForGlyph(visibleGlyph));
continue;
}
- adjustForSyntheticBold(glyphBuffer, i);
+ adjustForSyntheticBold(i);
if ((characterResponsibleForThisGlyph >= nullCharacter && characterResponsibleForThisGlyph < space)
|| (characterResponsibleForThisGlyph >= deleteCharacter && characterResponsibleForThisGlyph < noBreakSpace)) {
- m_runWidthSoFar -= width(glyphBuffer.advanceAt(i));
- glyphBuffer.deleteGlyphWithoutAffectingSize(i);
+ deleteGlyph(i);
continue;
}
@@ -629,8 +640,7 @@
case firstStrongIsolate:
case objectReplacementCharacter:
case zeroWidthNoBreakSpace:
- m_runWidthSoFar -= width(glyphBuffer.advanceAt(i));
- glyphBuffer.deleteGlyphWithoutAffectingSize(i);
+ deleteGlyph(i);
continue;
}
}
Modified: trunk/Source/WebCore/platform/graphics/WidthIterator.h (287207 => 287208)
--- trunk/Source/WebCore/platform/graphics/WidthIterator.h 2021-12-17 22:25:08 UTC (rev 287207)
+++ trunk/Source/WebCore/platform/graphics/WidthIterator.h 2021-12-17 22:41:39 UTC (rev 287208)
@@ -76,7 +76,6 @@
bool hasExtraSpacing() const;
void applyExtraSpacingAfterShaping(GlyphBuffer&, unsigned characterStartIndex, unsigned glyphBufferStartIndex, unsigned characterDestinationIndex, float startingRunWidth);
void applyCSSVisibilityRules(GlyphBuffer&, unsigned glyphBufferStartIndex);
- void adjustForSyntheticBold(GlyphBuffer&, unsigned index);
struct AdditionalWidth {
float left;
