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.

Sign up for GitHub

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

Support open/save dialogs on wayland. #2228

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Support open/save dialogs on wayland. #2228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions druid-shell/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ x11 = [
"x11rb",
]
wayland = [
"ashpd",
"futures",
"wayland-client",
"wayland-protocols/client",
"wayland-protocols/unstable_protocols",
Expand Down
1 change: 1 addition & 0 deletions druid-shell/src/backend/shared/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,6 @@ cfg_if::cfg_if! {
pub(crate) use timer::*;
pub(crate) mod xkb;
pub(crate) mod linux;
pub(crate) mod zbus;
}
}
17 changes: 7 additions & 10 deletions druid-shell/src/backend/x11/dialog.rs → druid-shell/src/backend/shared/zbus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,26 @@ use ashpd::{zbus, WindowIdentifier};
use futures::executor::block_on;
use tracing::warn;

use crate::{FileDialogOptions, FileDialogToken, FileInfo};

use super::window::IdleHandle;
use crate::{FileDialogOptions, FileDialogToken, FileInfo, IdleHandle};

pub(crate) fn open_file(
window: u32,
window: WindowIdentifier,
idle: IdleHandle,
options: FileDialogOptions,
) -> FileDialogToken {
dialog(window, idle, options, true)
}

pub(crate) fn save_file(
window: u32,
window: WindowIdentifier,
idle: IdleHandle,
options: FileDialogOptions,
) -> FileDialogToken {
dialog(window, idle, options, false)
}

