Bug 52403 – [chromium] drop backwards iteration in Linux complex text code

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Evan Martin]

[chromium] drop backwards iteration in Linux complex text code

[Evan Martin]

Created attachment 78866 [details]
Patch

[Tony Chang]

Comment on attachment 78866 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=78866&action=review

> Source/WebCore/platform/graphics/chromium/FontLinux.cpp:247
> +    for (int glyphIndex = 0; static_cast<unsigned>(glyphIndex) < controller.length(); ++glyphIndex) {
> +        int advance = truncateFixedPointToInteger(controller.advances()[glyphIndex]);
> +        int nextX = static_cast<int>(controller.xPositions()[glyphIndex]);
> +        nextX += advance / 2;
> +        if (std::min(nextX, lastX) <= targetX && targetX <= std::max(nextX, lastX))
> +            return glyphIndex;
> +        lastX = nextX;

A comment explaining this loop sounds like it would be worthwhile.

[Adam Langley]

Comment on attachment 78866 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=78866&action=review

> Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.cpp:163
> +        const SimpleFontData* nextFontData = m_font->glyphDataForCharacter(m_item.string[m_item.item.pos + endOfRun], false).fontData;

If this is a UTF16 string, will this not consider surrogate-pairs as two code points?

[Evan Martin]

Comment on attachment 78866 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=78866&action=review

>> Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.cpp:163
>> +        const SimpleFontData* nextFontData = m_font->glyphDataForCharacter(m_item.string[m_item.item.pos + endOfRun], false).fontData;
> 
> If this is a UTF16 string, will this not consider surrogate-pairs as two code points?

Probably.  This is just a reindent of L184 in the previous code, so at least I don't think I'm regressing anything.

[Evan Martin]

Created attachment 78875 [details]
Patch

[Evan Martin]

BTW, this is a good page for thinking about the logic with respect to glyphIndexForXPositionInScriptRun:
http://www.aestheticallyloyal.com/public/optimize-legibility/

With this patch, selection in the non-complex case matches the complex case.

[Evan Martin]

Committed r75756: <http://trac.webkit.org/changeset/75756>