Fix UI hangs on dialog boxes on macOS

[chris-marrero]

Problem

See #41 for a detailed explanation of the root cause. In brief, dialog boxes spin the thread which causes the ui to freeze and macOS functionality to be broken.

My Solution¹

It starts with DialogBinds and DialogBind, a thin wrapper around egui_async::Bind. It is a bevy Resource to allow access to it every update. Bind serves as a way to keep a future between updates. See here for more information on how I use it.

#[derive(Resource)]
pub struct DialogBinds {
    binds: Vec<DialogBind>,
    ...
}
#[derive(Debug)]
struct DialogBind {
    kind: DialogKind,
    bind: Bind<FileHandle, ()>,
}

When we want a new dialog box, we call DialogBinds::spawn_and_poll which, among other things, does what is shown below.

let bind = self.get_bind(kind);
if let Some(file) =
bind.read_or_request(|| async move { // Spawns future if the bind is empty
        kind.open(file_dialog) // Future to open the dialog box. Depends on the kind of dialog box
            .await
            .ok_or(())
    }
) {
    let r = file.clone().map(|file| file.path().to_path_buf()).ok().clone(); // Transform into `PathBuf`
    bind.clear(); // Reset the bind to allow for a new dialog box
    r
} else {
    None
}       

There are many dialog kinds, one per original call to rfd::FileDialog::new. Each kind is mapped to both a function call to rfd::AsyncFileDialog and a file filter. For example, if evaluating kind.open(file_dialog) when DialogKind::Open, this would be called: file_dialog.pick_file().

Certain dialog boxes require data to be opened properly, such as setting the default name for a new file. This is handled by adding an Option to DialogBinds and adding explicit cases for each piece of data.

Finally, this method relies on polling the future every update in order to progress. The first poll of the future will essentially never return a value. Therefore, the poll must kept being called every frame that we should expect a result. Each dialog box call must handle polling, so each call has a bespoke method for keeping the call alive every frame.

There are two main methods. If the area of code the dialog box is in is being run every update, we can just check if the bind is in_progress and keep polling if it is.

if ui.button("Browse...").clicked() || dialog_binds.in_progress(DialogKind::NewProject) { ... }

However, render_dialogs will take the pending_action from the state, meaning unless the action is replaced, that branch will not run the next update and the future will not be polled again. In this case, it is a simple matter of setting the pending_action back.

In the end, it leads to a call that looks something like this:

if ui.button("Browse...").clicked()
    || dialog_binds.in_progress(DialogKind::NewProject)
{
    let file_name = ...;

    if let Some(path) = dialog_binds
        .set_file_name(file_name)
        .spawn_and_poll(DialogKind::NewProject)
    {
        editor_state.new_project_save_path = Some(path.clone());
    }
}

Todo and Thoughts

• Must complete before publishing: In some situations, canceling a dialog box does not work because DialogBind makes no distinction between a finished bind which returned nothing and an in progress bind.
• Somewhat out of scope of the issue, but I'd like to go in and separate out all the platform specific code. There are a lot of #[cfg(target_os = foo)], but not applied equally. AsyncFileDialog should also allow it to work in the browser.
• I'd like a review on names for the dialog kinds. I just came up with whatever seemed right from the context of the code, but I'm not sure I quite captured the essence of what each dialog box is
• I'm strongly considering refactoring DialogKind to not have filter and open. Instead, the logic is returned to the call site as it was before this commit. So you'd have a call something like below. The problem is that we don't know which bind to use. The bind must be stable across updates for dialog box's lifetime. It would be easier if the next list item was implemented.

  if let Some(path) = dialog_binds
      .add_filters(MAP_PROJECT)
      .open(DialogKind::SaveFile)
      .spawn_and_poll() { // Do something with path }

• While I did my best to make sure the futures get polled until completion, I'm still not sure I got every dialog exactly right. It feels like a hack. One way to guarantee that the future is completed is to use a callback for when dialog sucessfully completes. That means that even if that future is never polled again by the ui, the callback is still called. That would look something like this:

  dialog_binds.spawn_and_poll(DialogKind::NewProject, move |path| {
      // Do something with path
  });

  However, this has a few implications. First, we'd need some resource to track pending dialog box futures. Second, we need a way to associate each dialog box future with a specific action and element in the ui so that we can react to it. Third, and most importantly, most of these callbacks require mutable access to local or bevy state. Cue lifetime and generic nightmares. It would definitely lead to a more resilient system with more capabilities to expand, but..... is it really worth it?
• Testing? Docs? ¯\_(ツ)_/¯

Footnotes

1. Sorry for the long explanation. It's a complex problem. Feel free to skip the code and come back to the explanation for specifics. ↩

[jbuehler23]

Overall, I'm happy with these changes - it would be good to just have some test evidence that the UI is not affecting on Windows machines as well. I can try and test locally later on this branch

[jbuehler23]

"Somewhat out of scope of the issue, but I'd like to go in and separate out all the platform specific code. There are a lot of #[cfg(target_os = foo)], but not applied equally. AsyncFileDialog should also allow it to work in the browser."

I would be very happy with these changes. I had to a hack a lot of this stuff together, and I would love to see it removed

[chris-marrero]

In addition to resolving your commets, I also completed the first todo and got dialog cancellation working. Also, can confirm that the dialog boxes work as usual on Windows. In fact, now, the window doesnt freeze when you open a dialog box on windows! You can still interact with the editor and open other dialogs.

This may lead to unintended bugs with weird interactions. For example, opening a dialog box in a menu, interacting with the UI such that the menu is no longer being rendered, and then submitting the dialog box. It may not propogate into the system until menu is opened again. Maybe we should consider moving this completely to a bevy observer and resource. Not sure it's a big enough issue fix, but we should be aware of it when building out any more UI with dialog boxes.

[jbuehler23]

Excellent work @chris-marrero! Let's get this merged in once CI is passing. I'll cut a new release likely tomorrow as well and push to crates.io

[jbuehler23]

Merging this in, once CI passes :)