Bug 174813 – [WinCairo] Implement Font::platformBoundsForGlyph

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Fujii Hironori]

(Forked from Bug 82798)

Font::platformBoundsForGlyph in Source/WebCore/platform/graphics/win/SimpleFontDataCairoWin.cpp is not implemented yet.

> FloatRect Font::platformBoundsForGlyph(Glyph glyph) const
> {
>     if (m_platformData.useGDI())
>         return boundsForGDIGlyph(glyph);
>     //FIXME: Implement this
>     return FloatRect();
> }

[Fujii Hironori]

Created attachment 316505 [details]
Wrongly placed text-emphasis (fast/text/emphasis.html)

text-emphasis should be placed at horizontally center of the character.

[Fujii Hironori]

Created attachment 316514 [details]
Patch

[Alex Christensen]

Comment on attachment 316514 [details]
Patch

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

> Source/WebCore/platform/graphics/win/SimpleFontDataCairoWin.cpp:114
> +    SaveDC(dc);

Would it be more efficient to use GetCurrentObject and GetGraphicsMode instead of saving the entire dc?

[Fujii Hironori]

Thank you for the review.

I'm reading the spec of cairo_win32_scaled_font_select_font and the source code.
https://www.cairographics.org/manual/cairo-Win32-Fonts.html#cairo-win32-scaled-font-select-font

cairo_win32_scaled_font_select_font changes not only current font, but also the graphics mode, the world transform and the map mode.
And, the spec recommends to use SaveDC() and RestoreDC(). I think it's safer to use SaveDC() and RestoreDC().

I realized I should call cairo_win32_scaled_font_done_font. (even though, this function does nothing actually)
I'll revise the patch.

[Fujii Hironori]

Created attachment 316753 [details]
Patch

[Alex Christensen]

Comment on attachment 316753 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Test: fast/text/emphasis.html

This usually indicates that you added a new test.  Do you mean this test failed with WinCairo before and not after?

[Fujii Hironori]

Created attachment 316834 [details]
WinCairo port's result of fast/text/emphasis.html after this change

Still fails after this change.
Mostly because the expectation are created for AppleWin port.
I think it's too early for WinCairo port to add expectations because WinCairo BuildBot is not ready for layout tests.

OK, I'll create a test for this change.

[Alex Christensen]

Comment on attachment 316753 [details]
Patch

Let's just land this.

[WebKit Commit Bot]

Comment on attachment 316753 [details]
Patch

Clearing flags on attachment: 316753

Committed r220117: <http://trac.webkit.org/changeset/220117>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/33667541>