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

Add PngDecoder::gamma_value method #2007

Merged
merged 2 commits into from
Sep 28, 2023
Merged

Conversation

fintelia
Copy link
Contributor

@fintelia fintelia commented Sep 19, 2023

The Result return type was chosen so that the method can return I/O errors once the PNG decoder no longer reads all metadata in the new() method. And an f64 was used so that all gamma values can be exactly represented (PNG encodes the value as a fixed precision decimal stored as a u32 which doesn't directly map to a f32).

cc: @anforowicz

Fixes #2001

/// > the values given above as if they had appeared in gAMA and cHRM chunks.
pub fn gamma_value(&self) -> ImageResult<Option<f64>> {
let gamma = if self.reader.info().srgb.is_some() {
Some(0.45455)

Choose a reason for hiding this comment

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

I wonder why you have chosen to manually consider srgb and gama_chunk, instead of using Info::source_gamma, which AFAICT takes both srgb (here) and gama_chunk (here) into account. FWIW, png::srgb::substitute_gamma is also 45455

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point point... I didn't notice the png crate already implemented that logic

@anforowicz
Copy link

Thanks - LGTM!

@fintelia fintelia merged commit 90cf937 into image-rs:master Sep 28, 2023
35 checks passed
@fintelia fintelia deleted the png-gamma branch September 28, 2023 16:08
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.

Can image::codecs::png::PngDecoder somehow preserve png::Info::source_gamma?
2 participants