gpu: Pepper3DImageChromium support on ChromeOS.

gpu: Pepper3DImageChromium support on ChromeOS.

This adds support for Pepper3DImageChromium feature on ChromeOS. The
feature is disabled by default but can be enabled on devices with
support for RGBA8888 native GpuMemoryBuffers.

This allows HW overlays to be used for Pepper3D output.

BUG=680851
TEST=chrome --enable-features=Pepper3DImageChromium
CQ_INCLUDE_TRYBOTS=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/2632903002
Cr-Commit-Position: refs/heads/master@{#444563}
Committed: https://chromium.googlesource.com/chromium/src/+/b288ced458b070ee2785301c4600d65806362557

[reveman, 2017-01-14 22:45:54]

Description was changed from

==========
gpu: Pepper3DImageChromium support on ChromeOS.

This adds support for Pepper3DImageChromium feature on ChromeOS. The
feature is disabled by default but can be enabled on devices with
support for RGBA8888 native GpuMemoryBuffers.

This allows HW overlays to be used for Pepper3D output.

BUG=680851
TEST=chrome --enable-features=Pepper3DImageChromium
==========

to

==========
gpu: Pepper3DImageChromium support on ChromeOS.

This adds support for Pepper3DImageChromium feature on ChromeOS. The
feature is disabled by default but can be enabled on devices with
support for RGBA8888 native GpuMemoryBuffers.

This allows HW overlays to be used for Pepper3D output.

BUG=680851
TEST=chrome --enable-features=Pepper3DImageChromium
CQ_INCLUDE_TRYBOTS=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
==========

[reveman, 2017-01-14 22:46:47]

reveman@chromium.org changed reviewers:
+ dcastagna@chromium.org

[Daniele Castagna, 2017-01-14 22:55:50]

https://codereview.chromium.org/2632903002/diff/1/content/renderer/pepper/ppb...
File content/renderer/pepper/ppb_graphics_3d_impl.cc (right):

https://codereview.chromium.org/2632903002/diff/1/content/renderer/pepper/ppb...
content/renderer/pepper/ppb_graphics_3d_impl.cc:53: #if defined(OS_MACOSX) ||
defined(OS_CHROMEOS)
Can you we remove the ifdef here now that the feature is disabled by default on
every platform but mac?

https://codereview.chromium.org/2632903002/diff/1/content/renderer/pepper/ppb...
content/renderer/pepper/ppb_graphics_3d_impl.cc:198: use_image_chromium_ ?
GL_TEXTURE_EXTERNAL_OES : GL_TEXTURE_2D,
Should this be elif CHROME_OS?
Can we add a todo: get the right texture target like we're doing for content?

https://codereview.chromium.org/2632903002/diff/1/gpu/ipc/service/gpu_memory_...
File gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.cc (right):

https://codereview.chromium.org/2632903002/diff/1/gpu/ipc/service/gpu_memory_...
gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.cc:130: <<
size.height() << " format " << static_cast<int>(format);
nit: It'd be nice to have a string for the format instead of the enum number.

https://codereview.chromium.org/2632903002/diff/1/gpu/ipc/service/gpu_memory_...
gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.cc:136: LOG(ERROR)
<< "Failed to create GLImage";
nit: format and size might still be useful here.

[reveman, 2017-01-14 23:23:46]

https://codereview.chromium.org/2632903002/diff/1/content/renderer/pepper/ppb...
File content/renderer/pepper/ppb_graphics_3d_impl.cc (right):

https://codereview.chromium.org/2632903002/diff/1/content/renderer/pepper/ppb...
content/renderer/pepper/ppb_graphics_3d_impl.cc:53: #if defined(OS_MACOSX) ||
defined(OS_CHROMEOS)
On 2017/01/14 at 22:55:49, Daniele Castagna wrote:
> Can you we remove the ifdef here now that the feature is disabled by default
on every platform but mac?

Done.

https://codereview.chromium.org/2632903002/diff/1/content/renderer/pepper/ppb...
content/renderer/pepper/ppb_graphics_3d_impl.cc:198: use_image_chromium_ ?
GL_TEXTURE_EXTERNAL_OES : GL_TEXTURE_2D,
On 2017/01/14 at 22:55:49, Daniele Castagna wrote:
> Should this be elif CHROME_OS?
> Can we add a todo: get the right texture target like we're doing for content?

Done. Avoiding the CHROMEOS ifdef as TEXTURE_EXTERNAL_OES is considered the
standard target for image backed textures and Mac is the outlier with its
RECTANGLE_ARB requirement.

https://codereview.chromium.org/2632903002/diff/1/gpu/ipc/service/gpu_memory_...
File gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.cc (right):

https://codereview.chromium.org/2632903002/diff/1/gpu/ipc/service/gpu_memory_...
gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.cc:130: <<
size.height() << " format " << static_cast<int>(format);
On 2017/01/14 at 22:55:49, Daniele Castagna wrote:
> nit: It'd be nice to have a string for the format instead of the enum number.

Agreed but adding utility function for that is outside the scope of this CL as
discussed.

https://codereview.chromium.org/2632903002/diff/1/gpu/ipc/service/gpu_memory_...
gpu/ipc/service/gpu_memory_buffer_factory_ozone_native_pixmap.cc:136: LOG(ERROR)
<< "Failed to create GLImage";
On 2017/01/14 at 22:55:49, Daniele Castagna wrote:
> nit: format and size might still be useful here.

Done.

[Daniele Castagna, 2017-01-14 23:26:54]

lgtm

[Daniele Castagna, 2017-01-14 23:30:26]

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

[Daniele Castagna, 2017-01-14 23:30:26]

+ccameron@ for ownership.

[reveman, 2017-01-18 16:01:52]

ping for owner review

[ccameron, 2017-01-18 17:01:42]

lgtm

[reveman, 2017-01-18 17:16:00]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2017-01-18 17:16:23]

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-01-18 17:23:41]

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

[commit-bot: I haz the power, 2017-01-18 17:23:42]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[reveman, 2017-01-18 19:25:16]

reveman@chromium.org changed reviewers:
+ avi@chromium.org, jbauman@chromium.org

[reveman, 2017-01-18 19:25:17]

+avi for content/
+jbauman for gpu/

[Avi (use Gerrit), 2017-01-18 20:43:57]

lgtm

stamp

[jbauman, 2017-01-19 00:16:12]

lgtm

[reveman, 2017-01-19 00:20:24]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2017-01-19 00:21: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-01-19 00:26:14]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484785224373310,
"parent_rev": "3ad6fbee118e858145e28d4f5720f695069c956b", "commit_rev":
"b288ced458b070ee2785301c4600d65806362557"}

[commit-bot: I haz the power, 2017-01-19 00:26:47]

Description was changed from

==========
gpu: Pepper3DImageChromium support on ChromeOS.

This adds support for Pepper3DImageChromium feature on ChromeOS. The
feature is disabled by default but can be enabled on devices with
support for RGBA8888 native GpuMemoryBuffers.

This allows HW overlays to be used for Pepper3D output.

BUG=680851
TEST=chrome --enable-features=Pepper3DImageChromium
CQ_INCLUDE_TRYBOTS=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

==========
gpu: Pepper3DImageChromium support on ChromeOS.

This adds support for Pepper3DImageChromium feature on ChromeOS. The
feature is disabled by default but can be enabled on devices with
support for RGBA8888 native GpuMemoryBuffers.

This allows HW overlays to be used for Pepper3D output.

BUG=680851
TEST=chrome --enable-features=Pepper3DImageChromium
CQ_INCLUDE_TRYBOTS=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/2632903002
Cr-Commit-Position: refs/heads/master@{#444563}
Committed:
https://chromium.googlesource.com/chromium/src/+/b288ced458b070ee2785301c4600...
==========

[commit-bot: I haz the power, 2017-01-19 00:26:50]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/b288ced458b070ee2785301c4600...