Make open/save dialogs no longer synchronous (in GTK) #1302
Conversation
There was a problem hiding this comment.
I think this looks good; my only hesitation is that it leaves us in a slightly inconsistent state in terms of what API is available; save_as and open_as are both used in runebender, and I'd like to just make this change once and be done with it.
That can be in follow-up PRs, but I'd like to at least have a plan.
| } | ||
| } | ||
|
|
||
| pub fn save_as_sync(&mut self, _options: FileDialogOptions) -> Option<FileInfo> { |
There was a problem hiding this comment.
am I understanding his correctly that with this patch, save_as_sync won't work on gtk, and save_as won't work elsewhere?
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
Right. The current situation is that druid papers over the druid-shell API nastiness. I did a quick rg on runebender and it looks like it should still work, because AFAICT you're using the druid commands instead of manually calling the WIndowHandle methods?
The plan is to implement save_as and open_file for the other backends (just windows and mac, really, because X11 and web don't support file dialogs yet anyway; and I can do windows soon). Then we delete the xxx_sync methods on WindowHandle and remove the fallback hack in druid.
Any druid users that are using the SHOW_OPEN_PANEL, etc, commands shouldn't even notice the change (except that they'll stop getting dropped event warnings).
|
Also: I'm looking in the macOS reentrancy question. |
|
As currently written, macOS suspends delivery of events to our runloop while we are displaying a dialog; things like timers are delivered when the modal returns. It would be nice to revisit this, because the way things like our animation API work mean that we aren't currently able to animate while displaying a modal. |
There was a problem hiding this comment.
Ah okay I'd not noticed the fallback call to the sync version in druid, that seems like a very reasonable approach for now.
if you're not planning on doing the other backends, or not planning on doing them imminently, I would open a tracking issue for the missing implementations.
Thanks!
This is a first step towards #1068: it introduces a new non-sync API to druid-shell and implements it in GTK.
One aspect that I'm not thrilled with (but don't have a better idea for) is that I need to keep around the old API for now. Druid tries the new one first, and then falls back to the old one if the new one doesn't work. The reason for this is that I can't do the mac implementation on my own -- it's non-trivial enough that I don't feel comfortable about the fact that I can't test it. (Also, I don't understand how re-entrancy is handled in the mac backend because there aren't any refcells, just unsafe casts to
&mut ViewState-- does this mean that re-entrant calls result in UB?)