Bug 59817 – skia: replace path-drawing-of-glyphs with standard drawText

• Component: New Bugs
• Status: RESOLVED DUPLICATE

[Mike Reed]

skia: replace path-drawing-of-glyphs with standard drawText

[Mike Reed]

Created attachment 91711 [details]
Patch

[Mike Reed]

Am (trying to) run DRT locally on my windows machine, to identify which images will require rebaselining (and therefore which will need to be added to test_expectations.txt temporarily.

[Mike Reed]

... therefore I would like to start the code-review, but will not request a commit until the test_expectations.txt is added to the CL.

[Stephen White]

Adding jamesr since he's done some work in the advances stuff recently.

[Stephen White]

Comment on attachment 91711 [details]
Patch

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

I haven't looked at the path cache code much before, so I defer to someone with greater experience here.

> Source/WebCore/platform/graphics/skia/SkiaFontWin.cpp:114
> +    SkAutoSTArray<kLocalGlyphMax, SkPoint> posStorage(numGlyphs);

In WebKit, when the size is known up front, we usually use WTF::OwnArrayPtr.  If you think the stack allocation is a crucial feature, you could stick with SkAutoSTArray, but usually we try to use WTF types in WebKit code.

> Source/WebCore/platform/graphics/skia/SkiaFontWin.cpp:121
> +        x += SkIntToScalar(advances[i]);

Kind of OT, but I'm wondering if we want to convert these advances to float at some point.  I think they start at floats over in drawGlyphsWin() and are cast to ints before this call, and then appear to be cast back to SkScalar (float) here.  Of course, if this codepath is always used with ClearType-style LCD subpixel AA enabled, maybe it makes no difference since we want to snap to pixels anyway.  If not, we might be able to get subpixel positioning with that change.  jamesr might have some insight here.

[Eric Seidel (no email)]

I like deleting code... but what are the performance changes from this change?

Are the new image results better?

[Mike Reed]

The general performance diff between paths and text is about 9x (faster for text). The quality will be slightly different, as we are now using GDI to rasterize the individual glyphs.

[Eric Seidel (no email)]

Comment on attachment 91711 [details]
Patch

I'm happy to rubber-stamp this.  If you need a more detailed review, you'll need to track down brettw or pkasting.

[Mike Reed]

working on the test_expectations part now...

[Mike Reed]

*** This bug has been marked as a duplicate of bug 64368 ***