8332895: Support interpolation for backgrounds and borders

[mstr2]

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
• ImagePattern
• LinearGradient
• RadialGradient
• Stop

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 missing elements are copied from the target list. If the start list has more elements than the target list, the excess elements are discarded.
(see prose)	Some component types are interpolated in specific ways not covered here. Refer to their respective documentation for more information.

Every component of an interpolatable class specifies its interpolation type with the new @interpolationType javadoc tag. For example, this is the specification of the CornerRadii.topLeftHorizontalRadius component:

    /**
     * The length of the horizontal radii of the top-left corner.
     *
     * @return the length of the horizontal radii of the top-left corner
     * @interpolationType <a href="../../animation/Interpolatable.html#linear">linear</a> if both values are
     *                    absolute or both values are {@link #isTopLeftHorizontalRadiusAsPercentage() percentages},
     *                    <a href="../../animation/Interpolatable.html#discrete">discrete</a> otherwise
     */
    public double getTopLeftHorizontalRadius()

In the generated documentation, this javadoc tag shows up as a new entry in the specification list:

Limitations

Implementations usually fall back to discrete interpolation when the start value is an absolute value, and the end value is a percentage (see the example of CornerRadii.topLeftHorizontalRadius above). However, we can often solve these scenarios by first canonicalizing the values before interpolation (i.e. converting percentages to absolute values). This will be a future enhancement.

/reviewers 2
/csr

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change requires CSR request JDK-8333455 to be approved
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issues

• JDK-8332895: Support interpolation for backgrounds and borders (Enhancement - P4)
• JDK-8226911: Interpolatable's contract should be reexamined (Enhancement - P4)
• JDK-8333455: Support interpolation for backgrounds and borders (CSR)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1471/head:pull/1471
$ git checkout pull/1471

Update a local copy of the PR:
$ git checkout pull/1471
$ git pull https://git.openjdk.org/jfx.git pull/1471/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1471

View PR using the GUI difftool:
$ git pr show -t 1471

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1471.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back mstrauss! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@mstr2
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[openjdk]

@mstr2 has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@mstr2 please create a CSR request for issue JDK-8332895 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

[mlbridge]

Webrevs

• 02: Full - Incremental (08ed751b)
• 01: Full - Incremental (bb84c57d)
• 00: Full (25bcb1df)

[FlorianKirmaier]

First, Thank you for working on these transitions!

Note that this PR also changes the specification of Interpolatable to make users aware that they shouldn't assume any particular identity of the object returned from the interpolate() method. This allows the implementation to re-use objects and reduce the number of object allocations.

Can you elaborate on this?
Are changes in the Region.background still trigger change events?
If not, is there a mechanism to get them? Like Transform.onTransformChanged?
Are the previous immutable objects like Background and Border now mutable?

[mstr2]

Can you elaborate on this? Are changes in the Region.background still trigger change events? If not, is there a mechanism to get them? Like Transform.onTransformChanged? Are the previous immutable objects like Background and Border now mutable?

I'm not entirely sure what you mean here, but nothing how backgrounds and borders work has changed. Both are still deeply immutable, what's new is that they implement Interpolatable.

For example, if you want to change the background color of a region, you can't reassign BackgroundFill.fill. Instead, you will have to create a new Background instance that contains a new BackgroundFill, which contains the new color. This is what CSS does when it applies a new background color.

This PR allows the CSS system to also create intermediate backgrounds that represent the transition from one color to the next.

The interpolate() method may or may not return a new instance of the interpolatable object. For example, consider the current version of Color.interpolate(), which returns this for t<=0 and endValue for t>=1 (in other words, a new instance is not returned in all cases):

    @Override
    public Color interpolate(Color endValue, double t) {
        if (t <= 0.0) return this;
        if (t >= 1.0) return endValue;
        ...
    }

However, the current specification of Interpolatable.interpolate() is a bit unclear on that:

    /*
     * The function calculates an interpolated value along the fraction
     * {@code t} between {@code 0.0} and {@code 1.0}. When {@code t} = 1.0,
     * {@code endVal} is returned.
     */

