-
Notifications
You must be signed in to change notification settings - Fork 12
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
Remove Default
in Color
#31
base: main
Are you sure you want to change the base?
Conversation
I tend to agree that at the very least I don't have any strong opinion on it though. |
The derive seems to have been originally added in vello#168 by @dfrg, but there's no mentioned rationale. The all-zero value, even though transparent, is still a valid value. It may be worth adding a custom default implementation with an opaque color, but the transparent color does work visually as "no color", revealing whatever is underneath as if there isn't any color. As for why have I think there is enough value and very little cost to keep |
In general, I’m opposed to removing the |
Sure, maybe just (opaque) black or white? But @xStrom made good arguments for it being transparent. I'm indifferent with keeping, removing or changing the default value, this PR just resulted out of a conversation in xilem. |
I'm don't have a strong opinion on transparent/opaque, I think if kurbo's shape impls had custom default implementations returning unit shapes, rather than zeros opaque black would be a better default. because then something like
Would give a black unit rect with an identify transform, which I think would be really convenient. |
Bevy's default here is I don't have the reasoning to hand for that |
My thinking was just make it contrast with whatever the default unfilled viewport color is. I off hand forget what vello uses, I seem to recall transparent (but thought it was either transparent or white). But this also begs the question of whether different backends like bevy_vello and vello proper might use different unfilled viewport background. So there might very well not be any good choice for a contrasting default, since logical colors like foreground/background don't exist at this level of the stack. |
Black/white background will vary between projects, so in that sense maybe some annoying neon color will work better for the intention of being immediately visible. |
I think the conclusion we reached is that we probably will keep it, but change it to something non-transparent?
|
So we had this argument in linebender/xilem#226 (comment)
And I think
Default
forColor
doesn't really make sense, especially ifColor::a
is0
.Is there a reason for implementing
Default
for it?(CI breaks currently because of this, I'm waiting for initial feedback before fixing it (i.e.
impl Default
for the types depending onColor::default
))