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

Expand stetch_contrast() to color images also using u8 #670

Merged
merged 1 commit into from
Aug 18, 2024

Conversation

ripytide
Copy link
Member

Ideally we'd want to use the Enlargeable trait currently private inside image to make this work for all types rather than just u8, but this is still an improvement that should be good enough for image-rs/image#2238.

@ripytide
Copy link
Member Author

It seems we need to bump the minimal rust version now since one of our dependencies, I think bitstream-io now requires v1.79

@theotherphil
Copy link
Contributor

Thanks! What impact does this have on function run time for the greyscale case?

@ripytide
Copy link
Member Author

ripytide commented Aug 17, 2024

Looks like no impact:
Before PR:

test contrast::benches::bench_stretch_contrast                                        ... bench:         660.08 ns/iter (+/- 2.24)
test contrast::benches::bench_stretch_contrast_mut                                    ... bench:           0.25 ns/iter (+/- 0.00)

After PR:

test contrast::benches::bench_stretch_contrast                                        ... bench:         658.86 ns/iter (+/- 7.36)
test contrast::benches::bench_stretch_contrast_mut                                    ... bench:           0.25 ns/iter (+/- 0.00)

@theotherphil
Copy link
Contributor

Thanks!

(Aside: it looks like the _mut variant of that benchmark is optimising out the function and so not actually running .)

@theotherphil theotherphil merged commit 241bf5c into image-rs:master Aug 18, 2024
15 checks passed
@ripytide ripytide deleted the color-stretch-contrast branch August 18, 2024 10:38
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.

2 participants