Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GL_BGRA8_EXT woes #92

Open
kusma opened this issue Feb 9, 2024 · 2 comments
Open

GL_BGRA8_EXT woes #92

kusma opened this issue Feb 9, 2024 · 2 comments

Comments

@kusma
Copy link

kusma commented Feb 9, 2024

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
Copy link
Author

kusma commented Feb 19, 2024

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: KhronosGroup/OpenGL-Registry#603

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

@kusma
Copy link
Author

kusma commented Feb 20, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant