RFR: 8332895: Support interpolation for backgrounds and borders [v36]

[Nir Lisker]

On Sun, 8 Sep 2024 20:55:53 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> This PR completes the CSS Transitions story (see #870) by adding interpolation support for backgrounds and borders, making them targetable by transitions.
>> 
>> `Background` and `Border` objects are deeply immutable, but not interpolatable. Consider the following `Background`, which describes the background of a `Region`:
>> 
>> 
>> Background {
>>     fills = [
>>         BackgroundFill {
>>             fill = Color.RED
>>         }
>>     ]
>> }
>> 
>> 
>> Since backgrounds are deeply immutable, changing the region's background to another color requires the construction of a new `Background`, containing a new `BackgroundFill`, containing the new `Color`.
>> 
>> Animating the background color using a CSS transition therefore requires the entire Background object graph to be interpolatable in order to generate intermediate backgrounds.
>> 
>> More specifically, the following types will now implement `Interpolatable`.
>> 
>> - `Insets`
>> - `Background`
>> - `BackgroundFill`
>> - `BackgroundImage`
>> - `BackgroundPosition`
>> - `BackgroundSize`
>> - `Border`
>> - `BorderImage`
>> - `BorderStroke`
>> - `BorderWidths`
>> - `CornerRadii`
>> - `Stop`
>> - `Paint` and all of its subclasses
>> - `Margins` (internal type)
>> - `BorderImageSlices` (internal type)
>> 
>> ## Interpolation of composite objects
>> 
>> As of now, only `Color`, `Point2D`, and `Point3D` are interpolatable. Each of these classes is an aggregate of `double` values, which are combined using linear interpolation. However, many of the new interpolatable classes comprise of not only `double` values, but a whole range of other types. This requires us to more precisely define what we mean by "interpolation".
>> 
>> Mirroring the CSS specification, the `Interpolatable` interface defines several types of component interpolation:
>> 
>> | Interpolation type | Description |
>> |---|---|
>> | default | Component types that implement `Interpolatable` are interpolated by calling the `interpolate(Object, double)}` method. |
>> | linear | Two components are combined by linear interpolation such that `t = 0` produces the start value, and `t = 1` produces the end value. This interpolation type is usually applicable for numeric components. |
>> | discrete | If two components cannot be meaningfully combined, the intermediate component value is equal to the start value for `t < 0.5` and equal to the end value for `t >= 0.5`. |
>> | pairwise | Two lists are combined by pairwise interpolation. If the start list has fewer elements than the target list, the...
>
> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
> 
>   small doc changes, copyright header

I went over the API and some of the implementation, looks good.

One sore thumb I saw were the private constructors with the `ignored` parameter to avoid name clash. I think there could have been a better way to do that, but since it's in the implementation I won't get into it here.

modules/javafx.graphics/src/main/java/javafx/animation/Interpolatable.java line 54:

> 52:  *             <td>Two lists are combined by pairwise interpolation. If the start list has fewer elements than
> 53:  *                 the target list, the missing elements are copied from the target list. If the start list has
> 54:  *                 more elements than the target list, the excess elements are discarded.

Are the elements within the lists interpolated as well? For example, if one list is `[red, blue]` and the other is `[green]`, then `blue` is discarded as excess, but will `red` be linearly interpolated to `green`?

modules/javafx.graphics/src/main/java/javafx/animation/Interpolatable.java line 73:

> 71:      * to 1 (inclusive).
> 72:      * <p>
> 73:      * The returned value may not be a new instance; the implementation might also return one of the

"might not be a new instance". "may not" can mean that it's disallowed.

modules/javafx.graphics/src/main/java/javafx/css/TransitionEvent.java line 109:

> 107:     public TransitionEvent(EventType<? extends Event> eventType,
> 108:                            StyleableProperty<?> property,
> 109:                            String propertyName,

Does this break backwards compatibility?

modules/javafx.graphics/src/main/java/javafx/scene/layout/Background.java line 292:

> 290:     private Background(List<BackgroundFill> fills, List<BackgroundImage> images, int ignored) {
> 291:         Objects.requireNonNull(fills, "fills cannot be null");
> 292:         Objects.requireNonNull(images, "images cannot be null");

Are these reached if there is an interpolation from/to null with a non-null?

-------------

PR Review: https://git.openjdk.org/jfx/pull/1522#pullrequestreview-2300363858
PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1756900815
PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1759655761
PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1756906715
PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1759665107