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

Select a Wayland compositor toolkit for the GUI agent #6715

Closed
DemiMarie opened this issue Jun 18, 2021 · 23 comments
Closed

Select a Wayland compositor toolkit for the GUI agent #6715

DemiMarie opened this issue Jun 18, 2021 · 23 comments
Labels
C: gui-domain P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: task Type: task. An action item that is neither a bug nor an enhancement.
Milestone

Comments

@DemiMarie
Copy link

DemiMarie commented Jun 18, 2021

Qubes OS version (if applicable)

R4.2

Affected component(s) or functionality (if applicable)

qubes-gui-agent

Brief summary

The Qubes GUI agent needs to support Wayland. While it is practical to support the entire core protocol using only first-party code, this will severely limit the protocols (roughly analogous to X extensions) that can be supported. As a result, applications will have less functionality than they would under X11, which would be bad. Therefore, the Qubes GUI agent should use a third-party compositor toolkit, such as wlroots or smithay.

Additional context

One of the major motivations for switching to Wayland in the GUI agent is to achieve at least some degree of isolation between sandboxed applications, such as Flatpaks. wlroots appears to be the most mature Wayland compositor toolkit, but is (according to its own documentation) 50,000 lines of C code. Smithay is written in Rust, but is not in distribution package repositories (yet) and may have fewer features and a smaller community.

Relevant documentation you've consulted

Related, non-duplicate issues

@DemiMarie DemiMarie added T: task Type: task. An action item that is neither a bug nor an enhancement. P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. labels Jun 18, 2021
@andrewdavidwong andrewdavidwong added this to the Release 4.2 milestone Jun 18, 2021
@DemiMarie
Copy link
Author

To quote @vberger:

Before I start, I should warn you that wlroots is a much more advanced and mature project than Smithay, which is on some regards still pretty experimental. That said, if you're interested anyway, we'd be glad to work in collaboration to bring to Smithay the missing pieces you may need.

@marmarek: from a project management perspective, would this be a good trade-off for us?

@marmarek
Copy link
Member

An idea: lets try to build a table of functionalities (interfaces, extensions) and for each lets write:

  • do we need to expose it to clients in VM?
  • if yes, should it be handled mostly at our compositor side (framework will be more helpful), or proxied to GUI VM (framework will be less helpful)?
  • is is supported by wlroot?
  • is it supported by Smithay?

This will give us some idea how much they differ for us in practice, plus will help with planning development at the same time.

@DemiMarie
Copy link
Author

DemiMarie commented Jun 20, 2021

An idea: lets try to build a table of functionalities (interfaces, extensions) and for each lets write:

  • do we need to expose it to clients in VM?
  • if yes, should it be handled mostly at our compositor side (framework will be more helpful), or proxied to GUI VM (framework will be less helpful)?
  • is is supported by wlroot?
  • is it supported by Smithay?

This will give us some idea how much they differ for us in practice, plus will help with planning development at the same time.

Here is my current idea. This is PROVISIONAL and anyone in the Qubes OS team should feel free to edit it. It may be inaccurate and should not be relied upon.

Interface Expose in qubes? Where handled? Supported by wlroots? Supported by Smithay? Comment
Core protocol Yes GUI qube Yes Yes Mandatory
xdg-shell Yes GUI qube Yes Yes Mandatory
xdg-decoration Yes Compositor Yes No To alert apps that they should not draw client-side decorations
server-decoration Yes Compositor Yes No To alert apps that they should not draw client-side decorations
viewporter ? ? Yes No Surface cropping and scaling
presentation-time ? ? Yes No Presentation timing feedback for synchronization
fullscreen-shell No N/A N/A N/A Not intended for desktop applications
idle-inhibit Optional (per-qube configuration) GUI qube Yes No This is a useful feature, but should be disabled by default.
input-method Maybe Compositor, GUI qube, or both! Yes No This is handled entirely in the GUI qube and the compositor need not be aware of it
input-timestamps No N/A No No The usefulness of this is unclear; it seems most useful for fingerprinting. In fact, Whonix would like to deliberately reduce this precision.
keyboard-shortcuts-inhibit No N/A Yes No Being able to disable keyboard shortcuts is an obvious security risk
linux-dmabuf Yes GUI qube Yes Yes DMA buffers, an alternative form shared memory.
linux-explicit-synchronization Probably not (yet)? GUI qube No Yes Explicit synchronization of GPU buffer use; probably not relevant as long as rendering is on the CPU
pointer-constraints No N/A Yes No Being able to disable keyboard shortcuts is an obvious security risk
pointer-gestures Maybe GUI qube Yes No Touchpad gestures
primary-selection Configurable Compositor Yes No Primary selection
relative-pointer No N/A Yes No This protocol provides support for receiving off-screen pointer events, but offscreen areas are (by definition) not assigned to any qube, so no qube is allowed to receive input to them
tablet Yes, but not immediately PV tablet protocol? Unshure! Yes No Graphics tablet protocol (useful for art)
text-input Maybe???? Compositor, GUI qube, or both! Yes No Text input and input methods
xdg-foreign Yes Compositor Yes No Useful for Flatpak portals
xdg-output Yes GUI qube Yes Yes Needed to provide monitor layout information
xwayland-keyboard-grab No Compositor No No XWayland keyboard grabs
virtual-keyboard Maybe Compositor? Yes No Virtual keyboards

@elinorbgr
Copy link

I gave you some details regarding protocol extension support, and how you can think about it as an answer to Smithay/smithay#300

I'd just like to add one thing, regarding this:

from a project management perspective, would this be a good trade-off for us?

You should also keep in mind that Smithay is, as a whole still rather in flux. We have at least one big API refactor still coming up before our 0.3 release. And while I do hope that we're settling on good design overall, given the low maturity of the project I cannot promise that there will not be significant changes in our API in the future.

@marmarek
Copy link
Member

Qubes OS use case is rather unusual, and in fact having an API not written in stone may be a feature - there maybe some cases where the easiest solution would be to adjust the API, instead of working around it.
That said, big API changes are (almost?) always an issue for building things on top of it.

@vberger do you have any timeline for the planned refactor? Do you plan at some point to provide documentation for migration (like "function X is removed, use function Y with those arguments instead")?

@elinorbgr
Copy link

My plan wrt this refractor is to try and tackle it in the next few days / weeks (depending on how smoothly that goes). Following that, we'll only have a few bug fixes to handle before we pin version 0.3 of Smithay. That will be the first "real" release of Smithay (that is more than just a prototype), so with it we'll start handling breaking changes more seriously (with migration guides and such).

@DemiMarie
Copy link
Author

One major difficulty with wlroots is that it is extremely difficult to write safe bindings for, to the point that the only person I know of to have tried gave up and rewrote their project in C instead. Their blog post explains (in much better detail than I could) what went wrong.

@PolyMeilex
Copy link

PolyMeilex commented Jun 22, 2021

I definitely agree, I have attempted to write safe wrapper around wlroots just like Timidger did, after reading his blog post I thought that my attempt will have much higher chance of success, because I decided to write my own lib on top of wlroots instead of trying to expose whole wlroots API, I wasted around a month of my life on that and failed miserably. I will not attempt to explain what went wrong, because Timidger already did it way better than I could.

EDIT: The only difference to Timidger story is that I ended up betting on Smithay instead of going c route

@DemiMarie
Copy link
Author

I definitely agree, I have attempted to write safe wrapper around wlroots just like Timidger did, after reading his blog post I thought that my attempt will have much higher chance of success, because I decided to write my own lib on top of wlroots instead of trying to expose whole wlroots API, I wasted around a month of my life on that and failed miserably. I will not attempt to explain what went wrong, because Timidger already did it way better than I could.

Would being willing to if x.destroyed() { panic!("Using a destroyed object") } help? I imagine that would be fine for Qubes OS.

EDIT: The only difference to Timidger story is that I ended up betting on Smithay instead of going c route

Was Smithay able to do what you needed? I am worried about how few protocols it supports.

@PolyMeilex
Copy link

Would being willing to if x.destroyed() { panic!("Using a destroyed object") } help? I imagine that would be fine for Qubes OS.

It probably would be fine, I just find it too cumbersome, it does not fix the underlying problem, it simply sweeps it under the rug, and of course, compositor is the last piece of software I would like to panic on me because of some deeply hidden API misuse.

Was Smithay able to do what you needed? I am worried about how few protocols it supports.

It's too soon to tell in my case, but I believe it's a solid base that is easy to build upon. My plan is to simply implement any missing protocols as I go, and merge them upstream at some point. (at the moment I'm planing to implement wlr_layer_shell,xdg-decoration, and tablet (already mostly working in my private branch))

@DemiMarie
Copy link
Author

Would being willing to if x.destroyed() { panic!("Using a destroyed object") } help? I imagine that would be fine for Qubes OS.

It probably would be fine, I just find it too cumbersome, it does not fix the underlying problem, it simply sweeps it under the rug, and of course, compositor is the last piece of software I would like to panic on me because of some deeply hidden API misuse.

Good point. People do manage to write seemingly-reliable compositors based on wlroots, but that is still a major footgun.

Was Smithay able to do what you needed? I am worried about how few protocols it supports.

It's too soon to tell in my case, but I believe it's a solid base that is easy to build upon. My plan is to simply implement any missing protocols as I go, and merge them upstream at some point. (at the moment I'm planing to implement wlr_layer_shell,xdg-decoration, and tablet (already mostly working in my private branch))

How much code was needed for each protocol?

@marmarek
Copy link
Member

Would being willing to if x.destroyed() { panic!("Using a destroyed object") } help? I imagine that would be fine for Qubes OS.

I may be missing something, but in context of the linked post, I guess an example would be disconnected output, right? Are you proposing to panic the compositor whenever someone disconnects an external output (VMs are aware of physical outputs, to some degree)? That doesn't sound like a good idea...

@DemiMarie
Copy link
Author

Would being willing to if x.destroyed() { panic!("Using a destroyed object") } help? I imagine that would be fine for Qubes OS.

I may be missing something, but in context of the linked post, I guess an example would be disconnected output, right? Are you proposing to panic the compositor whenever someone disconnects an external output (VMs are aware of physical outputs, to some degree)? That doesn't sound like a good idea...

Obviously not 🙂. I am suggesting that we panic the compositor if the compositor uses an output that has already been disconnected. That indicates a compositor bug, as the compositor should have removed all references to that output from its internal data structures in the destroy callback.

@PolyMeilex
Copy link

How much code was needed for each protocol?

It depends... wayland-rs has a really nice abstraction for implementing protocols, so it's shorter in comparison to c counterpart, tablet is quite simple protocol so it's around 300LOC (not including code automatically generated by wayland-rs protocol scanner), keep in mind that it is not finished, so it is expected to still grow a little.
xdg-decoration is even simpler, so I expect it to be even shorter than tablet.

@marmarek
Copy link
Member

viewporter

Do you know how many applications require it?

fullscreen-shell

We do allow VM to make fullscreen window, as opt-in feature. That's quite useful for video players or presentations.
But I think we can live without it in the first iteration.

@elinorbgr
Copy link

We do allow VM to make fullscreen window, as opt-in feature. That's quite useful for video players or presentations.

Note that making applications full-screen is already part of the xdg-shell protocol, and is what most applications use. fullscreen-shell is a protocol extension meant for more specific uses.

@PolyMeilex
Copy link

PolyMeilex commented Jun 23, 2021

As far as I remember it's mainly meant for compositors that don't have window management and are meant to simply display one thing, for example something like a splash screen of some sort.
xdg-shell has both set_maximized and set_fullscreen and that should be used for typical desktop use-cases.

@marmarek
Copy link
Member

xdg-shell has both set_maximized and set_fullscreen and that should be used for typical desktop use-cases.

Ah, ok.

@marmarek
Copy link
Member

It seems we have two main options:

  • writing in Rust using Smithay
  • writing in C using wlroots

Going with Smithay sounds like an interesting adventure. The gui-agent side (where the compositor will be) isn't the most security critical part, so from the security point of view, having it in C would be acceptable too. But on the other hand, we do want to slowly migrate from C to Rust, wherever it makes sense. This project, which is mostly writing new thing from scratch, sounds like a good candidate to apply this rule.
But, since this may be technically challenging and full of surprises, I want to setup some safety fuse. Lets say, try to get basic functionality (get it to display a simple application window, maybe also some kind of input) working in about a month. If we hit too many obstacles in this time, lets reconsider the choice. Does this plan make sense to you @DemiMarie ?

There is also a question whether we should start now, or after the refactor @vberger mentioned earlier. Can you give some insights about what areas will be affected? Basically the question is whether will it affect hello-world kind of compositor (with totally custom backend)?

@elinorbgr
Copy link

The refractor in question is here Smithay/smithay#304, so if you're willing to wait a few days that we review and merge our pending PRs, that should be good for you. Once this is done, the remaining changes on our 0.3 roadmap are minor changes & bugfixes, so you should be good starting prototyping from our master branch before the release.

Also feel free to come in our chat room (#smithay:matrix.org on Matrix, and it is bridged on #smithay on libera.chat if you prefer IRC, we're mostly active on European hours) if you have questions about Smithay and how to achieve what you want. We try to have detailed documentation, but it is geared towards classic compositors so I don't know how well adapted it would be to your project.

@DemiMarie
Copy link
Author

But, since this may be technically challenging and full of surprises, I want to setup some safety fuse. Lets say, try to get basic functionality (get it to display a simple application window, maybe also some kind of input) working in about a month. If we hit too many obstacles in this time, lets reconsider the choice. Does this plan make sense to you @DemiMarie ?

It does make sense, and I support it!

@DemiMarie
Copy link
Author

Update: The goal of getting an implementation done by now was not met. I will work on this this month.

@DemiMarie
Copy link
Author

Currently wlroots has been chosen, as Smithay lacks a scene graph with damage tracking support. Closing, though this will be reconsidered if Smithay gets the needed support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: gui-domain P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: task Type: task. An action item that is neither a bug nor an enhancement.
Projects
None yet
Development

No branches or pull requests

5 participants