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

Don't create new screencopy session or wl_buffer each frame #30

Merged
merged 5 commits into from
Mar 14, 2024

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Mar 13, 2024

This shouldn't change behavior, except that WAYLAND_DEBUG=1 will show a lot fewer Wayland requests while capturing the screen. And perhaps some performance benefit, but I haven't specifically verified that.

The changes to how screencopy is handled should help for moving to the v2 screencopy protocol (pop-os/cosmic-protocols#24).

ids1024 added 5 commits March 13, 2024 13:40
`Session` type now contains a reference to the
`ZCosmicScreencopySessionV1`, and is reference counted internally.

`SessionData` now uses a weak reference to `Session`, and the wayland
object is destroyed when the strong references are all dropped.

This should help for re-using the screencopy session, and moving to v2
of cosmic screencopy. Later this logic should be combined with the
screencopy code in cosmic-workspaces to have a higher-level abstraction
that can be used in both.
It still should also avoid creating a new `wl_buffer` each frame.
Not compatible with re-use of of screencopy session.
Now we create a `wl_buffer` in the pipewire `add_buffer` handler, and
store it in the `pw_buffer` user data. And destroy the buffer in the
`remove_buffer` handler.

`pipewire-rs` still needs more safe APIs for `pw_buffer`, but this is
alright.
@ids1024 ids1024 requested a review from a team March 13, 2024 23:10
@ids1024 ids1024 merged commit ff58e57 into master Mar 14, 2024
5 checks passed
@ids1024 ids1024 deleted the screencopy-session branch March 14, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants