KhronosGroup / OpenGL-API

OpenGL and OpenGL ES API Issue Tracker
34 stars 5 forks source link

GL_BGRA8_EXT woes #92

Open kusma opened 6 months ago

kusma commented 6 months ago

TLDR; I believe implementations implement wider support for GL_BGRA8_EXT than the published extension specifications allow, and applications depend on this behavior. As such, it probably makes sense to consider to widen the specs (or introduce new specs that allows what applications depend on).


EXT_texture_storage + EXT_texture_format_BGRA8888

I recently added support for EXT_texture_storage to Mesa, in order to let applications use GL_BGRA8_EXT for glTexStorage2D(). Mesa already supported EXT_texture_format_BGRA8888, and the EXT_texture_storage-spec says that the interaction with EXT_texture_format_BGRA8888 allows just that.

However, adding this broke Chrome and all derivatives, as well as Firefox. It seems both those browsers assume that the combination of those two specs also allow GL_BGRA8_EXT for glTex(Sub)Image2D() etc, which the specs don't.

It turns out, Chrome has expected this behavior since 2016. This means that's either unused code (which I doubt), or other implementations allow the behavior. I can in fact not find any combination of extensions that allows using an internalformat of GL_BGRA8_EXT for e.g glTexImage2D(). If someone knows of one, please let me know.

By the way, Firefox seems to have the exact same issue.

glGenerateMipmap

In addition, it seems like functions like glGenerateMipmap() are also left in the cold with the above combination of extensions, because the EXT_texture_format_BGRA8888-spec doesn't actually define GL_BGRA8_EXT as an internalformat. Instead if defines (confusingly) GL_BGRA_EXT, which is a different enum-value. It's defined as a color-renderable sized internalformat. But GL_BGRA8_EXT is not defined as such.

The APPLE_texture_format_BGRA8888 extension resolves that latter part - it's wording is clear that this is meant to be a color-renderable format. But it's written against the GLES 1.1 spec, so one has to extrapolate a bit.

References

Mesa issue: https://gitlab.freedesktop.org/mesa/mesa/-/issues/10550 Chromium issue: https://issues.chromium.org/issues/41497267 Firefox issue: https://bugzilla.mozilla.org/show_bug.cgi?id=1879496

kusma commented 6 months ago

This was discussed at the joint OpenGL / OpenGL ES working group meeting last week, and the consensus was to change the EXT_texture_format_BGRA8888 spec to allow GL_BGRA8_EXT as well as GL_BGRA_EXT. This should fix "both" problems listed above:

Here's a pull-request for the spec change: https://github.com/KhronosGroup/OpenGL-Registry/pull/603

I'll post a pull-request for the CTS as soon as I've written it ;)

kusma commented 6 months ago

Just some additional details (just writing this down while poking at the details for the CTS case here):

What it seems Chrome (and Firefox) does, which is currently out of spec, is to use glTexStorage2D(..., GL_BGRA8_EXT, ...);, and then try to change parts of that texture using glTexSubImage2D(). The glTexSubImage2D()-call itself doesn't use GL_BGRA8_EXT, but the error handling is "inherited" from glTexImage2D, thanks to this wording in the OpenGL ES 3.2 spec (from section 8.6 "Alternate Texture Image Specification Commands"):

The same constraints and errors apply to the TexSubImage commands’ argument format and the internalformat of the texture image being respecified as apply to the format and internalformat arguments of its TexImage counterparts.

And since GL_BGRA8_EXT isn't allowed for glTexImage2D, it's also not allowed for glTexSubImage2D.

The only reasonable way of fixing this is to allow it for both, otherwise we'd need to change the wording quite a lot here.