linebender / xilem

An experimental Rust native UI framework
Apache License 2.0
3.55k stars 111 forks source link

Add winit event callbacks to masonry and xilem application #404

Closed cfagot closed 3 months ago

cfagot commented 3 months ago

The purpose of this pull request is to allow masonry and xilem to share the event loop with the rest of the application but still essentially own and manage the event loop. There is also a small hack to allow widgets to always repaint, but that isn't meant to be included in the final pull request (left it in for information purposes).

Breakdown of changes:

Masonry

Xilem

Example

In the following example, the application hooks into resumed/suspended to change the control flow of the event loop, block redraw events coming from the event loop, and uses about_to_wait to update game world and push a redraw request to xilem.

type GameState = Arc<Mutex<GameWorld>>;

struct AppInterface {}

impl XilemToAppInterface<GameState> for AppInterface {
    fn resumed(&self, event_loop: &winit::event_loop::ActiveEventLoop, _: &mut dyn AppToXilemInterface<GameState>, _: &mut masonry::event_loop_runner::MasonryState<'_>) {
        event_loop.set_control_flow(winit::event_loop::ControlFlow::Poll);
    }

    fn suspended(&self, event_loop: &winit::event_loop::ActiveEventLoop, _: &mut dyn AppToXilemInterface<GameState>, _: &mut masonry::event_loop_runner::MasonryState<'_>) {
        event_loop.set_control_flow(winit::event_loop::ControlFlow::Wait);
    }

    fn window_event(&self, _event_loop: &winit::event_loop::ActiveEventLoop, _window_id: winit::window::WindowId, event: &winit::event::WindowEvent, xilem_interface: &mut dyn AppToXilemInterface<GameState>, masonry_state: &mut masonry::event_loop_runner::MasonryState<'_>) -> bool { 
        if *event == winit::event::WindowEvent::RedrawRequested {
            // we block redraw requests because we draw during about_to_wait
            true
        }
        else {
            false
        }
     }

    fn about_to_wait(&self, event_loop: &winit::event_loop::ActiveEventLoop, xilem_interface: &mut dyn AppToXilemInterface<GameState>, masonry_state: &mut masonry::event_loop_runner::MasonryState<'_>) {
        // this is safe because xilem and masonry don't track window id, but we should instead remember our window id and use that
        let fake_window_id = unsafe { winit::window::WindowId::dummy() };

        let game_state = xilem_interface.get_state();
        game_state.lock().unwrap().update();

        xilem_interface.submit_window_event(event_loop, masonry_state, fake_window_id, winit::event::WindowEvent::RedrawRequested);
    }
}

fn main() -> Result<(), EventLoopError> {
    let game_state = GameState::new(Mutex::new(create_game_world()));

    let app = Xilem::new(game_state, app_logic).with_app_interface(Box::new(AppInterface{}));
    app.run_windowed(EventLoop::with_user_event(), "test".into())?;
    Ok(())
}

Note that GameState is an Arc<Mutex> because it needs to be shared between xilem app, widget view, and widget. If there is a better way to do that let me know.

cfagot commented 3 months ago

I added an example game in a new branch: https://github.com/cfagot/xilem/tree/space_survival. I intend to contribute that in a new pull request if/when this pull request is approved. It might be too large as an example (not huge but is about 2000 lines small). But I think it would be good because it shows off the capabilities of xilem/masonry/vello and helps establish a guide post for how apps like this can be made with xilem (hopefully not guiding in the wrong direction though :).

All the code is clean room (other than a few snippets I pulled form my own code) so no license issues. And no external dependencies either.

PoignardAzur commented 3 months ago

I haven't looked at this PR in-depth, so I might be missing things, but so far I'd be against merging it.

Overall the fact that this introduces new interfaces which closely follow the API of another crate and by default do nothing but delegate to that another identical method (eg MasonryState::submit_user_event does nothing but call MasonryState::handle_user_event) strikes me as a major code smell.

Similarly, this PR introduces like 5 different methods with the same name or similar name for window events, device events and user events. Having worked on projects with similar structures, I can tell you from experience the DX of maintaining these APIs in painful.

My recommendation would be to nix the AppToXilemInterface trait and the AppToXilemInterface and look for a way to give your app direct access to the Winit event loop and whatever else you need. Doing so means you might need to create your own composition root, eg you won't end up using Xilem or AppDriver and will have to create your own root types instead. I think that's fine: if you have a direct dependency on Xilem and Masonry, you're already creating a game engine anyway.