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

Default color types to opaque white #85

Closed
wants to merge 1 commit into from

Conversation

waywardmonkeys
Copy link
Collaborator

We choose an opaque color because otherwise the OpaqueColor would have a substantially different default value than the other color types.

White is the same as the default color within bevy_color.

The default color is used by the Brush in Parley and likely other places.

Copy link
Contributor

@raphlinus raphlinus left a comment

Choose a reason for hiding this comment

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

I'm ok with this, though I believe the canonical default is mediumorchid.

color/src/dynamic.rs Outdated Show resolved Hide resolved
@waywardmonkeys
Copy link
Collaborator Author

I'm going to keep this approval in my pocket for now and see if we can avoid needing it! :)

@DJMcNab
Copy link
Member

DJMcNab commented Dec 10, 2024

In our current world without rust-lang/rust#132162 on stable, I think having this is fine. I'd personally still advocate for a vile default, so that users can notice not overriding it, but not enough to block the "safe" white.

Copy link
Member

@tomcur tomcur left a comment

Choose a reason for hiding this comment

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

Doing a vile default is a little bit tricky, and may require specifying the color on the color space: doing a conversion at runtime from some vile color in a specific color space is quite a bit of operations for a color that is likely to be overwritten, and it may fall outside that color space's natural gamut, making it a bit messy.

color/src/dynamic.rs Outdated Show resolved Hide resolved
@waywardmonkeys
Copy link
Collaborator Author

Many color libraries don’t provide a default… I am fixing the things that need it for now and my inclination is to only merge this under duress.

We choose an opaque color because otherwise the `OpaqueColor`
would have a substantially different default value than the
other color types.

White is the same as the default color within `bevy_color`.

The default color is used by the Brush in Parley and likely other places.
@waywardmonkeys waywardmonkeys marked this pull request as draft December 11, 2024 17:01
@waywardmonkeys
Copy link
Collaborator Author

Converting to draft as there is no intent to merge at this time.

@waywardmonkeys waywardmonkeys deleted the default-color branch December 29, 2024 23:55
@waywardmonkeys
Copy link
Collaborator Author

I hope to never need this.

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.

4 participants