gpu: support R16 GPUMemoryBuffer

gpu: support R16 GPUMemoryBuffer

9-16 bit video channel will use it.

TEST=gl_tests --gtest_filter=GpuMemoryBufferTest*

BUG=445071, 624436
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

Review-Url: https://codereview.chromium.org/2920793005
Cr-Commit-Position: refs/heads/master@{#481176}
Committed: https://chromium.googlesource.com/chromium/src/+/65f143da57bb9367c5004c659d368642bd9969f6

[riju_, 2017-06-02 10:32:46]

Description was changed from

==========
gpu: support R16 GPUMemoryBuffer

9-16 bit video channel will use it.

TEST=gl_tests --gtest_filter=GpuMemoryBufferTest*

BUG=445071, 624436
==========

to

==========
gpu: support R16 GPUMemoryBuffer

9-16 bit video channel will use it.

TEST=gl_tests --gtest_filter=GpuMemoryBufferTest*

BUG=445071, 624436
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
==========

[riju_, 2017-06-02 12:24:32]

The CQ bit was checked by rijubrata.bhaumik@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-02 12:24:51]

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-06-02 13:20:47]

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

[commit-bot: I haz the power, 2017-06-02 13:20:50]

Dry run: 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_...)

[aleksandar.stojiljkovic, 2017-06-07 20:11:54]

aleksandar.stojiljkovic@intel.com changed reviewers:
+ aleksandar.stojiljkovic@intel.com

[aleksandar.stojiljkovic, 2017-06-07 20:11:55]

https://codereview.chromium.org/2920793005/diff/1/gpu/ipc/host/gpu_memory_buf...
File gpu/ipc/host/gpu_memory_buffer_support.cc (right):

https://codereview.chromium.org/2920793005/diff/1/gpu/ipc/host/gpu_memory_buf...
gpu/ipc/host/gpu_memory_buffer_support.cc:42: gfx::BufferFormat::R_16,     
gfx::BufferFormat::R_16,
typo here? No need to have it twice.

[riju_, 2017-06-08 06:13:30]

The CQ bit was checked by rijubrata.bhaumik@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-08 06:13:45]

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-06-08 07:10:28]

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

[commit-bot: I haz the power, 2017-06-08 07:10:30]

Dry run: 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_...)

[riju_, 2017-06-08 08:41:51]

The CQ bit was checked by rijubrata.bhaumik@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-08 08:42:01]

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-06-08 10:22:19]

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

[commit-bot: I haz the power, 2017-06-08 10:22:20]

Dry run: This issue passed the CQ dry run.

[aleksandar.stojiljkovic, 2017-06-08 10:25:03]

https://codereview.chromium.org/2920793005/diff/40001/gpu/command_buffer/clie...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/2920793005/diff/40001/gpu/command_buffer/clie...
gpu/command_buffer/client/gles2_implementation.cc:6233: case GL_R16_EXT:
As it is not available always, maybe:
  return capabilities.texture_norm16;

e.g.
https://cs.chromium.org/chromium/src/media/renderers/skcanvas_video_renderer....

[riju_, 2017-06-08 12:56:30]

Patchset #4 (id:60001) has been deleted

[riju_, 2017-06-08 12:59:57]

The CQ bit was checked by rijubrata.bhaumik@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-08 13:00:07]

Dry run: CQ is trying da patch.

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

[aleksandar.stojiljkovic, 2017-06-08 14:27:56]

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_io_surfa...
File ui/gl/gl_image_io_surface.mm (right):

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_io_surfa...
ui/gl/gl_image_io_surface.mm:147: case gfx::BufferFormat::R_16:
should return GL_UNSIGNED_SHORT

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_memory.cc
File ui/gl/gl_image_memory.cc (right):

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_memory.c...
ui/gl/gl_image_memory.cc:179: case gfx::BufferFormat::R_16:
GL_UNSIGNED_SHORT

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_memory.c...
ui/gl/gl_image_memory.cc:216: return base::checked_cast<GLint>(stride);
return base::checked_cast<GLint>(stride) => return
base::checked_cast<GLint>(stride) / 2. Reconsider moving it up to case
gfx::BufferFormat::RG_88:

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/test/gl_image_tes...
File ui/gl/test/gl_image_test_support.cc (right):

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/test/gl_image_tes...
ui/gl/test/gl_image_test_support.cc:57: case gfx::BufferFormat::R_16:
are there any tests using this?

