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

Thread panics on EXR containing NAN #2275

Open
gyk opened this issue Jul 1, 2024 · 8 comments
Open

Thread panics on EXR containing NAN #2275

gyk opened this issue Jul 1, 2024 · 8 comments

Comments

@gyk
Copy link

gyk commented Jul 1, 2024

Some EXR files might contain NaNs, e.g., AllHalfValues.exr. Calling to_rgba8 on such images causes a panic:

thread 'main' panicked at image-0.25.1\src\color.rs:435:30:
called `Option::unwrap()` on a `None` value
@fintelia
Copy link
Contributor

fintelia commented Jul 2, 2024

Shorter repro:

let img = Rgba32FImage::from_raw(1, 1, vec![1.0 / 0.]).expect("create nan image");
DynamicImage::ImageRgba32F(img).to_rgba8();

@FlareFlo
Copy link
Contributor

According to my research, there is no defined operation for how to handle NaN when converting to another format. I can open a PR against my branch if the below behavior is agreed upon.
Firefox render of the linked sample image
image
PNG conversion with clamp applied, from the fixed branch
out

@awxkee
Copy link
Contributor

awxkee commented Nov 16, 2024

Num traits does not support Positive infinites, Negative infinites also, only subnormal numbers are supported.
Usually most hardware flushes NaNs towards negative infinity ( zero for unsigned, max possible negative value for signed ).

@FlareFlo
Copy link
Contributor

Num traits does not support Positive infinites, Negative infinites also, only subnormal numbers are supported.
Usually most hardware flushes NaNs towards negative infinity ( zero for unsigned, max possible negative value for signed ).

Well, in this case the image parses just fine when filtering out NaNs, all other funny values appear to be handled (as this image contains every possible bitrepr)

@awxkee
Copy link
Contributor

awxkee commented Nov 16, 2024

Not really, here two immediate fails

    let img1 = Rgba32FImage::from_raw(1, 1, vec![f32::INFINITY]).expect("create +inf image");
    DynamicImage::ImageRgba32F(img1).to_rgba8();

    let img2 = Rgba32FImage::from_raw(1, 1, vec![f32::NEG_INFINITY]).expect("create -inf image");
    DynamicImage::ImageRgba32F(img2).to_rgba8();

@FlareFlo
Copy link
Contributor

FlareFlo commented Nov 16, 2024

    let img2 = Rgba32FImage::from_raw(1, 1, vec![f32::NEG_INFINITY]).expect("create -inf image");
    DynamicImage::ImageRgba32F(img2).to_rgba8();

I believe this is a tangential issue, as this doesnt work either:

    let img2 = Rgba32FImage::from_raw(1, 1, vec![0.0]).expect("create any image");
    DynamicImage::ImageRgba32F(img2).to_rgba8();

(fails on the expect)
see below comment

@FlareFlo
Copy link
Contributor

FlareFlo commented Nov 16, 2024

Specifically, image_buffer_len yields Some(4) with input 1,1.
The shorter repro was simply bugged as it did not take the 4 color channels into account.
image

@FlareFlo
Copy link
Contributor

    let img2 = Rgba32FImage::from_raw(1,1, vec![f32::NEG_INFINITY, f32::INFINITY, f32::NAN, f32::MAX]).expect("create funny image");
    dbg!(DynamicImage::ImageRgba32F(img2).to_rgba8());

runs perfectly fine

[src/main.rs:9:5] DynamicImage::ImageRgba32F(img2).to_rgba8() = ImageBuffer {
    width: 1,
    height: 1,
    _phantom: PhantomData<image::color::Rgba<u8>>,
    data: [
        0,
        255,
        1,
        255,
    ],
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants