Partial swaps on OSX

Partial swaps on OSX

In combination with shawnsingh's scissoring, this is a huge win on 10.5 which uses a readback through main memory. Some numbers given on https://bugs.webkit.org/show_bug.cgi?id=67341

The win on >= 10.6 is probably less because there is not readback through main memory. The 10.6 implementation assumes that nothing outside the partial swap region was damaged (i.e. it's a SwapBuffers that always preserves the backbuffer --- it's not a true partial swap a la GLX_MESA_copy_sub_buffers). This is all that the scissoring needs.

BUG=none
TEST=ran with shawnsingh's patch on http://www.webkit.org/blog-files/3d-transforms/poster-circle.html on 10.6 and 10.5

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

[apatrick_chromium, 2011-12-05 22:42:45]

Some nits

http://codereview.chromium.org/8726046/diff/8001/content/browser/renderer_hos...
File content/browser/renderer_host/accelerated_surface_container_mac.h (right):

http://codereview.chromium.org/8726046/diff/8001/content/browser/renderer_hos...
content/browser/renderer_host/accelerated_surface_container_mac.h:86: void
set_was_painted_to(uint64 surface_id,
nit: const gfx::Rect&

http://codereview.chromium.org/8726046/diff/8001/content/browser/renderer_hos...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

http://codereview.chromium.org/8726046/diff/8001/content/browser/renderer_hos...
content/browser/renderer_host/render_widget_host_view_mac.mm:900: j 
DCHECK(view);
j at beginning of line

http://codereview.chromium.org/8726046/diff/8001/content/browser/renderer_hos...
content/browser/renderer_host/render_widget_host_view_mac.mm:911: // The surface
is hidden until its first paint, to not show gargabe.
gargabe -> garbage

http://codereview.chromium.org/8726046/diff/8001/content/common/gpu/image_tra...
File content/common/gpu/image_transport_surface_mac.cc (right):

http://codereview.chromium.org/8726046/diff/8001/content/common/gpu/image_tra...
content/common/gpu/image_transport_surface_mac.cc:478: GLint current_alignment =
0, current_pack_row_length=0;
Missing spaces in
current_pack_row_length=0

[jonathan.backer, 2011-12-06 18:29:31]

http://codereview.chromium.org/8726046/diff/8001/content/browser/renderer_hos...
File content/browser/renderer_host/accelerated_surface_container_mac.h (right):

http://codereview.chromium.org/8726046/diff/8001/content/browser/renderer_hos...
content/browser/renderer_host/accelerated_surface_container_mac.h:86: void
set_was_painted_to(uint64 surface_id,
On 2011/12/05 22:42:45, apatrick_chromium wrote:
> nit: const gfx::Rect&

Done.

http://codereview.chromium.org/8726046/diff/8001/content/browser/renderer_hos...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

http://codereview.chromium.org/8726046/diff/8001/content/browser/renderer_hos...
content/browser/renderer_host/render_widget_host_view_mac.mm:900: j 
DCHECK(view);
On 2011/12/05 22:42:45, apatrick_chromium wrote:
> j at beginning of line

<whine> I want focus follows mouse on OSX </whine>

Done.

http://codereview.chromium.org/8726046/diff/8001/content/browser/renderer_hos...
content/browser/renderer_host/render_widget_host_view_mac.mm:911: // The surface
is hidden until its first paint, to not show gargabe.
On 2011/12/05 22:42:45, apatrick_chromium wrote:
> gargabe -> garbage

Done.

http://codereview.chromium.org/8726046/diff/8001/content/common/gpu/image_tra...
File content/common/gpu/image_transport_surface_mac.cc (right):

http://codereview.chromium.org/8726046/diff/8001/content/common/gpu/image_tra...
content/common/gpu/image_transport_surface_mac.cc:478: GLint current_alignment =
0, current_pack_row_length=0;
On 2011/12/05 22:42:45, apatrick_chromium wrote:
> Missing spaces in
> current_pack_row_length=0

Done.

[Ken Russell (switch to Gerrit), 2011-12-06 18:38:59]

LGTM with one issue fixed.

http://codereview.chromium.org/8726046/diff/17001/content/common/gpu/image_tr...
File content/common/gpu/image_transport_surface_mac.cc (right):

http://codereview.chromium.org/8726046/diff/17001/content/common/gpu/image_tr...
content/common/gpu/image_transport_surface_mac.cc:480:
glGetIntegerv(GL_PACK_ALIGNMENT, &current_pack_row_length);
GL_PACK_ROW_LENGTH

[apatrick_chromium, 2011-12-06 19:23:47]

What kbr said then LGTM.

[jonathan.backer, 2011-12-06 19:36:12]

http://codereview.chromium.org/8726046/diff/17001/content/common/gpu/image_tr...
File content/common/gpu/image_transport_surface_mac.cc (right):

http://codereview.chromium.org/8726046/diff/17001/content/common/gpu/image_tr...
content/common/gpu/image_transport_surface_mac.cc:480:
glGetIntegerv(GL_PACK_ALIGNMENT, &current_pack_row_length);
On 2011/12/06 18:38:59, kbr wrote:
> GL_PACK_ROW_LENGTH

Awesome catch. That would have been an impossible bug to track down!

Done.

[sky, 2011-12-06 19:40:39]

LGTM

http://codereview.chromium.org/8726046/diff/18002/content/browser/renderer_ho...
File content/browser/renderer_host/accelerated_surface_container_mac.cc (right):

http://codereview.chromium.org/8726046/diff/18002/content/browser/renderer_ho...
content/browser/renderer_host/accelerated_surface_container_mac.cc:244: void
AcceleratedSurfaceContainerMac::set_was_painted_to(uint64 surface_id) {
nit: position should match header.

http://codereview.chromium.org/8726046/diff/18002/content/browser/renderer_ho...
File content/browser/renderer_host/accelerated_surface_container_mac.h (right):

http://codereview.chromium.org/8726046/diff/18002/content/browser/renderer_ho...
content/browser/renderer_host/accelerated_surface_container_mac.h:155: void
set_was_painted_to_common(uint64 surface_id);
methods before variables.

[jonathan.backer, 2011-12-06 20:24:41]

http://codereview.chromium.org/8726046/diff/18002/content/browser/renderer_ho...
File content/browser/renderer_host/accelerated_surface_container_mac.cc (right):

http://codereview.chromium.org/8726046/diff/18002/content/browser/renderer_ho...
content/browser/renderer_host/accelerated_surface_container_mac.cc:244: void
AcceleratedSurfaceContainerMac::set_was_painted_to(uint64 surface_id) {
On 2011/12/06 19:40:39, sky wrote:
> nit: position should match header.

Done.

http://codereview.chromium.org/8726046/diff/18002/content/browser/renderer_ho...
File content/browser/renderer_host/accelerated_surface_container_mac.h (right):

http://codereview.chromium.org/8726046/diff/18002/content/browser/renderer_ho...
content/browser/renderer_host/accelerated_surface_container_mac.h:155: void
set_was_painted_to_common(uint64 surface_id);
On 2011/12/06 19:40:39, sky wrote:
> methods before variables.

Done.