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

desktop: Add GUI using egui #10426

Merged
merged 46 commits into from May 24, 2023
Merged

desktop: Add GUI using egui #10426

merged 46 commits into from May 24, 2023

Conversation

Herschel
Copy link
Member

@Herschel Herschel commented Mar 28, 2023

This is a rough WIP for discussion and collaboration. Code could use some cleanup as I'm still learning my way around egui.

image

  • Ruffle launches into an empty movie instead of showing the file picker dialog.
  • Main menu bar can be used to play a movie.
  • File menu
    • Open File
    • Open URL
    • Exit
  • Help menu
    • Discord
    • About
  • Right-click context menu

TODO:

  • Currently uses fetch_root_movie to change movies, but it'd be safer to recreate the entire Player when loading a new movie for now. Have to add a method to reset and re-use some of the backends.
  • Pause and darken movie when UI is visible. no need anymore
  • Embed Ruffle logo animation and use it as default movie on startup.
  • Decide on rules for showing the menu bar.
    • Currently it will hide whenever a movie is playing, and holding Esc for half a second will display it.
    • This interacts badly with full-screen mode.
    • Other possibilities:
      • Always show
      • Show when mouse is at top of screen
  • In general, how should we handle keyboard shortcuts, because a game could also use the same keys?
  • Change egui styles to match Ruffle color scheme.
  • Settings menu
  • Improve About screen
    • Ruffle logo
    • Open source licenses/acknowledgements
  • Move file picker code out of main to gui (still using rfd)
  • Move full-screen shortcuts into gui
  • Update navigator base URL when a new movie is loaded.
  • Prevent GUI mouse event from interacting with the movie.
  • egui rendering is done directly to the final wgpu surface so no AA and possible color space issues.
  • General polish of the layout.

Other UI crate possibilities

I'm not settled on egui as the best solution, this is experimental to see how the crate works. Open to opinions. Other possibilites:

  • iced This is the most likely alternative. egui seemed a little more active, and at the time I started this work, iced didn't have a built-in menu bar widget. Looks like it does now in iced_aw.
  • imgui-rs Industry standard, very mature, but some awkwardness from C++<->Rust interop
  • gtk
  • xilem Formerly druid, in flux but maybe something to keep an eye on in the future

@Herschel Herschel marked this pull request as draft March 28, 2023 09:36
@SN902
Copy link

SN902 commented Mar 28, 2023

My rough WIP, I think, all stuff must be in the center of screen, it will be useful in full-screen mode and on high res monitors

image

We can make buttons with any colors, include transparency of the buttons

image

Rough "options" in ms paint. If it looks clunky (in very small window size), we can make all options just in single line in center, with drop boxes (scale, load behavior) and move "back" button in center on bottom side of screen

@n0samu
Copy link
Member

n0samu commented Mar 28, 2023

This is great, thanks so much! Don't forget that we can always use the Flash projector as a reference - I think the way they did things is generally pretty good.

Two issues I noticed:

  1. The About Ruffle and Open URL dialogs have a resizable border when they probably shouldn't.
  2. Clicking the Open File button doesn't dismiss the submenu until after a file is selected, which isn't consistent with how the other menu buttons work.

Currently uses fetch_root_movie to change movies, but it'd be safer to recreate the entire Player when loading a new movie for now. Have to add a method to reset and re-use some of the backends.

Yeah, right now the background music from one game persists after loading the next game, so this is definitely needed.

Pause and darken movie when UI is visible.

The Flash projector doesn't do that, so I wouldn't consider that an essential feature, but it might be nice.

Decide on rules for showing the menu bar.

  • In windowed mode, the Flash projector shows the menu bar at all times above the current movie, except when the movie sets Stage.showMenu = false (AS2) or stage.showDefaultContextMenu = false (AS3), in which case the menu bar is hidden. I find it annoying when movies hide the projector's menu bar, so I think we should have those properties only affect the actual context menu. Other than that, the Flash projector's behavior is good and we should do things the same way, I think.
  • In fullscreen mode, the Flash projector hides the menu bar. I think we should do the same. I don't think we need a way to show the menu in fullscreen mode.
  • Side note: the Flash projector has two ways of entering fullscreen mode: Alt-Enter and Ctrl-F. Weirder still, the Alt-Enter fullscreen mode can only be exited by pressing Alt-Enter again, and the Ctrl-F mode can only be exited by pressing Esc. And very weird problems occur when you try combining both fullscreen modes. This is not something we should emulate 😆 - our current keyboard shortcut setup for entering/exiting fullscreen is good.

In general, how should we handle keyboard shortcuts, because a game could also use the same keys?

I guess we should test what the Flash projector does, I'm not sure.

Prevent GUI mouse event from interacting with the movie.

It seems like mouse clicks are already blocked correctly but I guess we should do the same with hover events, etc.

Finally, we should definitely look into adding some more of the Flash projector's menu options:

@waspennator
Copy link

waspennator commented Mar 28, 2023

Could touch input also be looked into for the revamp if possible? According to Nosamu it doesnt work on the original desktop version of ruffle, but did on the flash player.

@n0samu
Copy link
Member

n0samu commented Mar 29, 2023

Could touch input also be looked into for the revamp if possible? According to Nosamu it doesnt work on the original desktop version of ruffle, but did on the flash player.

I think that's quite unrelated to this PR - you should make a separate feature request for it.

@Tyviebrock
Copy link

Tyviebrock commented Mar 29, 2023

Flash also shows you the last 9 opened flash files from newest to oldest under file.
image-23

And if you pressed 1 to 9 on the keyboard with the drop down open it'll open that file.

@Herschel
Copy link
Member Author

Herschel commented Mar 30, 2023

Thanks for the feedback!

  • In fullscreen mode, the Flash projector hides the menu bar. I think we should do the same. I don't think we need a way to show the menu in fullscreen mode.

I like the idea of mimicking the standalone player, and it does keep things simple! The only thought I had is that it might be useful to be able to adjust settings without exiting fullscreen mode. For example, if we have a full settings menu with things like key rebinding, etc., it might be slightly annoying to have to exit to windowed mode to fiddle with this. But possibly this is thinking ahead too much!

I also have to properly resize the window and bump the player itself down to account for the menu size if the menu is always going to be visible.

In general, how should we handle keyboard shortcuts, because a game could also use the same keys?

To answer my own question, I was just playing through Typing of the Dead via RetroArch recently, and I like how they handle it. By default, keyboard shortcuts are captured by the UI, which is totally fine for most content. But you can press Scroll Lock to lock all keyboard input to the the game for content where it matters.

@Tyviebrock
Copy link

Looking at #10434, maybe even a new option to reload the swf we're playing?

@Dinnerbone
Copy link
Contributor

egui rendering is done directly to the final wgpu surface so no AA and possible color space issues.

Instead of rendering ui onto the wgpu ruffle backend, render ruffle onto a texture and project that onto the ui. This way we can have UI without a ruffle player loaded, and separation of MSAA

@Herschel
Copy link
Member Author

Instead of rendering ui onto the wgpu ruffle backend, render ruffle onto a texture and project that onto the ui. This way we can have UI without a ruffle player loaded, and separation of MSAA

I agree! This was just the easiest way to get started.

@Dinnerbone
Copy link
Contributor

I've rebased this onto master (with wgpu 0.16, so I've had to change the egui version to a commit ref until they release a new one).

I can take a look at the TODOs too.

@Dinnerbone
Copy link
Contributor

@n0samu

The About Ruffle and Open URL dialogs have a resizable border when they probably shouldn't.

Fixed!

Clicking the Open File button doesn't dismiss the submenu until after a file is selected, which isn't consistent with how the other menu buttons work.

Not easy to fix, it's because the window is "paused" (OS level) when you click it, so we aren't re-rendering and you're seeing the state of the screen before the dialog was opened.

Yeah, right now the background music from one game persists after loading the next game, so this is definitely needed.

Should be fixed.

For keeping the menu bar open all the time and not overlaying the movie, it's not trivial right now but I can look into it. It'd certainly make the question of how to open it much simpler.

@crumblingstatue
Copy link
Contributor

This is really awesome.
Due to the immediate mode nature of egui, it should be fairly easy to walk the data structures of ruffle and expose a powerful debugging ui for the ruffle developers, and also enthusiasts to play with.

@Dinnerbone Dinnerbone force-pushed the egui branch 3 times, most recently from 96fed9d to c502c7a Compare May 22, 2023 20:30
@Dinnerbone
Copy link
Contributor

Opening this up for review now. It's not great, there's a lot I'd like to do in further PRs, but it should now be a strict improvement over the current desktop!

@Dinnerbone Dinnerbone marked this pull request as ready for review May 22, 2023 22:13
@Dinnerbone Dinnerbone changed the title [wip] desktop: Add GUI using egui desktop: Add GUI using egui May 22, 2023
@n0samu
Copy link
Member

n0samu commented May 23, 2023

Just tested this out and it's working great, thanks so much! From a UX standpoint I think it's definitely ready to merge.

@thisisjonathan
Copy link

This crashes on startup for me on linux, X11 with the following error:

thread 'main' panicked at 'Surface was not configured?', /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.16.0/src/backend/direct.rs:754:14
stack backtrace:
   0: rust_begin_unwind
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
   2: core::panicking::panic_display
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:147:5
   3: core::panicking::panic_str
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:131:5
   4: core::option::expect_failed
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/option.rs:1924:5
   5: core::option::Option<T>::expect
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/option.rs:786:21
   6: <wgpu::backend::direct::Context as wgpu::context::Context>::surface_get_current_texture
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.16.0/src/backend/direct.rs:751:25
   7: <T as wgpu::context::DynContext>::surface_get_current_texture
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.16.0/src/context.rs:2072:13
   8: wgpu::Surface::get_current_texture
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.16.0/src/lib.rs:4172:13
   9: ruffle_desktop::gui::controller::GuiController::render
             at ./desktop/src/gui/controller.rs:137:31
  10: ruffle_desktop::app::App::run::{{closure}}
             at ./desktop/src/app.rs:150:29
  11: winit::platform_impl::platform::sticky_exit_callback
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.28.6/src/platform_impl/linux/mod.rs:884:9
  12: winit::platform_impl::platform::x11::EventLoop<T>::run_return::single_iteration
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.28.6/src/platform_impl/linux/x11/mod.rs:375:21
  13: winit::platform_impl::platform::x11::EventLoop<T>::run_return
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.28.6/src/platform_impl/linux/x11/mod.rs:443:31
  14: winit::platform_impl::platform::x11::EventLoop<T>::run
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.28.6/src/platform_impl/linux/x11/mod.rs:498:25
  15: winit::platform_impl::platform::EventLoop<T>::run
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.28.6/src/platform_impl/linux/mod.rs:792:56
  16: winit::event_loop::EventLoop<T>::run
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.28.6/src/event_loop.rs:305:9
  17: ruffle_desktop::app::App::run
             at ./desktop/src/app.rs:108:9
  18: ruffle_desktop::main::{{closure}}
             at ./desktop/src/main.rs:161:33
  19: core::result::Result<T,E>::map
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/result.rs:774:25
  20: ruffle_desktop::main
             at ./desktop/src/main.rs:161:9
  21: core::ops::function::FnOnce::call_once
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

I think the code assumes that a resize event is send before rendering the window and that that isn't true on X11.
If I add the following code to desktop/src/gui/controller.rs in GuiController::new:

    surface.configure(
               &descriptors.device,
                &wgpu::SurfaceConfiguration {
                    usage: wgpu::TextureUsages::RENDER_ATTACHMENT,
                    format: surface_format,
                    width: 800,
                    height: 600,
                    present_mode: Default::default(),
                    alpha_mode: Default::default(),
                    view_formats: Default::default(),
                },
            );

