Implement EXT_texture_format_BGRA8888

[GoogleCodeExporter]

EXT_texture_formatBGRA8888 matches the D3D9 A8R8G8B8 format directly, and as 
such texture upload and ReadPixels() can be made much faster, turning into a 
simple memcpy.  The attached patch implements this extension in ANGLE.

Original issue reported on code.google.com by vladim...@gmail.com on 31 Jul 2010 at 7:00

Attachments:

• bgra.patch

[GoogleCodeExporter]

Oops, forgot to regenerate that patch.  Updated patch attached.

Original comment by vladim...@gmail.com on 31 Jul 2010 at 7:02

Attachments:

• bgra.patch

[GoogleCodeExporter]

Daniel, Alok, can one of you take this bug and integrate this patch?

Original comment by kbr@chromium.org on 2 Aug 2010 at 4:52

[GoogleCodeExporter]

i'll review it

Original comment by dan...@transgaming.com on 2 Aug 2010 at 5:15

[GoogleCodeExporter]

Hi Vlad,

* The actual change themselves seem fine, however it seems they go too far
for just supporting EXT_texture_format_BGRA888
 * the spec doesn't say this extension extends glCopyTexImage2D to support
this.  It's possible it is supposed to be implied since CopyTexImage2D
formats are defined in terms of TexImage2D, but I'm going to ask on the ES
mailing list for clarification from existing implementors.
 * the extension doesn't extend the supported read formats for ReadPixels.
It would need to support something like EXT_read_format_bgra to expose this
functionality.  However EXT_read_format_bgra also exposes the
UNSIGNED_SHORT_4_4_4_4_REV_EXT and UNSIGNED_SHORT_1_5_5_5_REV_EXT types as
possible combinations with the BGRA_EXT format.
 * the alternative is writing an ANGLE-specific extension which adds all
of this functionality.

Daniel

Original comment by dan...@transgaming.com on 4 Aug 2010 at 6:59

[GoogleCodeExporter]

Ah, good point; I made the CopyTexImage2D assumption, and I wasn't thinking in 
terms of the read format for ReadPixels.  I could also just add support for 
UNSIGNED_SHORT_4_4_4_4_REV_EXT and UNSIGNED_SHORT_1_5_5_5_REV_EXT fairly 
easily, to avoid having to define a new extension for this; would that be 
better?

Original comment by vladim...@gmail.com on 4 Aug 2010 at 7:05

[GoogleCodeExporter]

Yes, that seems reasonable.

Original comment by dan...@transgaming.com on 4 Aug 2010 at 7:14

[GoogleCodeExporter]

Consensus is that the CopyTexImage2D does not accept the BGRA_EXT token for 
<internalformat>.
EXT_texture_formatBGRA8888 appears to be purely for the convenience for loading 
textures from client memory and doesn't really add a new internal format, it 
behaves like any other RGBA texture.

Original comment by dan...@transgaming.com on 5 Aug 2010 at 12:56

[GoogleCodeExporter]

Ok, updated to remove the CopyTexImage2D bit, and to fully implement 
EXT_read_format_bgra.

Original comment by vladim...@gmail.com on 7 Aug 2010 at 4:05

Attachments:

• bgra.patch

[GoogleCodeExporter]

Patch looks good with the following exceptions:

 * unused finish: label was added (I've removed it).
 * I'm fairly certain the component orderings for GL_UNSIGNED_SHORT_4_4_4_4_REV_EXT and GL_UNSIGNED_SHORT_1_5_5_5_REV_EXT were backwards, so I fixed them and added in comments explaining the bit ordering.  Please let me know if you disagree and/or have tests which prove otherwise.

Changes committed @ r370

Original comment by dan...@transgaming.com on 8 Aug 2010 at 4:52

• Changed state: Fixed

[GoogleCodeExporter]

Great, thanks -- yeah, missed the finish label (was part of something else that 
snuck in).

For the component ordering, you're probably right; I started at the docs for a 
while, but I had a hard time finding code that actually used either of those, 
so couldn't find a concrete example to check expectations.

Original comment by vladim...@gmail.com on 8 Aug 2010 at 6:43