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.

Sign up for GitHub

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

Jump to bottom

Design considerations for HDR colorspace support #1653

Open
MarijnS95 opened this issue Dec 15, 2023 · 3 comments · May be fixed by #1658
Open

Design considerations for HDR colorspace support #1653

MarijnS95 opened this issue Dec 15, 2023 · 3 comments · May be fixed by #1658
Labels
api

Comments

@MarijnS95
Copy link
Member

As announced in #1652 I'm working on HDR support for glutin 🎉. I have a working branch for this but with some caveats that I'd like to discuss before submitting a draft PR.

It turns out to only be a thing on EGL. The existing "colorspace" support (to select sRGB vs Linear) simply gets more elaborate, with more enum values. It is an attribute on the surface, and it can be queried after the fact.

There does not seem to be a way to query which color spaces a certain config/surface supports, like the existing "fake-ish" srgb_capable() check on EGL which only checks if the display supports the extension.
(Please correct me if I missed something obvious!)

Certain color spaces might benefit from specific output formats, or might not work at all on certain ColorBufferTypes (citation/testing needed, but on Vulkan there is an explicit list of format<->colorspace mapping and only few combinations are allowed).

There is some slight overlap with WGL, where it seems to be possible to select a colorspace (only linear or sRGB) in the config via WGL_COLORSPACE_EXT (even if the extension is not said to be supported...). Again, EGL selects it much later, on the surface (but assuming you might want to keep an 8-bit and 10-bit config around to also take advantage of higher bit-depths on HDR if supported?).

There is however no relation to srgb_capable(), which I think (see #1652) is supposed to denote if GL_FRAMEBUFFER_SRGB is supported which simply does automatic gamma conversion (as it was found to be totally detached from FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING values).

So the questions are probably as follows:

  • Should we replace the existing with_srgb() support on SurfaceAttributes to with_color_space() (it's still only EGL support)?
    • Or should we keep it as an EGL-specific API?
  • Should we add with_srgb() on Config for WGL?
  • Should we extend DisplayFeatures with many of the color spaces based on device extensions?
  • Should we have a generic Config implementation for DisplayFeatures given that the latter bitflags type is shared across all backends already anyway?

Or I could also open up a draft PR from my branch if that makes it easier to discuss about these points; but it needs to be cleaned up some more with the findings from this issue and #1652.

Note also that EGL_EXT_gl_colorspace_bt2020_hlg was added in April this year and requires a gl_generator upgrade to use a newer registry (and IIRC there was one more constant that is missing). All those crates currently seem unmaintained but there was a request for new maintainers, maybe we can fill the gap there.

And GL_ARB_framebuffer_sRGB does not exist in Api::Gles2 so the bindings do not contain GL_FRAMEBUFFER_SRGB, unless generated with Api::Gl (without extensions).

Thanks for your time!
@kchibisov
Copy link
Member

There does not seem to be a way to query which color spaces a certain config/surface supports, like the existing "fake-ish" srgb_capable() check on EGL which only checks if the display supports the extension.
(Please correct me if I missed something obvious!)

This is present because some platform determine the framebuffer type at the surface/context creation time and you can't switch at runtime. Basically GLES and that's why we have those checks, mostly. Since when you have OpenGL context you don't have GL_ARB_framebuffer_sRGB.

Should we replace the existing with_srgb() support on SurfaceAttributes to with_color_space() (it's still only EGL support)?

If it's present only on EGL we should probably use only EGL here, users willing to do HDR will likely have special pass for EGL if it's a EGL only API.

Should we extend DisplayFeatures with many of the color spaces based on device extensions?

Maybe a separate set with them, because it could be EGL specific. We have attempt with fences PR #1646 to split caps iirc.

Should we have a generic Config implementation for DisplayFeatures given that the latter bitflags type is shared across all backends already anyway?

Not sure, need to try and see how it looks.

Note also that EGL_EXT_gl_colorspace_bt2020_hlg was added in April this year and requires a gl_generator upgrade to use a newer registry (and IIRC there was one more constant that is missing).

I think gl_generator generally not maintained since it can't parse modern EGL specs, etc. We were adding functions/constants into egl-sys bindings manually, but that's about it. You can probably do the same.

@MarijnS95 MarijnS95 mentioned this issue Dec 18, 2023
Open
@MarijnS95
Copy link
Member Author

There does not seem to be a way to query which color spaces a certain config/surface supports, like the existing "fake-ish" srgb_capable() check on EGL which only checks if the display supports the extension.
(Please correct me if I missed something obvious!)

This is present because some platform determine the framebuffer type at the surface/context creation time and you can't switch at runtime.

Note that by "fake-ish" I'm referring to EGL having the same srgb_capable() EGLDisplay-wide, not per-config (so a user will either get all configs to return true or false unanimously). For other backends where a config attribute is read, this makes total sense.

I will reply to the rest about sRGB in #1652.

Should we replace the existing with_srgb() support on SurfaceAttributes to with_color_space() (it's still only EGL support)?

If it's present only on EGL we should probably use only EGL here, users willing to do HDR will likely have special pass for EGL if it's a EGL only API.

Sounds good, that makes the most sense and I will do it like that. On EGL the existing with_srgb() can then simply call into with_color_space() :)

Should we extend DisplayFeatures with many of the color spaces based on device extensions?

Maybe a separate set with them, because it could be EGL specific. We have attempt with fences PR #1646 to split caps iirc.

Yeah if we make it EGL-specific that makes little sense. I was hoping of a way to query for surface support, but maybe every surface supports all colorspaces if the extension is present (and as long as there is a supporting data format).

Should we have a generic Config implementation for DisplayFeatures given that the latter bitflags type is shared across all backends already anyway?

Not sure, need to try and see how it looks.

👍

Note also that EGL_EXT_gl_colorspace_bt2020_hlg was added in April this year and requires a gl_generator upgrade to use a newer registry (and IIRC there was one more constant that is missing).

I think gl_generator generally not maintained since it can't parse modern EGL specs, etc. We were adding functions/constants into egl-sys bindings manually, but that's about it. You can probably do the same.

Oh that would be unfortunate, there was a request for new maintainers and maybe we can take over? Or is there a newer project that does the same, if so glutin might want to migrate?

@kchibisov
Copy link
Member

Oh that would be unfortunate, there was a request for new maintainers and maybe we can take over? Or is there a newer project that does the same, if so glutin might want to migrate?

Probably shouldn't be me... There's https://crates.io/crates/phosphorus , but it can't parse EGL either. There was a feature request for it, but that's about it.

@kchibisov kchibisov added the api label Dec 22, 2023
This was referenced Jan 7, 2024
Draft
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
api
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

EGL: Provide color-space support MarijnS95/glutin
2 participants
@MarijnS95 @kchibisov