Skip to content

rp2040 zero rainbow using palette#82

Open
ChocolateLoverRaj wants to merge 1 commit intorp-rs:mainfrom
ChocolateLoverRaj:palette
Open

rp2040 zero rainbow using palette#82
ChocolateLoverRaj wants to merge 1 commit intorp-rs:mainfrom
ChocolateLoverRaj:palette

Conversation

@ChocolateLoverRaj
Copy link

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.

Comment on lines +87 to +89
// 0 is off and 1 is max brightness
let brightness = 0.05 as f64;
(brightness * u8::MAX as f64) as u8
Copy link
Member

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);
Copy link
Member

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let brightness = 0.05 as f64;
let brightness = 0.05f64;

},
))
.unwrap();
hue += 360.0 / 6.0 / u8::MAX as f64;
Copy link
Member

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:

Suggested change
hue += 360.0 / 6.0 / u8::MAX as f64;
hue += 1.0; // advance by 1°

Copy link
Author

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.

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