Add half-float IOSurface GpuMemoryBuffer support

Add half-float IOSurface GpuMemoryBuffer support

Add support for gpu rasterization to half-float RGBA textures.

Do not add software rasterization support for half-float RGBA textures.
While this is possible, software rasterization to half-float is not a good
idea for performance reasons.

BUG=655171
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2786103003
Cr-Commit-Position: refs/heads/master@{#464114}
Committed: https://chromium.googlesource.com/chromium/src/+/c1fcde58c22a5825c44cbad17eb3ed84b8c2bcce

[ccameron, 2017-03-30 19:28:05]

Description was changed from

==========
Add half-float IOSurface GpuMemoryBuffer support

Add support for gpu rasterization to half-float RGBA textures.

Do not add software support for half-float RGBA textures. While this is
possible, software rasterization to half-float is not a good idea for
performance reasons.

BUG=655171
==========

to

==========
Add half-float IOSurface GpuMemoryBuffer support

Add support for gpu rasterization to half-float RGBA textures.

Do not add software support for half-float RGBA textures. While this is
possible, software rasterization to half-float is not a good idea for
performance reasons.

BUG=655171
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[ccameron, 2017-03-30 19:28:05]

Description was changed from

==========
Add half-float IOSurface GpuMemoryBuffer support

Add support for gpu rasterization to half-float RGBA textures.

Do not add software support for half-float RGBA textures. While this is
possible, software rasterization to half-float is not a good idea for
performance reasons.

BUG=655171
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Add half-float IOSurface GpuMemoryBuffer support

Add support for gpu rasterization to half-float RGBA textures.

Do not add software support for half-float RGBA textures. While this is
possible, software rasterization to half-float is not a good idea for
performance reasons.

BUG=655171
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[ccameron, 2017-03-30 19:28:21]

The CQ bit was checked by ccameron@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 19:29:05]

Dry run: 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-03-30 19:47:50]

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

[commit-bot: I haz the power, 2017-03-30 19:47:50]

Dry run: Try jobs failed on following builders:
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[ccameron, 2017-03-30 20:41:35]

The CQ bit was checked by ccameron@chromium.org to run a CQ dry run

[ccameron, 2017-03-30 20:42:01]

Description was changed from

==========
Add half-float IOSurface GpuMemoryBuffer support

Add support for gpu rasterization to half-float RGBA textures.

Do not add software support for half-float RGBA textures. While this is
possible, software rasterization to half-float is not a good idea for
performance reasons.

BUG=655171
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Add half-float IOSurface GpuMemoryBuffer support

Add support for gpu rasterization to half-float RGBA textures.

Do not add software rasterization support for half-float RGBA textures.
While this is possible, software rasterization to half-float is not a good
idea for performance reasons.

BUG=655171
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2017-03-30 20:42:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ccameron, 2017-03-30 21:04:42]

ccameron@chromium.org changed reviewers:
+ dcastagna@chromium.org, reveman@chromium.org, zmo@chromium.org

[ccameron, 2017-03-30 21:04:43]

ptal -- this will eventually be needed for wide-color and HDR on Mac. For the
moment it isn't used (but the tests exercise it, and I manually forced GPU
raster to half-float).

[commit-bot: I haz the power, 2017-03-30 21:21:28]

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

[commit-bot: I haz the power, 2017-03-30 21:21:28]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[Daniele Castagna, 2017-03-30 21:47:10]

Would it make sense in the future to add a shared memory implementation of half
float RGBA GMBs too?

https://codereview.chromium.org/2786103003/diff/20001/cc/raster/raster_buffer...
File cc/raster/raster_buffer_provider.cc (left):

https://codereview.chromium.org/2786103003/diff/20001/cc/raster/raster_buffer...
cc/raster/raster_buffer_provider.cc:30: case RGBA_F16:
I'm curious, why was this here in the first place?

https://codereview.chromium.org/2786103003/diff/20001/gpu/command_buffer/comm...
File gpu/command_buffer/common/gpu_memory_buffer_support.cc (right):

https://codereview.chromium.org/2786103003/diff/20001/gpu/command_buffer/comm...
gpu/command_buffer/common/gpu_memory_buffer_support.cc:120: return
capabilities.texture_half_float_linear;
Does this return false on all the non-mac platforms?
Does having support for texture_half_float_linear means we can also allocate
Gmbs with that format?

What if we're allocating shmem GMBs?

[ccameron, 2017-03-30 23:06:48]

The CQ bit was checked by ccameron@chromium.org to run a CQ dry run

[ccameron, 2017-03-30 23:07:03]

https://codereview.chromium.org/2786103003/diff/20001/cc/raster/raster_buffer...
File cc/raster/raster_buffer_provider.cc (left):

https://codereview.chromium.org/2786103003/diff/20001/cc/raster/raster_buffer...
cc/raster/raster_buffer_provider.cc:30: case RGBA_F16:
On 2017/03/30 21:47:10, Daniele Castagna wrote:
> I'm curious, why was this here in the first place?

