cache offscreen to speedup generateImage

[caryclark1, 2011-11-02 17:38:58]

LGTM

http://codereview.appspot.com/5318073/diff/1001/src/ports/SkFontHost_mac_core...
File src/ports/SkFontHost_mac_coretext.cpp (right):

http://codereview.appspot.com/5318073/diff/1001/src/ports/SkFontHost_mac_core...
src/ports/SkFontHost_mac_coretext.cpp:35: while (height) {
looks like this will wipe memory if height is accidentally == 0. Worth an
assert? Or preloop check and exit?

http://codereview.appspot.com/5318073/diff/1001/src/ports/SkFontHost_mac_core...
src/ports/SkFontHost_mac_coretext.cpp:223: CGRGBPixel* Offscreen::getCG(const
SkGlyph& glyph, float grayColor,
grayColor is only ever 0 or 1 (also fGrayColor is set to -1 to show it is
uninitalized but could be set to !grayColor in that case) so could it be an int
or even a bool? Only when CGContextSetGrayFillColor is the float value required,
as far as I can tell.

http://codereview.appspot.com/5318073/diff/1001/src/ports/SkFontHost_mac_core...
src/ports/SkFontHost_mac_coretext.cpp:267: CGContextSetFontSize(fCG, 1); //
cgFont know's its size
s/know's/knows/

http://codereview.appspot.com/5318073/diff/1001/src/ports/SkFontHost_mac_core...
src/ports/SkFontHost_mac_coretext.cpp:311: *rowBytesPtr = fSize.fWidth * 4;
fSize.fWidth * 4 shows up in a few places -- worth (for documentation) having a
local called rowBytes?

[reed1, 2011-11-02 17:44:37]

http://codereview.appspot.com/5318073/diff/1001/src/ports/SkFontHost_mac_core...
File src/ports/SkFontHost_mac_coretext.cpp (right):

http://codereview.appspot.com/5318073/diff/1001/src/ports/SkFontHost_mac_core...
src/ports/SkFontHost_mac_coretext.cpp:35: while (height) {
If height is 0, this will do nothing (I think).

On 2011/11/02 17:38:59, caryclark1 wrote:
> looks like this will wipe memory if height is accidentally == 0. Worth an
> assert? Or preloop check and exit?

http://codereview.appspot.com/5318073/diff/1001/src/ports/SkFontHost_mac_core...
src/ports/SkFontHost_mac_coretext.cpp:223: CGRGBPixel* Offscreen::getCG(const
SkGlyph& glyph, float grayColor,
Agreed, it should be an int or bool. Will change.

On 2011/11/02 17:38:59, caryclark1 wrote:
> grayColor is only ever 0 or 1 (also fGrayColor is set to -1 to show it is
> uninitalized but could be set to !grayColor in that case) so could it be an
int
> or even a bool? Only when CGContextSetGrayFillColor is the float value
required,
> as far as I can tell.

http://codereview.appspot.com/5318073/diff/1001/src/ports/SkFontHost_mac_core...
src/ports/SkFontHost_mac_coretext.cpp:311: *rowBytesPtr = fSize.fWidth * 4;
Agreed.

On 2011/11/02 17:38:59, caryclark1 wrote:
> fSize.fWidth * 4 shows up in a few places -- worth (for documentation) having
a
> local called rowBytes?

[reed1, 2011-11-02 17:56:56]

will now write a bench to time font cache creation before checking this in.

[caryclark1, 2011-11-02 18:51:04]

LGTM

[bungeman, 2011-11-02 19:49:40]

On 2011/11/02 18:16:39, reed1 wrote:

LGTM