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

Add color animation support #12202

Closed
2 of 4 tasks
alice-i-cecile opened this issue Feb 29, 2024 · 10 comments · Fixed by #12575 or #12614
Closed
2 of 4 tasks

Add color animation support #12202

alice-i-cecile opened this issue Feb 29, 2024 · 10 comments · Fixed by #12575 or #12614
Labels
A-Animation Make things move and change over time A-Rendering Drawing game state to the screen C-Enhancement A new feature
Milestone

Comments

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Feb 29, 2024

There are four sections to this issue:

  • Implement Animatable for all of our color types.
  • Implement Point (in the sense of bevy_math for all of our color types.
  • Figure out the correct way to handle color addition/subtraction and scalar multiplication and division.
  • Add examples demonstrating color animation

Our options for handling alpha during color addition and subtraction from #12163 (comment):

  1. Average: leads to fairly surprising behavior, non-associative.
  2. Use the left-hand alpha: fairly surprising, non-commutative.
  3. Set the alpha to 1: very surprising, associative and commutative.
  4. Don't support adding colors with alpha: panic, remove the implementation, or create a set of alpha-free colors.
  5. Just add and subtract alpha like RGB: simple, but leads to values outside of 0-1.0 very easily.
  6. Apply alpha blending: a little more expensive, probably well-behaved.
@alice-i-cecile alice-i-cecile added C-Enhancement A new feature A-Rendering Drawing game state to the screen A-Animation Make things move and change over time labels Feb 29, 2024
@alice-i-cecile alice-i-cecile added this to the 0.14 milestone Feb 29, 2024
@viridia
Copy link
Contributor

viridia commented Feb 29, 2024

Note that Mix which internally uses addition and multiplication, is well-defined: the alpha is linearly interpolated just as the other channels are. This suggests that addition and subtraction are perhaps too low-level for colors, and offer too much generality. It might be better if we can identify higher-level operations that require addition and subtraction, but which don't have degenerate cases, and implement those directly.

As an example, a bezier or quadratic curve in color space would be a well-defined operation, where each color is a control point along the curve.

This was referenced Mar 13, 2024
@lynn-lumen
Copy link
Contributor

I would like to organize the discussions in #12460, #12534 and #12553 a bit more. We have got two basic ideas for how to handle color addition, subtraction, etc.:

Solution Pros Cons
Implement Add, Sub for colors using componentwise operations #12460
  • Easy to use
  • Easy to make mistakes for users
  • operations don't do what one might expect them to do
A custom trait providing all operations needed for splines, not implementing Add, Sub #12553
  • Could prevent some user errors
  • Moves the docs closer to a possible source of confusion
  • Hard to work with, especially for more complex maths operations

If you have another solution in mind, or some more pros/cons please feel free to add them here. Let's try to reach a consensus on how to do this here. If you want to discuss any specific attempt, please do so in the relevant PRs.

@alice-i-cecile
Copy link
Member Author

I think the solution is really clear:

  • Don't add math to Color
  • Do add math to linear spaces like OkLaba by implementing Add and ConvexLinearSpace (but this requires design work for alpha compositing)
  • Also make it easy to convert vectors or tuples of vectors into Color or individual spaces

I'll second that approach: it avoids complexity, implements the common traits for the spaces they make sense on, and pushes users to interpolating in the spaces that they should be using.

Classifying our existing color spaces:

  • Srgba(Srgba): Nonlinear due to gamma correction
  • LinearRgba(LinearRgba): linear, in a physical sense
  • Hsla(Hsla): nonlinear due to cylindrical hue
  • Hsva(Hsva: nonlinear due to cylindrical hue
  • Hwba(Hwba): nonlinear due to cylindrical hue
  • Laba(Laba): perceptually linear
  • Lcha(Lcha): : nonlinear due to cylindrical hue
  • Oklaba(Oklaba): : perceptually linear
  • Oklcha(Oklcha): : nonlinear due to cylindrical hue
  • Xyza(Xyza): physically linear

@viridia
Copy link
Contributor

viridia commented Mar 19, 2024

Ultimately, I think we need to take an artist-centric approach to this issue. Artists won't be calling APIs or manually coding control points for spline curves - they will be working in some kind of content creation tool, and generally in the context of some larger entity such as a 3d model.

Animating colors directly is in itself already an outlier: yes, you can attach an animation track to a material color property in Blender, but this is rare. I can imagine something like a pulsing traffic light, but even in this case it's only changing brightness, which means that it doesn't much matter which color space you pick.

@viridia
Copy link
Contributor

viridia commented Mar 19, 2024

Another point to bear in mind, from the standpoint of an artist working in Blender, "opacity" is a separate property unrelated to color. The combination of color + alpha is a historical accident related to compositing of 2d images.

github-merge-queue bot pushed a commit that referenced this issue Mar 19, 2024
# Objective

- Fixes #12202 

## Solution

- This PR implements componentwise (including alpha) addition,
subtraction and scalar multiplication/division for some color types.
- The mentioned color types are `Laba`, `Oklaba`, `LinearRgba` and
`Xyza` as all of them are either physically or perceptually linear as
mentioned by @alice-i-cecile in the issue.

---

## Changelog

- Scalar mul/div for `LinearRgba` may modify alpha now.

## Migration Guide

- Users of scalar mul/div for `LinearRgba` need to be aware of the
change and maybe use the `.clamp()` methods or manually set the `alpha`
channel.
@alice-i-cecile
Copy link
Member Author

Not quite done yet: we need examples and the Animatable trait still :)

@lynn-lumen
Copy link
Contributor

Oooh, I completely forgot about that 😅

I would like to give this a try

@NthTensor
Copy link
Contributor

Should probably be built with/along side the Mix trait.

@alice-i-cecile
Copy link
Member Author

Yep, I think we should just call the Mix trait in there.

@lynn-lumen
Copy link
Contributor

lynn-lumen commented Mar 20, 2024

Should probably be built with/along side the Mix trait.

I am not quite sure about that. Mix is only guaranteed to work for t in 0..1 which is explicitly not guaranteed to be the case for Animatable. I would just use the new operators and an implementation similar to the one for VecX.

Edit: the types implementing the operators could actually just use them inside Mix to reduce code duplication

github-merge-queue bot pushed a commit that referenced this issue Mar 22, 2024
# Objective

- Fixes #12202 

## Solution

- Implements `Animatable` for all color types implementing arithmetic
operations.
  - the colors returned by `Animatable`s methods are already clamped.
- Adds a `color_animation.rs` example.
- Implements the `*Assign` operators for color types that already had
the corresponding operators. This is just a 'nice to have' and I am
happy to remove this if it's not wanted.

---

## Changelog

- `bevy_animation` now depends on `bevy_color`.
- `LinearRgba`, `Laba`, `Oklaba` and `Xyza` implement `Animatable`.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Animation Make things move and change over time A-Rendering Drawing game state to the screen C-Enhancement A new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants