PR #1279 tried to throttle `send_toplevel_to_client`, which was rejected. This takes (what I think is) a better approach, and instead makes some optimizations. We delay grabbing the mutex until it's actually needed, and we also checks to see if the state actually needs to be resent. This resolves some lock contention, and also avoids having to generate the state in the first place. Since `wayland-rs` uses a single mutex to lock the entire state, this both massively reduces lock contention and avoids making (apparently quite a lot of) unnecessary computation.
I decided to give this one to AI since I'm a bit busy right now (although I am still manually writing the commit messages and descriptions). I think it generally did a pretty good job, although I did have to make a few manual tweaks. While I don't have any empirical data, cosmic-comp idle CPU has gone down from ~3% all-core to 0-0.5%, even stress testing it with far more windows open. I can't say it *liked* opening 30-some windows in 2 seconds, but after it finished opening them all, it was still buttery smooth. Overall very happy with the results.
Assisted-by: Claude:claude-4-opus
We want to merge this when updating Smithay to have
https://github.com/Smithay/smithay/pull/1961, so that we don't panic the
the output/toplevel already has no strong references when the source is
created.
This doesn't change much, since the Smithay implementation is based on
the `cosmic-comp` version, but made more generic. We provide our own
implementation for our workspace capture protocol, but otherwise Smithay
handles the boilerplate now.
This should not cause any change in behavior.
If we want to use the `org.freedesktop.a11y.KeyboardMonitor` protocol on
Pop!_OS, there is no need to support the Cosmic-specific protocol that
requires an `at-spi2-core` patch.
The `id` is defined to be sent only once, on creation of the handle or
later. And only for workspaces that are "likely to be stable across
multiple sessions".
Set we add an `id` initially for pinned workspaces, and add one when the
workspace is pinned.
The `id` is not supposed to be human readable, so we just use a random
value.
It seems previously, workspace migration due to output add could result in
a workspace being removed from a group, but not added to the new group
for existing clients, because the workspace group creation didn't happen
until `done`. And `send_workspace_to_client` didn't send
`workspace_enter` except when a workspace instance is newly created.
That logic worked with the old protocol, but now a workspace can be
moved to a different group.
Seems to fix the issue with workspaces disappearing from the workspaces
view in https://github.com/pop-os/cosmic-comp/issues/1470. I don't seem
to be able to reproduce the panel auto-hide issue, but have seen it in
the past. So it may or may not be prevented from happening now.
Previously, `Frame` was stored in KMS frame udata, but in some cases the
udata was dropped without a capture happening, and `Frame` did not
implement `Drop`, so `fail` was never sent.
Instead, rename `DropableFrame` to `Frame` and `Frame` to `FrameRef`, so
we can have a single instance of `Frame`, that will send `fail` on drop.
This guarantees either `.success` or `.fail` are send, as long as its
not leaked.
This seems to fix https://github.com/pop-os/cosmic-comp/issues/1305.
xdg-desktop-portal-cosmic prints an error, buy retries (as it should for
an `Unknown` error; though maybe there should be a retry limit) and the
session continues working.
(Not sure if it should be sending `failed`, or queing it with the next
frame so it can send `success` to the client, but this works and is
desirable as a failsafe anyway.)
`Session` and `CursorSession` are similiarly updated.
`.fail()`, `.success()`, and `.stop()` now consume
`Frame`/`Session`/`CursorSession`. So to stop a session, it is now
necessary to call `.remove_session()`, but then simply dropping with
send `.stop()`.
Factoring out some `Request::Capture` handling into a `capture_frame`
function seems to clean up error handling and such a bit.
Adds support for cosmic-workspace-v2 pin, unpin, move_after, and
move_before requests.
Both features need some work with workspaces span displays mode, so that
will need more fixes later.
We also want to generate a unique id for pinned workspaces to send in
the ext-workspace-v1 protocol. But that isn't a strict requirement for
anything. So I haven't yet fully implemented that. We'll also want to
persist other things, like workspace naming when that's added.
Overall, though, with separate workspaces per display, this is working
pretty well.
This is slightly simpler, if there's not some reason I'm missing to do
this as it was previously done. And in particular provides a cleaner
API (if we wanted to move this to Smithay; perhaps without the Cosmic
extension).
But it also should be more correct. Presumably if a client (unusually)
had multiple components with their own `ext_workspace_manager_v1`
instance, they should have their own queues, and
`ext_workspace_manager_v1::commit` should be independent.
Inevitably, there's a racy element to multiple components trying to
update the workspace state like this, but it should behave the same as
two clients with separate connections.
(This is different from `CompositorClientState`, since the commit queue
there is fundamentally tied to the client, and different components with
their own compositor instance way have related surfaces.)
It seems at least with Rust 1.82 (the version in `rust-toolchain.toml`)
these bounds are inferred from the `D: WorkspaceHandler` bound, so they
can be specified only there.
Clients have been updated to use ext-workspace-v1 and
cosmic-workspace-v2.
Removing this should save some duplication of having to update both
cosmic workspace v1 and ext workspace code, and should help with things
like using the same workspace handle when a workspace moves to a
different group (which is something that was changed before the final
version of the ext-workspace-v1 protocol.)
This means a change to an ext capability will not send a redundant
cosmic capability event, and vice versa.
This will be more important when cosmic-specific states are added. Since
those may change often.
This new protocol extends `ext-workspace-v1` with the same additional
functionality `cosmic-workspace-v1` provided. Toplevel info and toplevel
management are also updated to use ext handles, and there's an image
source for ext workspaces.
For now, the old protocol is still supported.
The protocol states that these should always be sent, but this was
not initially sending bitflags if they were empty. That works, but isn't
what the protocol states.
Not wrapping the bitflag fields in options works well for `Workspace`,
but not for `WorkspaceDataInner`.
