[Webkit-unassigned] [Bug 50619] [GTK] Glyphs in vertical text tests are rotated 90 degrees clockwise

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=50619





--- Comment #44 from Denis Nomiyama (dnomi) <d.nomiyama at samsung.com>  2013-11-05 05:54:22 PST ---
(From update of attachment 213358)
View in context: https://bugs.webkit.org/attachment.cgi?id=213358&action=review

>>> Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:367
>>> +    uint32_t tag = FT_MAKE_TAG(table & 0xff,  table & 0xff00, table & 0xff0000, table & 0xff000000);
>> 
>> Extra space here.
> 
> Ops sorry.

I found a bug in my simplification with FT_MAKE_TAG. I forgot to shift the bits when applying the bit mask.

>>> Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:385
>>> +void FontPlatformData::setOrientation(FontOrientation orientation)
>> 
>> Is this ever called for us? The only place I see this called is in SimpleFontData::verticalRightOrientationFontData, but that appears to only be called by the Mac port.
> 
> The code changed quite significantly since the initial patch, in particular after the Chromium code clean up. I will re-test this function and will double check the use cases.

SimpleFontData::verticalRightOrientationFontData() is used by glyphDataAndPageForNonCJKCharacterWithGlyphOrientation() when displaying non-CJK characters in vertical mode, which should stay rotated 90 degrees.

>> Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:402
>> +    setCairoFontOptionsFromFontConfigPattern(options, m_pattern.get());
> 
> Not sure I understand why this is called again.

Good catch, this call is not needed. I'll remove it.

>>> Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp:98
>>> +
>> 
>> This looks like a lot of repetition, so why not add two small helpers, heightFromTextExtentsAndOrientation and widthFromTextExtentsAndOrientation.
> 
> Ok I'll add them.

I'll simplify this code and helpers won't be necessary.

>> Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp:103
>> +        }
> 
> Not sure I understand why this is necessary now...

EM is used by OpenTypeVerticalData. In my tests without setting it, the space between glyphs in vertical text gets wrong.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.