Bug 112213 – Character orientation should follow UTR50 specs for vertical layout

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Enrica Casucci]

This bug track the remaining work to correct character orientation in vertical layout according to the specs in www.unicode.org/reports/tr50/tr50-6.Orientation.txt

<rdar://problem/12880943>

[Enrica Casucci]

Created attachment 192841 [details]
Patch

[Sam Weinig]

Comment on attachment 192841 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        This patch modifies shouldIgnoreRotation to include all the characters that
> +        should not be rotated sideways in vertical layout according to the UTR50 draft 6
> +        specifications. It also fixes rotation for Emojii.

You should note which tests this changes.

[Ryosuke Niwa]

Comment on attachment 192841 [details]
Patch

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

I trust you that you've updated the table correctly. Please add the new pixel results before you land.

> Source/WebCore/platform/graphics/FontFastPath.cpp:-47
> -    switch (character) {

I'm concerned that this might regress performance but I guess we can land and see if it does.

> Source/WebCore/platform/graphics/FontFastPath.cpp:53
> +    if (character >= 0x002E5 && character <= 0x002EB)

Maybe we can add a helper function like isInRange so that we can write this like isInRange(character, 0x002E5, 0x002EB)

> Source/WebCore/platform/graphics/mac/FontMac.mm:101
> +        CGAffineTransform savedMatrix;

I wich this entire block were broken up into smaller inline functions but that can be done in a separate patch.

> Source/WebCore/platform/graphics/mac/FontMac.mm:115
> +        } else {
> +            translationsTransform = rotateLeftTransform;
> +        }

Nit: no curly brackets around a single line statement as you pointed out in person.

[Enrica Casucci]

(> I'm concerned that this might regress performance but I guess we can land and see if it does.
This function gets called only for vertical layout, so if there is a regression, it should be limited to the vertical layout case.
> 
> > Source/WebCore/platform/graphics/FontFastPath.cpp:53
> > +    if (character >= 0x002E5 && character <= 0x002EB)
> 
> Maybe we can add a helper function like isInRange so that we can write this like isInRange(character, 0x002E5, 0x002EB)
>
That is a good idea. It will make the code more readable.

[Enrica Casucci]

Committed revision 145854.