-
Notifications
You must be signed in to change notification settings - Fork 90
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
rp2040 zero rainbow using palette #82
base: main
Are you sure you want to change the base?
Conversation
// 0 is off and 1 is max brightness | ||
let brightness = 0.05 as f64; | ||
(brightness * u8::MAX as f64) as u8 |
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 previous code uses 32 as brightness which is 12.5% not 5%.
Why not leaving the value 32
and only adding a comment like /* 12.5% */
(wheel_pos * 3, 255 - (wheel_pos * 3), 0).into() | ||
ws.write(brightness( | ||
once({ | ||
let hsl = Hsl::<Rgb, _>::new(hue as f64, 1.0, 0.5); |
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.
hue
is already an f64 from its declaration. No need to cast it here.
}), | ||
{ | ||
// 0 is off and 1 is max brightness | ||
let brightness = 0.05 as f64; |
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.
let brightness = 0.05 as f64; | |
let brightness = 0.05f64; |
}, | ||
)) | ||
.unwrap(); | ||
hue += 360.0 / 6.0 / u8::MAX as f64; |
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.
All these values are constants. What’s the intent in that math?
The current code turns the wheel by (1/255)rad
(~1.41°) per iteration.
We don’t need this to match strictly. This should be enough:
hue += 360.0 / 6.0 / u8::MAX as f64; | |
hue += 1.0; // advance by 1° |
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 intent is that there are 256 steps of changes in a value of either r, g, b in every 1/6 of 360 degrees, so adding the hue by 360/6/256 makes sure that there will actually be a change in pixels every iteration of the loop. This can be changed to 1 if that's what's prefered by this project.
Rather than do math with rgb numbers directly, it's much simpler to just use HSL and a color conversion library to convert HSL into RGB. Imo this makes the code simpler and easier to understand.