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

OnDamage screencopy issues #338

Closed
ids1024 opened this issue Mar 7, 2024 · 2 comments · Fixed by #358
Closed

OnDamage screencopy issues #338

ids1024 opened this issue Mar 7, 2024 · 2 comments · Fixed by #358

Comments

@ids1024
Copy link
Member

ids1024 commented Mar 7, 2024

I've noticed issues with using OnDamage for screencopy. Specifically it seems:

  • OnDamage screencopy doesn't work for window capture.
  • Capturing a workspace that isn't the current workspace with OnDamage only gets 1fps. While without OnDamage, it ends up sending frame callbacks and rendering at normal speed.
  • Less importantly, but should OnDamage determine if there's been damage since the last capture with the screencopy session, rather than getting the next damage from when the capture request is sent?
    • Keeping track damage from the last screencopy would also be necessary to handle damage/age with screencopy. It seems the age is currently ignored.
@Drakulix
Copy link
Member

Drakulix commented Mar 7, 2024

The original reference protocol (ext-screencopy https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/124) has dropped OnDamage recently in favour of simply returning an empty capture (no damage events). This means the client drives the frame-rate of the capture by timing it's capture requests.

Adapting a similar approach will ease transition once the protocol is merged and simplify the implementation in cosmic-comp. Perhaps we should introduce cosmic-screencopy-v2 and drop OnDamage? Then we can also always trigger frame-callbacks, when we get a capture request.

Similarly age was never an explicit concept in ext-screencopy, but rather something the compositor can implicitly track, since the capture buffers can be re-used. Additionally ext-screencopy now explicitly tells the compositor the damaged regions, that need to be re-drawn, so we could simply drop the as well, if we update cosmic-screencopy.

EDIT: Thinking about the ease of transitioning to a v2-protocol right now, the major users of both OnDamage and age are probably the xdg-desktop-portal-cosmic and cosmic-workspaces. All other users shouldn't bother, since they only do screenshots. So if you feel like this would be an improvement for either of them, I can just start making these changes right away.

@ids1024
Copy link
Member Author

ids1024 commented Mar 7, 2024

Some variation on this seems fairly important for cosmic-workspaces... if we want captures to update at more than 1 fps. Triggering renders of each window and workspace at 60fps isn't very efficient.

Avoiding an explicit age like ext-screencopy currently does seems reasonable.

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 a pull request may close this issue.

2 participants