-
-
Notifications
You must be signed in to change notification settings - Fork 46.2k
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
Fix greyscale computation and inverted coords #8905
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add some doctests to prove that it will reach 255 and works correctly please
Good idea, thanks. done :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
""" | ||
return 0.114 * blue + 0.587 * green + 0.2126 * red | ||
return 0.114 * blue + 0.587 * green + 0.299 * red |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to Internet, it's either (0.2126R + 0.7152G + 0.0722B) or (0.299R + 0.587G + 0.114B)
Could you add a source for these numbers so that we can avoid mistakes like this in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, done :)
* Fix greyscale computation and inverted coords * Fix test * Add test cases * Add reference to the greyscaling formula --------- Co-authored-by: Colin Leroy-Mira <colin.leroy-mira@sigfox.com>
Describe your change:
Checklist:
The change:
Hello there,
I've used your algorithm to re-implement it in C for a side-project of mine, and noticed two little bugs in it:
the luminance calculation is incorrect - according to Internet, it's either (0.2126R + 0.7152G + 0.0722B) or (0.299R + 0.587G + 0.114B). Here, we had a mix of both (and the greyscale value could never reach 255.
the current_error setting used inverted coordinates.
Hope this helps!