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

Spawning an AsyncMessageDialog in response to a winit WindowEvent::DroppedFile event does not work properly #6

Closed
francesco-cattoglio opened this issue Mar 17, 2021 · 11 comments

Comments

@francesco-cattoglio
Copy link
Contributor

I don't know if this is a rfd or a winit bug, but if you drop a file onto a winit window on MacOS and try to show a dialog as a response, the AsyncMessageDialog does not work. Instead, the "Hi! It looks like you are running async dialog in unsuported environment, I will fallback to sync dialog for you." gets printed. I think this is because the app.key_window() at line 58 of modal_future.rs is actually null.
It is easy to reproduce this by making a small change to the winit example, see my branch: https://github.com/francesco-cattoglio/rfd/tree/dropped_file_bug
To reproduce, just start the example and drag any file from the MacOS Finder onto the window.

@PolyMeilex
Copy link
Owner

PolyMeilex commented Mar 19, 2021

First, I haven't checked if sdl2::event::Event::DropFile works the same way. (yet)

So... I don't see any solution for this yet, the only solution I have in mind now is adding rfd::set_root_window() fn to the API, that user would call after winit window creation. Or adding set_root_window to dialog builder, but that's problematic as users don't always have access to their winit window.

But this sounds more like yet another workaround rather than a fix...
And I'm kinda tired of adding more and more workarounds one on top of another in macOS backend.

@francesco-cattoglio
Copy link
Contributor Author

First, I haven't checked if sdl2::event::Event::DropFile works the same way. (yet)

That might be interesting as an alternative to winit, if you happen to take a look let me know the results.

But this sounds more like yet another workaround rather than a fix...
And I'm kinda tired of adding more and more workarounds one on top of another in macOS backend.

I can agree. The entire idea that we cannot run sync dialogs on OsX on Winit feels like something that should be fixed in Winit itself.

@PolyMeilex
Copy link
Owner

Speaking of sync dialogs... every occasion is a good occasion to bump rust-windowing/winit#1779 XDD

Jokes aside, I will try to research this one more, (my hope is that it is not winit related) and If I figure something out I'll let you know.

@PolyMeilex
Copy link
Owner

So... it's not winit related, I acually like the way winit handles this a lot more than SDL2 (don't want to go to deep in to details)

The problem is purely related to the use of keyWindow, as long as you have your window in focus everything works as expected.

mainWindow sadly works the same way, so we can't use it.
windows array is our last chance, but it will be problematic in multi window setups.
User specified parent window is also an option, but that also gets problematic fast, for example if someone is using something like iced, winit windows is not so easy to acquire, and as far as I remember it's also no clonable.

@francesco-cattoglio
Copy link
Contributor Author

francesco-cattoglio commented Mar 20, 2021 via email

@PolyMeilex
Copy link
Owner

Just to be clear all the async shenanigans has nothing to do with winit, it's entirely my responsibility to make this work, but the question is, would rfd ever need async support if winit sync dialogs were not fundamentally broken... probably not.

So with that out of the way (once again it's not winit related), my plan is to...

  • Check for mainWindow of the app
  • If none is available get list of all windows in app
  • And pick the first/last one from the list

@PolyMeilex
Copy link
Owner

PolyMeilex commented Mar 21, 2021

So, I introduced 2 fixes for this:

  1. If you are using a single window, you can leave everything as is, and it should just work.
  2. If you need to specify which window rfd should use, you can by calling AsyncDialog::set_parent(&winit_window)

Both are available on master, I would appreciate if you could cross check those on your machine, just to make sure that everything works as expected.

The quickest way would probably be by playing around with winit-example, comment out set_parent to check if autodetection works, and uncomment to check if explicit parent works.

@francesco-cattoglio
Copy link
Contributor Author

I have tested this on my three machines, one Linux, one W10, one MacOS, and I can confirm that the winit-example works, both with and without the set_parent() line. I also tested my program and can confirm that the issue is gone!

@PolyMeilex
Copy link
Owner

Thanks, I will publish a new version shortly

@francesco-cattoglio
Copy link
Contributor Author

before you do, there is one other save-file related thing I am working on windows, I will post an issue and a pull request within a couple hours at most

@PolyMeilex
Copy link
Owner

Sure, thanks for a notice

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

No branches or pull requests

2 participants