Implement onCountGlyphs and onGetUPEM on Windows.

Implement onCountGlyphs and onGetUPEM on Windows.

R=vandebo@chromium.org

Committed: https://code.google.com/p/skia/source/detail?r=10089

[vandebo (ex-Chrome), 2013-07-15 17:32:56]

LGTM

https://codereview.chromium.org/19231003/diff/11001/src/core/SkTypeface.cpp
File src/core/SkTypeface.cpp (right):

https://codereview.chromium.org/19231003/diff/11001/src/core/SkTypeface.cpp#n...
src/core/SkTypeface.cpp:217: int SkTypeface::onGetUPEM() const {
Did you mean to remove this implementation as well since you made the method
pure virtual?

https://codereview.chromium.org/19231003/diff/11001/src/ports/SkFontHost_win.cpp
File src/ports/SkFontHost_win.cpp (right):

https://codereview.chromium.org/19231003/diff/11001/src/ports/SkFontHost_win....
src/ports/SkFontHost_win.cpp:149: textMetric.tmPitchAndFamily = TMPF_TRUETYPE;
Maybe put this on line 146 to make it more obvious what's going on?  It would
also avoid the if on 148.

https://codereview.chromium.org/19231003/diff/11001/src/ports/SkFontHost_win....
src/ports/SkFontHost_win.cpp:182: TEXTMETRIC textMetric;
If you pull this out into a helper function you could do:

TEXTMETRIC textMetric;
if (get_text_metric(hdc, &textMetrics) && !(textMetric.tmPitchAndFamily ...

https://codereview.chromium.org/19231003/diff/11001/tests/FontHostTest.cpp
File tests/FontHostTest.cpp (right):

https://codereview.chromium.org/19231003/diff/11001/tests/FontHostTest.cpp#ne...
tests/FontHostTest.cpp:38: if (size) {
Do you want to check that size >= 20 ?

https://codereview.chromium.org/19231003/diff/11001/tests/FontHostTest.cpp#ne...
tests/FontHostTest.cpp:60: if (size) {
size >= 6 ?

[bungeman-skia, 2013-07-15 18:21:36]

https://codereview.chromium.org/19231003/diff/11001/src/core/SkTypeface.cpp
File src/core/SkTypeface.cpp (right):

https://codereview.chromium.org/19231003/diff/11001/src/core/SkTypeface.cpp#n...
src/core/SkTypeface.cpp:217: int SkTypeface::onGetUPEM() const {
On 2013/07/15 17:32:56, vandebo wrote:
> Did you mean to remove this implementation as well since you made the method
> pure virtual?

Yes. I'm fairly certain the only reason this is here is to make things compile
on Windows. All of these 'on' guys should be pure virtual on SkTypeface, as the
entire point is to force subclasses to implement them.

https://codereview.chromium.org/19231003/diff/11001/src/ports/SkFontHost_win.cpp
File src/ports/SkFontHost_win.cpp (right):

https://codereview.chromium.org/19231003/diff/11001/src/ports/SkFontHost_win....
src/ports/SkFontHost_win.cpp:149: textMetric.tmPitchAndFamily = TMPF_TRUETYPE;
On 2013/07/15 17:32:56, vandebo wrote:
> Maybe put this on line 146 to make it more obvious what's going on?  It would
> also avoid the if on 148.

Done. I dislike unneeded stores, but I suppose I dislike unneeded compares and
branches more.

https://codereview.chromium.org/19231003/diff/11001/src/ports/SkFontHost_win....
src/ports/SkFontHost_win.cpp:182: TEXTMETRIC textMetric;
On 2013/07/15 17:32:56, vandebo wrote:
> If you pull this out into a helper function you could do:
> 
> TEXTMETRIC textMetric;
> if (get_text_metric(hdc, &textMetrics) && !(textMetric.tmPitchAndFamily ...

This doesn't want to be in a helper, it wants to be in the LogFontTypeface (and
it almost is). I was trying to avoid that this time around.

https://codereview.chromium.org/19231003/diff/11001/tests/FontHostTest.cpp
File tests/FontHostTest.cpp (right):

https://codereview.chromium.org/19231003/diff/11001/tests/FontHostTest.cpp#ne...
tests/FontHostTest.cpp:38: if (size) {
On 2013/07/15 17:32:56, vandebo wrote:
> Do you want to check that size >= 20 ?

Already did that in test_tables.

https://codereview.chromium.org/19231003/diff/11001/tests/FontHostTest.cpp#ne...
tests/FontHostTest.cpp:60: if (size) {
On 2013/07/15 17:32:56, vandebo wrote:
> size >= 6 ?

Already did that in test_tables.

[vandebo (ex-Chrome), 2013-07-15 18:25:31]

LGTM

[bungeman-skia, 2013-07-15 19:43:01]

Committed patchset #4 manually as r10089.

[bungeman-skia, 2013-07-15 21:19:14]

On 2013/07/15 19:43:01, bungeman1 wrote:
> Committed patchset #4 manually as r10089.

Minor follow up commit in r10090 to fix proxy.