ui: Add RGBX_8888 buffer format.

ui: Add RGBX_8888 buffer format.

This adds a new RGBX buffer format which is the RGBA equivalent of
BGRX. This is useful to have when importing buffers and it makes
sense to have this when we have BGRX.

This also makes our shared memory implementation of GpuMemoryBuffers
and GLImages support these formats without an alpha channel. This is
useful for testing purposes and not having to special case BGRX makes
for cleaner code.

The GLImage unit test have been updated to test all 32bpp buffer
formats.

BUG=538325
TEST=gl_unittest --gtest_filter=GLImage*/GLImageCopyTest/*.CopyTexImage

Committed: https://crrev.com/485728ba7af8e6519af075fd64de8e0295c09b93
Cr-Commit-Position: refs/heads/master@{#355839}

[reveman, 2015-10-22 20:53:17]

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

[piman, 2015-10-23 00:52:44]

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

https://codereview.chromium.org/1415723006/diff/1/ui/gl/gl_image_memory.cc#ne...
ui/gl/gl_image_memory.cc:149: return GL_UNSIGNED_INT_8_8_8_8_REV;
Hmm, I'm not sure what the expectation is here. Those 2 packed types are not
supported by GLES.

But even then, I can't work out why this is what you want.

The difference between GL_BGRA_EXT/GL_UNSIGNED_BYTE and
GL_BGRA_EXT/GL_UNSIGNED_INT_8_8_8_8_REV is inexistent on little-endian (and
would cause an undesirable swap on big-endian).

I'm even more confused by the RGBX_8888 one - on little-endian it would put the
4th byte (the X of RGBX) as the MSB of the unsigned int, which would then go
into the first component of the format (the R in GL_RGBA).


I realize that these end up not being used, but I think they make things
confusing.

[reveman, 2015-10-23 13:44:43]

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

https://codereview.chromium.org/1415723006/diff/1/ui/gl/gl_image_memory.cc#ne...
ui/gl/gl_image_memory.cc:149: return GL_UNSIGNED_INT_8_8_8_8_REV;
On 2015/10/23 at 00:52:44, piman (slow to review) wrote:
> Hmm, I'm not sure what the expectation is here. Those 2 packed types are not
supported by GLES.
> 
> But even then, I can't work out why this is what you want.
> 
> The difference between GL_BGRA_EXT/GL_UNSIGNED_BYTE and
GL_BGRA_EXT/GL_UNSIGNED_INT_8_8_8_8_REV is inexistent on little-endian (and
would cause an undesirable swap on big-endian).
> 
> I'm even more confused by the RGBX_8888 one - on little-endian it would put
the 4th byte (the X of RGBX) as the MSB of the unsigned int, which would then go
into the first component of the format (the R in GL_RGBA).
> 
> 
> I realize that these end up not being used, but I think they make things
confusing.

Yea, that made no sense. Thanks for noticing. Latest patch uses GL_UNSIGNED_BYTE
for both formats and checks if we have full GL and in that case avoids the
conversion to 24bpp.

[reveman, 2015-10-23 13:45:01]

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

[commit-bot: I haz the power, 2015-10-23 13:45:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1415723006/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1415723006/40001

[commit-bot: I haz the power, 2015-10-23 14:41:12]

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

[commit-bot: I haz the power, 2015-10-23 14:41:13]

Dry run: This issue passed the CQ dry run.

[reveman, 2015-10-23 16:16:10]

Description was changed from

==========
ui: Add RGBX_8888 buffer format.

This adds a new RGBX buffer format which is the RGBA equivalent of
BGRX. This is useful to have when importing buffers and it makes
sense to have this when we have BGRX.

This also makes our shared memory implementation of GpuMemoryBuffers
and GLImages support these formats without an alpha channel. This is
useful for testing purposes and not having to special case BGRX makes
for cleaner code.

The GLImage unit test have been updated to test all 32bpp buffer
formats.

BUG=
TEST=gl_unittest --gtest_filter=GLImage*/GLImageCopyTest/*.CopyTexImage
==========

to

==========
ui: Add RGBX_8888 buffer format.

This adds a new RGBX buffer format which is the RGBA equivalent of
BGRX. This is useful to have when importing buffers and it makes
sense to have this when we have BGRX.

This also makes our shared memory implementation of GpuMemoryBuffers
and GLImages support these formats without an alpha channel. This is
useful for testing purposes and not having to special case BGRX makes
for cleaner code.

The GLImage unit test have been updated to test all 32bpp buffer
formats.

BUG=538325
TEST=gl_unittest --gtest_filter=GLImage*/GLImageCopyTest/*.CopyTexImage
==========

[piman, 2015-10-23 17:24:53]

lgtm

[reveman, 2015-10-23 18:24:24]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2015-10-23 18:24:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1415723006/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1415723006/40001

[commit-bot: I haz the power, 2015-10-23 18:43:29]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-23 18:44:50]

Patchset 3 (id:??) landed as
https://crrev.com/485728ba7af8e6519af075fd64de8e0295c09b93
Cr-Commit-Position: refs/heads/master@{#355839}