Rust-SDL2 / rust-sdl2

SDL2 bindings for Rust
MIT License
2.77k stars 474 forks source link

No way to get a window from a window_id. #573

Open ideasman42 opened 7 years ago

ideasman42 commented 7 years ago

Events expose their associated window, however I couldn't find a convenient way to get the window.

Using sdl2_sys this should be possible:

match sdl_event_generic {
    sdl2::event::Event::MouseMotion { x, y, window_id, .. } => {
        use sdl2_sys::video::SDL_GetWindowFromID;
        let win_ptr = SDL_GetWindowFromID(window_id);
        let win = sdl2::video::Window::from_ll(video_subsystem, win_ptr);
        /* ...etc */
    } /* ...etc */

However in this case the window video_subsystem takes a value. so sdl2::video::Window::from_ll can't be used to access windows multiple times.

Is this an intended API limitation? or are developers meant to maintain their own window data which can match against window ID's?

Cobrand commented 7 years ago

I don't think it was intentional, this is relatively safe to use. It's merely something that was forgotten.

If you want to create your own PR go ahead ;)

Cobrand commented 7 years ago

After some thoughts I think this method can be implemented, but as unsafe only. Not because you might get an invalid window if you send an invalid window_id (which is a valid concern but can be solved by NULL-checking and returning an Option), but more importantly you'd have a borrow or a mutable borrow, (or even worse, a "Window" object itself !) of something that is owned by something else. This is clearly disallowed by Rust idiomatic way of thinking: if you have a borrow, the original object can't be used, and this wouldn't prevent this basic security check.

So basically the problem is even worse when multi threading, Rust wouldn't be able to detect if 2 Windows referring to the same SDL_Window are used at the same time. This is another huge problem.

If you can, please create your own Window-retriever and maintain your own window, you are sure to have 0 memory safety issue with this. Otherwise, we could do a function of the like as "unsafe" and explicitly writing why it should be used with care in the documentation.