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

replace OnceLock with AtomicPtr in NATIVE_DISPLAY #525

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joseluis
Copy link
Contributor

@joseluis joseluis commented Feb 3, 2025

I wanted to be able to start and stop a miniquad context and after trying several solutions to make NATIVE_DISPLAY droppable (e.g. using RwLock), I believe this one with AtomicPtr plus an static AtomicBool has a very good balance between performance and security.

A simple test:

use miniquad::*;

struct Stage {
    ctx: GlContext,
}
impl EventHandler for Stage {
    fn update(&mut self) {}
    fn draw(&mut self) {}
}
fn main() {
    println!("first context\nclose the window to create a second context.");
    miniquad::start( conf::Conf::default(), || { Box::new(Stage { ctx: GlContext::new(), }) });
    println!("second context");
    miniquad::start( conf::Conf::default(), || { Box::new(Stage { ctx: GlContext::new(), }) });
}

I also got the idea of the native_display function wrappers from #505 to improve ergonomics.

EDIT: This commit would effectively lower MSRV to 1.64. Related: #524.

- reduce locking overhead by replacing `OnceLock<Mutex<T>>` with `AtomicPtr<Mutex<T>>`.
- add `drop_display` to allow safely resetting `NATIVE_DISPLAY` while ensuring proper deallocation.
- implement `Drop` for `GlContext` and `MetalContext` to guarantee cleanup on context destruction.
- ensure memory safety using `Release/Acquire` ordering and explicit deallocation.
- add `native_display_blocking` and `native_display_nonblocking` for safer access.
- update documentation and rustfmt.
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

Successfully merging this pull request may close these issues.

1 participant