-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add basic backend support #108
Conversation
5c57702
to
2b94b6a
Compare
908a074
to
f15c031
Compare
ce276d1
to
19ddc77
Compare
9827ea8
to
c253032
Compare
a9dcfb0
to
36e348b
Compare
63c86e9
to
e81b0ff
Compare
e81b0ff
to
8a77700
Compare
use super::Color; | ||
|
||
#[dbus_proxy( | ||
interface = "org.gnome.Shell.Screenshot", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't use any private GNOME APIs in components that are not version locked with the rest of GNOME. These APIs are not stable and will only work for components in the same release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is barely a demo to try out the API, we are not planning to make use of that anywhere, so I think that is fine ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That demo will only work if you run the shell with the "unsafe" mode activated, until it changes the next time, unless you pretend to be a component you're not, but it still raises some warning flags to use these APIs outside the platform. Does a demo really need to use "real" implementations to demonstrate the API, instead of just returning some predefined image?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with what you said here. The scope of this PR went too big that it would take forever to ever land. I think we should just focus on providing building blocks of writing portals. Keep the demo as basic as possible.
// Avoid pointless and confusing recursion | ||
glib::unsetenv("GTK_USE_PORTAL"); | ||
glib::setenv("ADW_DISABLE_PORTAL", "1", true).unwrap(); | ||
glib::setenv("GSK_RENDERER", "cairo", true).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has really really bad performance on gtk4 in GNOME 44 so should be avoided. With that said, it's not possible to have GdkDisplays with different backends while still using hardware acceleration. See https://gitlab.gnome.org/GNOME/xdg-desktop-portal-gnome/-/merge_requests/68.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am confused by this, we need x11 displays to have modal windows on windows using xwalyand. Does this means that xdp-gnome does not do modals on them anymore? By the way note that initializing gtk with the GL backaend takes extremely long (~0.5s-1s on my machines), not a big problem if it is only done once but still.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
xdp-gnome 44 uses Wayland for modals on top of both Wayland and X11 windows if it's a Wayland session, and X11 otherwise. It uses a custom protocol just for that.
Is this functional enough to try out with a custom portal, or should I instead look to making my own helpers on top of zbus? |
8a77700
to
3d6d1de
Compare
5e8344b
to
4c7fd4f
Compare
We have diverged quite a lot in main and simplified the whole process, it should be much simpler to add new portals backend support. Regarding the demo, I think we should keep it as simple as possible, so bringing gtk is a bit too much. Thanks for starting this initiative though, closing. |
No description provided.