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

reduce how many colors are possible by limiting channels to what an u8 can hold #4357

Closed
wants to merge 1 commit into from

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented Mar 28, 2022

Objective

Solution

  • Limit conversion of colors to wgpu colors to what an u8 can hold instead of directly as a f64

This is not an ideal fix as it reduces what is possible to do, but having color channels in u8 is what is used in a lot of places

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Mar 28, 2022
@mockersf mockersf force-pushed the reduce-color-space branch from 555f2cb to c03d476 Compare March 28, 2022 22:09
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior and removed S-Needs-Triage This issue needs to be labelled labels Mar 28, 2022
@alice-i-cecile
Copy link
Member

Right, makes sense as a fix. I don't love it though; maybe the real solution is just to finally migrate to a proper color crate.

@mockersf
Copy link
Member Author

not sure a proper color crate would change that, we still go through "one u8 per channel" when sending the color to gpu

@robtfm
Copy link
Contributor

robtfm commented Mar 29, 2022

i disagree with this - users can send color data to the gpu in other ways too, which would no longer match if we only constrain in conversion to wgpu::Color.

if we want to enforce u8 space we should use u8s directly within the Color struct.

A less restrictive approach would be to constrain the f32s in the constructors. then we could add something like a rgb_raw constructor that doesn't constrain the f32s. (except this would only work for linear formats)

@mockersf
Copy link
Member Author

I also disagree with this 🙂
I'm not even sure the original bug should be "fixed" - maybe documentation would be enough

@mockersf
Copy link
Member Author

this is not meant to be merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ClearColor and Sprite same value yields different color
3 participants