O3D Mac: Add CoreAnimation support

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

[maf, 2010-04-09 02:36:09]

Hey Ken, have a look at this. 
It's only using the new CA code in the Safari/10.6+ case. 



Maf

[Ken Russell (switch to Gerrit), 2010-04-09 17:11:05]

Generally looks good. A few comments.

http://codereview.chromium.org/577038/diff/29002/53003
File plugin/mac/main_mac.mm (right):

http://codereview.chromium.org/577038/diff/29002/53003#newcode659
plugin/mac/main_mac.mm:659: // If we're building for Core Animation then we
prefer the Cocoa event model.
>80 chars

http://codereview.chromium.org/577038/diff/29002/53003#newcode730
plugin/mac/main_mac.mm:730: // In order of preference. Preference is now
determined by compatibility,
This block of code appears to be duplicated now.

http://codereview.chromium.org/577038/diff/29002/53001
File plugin/mac/o3d_layer.mm (right):

http://codereview.chromium.org/577038/diff/29002/53001#newcode90
plugin/mac/o3d_layer.mm:90: #define DRAW_REAL_O3D_GRAPHICS 1
I think the test code above and the references to it below should be deleted. If
you want to leave it in for one revision and remove it afterward, that's fine;
the #define DRAW_REAL_O3D_GRAPHICS should be removed regardless since it's
unused.

http://codereview.chromium.org/577038/diff/29002/53007
File plugin/mac/plugin_mac.mm (right):

http://codereview.chromium.org/577038/diff/29002/53007#newcode280
plugin/mac/plugin_mac.mm:280: //CGLSetParameter(obj->mac_cgl_context_,
kCGLCPSwapInterval, &sync);
This should be uncommented at this point since we don't hit this code any more
when in Core Animation mode.

[maf, 2010-04-10 01:30:18]

http://codereview.chromium.org/577038/diff/29002/53003
File plugin/mac/main_mac.mm (right):

http://codereview.chromium.org/577038/diff/29002/53003#newcode659
plugin/mac/main_mac.mm:659: // If we're building for Core Animation then we
prefer the Cocoa event model.
On 2010/04/09 17:11:05, kbr wrote:
> >80 chars
> 

Done.

http://codereview.chromium.org/577038/diff/29002/53003#newcode730
plugin/mac/main_mac.mm:730: // In order of preference. Preference is now
determined by compatibility,
On 2010/04/09 17:11:05, kbr wrote:
> This block of code appears to be duplicated now.
> 

Done.

http://codereview.chromium.org/577038/diff/29002/53001
File plugin/mac/o3d_layer.mm (right):

http://codereview.chromium.org/577038/diff/29002/53001#newcode90
plugin/mac/o3d_layer.mm:90: #define DRAW_REAL_O3D_GRAPHICS 1
On 2010/04/09 17:11:05, kbr wrote:
> I think the test code above and the references to it below should be deleted.
If
> you want to leave it in for one revision and remove it afterward, that's fine;
> the #define DRAW_REAL_O3D_GRAPHICS should be removed regardless since it's
> unused.
> 

Done.

[Ken Russell (switch to Gerrit), 2010-04-11 03:46:45]

http://codereview.chromium.org/577038/diff/53010/14010
File plugin/mac/main_mac.mm (right):

http://codereview.chromium.org/577038/diff/53010/14010#newcode730
plugin/mac/main_mac.mm:730: // not by modernity, and so is the opposite of the
order I first used.
This comment is still duplicated.

http://codereview.chromium.org/577038/diff/53010/14010#newcode1017
plugin/mac/main_mac.mm:1017: O3DLayer * o3dLayer = ObjO3DLayer(obj);
"O3DLayer * " doesn't conform to (C++) style guide.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Pointer_and_Re...

http://codereview.chromium.org/577038/diff/53010/14014
File plugin/mac/plugin_mac.mm (right):

http://codereview.chromium.org/577038/diff/53010/14014#newcode242
plugin/mac/plugin_mac.mm:242: O3DLayer * o3dLayer =
static_cast<O3DLayer*>(obj->gl_layer_);
"O3DLayer * " doesn't conform to (C++) style guide.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Pointer_and_Re...

http://codereview.chromium.org/577038/diff/53010/14014#newcode280
plugin/mac/plugin_mac.mm:280: //CGLSetParameter(obj->mac_cgl_context_,
kCGLCPSwapInterval, &sync);
Uncomment.

[maf, 2010-04-13 01:25:52]

http://codereview.chromium.org/577038/diff/53010/14010
File plugin/mac/main_mac.mm (right):

http://codereview.chromium.org/577038/diff/53010/14010#newcode730
plugin/mac/main_mac.mm:730: // not by modernity, and so is the opposite of the
order I first used.
On 2010/04/11 03:46:45, kbr wrote:
> This comment is still duplicated.
> 

Done.

http://codereview.chromium.org/577038/diff/53010/14010#newcode1017
plugin/mac/main_mac.mm:1017: O3DLayer * o3dLayer = ObjO3DLayer(obj);
On 2010/04/11 03:46:45, kbr wrote:
> "O3DLayer * " doesn't conform to (C++) style guide.
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Pointer_and_Re...

Done.

http://codereview.chromium.org/577038/diff/53010/14014
File plugin/mac/plugin_mac.mm (right):

http://codereview.chromium.org/577038/diff/53010/14014#newcode242
plugin/mac/plugin_mac.mm:242: O3DLayer * o3dLayer =
static_cast<O3DLayer*>(obj->gl_layer_);
On 2010/04/11 03:46:45, kbr wrote:
> "O3DLayer * " doesn't conform to (C++) style guide.
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Pointer_and_Re...
> 

Done.

http://codereview.chromium.org/577038/diff/53010/14014#newcode280
plugin/mac/plugin_mac.mm:280: //CGLSetParameter(obj->mac_cgl_context_,
kCGLCPSwapInterval, &sync);
On 2010/04/11 03:46:45, kbr wrote:
> Uncomment.
> 

Done.

[Ken Russell (switch to Gerrit), 2010-04-13 01:29:16]

LGTM. Nice work.