Expand colour histogram past rgba8

[Stoppedpuma]

No description provided.

[awxkee]

@B-LechCode

Since we once did it for Rgba8 it was interesting to try it out to make that more abstract https://gist.github.com/awxkee/6bf5ba717365dcfeb32d82486b0dfc08.

I'm not sure if this is the best one, but this is what I got.

[B-LechCode]

@B-LechCode

Since we once did it for Rgba8 it was interesting to try it out to make that more abstract https://gist.github.com/awxkee/6bf5ba717365dcfeb32d82486b0dfc08.

I'm not sure if this is the best one, but this is what I got.

Thank you very much! I will have a look at it, as soon as I can! On the first glance it looks awesome!

[B-LechCode]

Hey @awxkee,

I had a look at it. Really good job, I will try to extend it with some additional things:

• Histogram bins according to the min/max value in the image, if the Input is not u8
• Transparent colors counting.

Thank you very much for providing me this awesome base implementation :)

[awxkee]

Hi, @B-LechCode

Glad to hear it helps!

Histogram bins according to the min/max value in the image, if the Input is not u8

This one might be tricky, because you're using DynamicImage, image crate permamently extends any image to 16 bit-depth on load. And floating point images are usually normalized on [0, 1] range, however there are some images that contains NaNs, +-Inf and you'll have to deal with it.

Transparent color counting is easy, except distinct transparent colors. For distinct transparent colors count you'll have to add 2^8 size to distinct_map and this is will a huge one.

[B-LechCode]

@awxkee, I reduced the code a little bit and added transparent color counting, matching the old implementation.
Is there a deeper reason why you iterated the rows instead of raw pixel chunks directly? Did I miss something there?

Please look at: 087fb74

Edit: I use float images which aren't normalized often, so that's a thing I'd like to consider here :)

[awxkee]

Is there a deeper reason why you iterated the rows instead of raw pixel chunks directly? Did I miss something there?

Yes, I believe complex computing won't be autovectorized properly and will hurt performance. OTH iterating by rows creates 2 very simple loops that will likely be exceptionally fast.

[awxkee]

I think if there is some reason to handle not normalized on [0..1] f32 images there is special case will be needed, because this is 2 completely different cases.
We'll need to scan f32 image first to check normalization, then if it is normalized use reduce specialization value * u16::MAX as ImageMagick does, if image is not normalized, then use passthrough specialization. What do you think about this one?

[B-LechCode]

Is there a deeper reason why you iterated the rows instead of raw pixel chunks directly? Did I miss something there?

Yes, I believe complex computing won't be autovectorized properly and will hurt performance. OTH iterating by rows creates 2 very simple loops that will likely be exceptionally fast.

Ah, I see! That's a good point - I will make it work firstly and then I should have a look at godbolt to optimize the code. Since this is really performance critical here that's what I think we should do. Did you compare that initially?

I think if there is some reason to handle not normalized on [0..1] f32 images there is special case will be needed, because this is 2 completely different cases. We'll need to scan f32 image first to check normalization, then if it is normalized use reduce specialization value * u16::MAX as ImageMagick does, if image is not normalized, then use passthrough specialization. What do you think about this one?

Ahh, I see. Do you think adding one multiplication and one addition would hurt more than using branching inside the loop?
Another Idea could be, to add a min/max specialization to the Reducers/Converters for float32.

[awxkee]

Did you compare that initially?

I think, yes, but I'm already not truly sure :)

Ahh, I see. Do you think adding one multiplication and one addition would hurt more than using branching inside the loop?
Another Idea could be, to add a min/max specialization to the Reducers/Converters for float32.

Now, Rust always ensures that value fit to u16, because it guarantee of language that if you're using u16, there is u16, not i32 under the hood as C/C++ does.

impl ArgumentReducer<f32, u16> for ArgumentReducerFloat32 {
    #[inline(always)]
    fn reduce(&self, a: f32) -> u16 {
        (a * (u16::MAX as f32)) as u16
    }
}

However for passthrough non normalized value you'll probably need to clamp to u16::MAX. If you don't want to do normalization of non normalized f32 values into a range [0,65535] you'll also probably should find the maximum value for image also. There is also might be an issue many values are higher than 65535. It may also have sense if image is not normalized, to normalize them first.

[awxkee]

Do you think adding one multiplication and one addition would hurt more than using branching inside the loop?

I do not think this is important here, I believe handling values correct here is more important than 2-5% performance drop. In common, if loop will be vectorized properly, there is no real branching, vector could be just tested againts the values, it should be more or less the same.

[awxkee]

Here is some normalization test, if that makes sense

/// find if image has too many values not in the range [0...1]
fn deviation_test(image: &[f32]) -> bool {
    let mean = image.iter().sum::<f32>() / image.len() as f32;
    let variance = image
        .iter()
        .map(|&v| {
            let dm = v - mean;
            dm * dm
        })
        .sum::<f32>()
        / image.len() as f32;
    let stddev_scale = 1. / variance.sqrt();
    let z_score_outliers = image.iter().filter(|&&v| ((v - mean) * stddev_scale).abs() > 3.0).count();
    let outliers_fraction = z_score_outliers as f32 / image.len() as f32;
    // if more than 3% is outliers then image is not normalized
    outliers_fraction > 0.03
}

And this is not ideal .

[B-LechCode]

Here is some normalization test, if that makes sense

/// find if image has too many values not in the range [0...1]
fn deviation_test(image: &[f32]) -> bool {
let mean = image.iter().sum::() / image.len() as f32;
let variance = image
.iter()
.map(|&v| {
let dm = v - mean;
dm * dm
})
.sum::()
/ image.len() as f32;
let stddev_scale = 1. / variance.sqrt();
let z_score_outliers = image.iter().filter(|&&v| ((v - mean) * stddev_scale).abs() > 3.0).count();
let outliers_fraction = z_score_outliers as f32 / image.len() as f32;
// if more than 3% is outliers then image is not normalized
outliers_fraction > 0.03
}

And this is not ideal .

Thank you! Why shouldn't we treat images with at least one single value above 1.0 as not normalized, is there something I miss here?

[awxkee]

Thresholding by 1.0 will work if you don’t expect raw HDR content in your image.
However, if there is possibility of receiving raw HDR content this won’t work because HDR values may exceed 1.0.

[B-LechCode]

Thresholding by 1.0 will work if you don’t expect raw HDR content in your image. However, if there is possibility of receiving raw HDR content this won’t work because HDR values may exceed 1.0.

I understand, but if the values are > 1.0 - why should I clip them to 1.0 in the Histogram instead of plotting the Histogram till max value?

[awxkee]

Yes, this is fine, but at first you have to determine that image is still normalized, even if some values higher than 1.0, that’s my point.

[awxkee]

I'd also like to note that such HDR images may appear, but not necessarily will. I don't know what images users are viewing in the app, so handling case with HDR images may not be worth it at all.

[Stoppedpuma]

I'm going to say that handling HDR is a must. Excluding these is not an option in my books if HDR is to be supported (which is pretty high on both mine and woelpers list) as this would lead to a relatively half baked implementation.

[B-LechCode]

@awxkee I think it wouldn't be a problem to show the histogram between [min max] even though the display algorithms will clip them to [0,1]. In my opinion the histogram should be statistics on raw input data, and 65k bins should be sufficient?
@Stoppedpuma we will find a solution, which should fit that use case ;)

[awxkee]

HDR image usually does not have enough headroom to create a human understandable histogram within the range [min, max].
HDR images often contain bright regions where values exceed 1.0, reaching up to ~10.0–20.0, and most of regions still in [0,1].
For example, if you're making a city picture, the sky will likely contain HDR content, which sensor will capture as [0,~10], and the city ground region that will remain SDR content in the range [0,1].
If the image hasn't been pre-processed, these values can remain in the 10.0–20.0 range because most screens have a headroom cap somewhere around 2.0.
As a result, most of the HDR histograms clipped (without multiplication) to u16 [min,max] will have 40–80% of their values =0, with the remainder distributed between [1.0,~20].

In my opinion the histogram should be statistics on raw input data, and 65k bins should be sufficient?

There is also issue with multiplication because bright HDR regions after multiplication will always be clipped to u16::MAX.

My initial implementation handled those cases more easily because it treated all images as normalized and ignored the headroom.

I think solution here depends on what level of correctness you want. Clipping HDR histogram to [min, max] is an easy one, however, have some drawbacks.

[Stoppedpuma]

I believe that regardless we should still show the absolute values for those who desire it as that's what's actually correct. What about a middle point where it can be changed by the user? Currently the histogram can be zoomed in by dragging a selection area and it will zoom into that area, double left click and it resets the zoom. In this case, we could bind a double right click to normalise the histogram and show a notification to inform the user that it has normalised the histogram for this image, double right click again to restore the original and show another notification informing the user of the reset.

[B-LechCode]

@Stoppedpuma please give me some days to implement what I thought about. Then let's do another iteration whether you think it's ok or not :)
I'd like to use raw information - in that case the mentioned values of 10 or 20 für f32 images would be present in the histogram.

[awxkee]

@B-LechCode
This is a little unrelated to histograms.
As there were already issues how to handle HDR images, I ported mine HDR tone mapper with basic CMS to perform conversion Wide gamut -> SDR gamut.

https://github.com/awxkee/gainforge
This is should make relatively easy to perform basic, correct HDR handling on any display, at least it will work.
Since gain maps are becoming more popular, I might eventually write the math for Apple/Adobe gain maps, but others like AVIF, JXL, and UltraHDR(?) seem overly complicated at the moment.

[Stoppedpuma]

@B-LechCode could I ask you to look at #647 since you're currently working on the histogram? This seems to have broke some where along the way and doesn't update from filter changes anymore. If you're busy or don't feel like it, no worries! This can always be handled another time or by someone else.