Skip to content

kms/surface: Don't send screencopysuccess() until sync point is reached #1595

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

Merged
merged 5 commits into from
Aug 21, 2025

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Aug 19, 2025

Uses Smithay/smithay#1805 to get a sync point from the blit, and then waits on it in a calloop source without blocking the thread.

Like #1580, this should fix some screen capture synchronization issues, but without the overhead of a glFinish.

Copy link
Member

@Drakulix Drakulix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@ids1024
Copy link
Member Author

ids1024 commented Aug 20, 2025

Actually, PendingImageCopyData can also be used in the return value of render_session, then we can make sure this synchronization is handled the same way for every type of capture (without adding duplication).

@ids1024
Copy link
Member Author

ids1024 commented Aug 20, 2025

I think this should be good to merge pending Smithay/smithay#1805.

Copy link
Member

@Drakulix Drakulix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

There is always optimizations to be had here regarding deferring the copy, but those are difficult, when you have to deal with GLs global state.

Adding anything else to this tuple is awkward; defining a simple struct
makes this cleaner.

This also adds a `sync` property, which will come in handy later.
Containing simply the same-named argument that was passed to
`submit_buffer`.
A little annoying to add a `loop_handle` argument to `render_output`,
but generally straightforward.
Shouldn't really matter, but no need to check the fence after this.

It would be good if shm screencopy didn't block here...
@ids1024
Copy link
Member Author

ids1024 commented Aug 21, 2025

There is always optimizations to be had here regarding deferring the copy, but those are difficult, when you have to deal with GLs global state.

Yeah, that part could be easier with Vulkan, though synchronization is known to be one of the more challenging aspects of Vulkan, so it has it's own difficulties. Probably should be possible to improve this on GL too, but that can be done later.

Rebased and updated for merged smithay changes , so this should be good now.

@ids1024 ids1024 marked this pull request as ready for review August 21, 2025 14:52
@Drakulix Drakulix merged commit f2813f0 into master Aug 21, 2025
6 checks passed
@Drakulix Drakulix deleted the sync_noble branch August 21, 2025 15:09
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