Should be something like:
uint16_t red = (uint16_t)color[0] << 8; 
for (int y = 0; y < height; ++y) {
  uint16_t* row = reinterpret_cast<uint16_t*>(data + y * stride);
  for (int x = 0; x < width; ++x) {
    row[x] = red;
  }
}

[commit-bot: I haz the power, 2017-06-08 14:43:07]

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

[commit-bot: I haz the power, 2017-06-08 14:43:08]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[riju_, 2017-06-08 16:30:19]

The CQ bit was checked by rijubrata.bhaumik@intel.com to run a CQ dry run

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

Dry run: CQ is trying da patch.

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

[riju_, 2017-06-08 17:38:22]

Patchset #5 (id:100001) has been deleted

[commit-bot: I haz the power, 2017-06-08 17:39:12]

Dry run: 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_...)

[riju_, 2017-06-08 17:51:14]

The CQ bit was checked by rijubrata.bhaumik@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-08 17:51:33]

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-06-08 20:28:01]

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

[commit-bot: I haz the power, 2017-06-08 20:28:03]

Dry run: This issue passed the CQ dry run.

[riju_, 2017-06-09 08:20:10]

Thanks aleks.

https://codereview.chromium.org/2920793005/diff/40001/gpu/command_buffer/clie...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/2920793005/diff/40001/gpu/command_buffer/clie...
gpu/command_buffer/client/gles2_implementation.cc:6233: case GL_R16_EXT:
On 2017/06/08 10:25:03, aleksandar.stojiljkovic wrote:
> As it is not available always, maybe:
>   return capabilities.texture_norm16;
> 
> e.g.
>
https://cs.chromium.org/chromium/src/media/renderers/skcanvas_video_renderer....

Done.

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_io_surfa...
File ui/gl/gl_image_io_surface.mm (right):

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_io_surfa...
ui/gl/gl_image_io_surface.mm:147: case gfx::BufferFormat::R_16:
On 2017/06/08 14:27:56, aleksandar.stojiljkovic wrote:
> should return GL_UNSIGNED_SHORT

Done.

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_memory.cc
File ui/gl/gl_image_memory.cc (right):

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_memory.c...
ui/gl/gl_image_memory.cc:179: case gfx::BufferFormat::R_16:
On 2017/06/08 14:27:56, aleksandar.stojiljkovic wrote:
> GL_UNSIGNED_SHORT

Done.

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_memory.c...
ui/gl/gl_image_memory.cc:216: return base::checked_cast<GLint>(stride);
On 2017/06/08 14:27:56, aleksandar.stojiljkovic wrote:
> return base::checked_cast<GLint>(stride) => return
> base::checked_cast<GLint>(stride) / 2. Reconsider moving it up to case
> gfx::BufferFormat::RG_88:

Done.

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/test/gl_image_tes...
File ui/gl/test/gl_image_test_support.cc (right):

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/test/gl_image_tes...
ui/gl/test/gl_image_test_support.cc:57: case gfx::BufferFormat::R_16:
On 2017/06/08 14:27:56, aleksandar.stojiljkovic wrote:
> are there any tests using this?
> 
> Should be something like:
> uint16_t red = (uint16_t)color[0] << 8; 
> for (int y = 0; y < height; ++y) {
>   uint16_t* row = reinterpret_cast<uint16_t*>(data + y * stride);
>   for (int x = 0; x < width; ++x) {
>     row[x] = red;
>   }
> }

Done.

https://codereview.chromium.org/2920793005/diff/80001/ui/gl/test/gl_image_tes...
ui/gl/test/gl_image_test_support.cc:57: case gfx::BufferFormat::R_16:
On 2017/06/08 14:27:56, aleksandar.stojiljkovic wrote:
> are there any tests using this?
> 
> Should be something like:
> uint16_t red = (uint16_t)color[0] << 8; 
> for (int y = 0; y < height; ++y) {
>   uint16_t* row = reinterpret_cast<uint16_t*>(data + y * stride);
>   for (int x = 0; x < width; ++x) {
>     row[x] = red;
>   }
> }

Thanks.

[aleksandar.stojiljkovic, 2017-06-09 11:09:16]

On 2017/06/09 08:20:10, riju_ wrote:
> Thanks aleks.
> 
>
https://codereview.chromium.org/2920793005/diff/40001/gpu/command_buffer/clie...
> File gpu/command_buffer/client/gles2_implementation.cc (right):
> 
>
https://codereview.chromium.org/2920793005/diff/40001/gpu/command_buffer/clie...
> gpu/command_buffer/client/gles2_implementation.cc:6233: case GL_R16_EXT:
> On 2017/06/08 10:25:03, aleksandar.stojiljkovic wrote:
> > As it is not available always, maybe:
> >   return capabilities.texture_norm16;
> > 
> > e.g.
> >
>
https://cs.chromium.org/chromium/src/media/renderers/skcanvas_video_renderer....
> 
> Done.
> 
>
https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_io_surfa...
> File ui/gl/gl_image_io_surface.mm (right):
> 
>
https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_io_surfa...
> ui/gl/gl_image_io_surface.mm:147: case gfx::BufferFormat::R_16:
> On 2017/06/08 14:27:56, aleksandar.stojiljkovic wrote:
> > should return GL_UNSIGNED_SHORT
> 
> Done.
> 
> https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_memory.cc
> File ui/gl/gl_image_memory.cc (right):
> 
>
https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_memory.c...
> ui/gl/gl_image_memory.cc:179: case gfx::BufferFormat::R_16:
> On 2017/06/08 14:27:56, aleksandar.stojiljkovic wrote:
> > GL_UNSIGNED_SHORT
> 
> Done.
> 
>
https://codereview.chromium.org/2920793005/diff/80001/ui/gl/gl_image_memory.c...
> ui/gl/gl_image_memory.cc:216: return base::checked_cast<GLint>(stride);
> On 2017/06/08 14:27:56, aleksandar.stojiljkovic wrote:
> > return base::checked_cast<GLint>(stride) => return
> > base::checked_cast<GLint>(stride) / 2. Reconsider moving it up to case
> > gfx::BufferFormat::RG_88:
> 
> Done.
> 
>
https://codereview.chromium.org/2920793005/diff/80001/ui/gl/test/gl_image_tes...
> File ui/gl/test/gl_image_test_support.cc (right):
> 
>
https://codereview.chromium.org/2920793005/diff/80001/ui/gl/test/gl_image_tes...
> ui/gl/test/gl_image_test_support.cc:57: case gfx::BufferFormat::R_16:
> On 2017/06/08 14:27:56, aleksandar.stojiljkovic wrote:
> > are there any tests using this?
> > 
> > Should be something like:
> > uint16_t red = (uint16_t)color[0] << 8; 
> > for (int y = 0; y < height; ++y) {
> >   uint16_t* row = reinterpret_cast<uint16_t*>(data + y * stride);
> >   for (int x = 0; x < width; ++x) {
> >     row[x] = red;
> >   }
> > }
> 
> Done.
> 
>
https://codereview.chromium.org/2920793005/diff/80001/ui/gl/test/gl_image_tes...
> ui/gl/test/gl_image_test_support.cc:57: case gfx::BufferFormat::R_16:
> On 2017/06/08 14:27:56, aleksandar.stojiljkovic wrote:
> > are there any tests using this?
> > 
> > Should be something like:
> > uint16_t red = (uint16_t)color[0] << 8; 
> > for (int y = 0; y < height; ++y) {
> >   uint16_t* row = reinterpret_cast<uint16_t*>(data + y * stride);
> >   for (int x = 0; x < width; ++x) {
> >     row[x] = red;
> >   }
> > }
> 
> Thanks.
 non-owner lgtm

[riju_, 2017-06-09 11:31:41]

rijubrata.bhaumik@intel.com changed reviewers:
+ ccameron@chromium.org, kbr@chromium.org

[riju_, 2017-06-09 11:31:42]

ccameron@chromium.org: Please review changes in ui/

kbr@chromium.org: Please review changes in gpu/

[Ken Russell (switch to Gerrit), 2017-06-09 22:37:39]

Generally looks good. After researching the Mac-specific comment below, will
this capability be properly feature detected on all platforms (in particular,
both OpenGL and OpenGL ES based systems)?

How is this being tested?

