Use more than double buffering in SoftwareOutputDeviceMac

Use more than double buffering in SoftwareOutputDeviceMac

Previously, this would flip between two buffers, but that had issues
because the IOSurfaces would sometimes still be being read by the
window server while the next frame was writing (leading to flashes).

Change this to a buffer queue very similar to the GPU output surface
BufferQueue. Use the IOSurfaceIsInUse function to ensure that buffers
are not painted to until they are not being read by the window server.

Limit the number of buffers being held in the queue to 4 (determined
empirically). If we need more than 4, then they will thrash (by
frequent allocation and discard). The window server will keep more
than 4 IOSurfaces alive (as many as it needs) regardless of our
purging here.

BUG=650484
Review-Url: https://codereview.chromium.org/2866683003
Cr-Commit-Position: refs/heads/master@{#469876}
Committed: https://chromium.googlesource.com/chromium/src/+/469d0548dfb4c3304c03571ba3f0f1a9bc58e2a4

[ccameron, 2017-05-05 17:44:16]

ccameron@chromium.org changed reviewers:
+ jbauman@chromium.org

[ccameron, 2017-05-05 17:44:17]

I tried very hard to this share the logic of BufferQueue, but ultimately didn't
find a clean way to do it.

[jbauman, 2017-05-05 19:19:25]

lgtm with a couple of nits.

https://codereview.chromium.org/2866683003/diff/1/content/browser/compositor/...
File content/browser/compositor/software_output_device_mac.h (right):

https://codereview.chromium.org/2866683003/diff/1/content/browser/compositor/...
content/browser/compositor/software_output_device_mac.h:63: // This surface is
double-buffered. The two buffers are in |io_surfaces_|,
Fix comment.

https://codereview.chromium.org/2866683003/diff/1/content/browser/compositor/...
File content/browser/compositor/software_output_device_mac.mm (right):

https://codereview.chromium.org/2866683003/diff/1/content/browser/compositor/...
content/browser/compositor/software_output_device_mac.mm:61: }
Maybe add a DCHECK_NE(previous_paint_buffer, current_paint_buffer_); here.

[ccameron, 2017-05-05 22:06:08]

Thanks -- updated (and added a test for previous==next).

https://codereview.chromium.org/2866683003/diff/1/content/browser/compositor/...
File content/browser/compositor/software_output_device_mac.h (right):

https://codereview.chromium.org/2866683003/diff/1/content/browser/compositor/...
content/browser/compositor/software_output_device_mac.h:63: // This surface is
double-buffered. The two buffers are in |io_surfaces_|,
On 2017/05/05 19:19:25, jbauman wrote:
> Fix comment.

Done.

https://codereview.chromium.org/2866683003/diff/1/content/browser/compositor/...
File content/browser/compositor/software_output_device_mac.mm (right):

https://codereview.chromium.org/2866683003/diff/1/content/browser/compositor/...
content/browser/compositor/software_output_device_mac.mm:61: }
On 2017/05/05 19:19:25, jbauman wrote:
> Maybe add a DCHECK_NE(previous_paint_buffer, current_paint_buffer_); here.

Done.

[ccameron, 2017-05-05 22:06:15]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2017-05-05 22:06:15]

The patchset sent to the CQ was uploaded after l-g-t-m from jbauman@chromium.org
Link to the patchset: https://codereview.chromium.org/2866683003/#ps20001
(title: "Review feedback and tests")

[commit-bot: I haz the power, 2017-05-05 22:07:36]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ccameron, 2017-05-05 22:12:03]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2017-05-05 22:12:04]

The patchset sent to the CQ was uploaded after l-g-t-m from jbauman@chromium.org
Link to the patchset: https://codereview.chromium.org/2866683003/#ps40001
(title: "Review feedback and tests")

[commit-bot: I haz the power, 2017-05-05 22:13:14]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 22:31:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-05 22:31:31]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ccameron, 2017-05-06 06:29:46]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2017-05-06 06:29:46]

The patchset sent to the CQ was uploaded after l-g-t-m from jbauman@chromium.org
Link to the patchset: https://codereview.chromium.org/2866683003/#ps60001
(title: "Fix compile")

[commit-bot: I haz the power, 2017-05-06 06:30:16]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ccameron, 2017-05-06 09:15:33]

The CQ bit was unchecked by ccameron@chromium.org

[ccameron, 2017-05-06 09:15:38]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2017-05-06 09:16:16]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-06 19:15:47]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494062138899700,
"parent_rev": "fc330ee6e339230c34b73c54cb65f9ff6a4a20f1", "commit_rev":
"469d0548dfb4c3304c03571ba3f0f1a9bc58e2a4"}

[commit-bot: I haz the power, 2017-05-06 19:15:58]

Description was changed from

==========
Use more than double buffering in SoftwareOutputDeviceMac

Previously, this would flip between two buffers, but that had issues
because the IOSurfaces would sometimes still be being read by the
window server while the next frame was writing (leading to flashes).

Change this to a buffer queue very similar to the GPU output surface
BufferQueue. Use the IOSurfaceIsInUse function to ensure that buffers
are not painted to until they are not being read by the window server.

Limit the number of buffers being held in the queue to 4 (determined
empirically). If we need more than 4, then they will thrash (by
frequent allocation and discard). The window server will keep more
than 4 IOSurfaces alive (as many as it needs) regardless of our
purging here.

BUG=650484
==========

to

==========
Use more than double buffering in SoftwareOutputDeviceMac

Previously, this would flip between two buffers, but that had issues
because the IOSurfaces would sometimes still be being read by the
window server while the next frame was writing (leading to flashes).

Change this to a buffer queue very similar to the GPU output surface
BufferQueue. Use the IOSurfaceIsInUse function to ensure that buffers
are not painted to until they are not being read by the window server.

Limit the number of buffers being held in the queue to 4 (determined
empirically). If we need more than 4, then they will thrash (by
frequent allocation and discard). The window server will keep more
than 4 IOSurfaces alive (as many as it needs) regardless of our
purging here.

BUG=650484

Review-Url: https://codereview.chromium.org/2866683003
Cr-Commit-Position: refs/heads/master@{#469876}
Committed:
https://chromium.googlesource.com/chromium/src/+/469d0548dfb4c3304c03571ba3f0...
==========

[commit-bot: I haz the power, 2017-05-06 19:15:59]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/469d0548dfb4c3304c03571ba3f0...