Normalize font BBox to make sure it's correct when rendering a PDF on the mac.

Normalize font BBox to make sure it's correct when rendering a PDF on the mac.
BUG=crbug.com/124572

[abodenha, 2012-08-28 18:57:17]

My first Skia change.  Please let me know if I've done anything particularly
stupid here.

[caryclark1, 2012-08-28 19:00:36]

https://codereview.appspot.com/6501049/diff/1/ports/SkFontHost_mac_coretext.cpp
File ports/SkFontHost_mac_coretext.cpp (right):

https://codereview.appspot.com/6501049/diff/1/ports/SkFontHost_mac_coretext.c...
ports/SkFontHost_mac_coretext.cpp:1620: }
Use 

SkTSwap<uint16_t>(info->fBBox.fTop, info->fBBox.fBottom); 

instead.

[Steve VanDeBogart, 2012-08-28 19:06:01]

https://codereview.appspot.com/6501049/diff/1/ports/SkFontHost_mac_coretext.cpp
File ports/SkFontHost_mac_coretext.cpp (right):

https://codereview.appspot.com/6501049/diff/1/ports/SkFontHost_mac_coretext.c...
ports/SkFontHost_mac_coretext.cpp:1611: CGRect bbox =
CTFontGetBoundingBox(ctFont);
You may want to look at the other use of CTFontGetBoundingBox in this file.  It
uses some macros like CGRectGetMaxY_inline to compute the bounds.  You might be
able to use directly in the call to the SkIRect constructor.

[abodenha, 2012-08-28 19:24:55]

Please take another look.

https://codereview.appspot.com/6501049/diff/1/ports/SkFontHost_mac_coretext.cpp
File ports/SkFontHost_mac_coretext.cpp (right):

https://codereview.appspot.com/6501049/diff/1/ports/SkFontHost_mac_coretext.c...
ports/SkFontHost_mac_coretext.cpp:1611: CGRect bbox =
CTFontGetBoundingBox(ctFont);
Good call. That's much cleaner than what I had here.

On 2012/08/28 19:06:01, Steve VanDeBogart wrote:
> You may want to look at the other use of CTFontGetBoundingBox in this file. 
It
> uses some macros like CGRectGetMaxY_inline to compute the bounds.  You might
be
> able to use directly in the call to the SkIRect constructor.

https://codereview.appspot.com/6501049/diff/1/ports/SkFontHost_mac_coretext.c...
ports/SkFontHost_mac_coretext.cpp:1620: }
Cool.  I figured there was a swap utility somewhere.  Thanks.

On 2012/08/28 19:00:36, caryclark1 wrote:
> Use 
> 
> SkTSwap<uint16_t>(info->fBBox.fTop, info->fBBox.fBottom); 
> 
> instead.

[Steve VanDeBogart, 2012-08-28 19:40:01]

I assume you tested this change and it gave you the values you expected?

LGTM

I'll land the fix.

https://codereview.appspot.com/6501049/diff/4001/ports/SkFontHost_mac_coretex...
File ports/SkFontHost_mac_coretext.cpp (right):

https://codereview.appspot.com/6501049/diff/4001/ports/SkFontHost_mac_coretex...
ports/SkFontHost_mac_coretext.cpp:1613: CGToScalar( CGRectGetMinX_inline(bbox)),
 // Left
nit: extra space.

[Steve VanDeBogart, 2012-08-28 19:48:53]

Landed as r5325

FYI: your patch was one level to high, path should have been
src/ports/SkFontHost_mac_coretext.cpp

[abodenha, 2012-08-28 19:54:08]

On Tue, Aug 28, 2012 at 12:40 PM, <vandebo@chromium.org> wrote:

> I assume you tested this change and it gave you the values you expected?
>
> Yep.


> LGTM
>
> I'll land the fix.
>
>
> https://codereview.appspot.**com/6501049/diff/4001/ports/**
>
SkFontHost_mac_coretext.cpp<https://codereview.appspot.com/6501049/diff/4001/ports/SkFontHost_mac_coretext.cpp>
> File ports/SkFontHost_mac_coretext.**cpp (right):
>
> https://codereview.appspot.**com/6501049/diff/4001/ports/**
>
SkFontHost_mac_coretext.cpp#**newcode1613<https://codereview.appspot.com/6501049/diff/4001/ports/SkFontHost_mac_coretext.cpp#newcode1613>
> ports/SkFontHost_mac_coretext.**cpp:1613: CGToScalar(
> CGRectGetMinX_inline(bbox)),  // Left
> nit: extra space.
>
>
https://codereview.appspot.**com/6501049/<https://codereview.appspot.com/6501...
>



-- 
Albert Bodenhamer | Software Engineer | abodenha@chromium.<abodenha@google.com>
org