The updated specification is clearer:

    /**
     * Returns an intermediate value between the value of this {@code Interpolatable} and the specified
     * {@code endValue} using the linear interpolation factor {@code t}, ranging from 0 (inclusive)
     * to 1 (inclusive).
     * <p>
     * The returned value may not be a new instance; an implementation might also return one of the
     * two existing instances if the intermediate value would be equal to one of the existing values.
     * However, this is an optimization and applications should not assume any particular identity
     * of the returned value.
     * <p>
     * An implementation is not required to reject interpolation factors less than 0 or larger than 1,
     * but this specification gives no meaning to values returned outside of this range. For example,
     * an implementation might clamp the interpolation factor to [0..1], or it might continue the linear
     * interpolation outside of this range.
     */

[FlorianKirmaier]

Ok, great! Thank you for the feedback.
I feared some magic was happening, but based on your response, this looks good!
As far as I understand this, this is just command sense and, in my opinion, doesn't have to be documented.
It's somewhat confusing and irrelevant.

[kevinrushforth]

This seems like a reasonable enhancement. I'll review it (not immediately, though).

@nlisker would you be willing to be the second reviewer?

A couple general comments:

1. Since you propose interpolating objects that aren't simple sets of floating-point fields, the overridden interpolate method should specify the behavior of the fields that are not. For example, different types of Paint are sometimes interpolatable and sometimes return either the start or end; some fields of BackgroundImage are interpolatable and others (notably the image itself) return either the start or the end; and so forth.
2. I spotted at least one place where you override equals, but not hashCode, so you'll need to provide an override for that.

[nlisker]

@nlisker would you be willing to be the second reviewer?

I could do some of the review, but probably no time for a full one.

Since you propose interpolating objects that aren't simple sets of floating-point fields, the overridden interpolate method should specify the behavior of the fields that are not. For example, different types of Paint are sometimes interpolatable and sometimes return either the start or end; some fields of BackgroundImage are interpolatable and others (notably the image itself) return either the start or the end; and so forth.

When I did the (simple) interpolation work on Point2D/3D I also looked at Border and Background because animating the color of these is rather common. The problem is that they are rather complex and don't interpolate trivially as a whole. As a user, if I want to interpolate between images, I would think of a smooth transition between them, not a jump cut. There are other oddities as well.

[nlisker]

Note that this PR also changes the specification of Interpolatable to make users aware that they shouldn't assume any particular identity of the object returned from the interpolate() method. This allows the implementation to re-use objects and reduce the number of object allocations.

There is an issue for that already: https://bugs.openjdk.org/browse/JDK-8226911. You can take it.

[mstr2]

/issue add JDK-8226911

[openjdk]

@mstr2
Adding additional issue to issue list: 8226911: Interpolatable's contract should be reexamined.

[mstr2]

I've added a new "interpolation type" specification for all components of interpolatable classes. This mirrors the "animation type" of the CSS specification, which is listed as an entry in the property table. Please also read the updated top-level post of this PR, which goes into more detail about this new specification.

@kevinrushforth This might be a candidate for a late enhancement, because the risk is relatively low (it's mostly the addition of the interpolate() method to various types), and it seems to be important to make the CSS transitions feature useful (backgrounds and borders will probably be one of the most obvious things developers will want to animate).

[mstr2]

Note that a null color is now treated as TRANSPARENT for the following reasons:

1. The previous implementation was broken: if a Stop is constructed with a null color, the hashCode() method throws a NPE because it doesn't check for null.
2. It doesn't make sense to have a stop with null color. What does it even mean?
3. When the stop list is normalized, an empty or null list is treated as a two-stop list with TRANSPARENT color (see normalize()). So we already have a scenario where null is treated as TRANSPARENT.

[mstr2]

As proposed, this enhancement doesn't cover some edge cases. Back to the drawing board.