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

XDG Parenting Support #46

Open
PolyMeilex opened this issue Jan 19, 2022 · 8 comments
Open

XDG Parenting Support #46

PolyMeilex opened this issue Jan 19, 2022 · 8 comments

Comments

@PolyMeilex
Copy link
Owner

bilelmoussaoui/ashpd#40

@bilelmoussaoui
Copy link
Contributor

@PolyMeilex should be fixed from ashpd side, the wayland part is very untested but it should work i guess. Please give it a try and let me know how that goes, if all goes as expected I will do another beta and then aim for a 0.2 release.

@PolyMeilex
Copy link
Owner Author

Thanks for the heads-up!
I'll give it a try.

@PolyMeilex
Copy link
Owner Author

Just a small update for bystanders. I'm not going to depend on ashpd parenting support until we finish work on wayland-rs-0.30 release, we can't have alpha deps in the tree, especially as dynamically changing ones as wayland-rs 0.30 rework.

@bjorn
Copy link

bjorn commented Nov 9, 2023

I'm not going to depend on ashpd parenting support until we finish work on wayland-rs-0.30 release, we can't have alpha deps in the tree

With that issue closed a while back, what is the current status of this?

I've recently started using rfd in my Slint app and I noticed the parenting is entirely missing (though I'm likely using the direct gtk3 mode at the moment). I'd like to know if I could help improve the behavior.

@PolyMeilex
Copy link
Owner Author

Yeah, we released wayland-rs 0.30 a while ago, and ashpd already updated to it, so the only thing left is to wire it up in RFD xdg desktop backend

@bjorn
Copy link

bjorn commented Nov 10, 2023

@PolyMeilex I had a look into this, but I think it's a little beyond me due to my still limited Rust experience. I got the following to compile, but I'm not sure if this is the desired approach:

diff --git a/Cargo.toml b/Cargo.toml
index 10f7ca2..a6ab94d 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -11,7 +11,7 @@ repository = "https://github.com/PolyMeilex/rfd"
 documentation = "https://docs.rs/rfd"
 
 [features]
-default = ["gtk3"]
+default = ["xdg-portal"]
 file-handle-inner = []
 gtk3 = ["gtk-sys", "glib-sys", "gobject-sys"]
 xdg-portal = ["ashpd", "urlencoding", "pollster", "async-io", "futures-util"]
@@ -46,7 +46,7 @@ async-io = { version = "1.3", optional = true }
 futures-util = { version = "0.3", optional = true, default-features = false, features = ["io"] }
 
 # XDG Desktop Portal
-ashpd = { version = "0.6", optional = true }
+ashpd = { version = "0.6", optional = true, features = ["raw_handle"] }
 urlencoding = { version = "2.1.0", optional = true }
 pollster = { version = "0.3", optional = true }
 # GTK
diff --git a/src/backend/xdg_desktop_portal.rs b/src/backend/xdg_desktop_portal.rs
index 0bbabad..0bf9242 100644
--- a/src/backend/xdg_desktop_portal.rs
+++ b/src/backend/xdg_desktop_portal.rs
@@ -61,8 +61,15 @@ impl AsyncFilePickerDialogImpl for FileDialog {
     }
 
     fn pick_files_async(self) -> DialogFutureType<Option<Vec<FileHandle>>> {
+        let identifier = if let Some(parent) = self.parent {
+            Some(block_on(ashpd::WindowIdentifier::from_raw_handle(&parent, None)))
+        } else {
+            None
+        };
+
         Box::pin(async {
             OpenFileRequest::default()
+                .identifier(identifier)
                 .accept_label("Pick file(s)")
                 .multiple(true)
                 .title(

Open questions for me:

  • Is it alright to block on creating the WindowIdentifier? When I moved it into the async block, I got "*mut c_void cannot be shared between threads safely".
  • I just passed None for the display_handle, but that likely means Wayland is not going to be supported. I'm not sure where the display handle is supposed to come from.

@PolyMeilex
Copy link
Owner Author

I just passed None for the display_handle, but that likely means Wayland is not going to be supported. I'm not sure where the display handle is supposed to come from.

set_parent Should accept HasRawWindowHandle + HasRawDisplayHandle, the API predates existence of DisplayHandles that's why it only has WindowHandle.

Is it alright to block on creating the WindowIdentifier? When I moved it into the async block, I got "*mut c_void cannot be shared between threads safely".

Not sure to be honest, haven't looked into what it does on X and Wayland, perhaps it does some roundtrips to the display server, but I suppose it should not take significant amounts of time.

@jf2048
Copy link

jf2048 commented Nov 27, 2023

On X it doesn't require any server communication, on Wayland it spawns a thread to do a request + response with the server to generate the ID: src/window_identifier/wayland.rs#L87-L94

It should be possible to do this without any blocking by ensuring the window object (whatever it may be) stays alive as long as the future is alive. But to do that safely I think would require updating to use the borrowing traits. Otherwise the FileDialog could outlive whatever was used to generate the RawWindowHandle and then the pointer inside it would become dangling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants