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: add support for BGR888 image format #82

Merged
merged 2 commits into from
Jan 20, 2024
Merged

feat: add support for BGR888 image format #82

merged 2 commits into from
Jan 20, 2024

Conversation

vivienm
Copy link
Contributor

@vivienm vivienm commented Jan 8, 2024

Hello,

I use the dev version of Sway with NVidia proprietary drivers and I'm unable to take a screenshot using Wayshot.

2024-01-08T16:56:40.302884Z ERROR libwayshot: No suitable frame format found
Error: NoSupportedBufferFormat

Turns out that with my setup, the frame format is BGR888, which is not supported yet. So here is a small PR that seems to make it work for me.

BGR888 is a 3-byte-per-pixel frame format with no transparency, unlike other formats used so far. So in convert_inplace, there is no room in the buffer to turn it into an RGBA image. As a workaround, I use a DynamicImage type in subsequent operations to handle both RGB and RGBA formats.

I'm aware this may not be the best solution, please let me know if there is anything I can do to improve this code :)

@Shinyzenith
Copy link
Member

Hi I am on my phone so I can't give a full verdict yet. However the patch looks fine to me with just 1 concern.

Please correct me if my understanding is incorrect, but you're coercing the dynamic image into rgba8 - is it possible to let image crate pick the highest supported bit channel? Eg: 12 bit / 16 bit?

@Shinyzenith
Copy link
Member

@Decodetalkers any comments?

@vivienm
Copy link
Contributor Author

vivienm commented Jan 8, 2024

Please correct me if my understanding is incorrect, but you're coercing the dynamic image into rgba8 - is it possible to let image crate pick the highest supported bit channel? Eg: 12 bit / 16 bit?

You're right. I've pushed a new commit to avoid that. Most of this happens in the rotate_image_buffer function. I was not able to turn a DynamicImage into some kind of generic buffer, so I went the other way around so that each branch of the match is a DynamicImage.

@Decodetalkers
Copy link
Collaborator

@Decodetalkers any comments?

Emm. I think it should be ok

@Shinyzenith
Copy link
Member

Yep this seems fine to me.

@Shinyzenith
Copy link
Member

Shinyzenith commented Jan 9, 2024

I'll test the pr rq and go ahead with the merge once I get home.

@vivienm
Copy link
Contributor Author

vivienm commented Jan 9, 2024

I'll test the pr rq and go ahead with the merge once I get home.

TY :)

@Shinyzenith
Copy link
Member

Hey sorry for the late response! @Decodetalkers can you merge this PR? I'll mirror it to sourcehut when I'm home. Just drop a message in this thread once you merge.

@Decodetalkers
Copy link
Collaborator

Ok

@Decodetalkers Decodetalkers merged commit cb6bd68 into waycrate:main Jan 20, 2024
3 checks passed
@vivienm vivienm deleted the bgr888 branch January 20, 2024 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants