-
-
Notifications
You must be signed in to change notification settings - Fork 477
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
Implement priority hints for EGL #1700
base: master
Are you sure you want to change the base?
Conversation
656ab7f
to
2625fe2
Compare
2625fe2
to
c63d6a8
Compare
@@ -228,6 +241,8 @@ pub struct ContextAttributes { | |||
|
|||
pub(crate) api: Option<ContextApi>, | |||
|
|||
pub(crate) priority: Priority, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that drivers could confuse things, can we use Option
internally here? So when it's None
we don't push, but when it's e.g. Some(Medium)
it's pushed to ensure the Medium
.
/// | ||
/// # Api-specific | ||
/// | ||
/// - Only EGL at the moment implements context priorities. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably use the notation with not supported we use everywhere else, so say that glx
, wgl
, and cgl
is not supported.
attrs.push(match context_attributes.priority { | ||
Priority::Low => egl::CONTEXT_PRIORITY_LOW_IMG, | ||
Priority::High => egl::CONTEXT_PRIORITY_HIGH_IMG, | ||
Priority::Realtime => { | ||
if self.inner.display_extensions.contains("EGL_NV_context_priority_realtime") { | ||
egl::CONTEXT_PRIORITY_REALTIME_NV | ||
} else { | ||
egl::CONTEXT_PRIORITY_HIGH_IMG | ||
} | ||
}, | ||
_ => unreachable!(), | ||
} as EGLint); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put a match
on a separate line please and then just push with as
casting.
You need to add a |
I'm fine with adding such a check just for android target. |
Fixes #1645
Fixes #1694
This PR adds a way to specify the priority for contexts. I mostly used the term hint, as its not guaranteed for various reasons that the context actually gets the priority.
Don't know if it makes sense to guarantee the priority. In Chromium it's also only used as a hint.
I also probably should extend the extension check as it is not correctly reported on Android.
https://github.com/chromium/chromium/blob/main/ui/gl/gl_display.cc#L814
Theoretically the extension also exists for GLX but Mesa for example doesn't support it.
CHANGELOG.md
if knowledge of this change could be valuable to users