https://codereview.chromium.org/2920793005/diff/120001/ui/gl/gl_image_io_surf...
File ui/gl/gl_image_io_surface.mm (right):

https://codereview.chromium.org/2920793005/diff/120001/ui/gl/gl_image_io_surf...
ui/gl/gl_image_io_surface.mm:35: case GL_R16_EXT:
Note: I checked CGLIOSurface.h and confirmed that the format/type/internal
format tuple (GL_UNSIGNED_SHORT, GL_RED, GL_R16) is supported. However, it was
only subsumed into the Core Profile in OpenGL 3.0. If Chrome happens to fall
back to the OpenGL compatibility profile (which is feature level 2.1 on macOS),
do we need to feature detect its availability (via GL_ARB_texture_rg)?

https://codereview.chromium.org/2920793005/diff/120001/ui/gl/gl_image_native_...
File ui/gl/gl_image_native_pixmap.cc (right):

https://codereview.chromium.org/2920793005/diff/120001/ui/gl/gl_image_native_...
ui/gl/gl_image_native_pixmap.cc:17: #define DRM_FORMAT_R16 FOURCC('R', '1', '6',
' ')
Note: I did a quick web search and confirmed that this FOURCC code seems to be
valid.

[riju_, 2017-06-13 15:34:14]

We plan to use R16 GMB in a reworked version of 
https://codereview.chromium.org/2122573003/.
As for this patch, I am following the approach as shown in the patch
https://codereview.chromium.org/2376293003/ which added RG_88 GBM

https://codereview.chromium.org/2920793005/diff/120001/ui/gl/gl_image_io_surf...
File ui/gl/gl_image_io_surface.mm (right):

https://codereview.chromium.org/2920793005/diff/120001/ui/gl/gl_image_io_surf...
ui/gl/gl_image_io_surface.mm:35: case GL_R16_EXT:
On 2017/06/09 22:37:39, Ken Russell wrote:
> Note: I checked CGLIOSurface.h and confirmed that the format/type/internal
> format tuple (GL_UNSIGNED_SHORT, GL_RED, GL_R16) is supported. However, it was
> only subsumed into the Core Profile in OpenGL 3.0. If Chrome happens to fall
> back to the OpenGL compatibility profile (which is feature level 2.1 on
macOS),
> do we need to feature detect its availability (via GL_ARB_texture_rg)?

Thanks, I am doing the check here->
https://codereview.chromium.org/2933313003

[Ken Russell (switch to Gerrit), 2017-06-13 23:52:36]

lgtm from my standpoint

https://codereview.chromium.org/2920793005/diff/120001/ui/gl/gl_image_io_surf...
File ui/gl/gl_image_io_surface.mm (right):

https://codereview.chromium.org/2920793005/diff/120001/ui/gl/gl_image_io_surf...
ui/gl/gl_image_io_surface.mm:35: case GL_R16_EXT:
On 2017/06/13 15:34:14, riju_ wrote:
> On 2017/06/09 22:37:39, Ken Russell wrote:
> > Note: I checked CGLIOSurface.h and confirmed that the format/type/internal
> > format tuple (GL_UNSIGNED_SHORT, GL_RED, GL_R16) is supported. However, it
was
> > only subsumed into the Core Profile in OpenGL 3.0. If Chrome happens to fall
> > back to the OpenGL compatibility profile (which is feature level 2.1 on
> macOS),
> > do we need to feature detect its availability (via GL_ARB_texture_rg)?
> 
> Thanks, I am doing the check here->
> https://codereview.chromium.org/2933313003

Thanks. Reviewed that other change; it isn't necessary, since the other code
path checks for is_desktop_core_profile, which implies a minimum version of
OpenGL 3.2.

[riju_, 2017-06-14 07:14:46]

rijubrata.bhaumik@intel.com changed reviewers:
+ reveman@chromium.org

[riju_, 2017-06-14 07:14:47]

reveman@chromium.org: Please review changes in components/exo/buffer.cc 

meacer@chromium.org: please review changes in ui/gfx/mojo

https://codereview.chromium.org/2920793005/diff/120001/ui/gl/gl_image_io_surf...
File ui/gl/gl_image_io_surface.mm (right):

