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

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.

/reviewers 2 /csr

---

Progress

• [x] Change must not contain extraneous whitespace
• [x] 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-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[bot]]

:wave: 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[bot]]

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

[openjdk[bot]]

@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[bot]]

@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[bot]]

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[bot]]

@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]

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