Faster Rgb to Hsl and Hsv conversions

[sidred]

Faster conversion from Rgb to Hsl and Rgb to Hsv

[sidred]

I've added the before and after benchmark and tests code to the benches folder. Let me know if they should be removed.

Rgb to Hsl benchmarks

test rgb_to_hsl_original_f32  ... bench:          14 ns/iter (+/- 2)
test rgb_to_hsl_optimized_f32 ... bench:           9 ns/iter (+/- 1)

test rgb_to_hsl_original_f64  ... bench:          14 ns/iter (+/- 5)
test rgb_to_hsl_optimized_f64 ... bench:           9 ns/iter (+/- 1)

Rgb to Hsv benchmarks

test rgb_to_hsv_original_f32  ... bench:          12 ns/iter (+/- 0)
test rgb_to_hsv_optimized_f32 ... bench:           9 ns/iter (+/- 0)

test rgb_to_hsv_original_f64  ... bench:          12 ns/iter (+/- 1)
test rgb_to_hsv_optimized_f64 ... bench:           9 ns/iter (+/- 1)

[Ogeon]

Cool! And nice speedup. Are these new algorithms hand crafted or is there a source for them? (just curious) The benchmarks can stay, but how come they aren't using the rgb.into_hsl() and rgb.into_hsv()? Another inlining issue?

[sidred]

Are these new algorithms hand crafted or is there a source for them? (just curious)

It started with me noticing that there are unnecessary comparisons for finding both max and min and trying to optimize them.I also a stackoverflow answer that basically said the same, you can find max and min in 3 comparisons instead of 4.

The benchmarks can stay, but how come they aren't using the rgb.into_hsl() and rgb.into_hsv()?

I didn't check with the into_hsl and hsv. I guess because running the benchmarks will then no longer make sense once this is merged.

[Ogeon]

Yea, it was pretty naive before, since the first goal was to just get the basics down.

I didn't check with the into_hsl and hsv. I guess because running the benchmarks will then no longer make sense once this is merged.

What I meant was to run the into_* as the new algorithm. Thinking more about it makes me realize that these benchmarks will make little sense in the future, unless they are kept as some kind of regression tests. A proper performance test would compare to the previous release, and not just a previous algorithm, so this doesn't really work for that either... May be better to just remove them, after all...

Thank you for including them, anyway. It's nice to see some numbers.

[sidred]

Just checked. The trait version is equally fast.

test rgb_to_hsv_optimized_f32 ... bench:           8 ns/iter (+/- 0)
test rgb_to_hsv_optimized_f64 ... bench:          10 ns/iter (+/- 0)
test rgb_to_hsv_original_f32  ... bench:          12 ns/iter (+/- 0)
test rgb_to_hsv_original_f64  ... bench:          12 ns/iter (+/- 1)
test rgb_to_hsv_trait_f32     ... bench:           8 ns/iter (+/- 0)
test rgb_to_hsv_trait_f64     ... bench:          10 ns/iter (+/- 0)

test rgb_to_hsl_optimized_f32 ... bench:           9 ns/iter (+/- 0)
test rgb_to_hsl_optimized_f64 ... bench:           9 ns/iter (+/- 0)
test rgb_to_hsl_original_f32  ... bench:          13 ns/iter (+/- 4)
test rgb_to_hsl_original_f64  ... bench:          13 ns/iter (+/- 4)
test rgb_to_hsl_trait_f32     ... bench:           9 ns/iter (+/- 0)
test rgb_to_hsl_trait_f64     ... bench:           9 ns/iter (+/- 1)

[Ogeon]

Alright, seems like inlining works as expected in this case.

[sidred]

Removed the benches. Should be good to go.

[Ogeon]

The RgbHue hiccups are probably related to the nested calls, while this is quite shallow. Anyway, I'll inspect this a bit deeper in a moment, but I think the tests has given this an OK stamp, already.

[Ogeon]

This looks great! Can't find anything to fix here 👍 Thank you!

@homu r+

[homu]

📌 Commit cdb4a3f has been approved by Ogeon

[homu]

⚡ Test exempted - status