Refactor blits

Refactor blits to be more cross-platform.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=21565

[Antoine Labour, 2009-07-24 06:31:09]

Amanda: I don't have a mac, and could only check that the trybots came out
green, so please review carefully the mac parts. Also, please let me know if
you'd rather have me wait until you get your changes in.
John and Evan: please review.

[Amanda Walker (Google), 2009-07-24 13:14:03]

Nice, much cleaner :-).  Mac stuff LGTM, and no need to wait for my
changes--there shouldn't be any destructive overlap.

[Amanda Walker (Google), 2009-07-24 13:14:18]

http://codereview.chromium.org/159190/diff/1001/15
File base/gfx/blit.cc (right):

http://codereview.chromium.org/159190/diff/1001/15#newcode55
Line 55: #endif
I don't understand this typedef in two respects.  For OS_MACOSX,
gfx::NativeDrawingContext is already a typedef for CGContextRef, so the #if
should not be needed.  Also, what is the purpose of adding "scoped" without
wrapping it in a scoping template or some such?  If the following routines are
all this is used for, I think you can safely get rid of the typedef and change
GetContextFromCanvas to just return a NativeDrawingContext.

[Evan Martin, 2009-07-24 14:56:57]

LGTM, though I share Amanda's puzzlement about the scoped bits.

[pimanttr, 2009-07-24 16:26:09]

http://codereview.chromium.org/159190/diff/1001/15
File base/gfx/blit.cc (right):

http://codereview.chromium.org/159190/diff/1001/15#newcode55
Line 55: #endif
On 2009/07/24 13:14:18, Amanda Walker (Google) wrote:
> I don't understand this typedef in two respects.  For OS_MACOSX,
> gfx::NativeDrawingContext is already a typedef for CGContextRef, so the #if
> should not be needed.  Also, what is the purpose of adding "scoped" without
> wrapping it in a scoping template or some such?  If the following routines are
> all this is used for, I think you can safely get rid of the typedef and change
> GetContextFromCanvas to just return a NativeDrawingContext.

Unless I'm crazy NativeDrawingContext is a CGContext* on Mac - whereas
device.GetBitmapContext() returns a CGContextRef. I assumed the CGContextRef was
doing ref-counting, so I wanted to make sure we didn't lose the reference.

[Amanda Walker (Google), 2009-07-24 16:32:33]

Ah--that's a misunderstanding.  In native Mac API terms, a "Ref"
suffix just denotes an opaque pointer.  It is interchangeable with a
CGContext * (struct CGContext is private, so you can't actually access
its fields directly, or create one except via system APIs).

--Amanda

On Fri, Jul 24, 2009 at 12:26 PM, <pimanttr@gmail.com> wrote:
> Unless I'm crazy NativeDrawingContext is a CGContext* on Mac - whereas
> device.GetBitmapContext() returns a CGContextRef. I assumed the
> CGContextRef was doing ref-counting, so I wanted to make sure we didn't
> lose the reference.
>
> http://codereview.chromium.org/159190
>



-- 
"Portability is generally the result of advance planning rather than trench
warfare involving #ifdef" -- Henry Spencer (1992)

[Antoine Labour, 2009-07-24 16:35:13]

On Fri, Jul 24, 2009 at 4:32 PM, Amanda Walker<awalker@google.com> wrote:
> Ah--that's a misunderstanding. =A0In native Mac API terms, a "Ref"
> suffix just denotes an opaque pointer. =A0It is interchangeable with a
> CGContext * (struct CGContext is private, so you can't actually access
> its fields directly, or create one except via system APIs).

Ah, got it. I'll remove that Scoped thing then.

Antoine

>
> --Amanda
>
> On Fri, Jul 24, 2009 at 12:26 PM, <pimanttr@gmail.com> wrote:
>> Unless I'm crazy NativeDrawingContext is a CGContext* on Mac - whereas
>> device.GetBitmapContext() returns a CGContextRef. I assumed the
>> CGContextRef was doing ref-counting, so I wanted to make sure we didn't
>> lose the reference.
>>
>> http://codereview.chromium.org/159190
>>
>
>
>
> --
> "Portability is generally the result of advance planning rather than tren=
ch
> warfare involving #ifdef" -- Henry Spencer (1992)
>



--=20
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

[jam, 2009-07-24 18:28:37]

lgtm, nice cleanup

[Antoine Labour, 2009-07-24 20:24:21]

On Fri, Jul 24, 2009 at 4:26 PM, <pimanttr@gmail.com> wrote:
>
> http://codereview.chromium.org/159190/diff/1001/15
> File base/gfx/blit.cc (right):
>
> http://codereview.chromium.org/159190/diff/1001/15#newcode55
> Line 55: #endif
> On 2009/07/24 13:14:18, Amanda Walker (Google) wrote:
>>
>> I don't understand this typedef in two respects. =A0For OS_MACOSX,
>> gfx::NativeDrawingContext is already a typedef for CGContextRef, so
>
> the #if
>>
>> should not be needed. =A0Also, what is the purpose of adding "scoped"
>
> without
>>
>> wrapping it in a scoping template or some such? =A0If the following
>
> routines are
>>
>> all this is used for, I think you can safely get rid of the typedef
>
> and change
>>
>> GetContextFromCanvas to just return a NativeDrawingContext.
>
> Unless I'm crazy NativeDrawingContext is a CGContext* on Mac - whereas
> device.GetBitmapContext() returns a CGContextRef. I assumed the
> CGContextRef was doing ref-counting, so I wanted to make sure we didn't
> lose the reference.
>
> http://codereview.chromium.org/159190
>

Updated the CL with that.

Antoine