I had added it when I added RGBA_F16 as a ResourceFormat, thinking "well, F16
should be render-able by software".

I re-thought this in this patch for two reasons. First is that software raster
to F16 is really slow. Second is that we don't have all of the plumbing
necessary to enable it (we don't set up skia to draw F16, etc).

https://codereview.chromium.org/2786103003/diff/20001/gpu/command_buffer/comm...
File gpu/command_buffer/common/gpu_memory_buffer_support.cc (right):

https://codereview.chromium.org/2786103003/diff/20001/gpu/command_buffer/comm...
gpu/command_buffer/common/gpu_memory_buffer_support.cc:120: return
capabilities.texture_half_float_linear;
On 2017/03/30 21:47:10, Daniele Castagna wrote:
> Does this return false on all the non-mac platforms?
> Does having support for texture_half_float_linear means we can also allocate
> Gmbs with that format?
> 
> What if we're allocating shmem GMBs?

Ops, this affected the behavior of the above cases. Fixed that.

IIUC this returns true if we can texture out of this GMB type, which should be
okay for any platform (even the ones where we can't create native half-float
GMBs).

But I may not understand the function's purpose completely.

[commit-bot: I haz the power, 2017-03-30 23:08:04]

Dry run: 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-03-31 00:10:23]

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

[commit-bot: I haz the power, 2017-03-31 00:10:24]

Dry run: Try jobs failed on following builders:
  android_optional_gpu_tests_rel on master.tryserver.chromium.android
(JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_opti...)

[Daniele Castagna, 2017-03-31 00:25:20]

https://codereview.chromium.org/2786103003/diff/20001/cc/raster/raster_buffer...
File cc/raster/raster_buffer_provider.cc (left):

https://codereview.chromium.org/2786103003/diff/20001/cc/raster/raster_buffer...
cc/raster/raster_buffer_provider.cc:30: case RGBA_F16:
On 2017/03/30 at 23:07:03, ccameron wrote:
> On 2017/03/30 21:47:10, Daniele Castagna wrote:
> > I'm curious, why was this here in the first place?
> 
> I had added it when I added RGBA_F16 as a ResourceFormat, thinking "well, F16
should be render-able by software".
> 
> I re-thought this in this patch for two reasons. First is that software raster
to F16 is really slow. Second is that we don't have all of the plumbing
necessary to enable it (we don't set up skia to draw F16, etc).

I didn't even know that Skia could do F16, that's why I asked. I thought they
would raster only BGR. Thanks for the explanation.

https://codereview.chromium.org/2786103003/diff/20001/gpu/command_buffer/comm...
File gpu/command_buffer/common/gpu_memory_buffer_support.cc (right):

https://codereview.chromium.org/2786103003/diff/20001/gpu/command_buffer/comm...
gpu/command_buffer/common/gpu_memory_buffer_support.cc:120: return
capabilities.texture_half_float_linear;
On 2017/03/30 at 23:07:03, ccameron wrote:
> On 2017/03/30 21:47:10, Daniele Castagna wrote:
> > Does this return false on all the non-mac platforms?
> > Does having support for texture_half_float_linear means we can also allocate
> > Gmbs with that format?
> > 
> > What if we're allocating shmem GMBs?
> 
> Ops, this affected the behavior of the above cases. Fixed that.
> 
> IIUC this returns true if we can texture out of this GMB type, which should be
okay for any platform (even the ones where we can't create native half-float
GMBs).
> 
> But I may not understand the function's purpose completely.

Looking at IsGpuMemoryBufferFormatSupported makes me think this returning true
means we can allocate a GBM with that format.

For gfx::BufferFormat::UYVY_422 for example, image_ycbcr_422 is true only on
mac:
https://cs.chromium.org/chromium/src/gpu/command_buffer/service/feature_info....

Should this return false on all the platforms where we can't allocate a RGBA_F16
GMB?

[reveman, 2017-03-31 09:13:55]

Looks good.

In case it has not been mentioned already; we shouldn't confuse basic GMB
support with GLImage support for a given GMB format as there will likely be
cases in the future where we'll use GMBs without requiring support for GLImages
(Vulkan might be one use case, scanout support on CrOS without compositing
support might be another). The preferred approach is to always provide shm
support as that's useful for testing purposes. GLImage support on the other hand
is optional and easy to limit support for using GLES2 decoder capabilities.
Looks like this patch is well aligned with this, thanks!

https://codereview.chromium.org/2786103003/diff/40001/gpu/command_buffer/comm...
File gpu/command_buffer/common/gpu_memory_buffer_support.cc (right):

https://codereview.chromium.org/2786103003/diff/40001/gpu/command_buffer/comm...
gpu/command_buffer/common/gpu_memory_buffer_support.cc:122: return
capabilities.texture_half_float_linear;
This looks fine but we should probably rename this function in a separate patch
to not have it confused with GMB support. The function answers the question; is
GMB backed GLImage support for the given format supported given a set of GLES
decode capabilities. Maybe "IsImageForGpuMemoryBufferFormatSupported".

[ccameron, 2017-03-31 18:23:21]

The CQ bit was checked by ccameron@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 18:24:24]

Dry run: 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-03-31 19:57:11]

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

[commit-bot: I haz the power, 2017-03-31 19:57:12]

Dry run: This issue passed the CQ dry run.

[ccameron, 2017-04-11 07:45:12]

Oops, I let this drop off the radar.

I've updated the helper function names to clear up the confusion WRT the meaning
of support.

https://codereview.chromium.org/2786103003/diff/40001/gpu/command_buffer/comm...
File gpu/command_buffer/common/gpu_memory_buffer_support.cc (right):

https://codereview.chromium.org/2786103003/diff/40001/gpu/command_buffer/comm...
gpu/command_buffer/common/gpu_memory_buffer_support.cc:122: return
capabilities.texture_half_float_linear;
On 2017/03/31 09:13:55, reveman wrote:
> This looks fine but we should probably rename this function in a separate
patch
> to not have it confused with GMB support. The function answers the question;
is
> GMB backed GLImage support for the given format supported given a set of GLES
> decode capabilities. Maybe "IsImageForGpuMemoryBufferFormatSupported".

Thanks for the clarification -- I've gone ahead and updated the name in this
patch.

[reveman, 2017-04-11 16:28:27]

lgtm + nit

https://codereview.chromium.org/2786103003/diff/60001/gpu/command_buffer/comm...
File gpu/command_buffer/common/gpu_memory_buffer_support.h (right):

https://codereview.chromium.org/2786103003/diff/60001/gpu/command_buffer/comm...
gpu/command_buffer/common/gpu_memory_buffer_support.h:28: GPU_EXPORT bool
IsImageFromGpuMemoryBufferFormatSupported(
nit: can update the comment above too? "Returns true if creating an image for a
GpuMemoryBuffer with |format| is supported by |capabilities|."

[ccameron, 2017-04-12 05:32:52]

The CQ bit was checked by ccameron@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-12 05:33:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ccameron, 2017-04-12 05:34:35]

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

[ccameron, 2017-04-12 05:34:35]

Thanks

zmo@, could you OWNERs gpu/
dcheng@, could you OWNERs IPC/mojo

https://codereview.chromium.org/2786103003/diff/60001/gpu/command_buffer/comm...
File gpu/command_buffer/common/gpu_memory_buffer_support.h (right):

https://codereview.chromium.org/2786103003/diff/60001/gpu/command_buffer/comm...
gpu/command_buffer/common/gpu_memory_buffer_support.h:28: GPU_EXPORT bool
IsImageFromGpuMemoryBufferFormatSupported(
On 2017/04/11 16:28:26, reveman wrote:
> nit: can update the comment above too? "Returns true if creating an image for
a
> GpuMemoryBuffer with |format| is supported by |capabilities|."

Done.

[commit-bot: I haz the power, 2017-04-12 06:39:10]

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

[commit-bot: I haz the power, 2017-04-12 06:39:11]

Dry run: This issue passed the CQ dry run.

[dcheng, 2017-04-12 07:32:08]

ipc/mojo lgtm

[Zhenyao Mo, 2017-04-12 16:21:05]

On 2017/04/12 07:32:08, dcheng wrote:
> ipc/mojo lgtm

gpu lgtm

[ccameron, 2017-04-12 17:51:02]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2017-04-12 17:51:02]

The patchset sent to the CQ was uploaded after l-g-t-m from reveman@chromium.org
Link to the patchset: https://codereview.chromium.org/2786103003/#ps80001
(title: "Review feedback")

[commit-bot: I haz the power, 2017-04-12 17:52:06]

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-04-12 19:40:54]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1492019462219060,
"parent_rev": "c043081029a5b1ecee573c50c5fa21aa4bd956b7", "commit_rev":
"a1f8f4f18db2226c2dbc244b2d22c5a475dd0e22"}

[commit-bot: I haz the power, 2017-04-12 19:47:03]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1492019462219060,
"parent_rev": "0c375deb97e7a347219d0fadc843131592285b93", "commit_rev":
"c1fcde58c22a5825c44cbad17eb3ed84b8c2bcce"}

[commit-bot: I haz the power, 2017-04-12 19:47:58]

Description was changed from

==========
Add half-float IOSurface GpuMemoryBuffer support

Add support for gpu rasterization to half-float RGBA textures.

Do not add software rasterization support for half-float RGBA textures.
While this is possible, software rasterization to half-float is not a good
idea for performance reasons.

BUG=655171
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Add half-float IOSurface GpuMemoryBuffer support

Add support for gpu rasterization to half-float RGBA textures.

Do not add software rasterization support for half-float RGBA textures.
While this is possible, software rasterization to half-float is not a good
idea for performance reasons.

BUG=655171
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2786103003
Cr-Commit-Position: refs/heads/master@{#464114}
Committed:
https://chromium.googlesource.com/chromium/src/+/c1fcde58c22a5825c44cbad17eb3...
==========

[commit-bot: I haz the power, 2017-04-12 19:48:00]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/c1fcde58c22a5825c44cbad17eb3...