fn dialog(
window: u32,
id: WindowIdentifier,
idle: IdleHandle,
mut options: FileDialogOptions,
open: bool,
Expand All @@ -37,7 +35,6 @@ fn dialog(
if let Err(e) = block_on(async {
let conn = zbus::Connection::session().await?;
let proxy = file_chooser::FileChooserProxy::new(&conn).await?;
let id = WindowIdentifier::from_xid(window as u64);
let multi = options.multi_selection;

let title_owned = options.title.take();
Expand Down Expand Up @@ -70,7 +67,7 @@ fn dialog(
format: None,
})
.collect();
idle.add_idle_callback(move |handler| handler.open_files(tok, infos));
idle.add_idle(move |handler| handler.open_files(tok, infos));
} else if !multi {
if uris.len() > 2 {
warn!(
Expand All @@ -83,9 +80,9 @@ fn dialog(
format: None,
});
if open {
idle.add_idle_callback(move |handler| handler.open_file(tok, info));
idle.add_idle(move |handler| handler.open_file(tok, info));
} else {
idle.add_idle_callback(move |handler| handler.save_as(tok, info));
idle.add_idle(move |handler| handler.save_as(tok, info));
}
} else {
warn!("cannot save multiple paths");
Expand Down
10 changes: 10 additions & 0 deletions druid-shell/src/backend/wayland/surfaces/layershell.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use wayland_protocols::xdg_shell::client::xdg_surface;

use crate::kurbo;
use crate::window;
use crate::FileDialogOptions;
use crate::FileDialogToken;

use super::super::error;
use super::super::outputs;
Expand Down Expand Up @@ -369,6 +371,14 @@ impl Handle for Surface {
return self.inner.wl_surface.borrow().invalidate_rect(rect);
}

fn open_file(&self, options: FileDialogOptions) -> Option<FileDialogToken> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor question: do we need to add these to every surface or can we just use the already implemented data() method to access the implementations directly vs expanding the api surface of handle.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was kinda wondering that too, but it seemed better to just follow what all the other methods were doing... I'd be happy to change it, though

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the PR is fine strictly speaking as is; since you're basically following the pattern every backend is doing.... its just a nit i have with druids internals. these methods shouldn't need to be nested as deeply into the backends as they are. a backend really should not care about opening file dialogs. i've covered the topic in detail in other issues.

looks like you're only using the idle handle. which is already exposed, and long term the wayland id. I'd try to keep the file stuff as lightly coupled as possible personally. but rather see functionality implemented than worry about architectural issues that impact every backend currently =)

self.inner.wl_surface.borrow().open_file(options)
}

fn save_as(&self, options: FileDialogOptions) -> Option<FileDialogToken> {
self.inner.wl_surface.borrow().save_as(options)
}

fn remove_text_field(&self, token: crate::TextFieldToken) {
return self.inner.wl_surface.borrow().remove_text_field(token);
}
Expand Down
4 changes: 3 additions & 1 deletion druid-shell/src/backend/wayland/surfaces/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ use wayland_protocols::xdg_shell::client::xdg_popup;
use wayland_protocols::xdg_shell::client::xdg_positioner;
use wayland_protocols::xdg_shell::client::xdg_surface;

use crate::kurbo;
use crate::Scale;
use crate::TextFieldToken;
use crate::{kurbo, FileDialogOptions, FileDialogToken};

use super::error;
use super::outputs;
Expand Down Expand Up @@ -63,6 +63,8 @@ pub trait Handle {
fn request_anim_frame(&self);
fn invalidate(&self);
fn invalidate_rect(&self, rect: kurbo::Rect);
fn open_file(&self, options: FileDialogOptions) -> Option<FileDialogToken>;
fn save_as(&self, options: FileDialogOptions) -> Option<FileDialogToken>;
fn remove_text_field(&self, token: TextFieldToken);
fn set_focused_text_field(&self, active_field: Option<TextFieldToken>);
fn get_idle_handle(&self) -> idle::Handle;
Expand Down
10 changes: 10 additions & 0 deletions druid-shell/src/backend/wayland/surfaces/popup.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use wayland_protocols::xdg_shell::client::xdg_surface;

use crate::kurbo;
use crate::window;
use crate::FileDialogOptions;
use crate::FileDialogToken;

use super::error;
use super::surface;
Expand Down Expand Up @@ -205,6 +207,14 @@ impl Handle for Surface {
self.inner.wl_surface.invalidate_rect(rect)
}

fn open_file(&self, options: FileDialogOptions) -> Option<FileDialogToken> {
self.inner.wl_surface.open_file(options)
}

fn save_as(&self, options: FileDialogOptions) -> Option<FileDialogToken> {
self.inner.wl_surface.save_as(options)
}

fn remove_text_field(&self, token: crate::TextFieldToken) {
self.inner.wl_surface.remove_text_field(token)
}
Expand Down
41 changes: 40 additions & 1 deletion druid-shell/src/backend/wayland/surfaces/surface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ use wayland_protocols::xdg_shell::client::xdg_popup;
use wayland_protocols::xdg_shell::client::xdg_positioner;
use wayland_protocols::xdg_shell::client::xdg_surface;

use crate::kurbo;
use crate::backend::shared::zbus;
use crate::window;
use crate::{kurbo, FileDialogOptions, FileDialogToken};
use crate::{piet::Piet, region::Region, scale::Scale, TextFieldToken};

use super::super::Changed;
Expand Down Expand Up @@ -190,6 +191,14 @@ impl Handle for Surface {
self.inner.invalidate_rect(rect)
}

fn open_file(&self, options: FileDialogOptions) -> Option<FileDialogToken> {
self.inner.open_file(options)
}

fn save_as(&self, options: FileDialogOptions) -> Option<FileDialogToken> {
self.inner.save_as(options)
}

fn run_idle(&self) {
self.inner.run_idle();
}
Expand Down Expand Up @@ -469,6 +478,26 @@ impl Data {
self.schedule_deferred_task(DeferredTask::Paint);
}

fn open_file(&self, options: FileDialogOptions) -> Option<FileDialogToken> {
// FIXME: current ashpd has issues with wayland versions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a link to the upstream issue we could put here as a reference?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this. Which is merged, but there's still an issue with asphd'd wayland-client version not matching ours (and depending on a pre-release, which is a bit of a pain). I think the sanest thing to do is to wait until version 0.3 of the wayland-client libraries.

Also, it looks like getting an identifier requires a round-trip to the compositor, and so the new function is async...

//let id = ashpd::WindowIdentifier::from_wayland(&self.wl_surface.borrow());
Some(zbus::open_file(
Default::default(),
crate::IdleHandle(self.get_idle_handle()),
options,
))
}

fn save_as(&self, options: FileDialogOptions) -> Option<FileDialogToken> {
// FIXME: current ashpd has issues with wayland versions
//let id = ashpd::WindowIdentifier::from_wayland(&self.wl_surface.borrow());
Some(zbus::save_file(
Default::default(),
crate::IdleHandle(self.get_idle_handle()),
options,
))
}

pub fn schedule_deferred_task(&self, task: DeferredTask) {
tracing::trace!("scedule_deferred_task initiated");
self.deferred_tasks.borrow_mut().push_back(task);
Expand Down Expand Up @@ -625,6 +654,16 @@ impl Handle for Dead {
tracing::warn!("invalidate_rect invoked on a dead surface")
}

fn open_file(&self, _options: FileDialogOptions) -> Option<FileDialogToken> {
tracing::warn!("open_file invoked on a dead surface");
None
}

fn save_as(&self, _options: FileDialogOptions) -> Option<FileDialogToken> {
tracing::warn!("save_as invoked on a dead surface");
None
}

fn run_idle(&self) {
tracing::warn!("run_idle invoked on a dead surface")
}
Expand Down
10 changes: 4 additions & 6 deletions druid-shell/src/backend/wayland/window.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,12 @@ impl WindowHandle {
None
}

pub fn open_file(&mut self, _options: FileDialogOptions) -> Option<FileDialogToken> {
tracing::warn!("unimplemented open_file");
None
pub fn open_file(&mut self, options: FileDialogOptions) -> Option<FileDialogToken> {
self.inner.surface.open_file(options)
}

pub fn save_as(&mut self, _options: FileDialogOptions) -> Option<FileDialogToken> {
tracing::warn!("unimplemented save_as");
None
pub fn save_as(&mut self, options: FileDialogOptions) -> Option<FileDialogToken> {
self.inner.surface.save_as(options)
}

/// Get a handle that can be used to schedule an idle task.
Expand Down
1 change: 0 additions & 1 deletion druid-shell/src/backend/x11/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ mod util;

pub mod application;
pub mod clipboard;
pub mod dialog;
pub mod error;
pub mod menu;
pub mod screen;
Expand Down
16 changes: 12 additions & 4 deletions druid-shell/src/backend/x11/window.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ use x11rb::wrapper::ConnectionExt as _;
use x11rb::xcb_ffi::XCBConnection;

#[cfg(feature = "raw-win-handle")]
use raw_window_handle::{unix::XcbHandle, HasRawWindowHandle, RawWindowHandle};
use raw_window_handle::{HasRawWindowHandle, RawWindowHandle, XcbHandle};

use crate::backend::shared::zbus;
use crate::backend::shared::Timer;
use crate::common_util::IdleCallback;
use crate::dialog::FileDialogOptions;
Expand All @@ -61,7 +62,6 @@ use crate::window::{
use crate::{window, KeyEvent, ScaledArea};

use super::application::Application;
use super::dialog;
use super::menu::Menu;

/// A version of XCB's `xcb_visualtype_t` struct. This was copied from the [example] in x11rb; it
Expand Down Expand Up @@ -1798,7 +1798,11 @@ impl WindowHandle {
pub fn open_file(&mut self, options: FileDialogOptions) -> Option<FileDialogToken> {
if let Some(w) = self.window.upgrade() {
if let Some(idle) = self.get_idle_handle() {
Some(dialog::open_file(w.id, idle, options))
Some(zbus::open_file(
ashpd::WindowIdentifier::from_xid(w.id as u64),
window::IdleHandle(idle),
options,
))
} else {
warn!("Couldn't open file because no idle handle available");
None
Expand All @@ -1811,7 +1815,11 @@ impl WindowHandle {
pub fn save_as(&mut self, options: FileDialogOptions) -> Option<FileDialogToken> {
if let Some(w) = self.window.upgrade() {
if let Some(idle) = self.get_idle_handle() {
Some(dialog::save_file(w.id, idle, options))
Some(zbus::save_file(
ashpd::WindowIdentifier::from_xid(w.id as u64),
window::IdleHandle(idle),
options,
))
} else {
warn!("Couldn't save file because no idle handle available");
None
Expand Down
2 changes: 1 addition & 1 deletion druid-shell/src/window.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl TextFieldToken {
//NOTE: this has a From<backend::Handle> impl for construction
/// A handle that can enqueue tasks on the window loop.
#[derive(Clone)]
pub struct IdleHandle(backend::IdleHandle);
pub struct IdleHandle(pub(crate) backend::IdleHandle);

impl IdleHandle {
/// Add an idle handler, which is called (once) when the message loop
Expand Down