Basic support for bitmap fonts on Windows.

[bungeman, 2012-04-17 19:07:50]

Adds very basic (but arguably ok) support for using bitmap fonts on Windows. The
advances are currently trasformed, but the glyphs themselves are not, mostly
because when gdi does the transformation it uses a substitute font which may not
have the same metrics.

[reed1, 2012-04-18 13:31:01]

http://codereview.appspot.com/6062044/diff/2001/src/ports/SkFontHost_win.cpp
File src/ports/SkFontHost_win.cpp (left):

http://codereview.appspot.com/6062044/diff/2001/src/ports/SkFontHost_win.cpp#...
src/ports/SkFontHost_win.cpp:117: static unsigned calculateGlyphCount(HDC hdc) {
Add comment(s) for why we want these two checks?

http://codereview.appspot.com/6062044/diff/2001/src/ports/SkFontHost_win.cpp
File src/ports/SkFontHost_win.cpp (right):

http://codereview.appspot.com/6062044/diff/2001/src/ports/SkFontHost_win.cpp#...
src/ports/SkFontHost_win.cpp:554: 
Can we just add this as an assert insert the else--bitmap case? What will happen
if we hit this in a release build?

http://codereview.appspot.com/6062044/diff/2001/src/ports/SkFontHost_win.cpp#...
src/ports/SkFontHost_win.cpp:646: if (0 == GetTextMetrics(fDDC, &tm)) {
If we're going to "error out", we need to set the glyph struct to empty I think.

[bungeman, 2012-04-18 13:59:38]

http://codereview.appspot.com/6062044/diff/2001/src/ports/SkFontHost_win.cpp
File src/ports/SkFontHost_win.cpp (left):

http://codereview.appspot.com/6062044/diff/2001/src/ports/SkFontHost_win.cpp#...
src/ports/SkFontHost_win.cpp:117: static unsigned calculateGlyphCount(HDC hdc) {
On 2012/04/18 13:31:01, reed1 wrote:
> Add comment(s) for why we want these two checks?

This is called from both the internal guy (SkScalerContext_Windows, which
already knows) and an external guy (getAdvancedTypefaceMetrics, which doesn't
know). So this is doing the same check done in the constructor. The first is
just testing if the call to GetTextMetrics succeeded. The second is testing if
this is not TrueType, PostScript, or Vector.

http://codereview.appspot.com/6062044/diff/2001/src/ports/SkFontHost_win.cpp
File src/ports/SkFontHost_win.cpp (right):

http://codereview.appspot.com/6062044/diff/2001/src/ports/SkFontHost_win.cpp#...
src/ports/SkFontHost_win.cpp:554: 
On 2012/04/18 13:31:01, reed1 wrote:
> Can we just add this as an assert insert the else--bitmap case? What will
happen
> if we hit this in a release build?

I actually have no idea what will happen. I think this case is only printer
hardware fonts, so I haven't been able to test what happens. I imagine we'll
just draw nothing and most calls to retrieve information will fail.

Since we only use get HFONTS from logfonts on memory device contexts, I don't
think this can ever happen in any event. The only way to get one of these is to
create a device context to a printer and enumerate the fonts available. The user
would then have to force us to use that particular HFONT. I don't think we
currently have any means of doing that. As a result maybe we don't really need
this test, we can just assert above that it should never happen.

http://codereview.appspot.com/6062044/diff/2001/src/ports/SkFontHost_win.cpp#...
src/ports/SkFontHost_win.cpp:646: if (0 == GetTextMetrics(fDDC, &tm)) {
On 2012/04/18 13:31:01, reed1 wrote:
> If we're going to "error out", we need to set the glyph struct to empty I
think.

Done.

Yes, we do.

[bungeman, 2012-04-18 14:42:19]

Make it a bit faster and have less duplicated code.

[reed1, 2012-04-18 14:54:17]

http://codereview.appspot.com/6062044/diff/2002/src/ports/SkFontHost_win.cpp
File src/ports/SkFontHost_win.cpp (right):

http://codereview.appspot.com/6062044/diff/2002/src/ports/SkFontHost_win.cpp#...
src/ports/SkFontHost_win.cpp:644: if (0 == GetTextMetrics(fDDC, &tm)) {
    glyph->zeroMetrics();
+
   It seems you should set the fAdvanceX,Y to something (zero?)

[reed1, 2012-04-18 14:55:20]

much clearer

[bungeman, 2012-04-18 15:17:26]

A comment from the future!

http://codereview.appspot.com/6062044/diff/2002/src/ports/SkFontHost_win.cpp
File src/ports/SkFontHost_win.cpp (right):

http://codereview.appspot.com/6062044/diff/2002/src/ports/SkFontHost_win.cpp#...
src/ports/SkFontHost_win.cpp:644: if (0 == GetTextMetrics(fDDC, &tm)) {
On 2012/04/18 14:54:17, reed1 wrote:
>     glyph->zeroMetrics();
> +
>    It seems you should set the fAdvanceX,Y to something (zero?)

If this does need to be done, then the outline code below would need to do it
too. With Patch Set 4 this code is gone and we only go down this path if we
already have fTM. With Patch Set 4 code, in the case where GetTextMetrics fails,
we'll fail in the same way outline always has.

[bungeman, 2012-04-18 15:41:22]

Committed revision 3723.