https://codereview.chromium.org/2920793005/diff/120001/ui/gl/gl_image_io_surf...
ui/gl/gl_image_io_surface.mm:35: case GL_R16_EXT:
On 2017/06/13 23:52:36, Ken Russell wrote:
> On 2017/06/13 15:34:14, riju_ wrote:
> > On 2017/06/09 22:37:39, Ken Russell wrote:
> > > Note: I checked CGLIOSurface.h and confirmed that the format/type/internal
> > > format tuple (GL_UNSIGNED_SHORT, GL_RED, GL_R16) is supported. However, it
> was
> > > only subsumed into the Core Profile in OpenGL 3.0. If Chrome happens to
fall
> > > back to the OpenGL compatibility profile (which is feature level 2.1 on
> > macOS),
> > > do we need to feature detect its availability (via GL_ARB_texture_rg)?
> > 
> > Thanks, I am doing the check here->
> > https://codereview.chromium.org/2933313003
> 
> Thanks. Reviewed that other change; it isn't necessary, since the other code
> path checks for is_desktop_core_profile, which implies a minimum version of
> OpenGL 3.2.

Acknowledged.

[riju_, 2017-06-14 07:15:32]

Description was changed from

==========
gpu: support R16 GPUMemoryBuffer

9-16 bit video channel will use it.

TEST=gl_tests --gtest_filter=GpuMemoryBufferTest*

BUG=445071, 624436
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

==========
gpu: support R16 GPUMemoryBuffer

9-16 bit video channel will use it.

TEST=gl_tests --gtest_filter=GpuMemoryBufferTest*

BUG=445071, 624436
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
==========

[riju_, 2017-06-14 07:15:32]

rijubrata.bhaumik@intel.com changed reviewers:
+ meacer@chromium.org

[reveman, 2017-06-14 17:47:22]

components/exo lgtm + GLImage test nits

https://codereview.chromium.org/2920793005/diff/120001/ui/gl/test/gl_image_te...
File ui/gl/test/gl_image_test_support.cc (right):

https://codereview.chromium.org/2920793005/diff/120001/ui/gl/test/gl_image_te...
ui/gl/test/gl_image_test_support.cc:66: red = (uint16_t)color[0] << 8;
nit: "uint16_t red = static_cast<uint16_t>(color[0] << 8)" although I don't
think a temporary variable is really needed here. doing "row[x] =
static_cast<uint16_t>(color[0] << 8)" at l.70 is fine imo.

https://codereview.chromium.org/2920793005/diff/120001/ui/ozone/platform/drm/...
File ui/ozone/platform/drm/common/drm_util.cc (right):

https://codereview.chromium.org/2920793005/diff/120001/ui/ozone/platform/drm/...
ui/ozone/platform/drm/common/drm_util.cc:22: #define DRM_FORMAT_R16
fourcc_code('R', '1', '6', ' ')
upstream seems to have this. can we instead just update libdrm in third_party
and on chromeos before we land this?

[meacer, 2017-06-15 00:16:54]

mojo lgtm

[riju_, 2017-06-20 11:17:19]

The CQ bit was checked by rijubrata.bhaumik@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-20 11:17:35]

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-06-20 12:51:39]

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

[commit-bot: I haz the power, 2017-06-20 12:51:40]

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

[riju_, 2017-06-20 13:21:32]

Thanks reveman for the comments. Answers inline.

ccameron@ Please review ->

ui/gfx/mac/io_surface.cc
ui/ozone/platform/drm/common/drm_util.cc

https://codereview.chromium.org/2920793005/diff/120001/ui/gl/test/gl_image_te...
File ui/gl/test/gl_image_test_support.cc (right):

https://codereview.chromium.org/2920793005/diff/120001/ui/gl/test/gl_image_te...
ui/gl/test/gl_image_test_support.cc:66: red = (uint16_t)color[0] << 8;
On 2017/06/14 17:47:21, reveman wrote:
> nit: "uint16_t red = static_cast<uint16_t>(color[0] << 8)" although I don't
> think a temporary variable is really needed here. doing "row[x] =
> static_cast<uint16_t>(color[0] << 8)" at l.70 is fine imo.

Done.

https://codereview.chromium.org/2920793005/diff/120001/ui/ozone/platform/drm/...
File ui/ozone/platform/drm/common/drm_util.cc (right):

https://codereview.chromium.org/2920793005/diff/120001/ui/ozone/platform/drm/...
ui/ozone/platform/drm/common/drm_util.cc:22: #define DRM_FORMAT_R16
fourcc_code('R', '1', '6', ' ')
On 2017/06/14 17:47:22, reveman wrote:
> upstream seems to have this. can we instead just update libdrm in third_party
> and on chromeos before we land this?

Created bug: https://bugs.chromium.org/p/chromium/issues/detail?id=733703

ChromiumOS CLs:
(libdrm: upgrade): https://chromium-review.googlesource.com/c/539275/
(libpciaccess: upgrade): https://chromium-review.googlesource.com/c/539031/

Not familiar with chromeos package upgrades, so upgraded in wrong overlay.
What I understood from Stéphane Marchesin in 
https://chromium-review.googlesource.com/c/539275/#message-a3485b4a0f5d5d5152...
is that, updating libdrm is already in their plans.
There is already libdrm 2.4.75 in a branch, where some patches have to
rebased/applied and then it would be moved to master.
https://chromium.googlesource.com/chromiumos/third_party/libdrm/+/chromeos-2....,

 
Next Step:
So I suggest we check-in this CL, and I will update DEPS (and fix this TODO)
when libdrm is updated.
This approach was taken in https://codereview.chromium.org/2758013002

[reveman, 2017-06-20 18:45:38]

sounds good. still lgtm with a nit

https://codereview.chromium.org/2920793005/diff/140001/ui/ozone/platform/drm/...
File ui/ozone/platform/drm/common/drm_util.cc (right):

https://codereview.chromium.org/2920793005/diff/140001/ui/ozone/platform/drm/...
ui/ozone/platform/drm/common/drm_util.cc:21: // TODO(riju): after libdrm has
this definition, remove it.
nit: add a bug number here

[ccameron, 2017-06-20 18:50:22]

Lgtm

There does exist IOSurface support for this format, so we could add it if we
wanted to.

[riju_, 2017-06-21 07:53:19]

https://codereview.chromium.org/2920793005/diff/140001/ui/ozone/platform/drm/...
File ui/ozone/platform/drm/common/drm_util.cc (right):

https://codereview.chromium.org/2920793005/diff/140001/ui/ozone/platform/drm/...
ui/ozone/platform/drm/common/drm_util.cc:21: // TODO(riju): after libdrm has
this definition, remove it.
On 2017/06/20 18:45:38, reveman wrote:
> nit: add a bug number here

Done.

[riju_, 2017-06-21 07:53:29]

The CQ bit was checked by rijubrata.bhaumik@intel.com

[riju_, 2017-06-21 07:53:29]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org,
aleksandar.stojiljkovic@intel.com, meacer@chromium.org, reveman@chromium.org,
ccameron@chromium.org
Link to the patchset: https://codereview.chromium.org/2920793005/#ps160001
(title: "Add bug number to TODO.")

[commit-bot: I haz the power, 2017-06-21 07:53:42]

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-06-21 09:46:51]

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

[commit-bot: I haz the power, 2017-06-21 09:46:53]

Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[riju_, 2017-06-21 10:37:53]

The CQ bit was checked by rijubrata.bhaumik@intel.com

[commit-bot: I haz the power, 2017-06-21 10:38:02]

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-06-21 11:50:44]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1498041473230310,
"parent_rev": "ed00eba523c0ee50375c789223a63c732992802c", "commit_rev":
"65f143da57bb9367c5004c659d368642bd9969f6"}

[commit-bot: I haz the power, 2017-06-21 11:50:59]

Description was changed from

==========
gpu: support R16 GPUMemoryBuffer

9-16 bit video channel will use it.

TEST=gl_tests --gtest_filter=GpuMemoryBufferTest*

BUG=445071, 624436
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

==========
gpu: support R16 GPUMemoryBuffer

9-16 bit video channel will use it.

TEST=gl_tests --gtest_filter=GpuMemoryBufferTest*

BUG=445071, 624436
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

Review-Url: https://codereview.chromium.org/2920793005
Cr-Commit-Position: refs/heads/master@{#481176}
Committed:
https://chromium.googlesource.com/chromium/src/+/65f143da57bb9367c5004c659d36...
==========

[commit-bot: I haz the power, 2017-06-21 11:51:01]

Committed patchset #7 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/65f143da57bb9367c5004c659d36...