-
Notifications
You must be signed in to change notification settings - Fork 306
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
Transparent is not white #142
base: main
Are you sure you want to change the base?
Conversation
@mourner any chance to get this merged? |
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.
Overall the change looks great! Just let me understand the color logic better and I think it'll be good to merge.
index.js
Outdated
|
||
const rBackground = 48 + 159 * (k % 2); | ||
const gBackground = 48 + 159 * (Math.floor(k / 1.616) % 2); | ||
const bBackground = 48 + 159 * (Math.floor(k / 2.612) % 2); |
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.
Hmm, left a comment here but it disappeared somewhere... Can you describe how these specific values / coefficients were picked, and maybe show a screenshot of how this looks?
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.
I think one of the tests shows that - for me it looked like how the color picker looks, and I guess the coefficients came from there...
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.
This is what the background looks like:
There are a million choices for what the background image could look like. Here are my considerations when creating this background image:
- The background should not be a uniform color.
- The background should not contain large areas of uniform color.
- The background should have a large amount of perceptual variability.
- The background should be deterministic.
- The background color should be easy to compute.
- The background color should be a function of pixel index only (not x,y), because that is what was already available in
colorDelta()
. - The background should not contain "common colors" (particularly white and black).
- The background should not contain lines.
- The background image should not contain anything we would expect to appear in test images.
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.
The 48
and 159
were picked to give large color variability, while avoiding white, black, and fully saturated colors.
The 1.616
and 2.612
were picked to keep the pattern from degenerating into lines at certain integer image widths. I've added some digits to make sure theses coefficients preserve their irrationality for very large images :)
Fixes #26, by blending transparent pixels with a color that varies with position (instead of always blending transparent pixels with white).