CreateTypefaceFromStream for GDI.

[bungeman, 2012-02-06 20:26:01]

This is more or less how webfonts are currently handled in Chromium, but the
code is now in Skia and cleaned up a bit.

Note that the test as written here will not be able to go in with this change,
as CreateTypefaceFromStream needs to be implemented in the CG fonthost as well.

The one detraction from all of this is that when a typeface created from a
stream is unrefed to 1 we want to remove it immediately from the cache instead
of waiting for the cache to eventually let it go.

[bungeman, 2012-05-17 20:48:55]

Now that all the pre-requisites are out of the way, this is something that might
actually go in.

[reed1, 2012-05-17 20:59:34]

hmmm. can we move the renaming functionality into its own file?

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

http://codereview.appspot.com/5616047/diff/16001/src/ports/SkFontHost_win.cpp...
src/ports/SkFontHost_win.cpp:167: LogFontTypeface* lface =
reinterpret_cast<LogFontTypeface*>(face);
Do you really mean const_cast?

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

http://codereview.appspot.com/5616047/diff/16001/src/ports/SkFontHost_win.cpp...
src/ports/SkFontHost_win.cpp:166: class CustomLogFontTypeface : public
LogFontTypeface {
FontMemResourceLogFontTypeface?

http://codereview.appspot.com/5616047/diff/16001/src/ports/SkFontHost_win.cpp...
src/ports/SkFontHost_win.cpp:170: 
/** This is a FontMemResource, which we assume ownership of in our constructor
*/

http://codereview.appspot.com/5616047/diff/16001/src/ports/SkFontHost_win.cpp...
src/ports/SkFontHost_win.cpp:1530: #ifdef UNICODE
are you converting utf8->utf16 here?

[bungeman, 2012-05-17 21:46:19]

Additionally, I will take a look at splitting this out into another file

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

http://codereview.appspot.com/5616047/diff/16001/src/ports/SkFontHost_win.cpp...
src/ports/SkFontHost_win.cpp:167: LogFontTypeface* lface =
reinterpret_cast<LogFontTypeface*>(face);
On 2012/05/17 20:59:34, reed1 wrote:
> Do you really mean const_cast?

Commenting on the old code? This is now static_cast, face is already non-const.

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

http://codereview.appspot.com/5616047/diff/16001/src/ports/SkFontHost_win.cpp...
src/ports/SkFontHost_win.cpp:166: class CustomLogFontTypeface : public
LogFontTypeface {
On 2012/05/17 20:59:34, reed1 wrote:
> FontMemResourceLogFontTypeface?

Changed to FontMemResourceTypeface.

http://codereview.appspot.com/5616047/diff/16001/src/ports/SkFontHost_win.cpp...
src/ports/SkFontHost_win.cpp:170: 
On 2012/05/17 20:59:34, reed1 wrote:
> /** This is a FontMemResource, which we assume ownership of in our constructor
> */

Changed name to fFontMemResource. Added comment to constructor about ownership.

http://codereview.appspot.com/5616047/diff/16001/src/ports/SkFontHost_win.cpp...
src/ports/SkFontHost_win.cpp:1530: #ifdef UNICODE
On 2012/05/17 20:59:34, reed1 wrote:
> are you converting utf8->utf16 here?

utf8 to ucs2 or something like that. Whatever Windows thinks WideChar is. This
is existing code which was factored out (see block at 1361 patch set 5).

[bungeman, 2012-05-18 13:51:23]

Split out the font rename code.

[reed1, 2012-05-18 14:07:35]

better

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

http://codereview.appspot.com/5616047/diff/24001/src/ports/SkFontHost_win.cpp...
src/ports/SkFontHost_win.cpp:1386: */
can fontData be const?

http://codereview.appspot.com/5616047/diff/24001/src/sfnt/SkOTUtils.h
File src/sfnt/SkOTUtils.h (right):

http://codereview.appspot.com/5616047/diff/24001/src/sfnt/SkOTUtils.h#newcode25
src/sfnt/SkOTUtils.h:25: */
What name is the fontName, in 'name' terms? family? full? etc.
Is it required to be in UTF8?
Seems like this guy returning a bool or error code is clearer than having to
check the size of the output param.
Can/should the output param be a SkWStream? Or (better yet) can this function
just return SkDataRef (which can be NULL for failure)?

[bungeman, 2012-05-18 18:34:48]

Perhaps a little less ugly now?

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

http://codereview.appspot.com/5616047/diff/24001/src/ports/SkFontHost_win.cpp...
src/ports/SkFontHost_win.cpp:1386: */
On 2012/05/18 14:07:35, reed1 wrote:
> can fontData be const?

Apparently, it can. However, changing to SkData*.

http://codereview.appspot.com/5616047/diff/24001/src/sfnt/SkOTUtils.h
File src/sfnt/SkOTUtils.h (right):

http://codereview.appspot.com/5616047/diff/24001/src/sfnt/SkOTUtils.h#newcode25
src/sfnt/SkOTUtils.h:25: */
On 2012/05/18 14:07:35, reed1 wrote:
> What name is the fontName, in 'name' terms? family? full? etc.
Added comment.
> Is it required to be in UTF8?
It is required to be ASCII, though UTF8 could be added at some point. Added
comment.

> Seems like this guy returning a bool or error code is clearer than having to
> check the size of the output param.
> Can/should the output param be a SkWStream? Or (better yet) can this function
> just return SkDataRef (which can be NULL for failure)?
Done.

http://codereview.appspot.com/5616047/diff/17006/src/sfnt/SkOTUtils.cpp
File src/sfnt/SkOTUtils.cpp (right):

http://codereview.appspot.com/5616047/diff/17006/src/sfnt/SkOTUtils.cpp#newco...
src/sfnt/SkOTUtils.cpp:12: #include "SkTDArray.h"
I've already removed this.

[bungeman, 2012-05-18 19:31:27]

Committed revision r4001 and r4002.