Add HDR screenshot option - #1149
Conversation
|
|
||
| let mut out = vec![]; | ||
| { | ||
| if is_hdr10 { |
There was a problem hiding this comment.
This mixes two things:
- If the format is XRGB2101010, then we need to do the decoding done in the branch.
- If the color description is hdr10, then we need to set the cICP chunk.
These should be handled separately. So the if here should be guarded on buf.format == XRGB2101010 directly. Then the writing of the chunk should be guarded on whether we requested hdr10 from the compositor.
The same chunk check should be added to the else branch.
The else branch should also be guarded on the format being XRGB8888 with an else branch added below that aborts with a fatal error saying that we don't know how to transform the format into PNG.
| let mut writer = encoder.write_header().unwrap(); | ||
| // BT.2020 primaries, ST 2084 (PQ) transfer characteristics. | ||
| writer.write_chunk(chunk::cICP, &CICP_HDR10).unwrap(); | ||
| writer.write_image_data(&image_data).unwrap(); |
There was a problem hiding this comment.
Since this code is now going to be identical for both branches, the if/else should probably return the encoder and the image data, with this code moved out of the if/else.
|
LGTM otherwise. |
|
Thank you for reviewing, I'll try my best to fix everything when I have time |
|
Pushed a new commit which should hopefully be an improvement. Apologies if not, I'm still quite new to git and rust. |
2 participants
Most likely needs to be cleaned up a little but adds
jay screenshot --hdr10as an option in jay. Closes #1146.