it starts and lets me load a movie (and resizes to the size of the movie) but crashes when I try to resize the window with the following error:

thread 'main' panicked at 'wgpu error: Validation Error

Caused by:
    In a RenderPass
      note: encoder = `egui encoder`
    In a set_scissor_rect command
    Scissor Rect { x: 0, y: 0, w: 665, h: 525 } is not contained in the render target Extent3d { width: 640, height: 504, depth_or_array_layers: 1 }

', /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.16.0/src/backend/direct.rs:3019:5
stack backtrace:
   0: rust_begin_unwind
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
   2: wgpu::backend::direct::default_error_handler
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.16.0/src/backend/direct.rs:3019:5
   3: core::ops::function::Fn::call
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/ops/function.rs:79:5
   4: <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/alloc/src/boxed.rs:2002:9
   5: wgpu::backend::direct::ErrorSinkRaw::handle_error
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.16.0/src/backend/direct.rs:3005:17
   6: wgpu::backend::direct::Context::handle_error
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.16.0/src/backend/direct.rs:287:9
   7: <wgpu::backend::direct::Context as wgpu::context::Context>::command_encoder_end_render_pass
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.16.0/src/backend/direct.rs:1909:13
   8: <T as wgpu::context::DynContext>::command_encoder_end_render_pass
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.16.0/src/context.rs:2663:9
   9: <wgpu::RenderPass as core::ops::drop::Drop>::drop
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.16.0/src/lib.rs:3485:13
  10: core::ptr::drop_in_place<wgpu::RenderPass>
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/ptr/mod.rs:490:1
  11: ruffle_desktop::gui::controller::GuiController::render
             at ./desktop/src/gui/controller.rs:224:9
  12: ruffle_desktop::app::App::run::{{closure}}
             at ./desktop/src/app.rs:145:29
  13: winit::platform_impl::platform::sticky_exit_callback
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.28.6/src/platform_impl/linux/mod.rs:884:9
  14: winit::platform_impl::platform::x11::EventLoop<T>::run_return::single_iteration
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.28.6/src/platform_impl/linux/x11/mod.rs:375:21
  15: winit::platform_impl::platform::x11::EventLoop<T>::run_return
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.28.6/src/platform_impl/linux/x11/mod.rs:483:27
  16: winit::platform_impl::platform::x11::EventLoop<T>::run
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.28.6/src/platform_impl/linux/x11/mod.rs:498:25
  17: winit::platform_impl::platform::EventLoop<T>::run
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.28.6/src/platform_impl/linux/mod.rs:792:56
  18: winit::event_loop::EventLoop<T>::run
             at /home/jonathan/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.28.6/src/event_loop.rs:305:9
  19: ruffle_desktop::app::App::run
             at ./desktop/src/app.rs:108:9
  20: ruffle_desktop::main::{{closure}}
             at ./desktop/src/main.rs:161:33
  21: core::result::Result<T,E>::map
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/result.rs:774:25
  22: ruffle_desktop::main
             at ./desktop/src/main.rs:161:9
  23: core::ops::function::FnOnce::call_once
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

However, resizing does work when no movie is loaded.

@Dinnerbone
Copy link
Contributor

I'll take a look shortly, thank you!

@Dinnerbone
Copy link
Contributor

@thisisjonathan can you see if this commit fixes it? It's the same thing you tried but a correct width/height. I'm curious if that'll be enough or if that second crash is another issue entirely

@thisisjonathan
Copy link

Starting now works correctly, but resizing still gives the same error as before. Going fullscreen with Alt-Enter does work however.

@Dinnerbone Dinnerbone merged commit b29a784 into ruffle-rs:master May 24, 2023
13 checks passed
@n0samu n0samu removed the waiting-on-review Waiting on review from a Ruffle team member label Jun 9, 2023
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.

None yet