Modified: trunk/Source/WebCore/ChangeLog (265414 => 265415)
--- trunk/Source/WebCore/ChangeLog 2020-08-09 06:40:00 UTC (rev 265414)
+++ trunk/Source/WebCore/ChangeLog 2020-08-09 07:36:19 UTC (rev 265415)
@@ -1,5 +1,24 @@
2020-08-08 Myles C. Maxfield <[email protected]>
+ Update OriginalAdvancesForCharacterTreatedAsSpace to work correctly in the presence of inserted or removed glyphs
+ https://bugs.webkit.org/show_bug.cgi?id=215302
+
+ Reviewed by Darin Adler.
+
+ OriginalAdvancesForCharacterTreatedAsSpace is trying to make sure that shaping doesn't cause
+ spaces to get wider or thinner. However, the way it was doing that is, for all the space
+ characters, overwrite that glyph index's advance after shaping to be what it was before shaping.
+ However, this is wrong, because shaping can insert or delete glyphs. Instead, now that we have
+ explicit string indices for each glyph, we can use those to determine which glyphs come from
+ space characters. These glyphs are the ones which should be overwritten.
+
+ * platform/graphics/WidthIterator.cpp:
+ (WebCore::WidthIterator::applyFontTransforms):
+ (WebCore::WidthIterator::advanceInternal):
+ * platform/graphics/WidthIterator.h:
+
+2020-08-08 Myles C. Maxfield <[email protected]>
+
Fix bad merge in r265241
https://bugs.webkit.org/show_bug.cgi?id=215051
Modified: trunk/Source/WebCore/platform/graphics/WidthIterator.cpp (265414 => 265415)
--- trunk/Source/WebCore/platform/graphics/WidthIterator.cpp 2020-08-09 06:40:00 UTC (rev 265414)
+++ trunk/Source/WebCore/platform/graphics/WidthIterator.cpp 2020-08-09 07:36:19 UTC (rev 265415)
@@ -28,6 +28,7 @@
#include "GlyphBuffer.h"
#include "Latin1TextIterator.h"
#include "SurrogatePairAwareTextIterator.h"
+#include <algorithm>
#include <wtf/MathExtras.h>
namespace WebCore {
@@ -58,14 +59,15 @@
}
struct OriginalAdvancesForCharacterTreatedAsSpace {
-public:
- explicit OriginalAdvancesForCharacterTreatedAsSpace(bool isSpace, float advanceBefore, float advanceAt)
- : characterIsSpace(isSpace)
+ explicit OriginalAdvancesForCharacterTreatedAsSpace(GlyphBufferStringOffset stringOffset, bool isSpace, float advanceBefore, float advanceAt)
+ : stringOffset(stringOffset)
+ , characterIsSpace(isSpace)
, advanceBeforeCharacter(advanceBefore)
, advanceAtCharacter(advanceAt)
{
}
+ GlyphBufferStringOffset stringOffset;
bool characterIsSpace;
float advanceBeforeCharacter;
float advanceAtCharacter;
@@ -113,18 +115,17 @@
if (!ltr)
glyphBuffer.reverse(lastGlyphCount, glyphBufferSize - lastGlyphCount);
- // https://bugs.webkit.org/show_bug.cgi?id=206208: This is totally, 100%, furiously, utterly, frustratingly bogus.
- // There is absolutely no guarantee that glyph indices before shaping have any relation at all with glyph indices after shaping.
- // One of the fundamental things that shaping does is insert glyph all over the place.
- for (size_t i = 0; i < charactersTreatedAsSpace.size(); ++i) {
- auto spaceOffset = charactersTreatedAsSpace[i].first;
- // Shaping may have deleted the glyph.
- if (spaceOffset >= glyphBufferSize)
+ for (unsigned i = lastGlyphCount; i < glyphBufferSize; ++i) {
+ auto characterIndex = glyphBuffer.stringOffsetAt(i);
+ auto iterator = std::lower_bound(charactersTreatedAsSpace.begin(), charactersTreatedAsSpace.end(), characterIndex, [](const OriginalAdvancesForCharacterTreatedAsSpace& l, GlyphBufferStringOffset r) -> bool {
+ return l.stringOffset < r;
+ });
+ if (iterator == charactersTreatedAsSpace.end() || iterator->stringOffset != characterIndex)
continue;
- const OriginalAdvancesForCharacterTreatedAsSpace& originalAdvances = charactersTreatedAsSpace[i].second;
- if (spaceOffset && !originalAdvances.characterIsSpace)
- glyphBuffer.advances(spaceOffset - 1)->setWidth(originalAdvances.advanceBeforeCharacter);
- glyphBuffer.advances(spaceOffset)->setWidth(originalAdvances.advanceAtCharacter);
+ const auto& originalAdvances = *iterator;
+ if (i && !originalAdvances.characterIsSpace)
+ glyphBuffer.advances(i - 1)->setWidth(originalAdvances.advanceBeforeCharacter);
+ glyphBuffer.advances(i)->setWidth(originalAdvances.advanceAtCharacter);
}
charactersTreatedAsSpace.clear();
@@ -308,8 +309,11 @@
auto transformsType = shouldApplyFontTransforms(glyphBuffer, lastGlyphCount, previousCharacter);
if (transformsType != TransformsType::None && FontCascade::treatAsSpace(character)) {
- charactersTreatedAsSpace.append(std::make_pair(glyphBuffer.size(),
- OriginalAdvancesForCharacterTreatedAsSpace(character == ' ', glyphBuffer.size() ? glyphBuffer.advanceAt(glyphBuffer.size() - 1).width() : 0, width)));
+ charactersTreatedAsSpace.constructAndAppend(
+ currentCharacterIndex,
+ character == ' ',
+ glyphBuffer.size() ? glyphBuffer.advanceAt(glyphBuffer.size() - 1).width() : 0,
+ width);
}
if (m_accountForGlyphBounds) {
Modified: trunk/Source/WebCore/platform/graphics/WidthIterator.h (265414 => 265415)
--- trunk/Source/WebCore/platform/graphics/WidthIterator.h 2020-08-09 06:40:00 UTC (rev 265414)
+++ trunk/Source/WebCore/platform/graphics/WidthIterator.h 2020-08-09 07:36:19 UTC (rev 265415)
@@ -35,7 +35,7 @@
struct GlyphData;
struct OriginalAdvancesForCharacterTreatedAsSpace;
-typedef Vector<std::pair<unsigned, OriginalAdvancesForCharacterTreatedAsSpace>, 64> CharactersTreatedAsSpace;
+using CharactersTreatedAsSpace = Vector<OriginalAdvancesForCharacterTreatedAsSpace, 64>;
struct WidthIterator {
WTF_MAKE_FAST_ALLOCATED;
