[Webkit-unassigned] [Bug 94822] OPENTYPE_VERTICAL support for Chromium Win

[bugzilla-daemon at webkit.org]

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


Tony Chang <tony at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #160187|review?, commit-queue?      |review+, commit-queue-
               Flag|                            |




--- Comment #3 from Tony Chang <tony at chromium.org>  2012-08-27 10:31:33 PST ---
(From update of attachment 160187)
View in context: https://bugs.webkit.org/attachment.cgi?id=160187&action=review

cq- for the changelog.  Otherwise seems fine.

At some point, we may want to add a perf test to the PerformanceTests directory.  I don't think the page cyclers actually have any pages with vertical text.

> Source/WebCore/ChangeLog:3
> +        [chromium] OPENTYPE_VERTICAL support for Chromium Win

I would remove [chromium] from the bug title since this patch modifies cross platform code.

> Source/WebCore/platform/graphics/chromium/FontChromiumWin.cpp:429
> +#if ENABLE(OPENTYPE_VERTICAL)
> +        if (verticalData) {

Nit: Checking verticalData for each glyph sounds slow. We can try it, but I bet it would be faster to have drawGlyphs check verticalData at the beginning of the function and call drawVerticalGlyphs that has all of the vertical code.

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