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

Support EGL_KHR_display_reference to clean up EGL properly #1588

Closed
kchibisov opened this issue Apr 7, 2023 · 3 comments
Closed

Support EGL_KHR_display_reference to clean up EGL properly #1588

kchibisov opened this issue Apr 7, 2023 · 3 comments

Comments

@kchibisov
Copy link
Member

This extension defines a ref counting for the EGLDisplay which we could use to properly call eglTerminate instead of leaking like we have it now.

@kchibisov
Copy link
Member Author

cc @i509VCB given that you had some concerns about not calling eglTerminate before.

@i509VCB
Copy link
Contributor

i509VCB commented Apr 12, 2023

This is definitely a solution to the problem for displays which glutin has created and owns.

For glutin do we want to always set that attribute if supported? What about things which borrow the display, should cloning the display increment the ref count of the display reference or should we continue with the interior mutability in that case?

For displays which glutin does not own or takes ownership of it appears we can query if EGL_TRACK_REFERENCES_KHR is set for the display.

Also what how does driver support look for this extension?

@kchibisov
Copy link
Member Author

it's not a driver extension, it's a platform extension, like a buffer_age.

We want to always set it if supported and don't anything otherwise.

We should increment on Clone, and we don't use interior mutability at all(we simply keep it inside the Arc).

The GetGlDisplay simply clones and rebuilds the display, so the reference must be incremented each time internal Arc is cloned.

kchibisov added a commit to kchibisov/glutin that referenced this issue Aug 16, 2023
This resolves a past issue where glutin would not be able to terminate
a display due to risk of two displays being created from the same
native display.

Besides not terminating EGL raises issue with nvidia on Wayland, thus
the `api::egl::Display::terminate` is added to help clients call this
function when calling `eglTerminate` is generally unsafe to do
automatically when dropping the `Display`.

Links: alacritty/alacritty#7146
Fixes: rust-windowing#1588
kchibisov added a commit to kchibisov/glutin that referenced this issue Aug 16, 2023
This resolves a past issue where glutin would not be able to terminate
a display due to risk of two displays being created from the same
native display.

Besides not terminating EGL raises issue with nvidia on Wayland, thus
the `api::egl::Display::terminate` is added to help clients call this
function when calling `eglTerminate` is generally unsafe to do
automatically when dropping the `Display`.

Links: alacritty/alacritty#7146
Fixes: rust-windowing#1588
Molytho added a commit to Molytho/glutin that referenced this issue Oct 9, 2023
This resolves a past issue where glutin would not be able to terminate
a display due to risk of two displays being created from the same
native display.

Besides not terminating EGL raises issue with nvidia on Wayland, thus
the `api::egl::Display::terminate` is added to help clients call this
function when calling `eglTerminate` is generally unsafe to do
automatically when dropping the `Display`.

Links: alacritty/alacritty#7146
Fixes: rust-windowing#1588
Molytho added a commit to Molytho/glutin that referenced this issue Oct 9, 2023
This resolves a past issue where glutin would not be able to terminate
a display due to risk of two displays being created from the same
native display.

Besides not terminating EGL raises issue with nvidia on Wayland, thus
the `api::egl::Display::terminate` is added to help clients call this
function when calling `eglTerminate` is generally unsafe to do
automatically when dropping the `Display`.

Links: alacritty/alacritty#7146
Fixes: rust-windowing#1588
Molytho added a commit to Molytho/glutin that referenced this issue Oct 10, 2023
This resolves a past issue where glutin would not be able to terminate
a display due to risk of two displays being created from the same
native display.

Besides not terminating EGL raises issue with nvidia on Wayland, thus
the `api::egl::Display::terminate` is added to help clients call this
function when calling `eglTerminate` is generally unsafe to do
automatically when dropping the `Display`.

Links: alacritty/alacritty#7146
Fixes: rust-windowing#1588
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants