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

feat: try xdg-desktop-portal before grim #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jokeyrhyme
Copy link

@jokeyrhyme jokeyrhyme commented Aug 11, 2023

  • implements some of what is discussed over in is working without grim in scope for this project? #21
  • we try the xdg-desktop-portal Screenshot approach first, and then fallback to grim as before
  • this PR adopts the async-std Futures runtime because it is the default used by ashpd, but we could switch this to tokio if this was more desirable
  • error handling is not elegant at all, the bare minimum was done here in order to work across multiple Result values
  • I've tested this under sway with xdg-desktop-portal-wlr (including commenting out the grim fallback) and can confirm that it works

@Kirottu
Copy link
Owner

Kirottu commented Aug 13, 2023

You seem to have missed the fact that right now Watershot uses grim to both take a big screenshot of all monitors (to get cross monitor screenshots easily) and a screenshot for each monitor specifically in Monitor::new to make sure the screenshot is as sharp as it should be for each monitor in HiDPI situations.

@jokeyrhyme
Copy link
Author

Thanks for pointing that out

I'll have to double-check some of the portal implementations and the documentation, but I think xdg-desktop-portal-wlr allows the user to choose which output at the time of the Screenshot request, and I'm not sure the request payload can include a suggestion of which Screenshot we want

It might not be practical to use the portal for capturing all outputs

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