RFR: 8358820: Allow interpolation outside of range [0,1] [v3]

Andy Goryachev angorya at openjdk.org
Fri Aug 29 16:43:52 UTC 2025


On Sun, 27 Jul 2025 18:48:18 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> JavaFX unnecessarily restricts interpolation in the following ways:
>> 1. `Interpolatable` implementations often clamp intermediate values to the interpolation factor range [0,1].
>> 2. `SplineInterpolator` doesn't accept Y coordinates outside of [0,1] for its control points. While this was probably done so that the computed interpolation factor doesn't exceed [0,1], the restriction goes far beyond that. For example, the following function is not representable, even though its values are all within the [0,1] range:<br>
>>    <img src="https://github.com/user-attachments/assets/368b6142-052d-4ead-8a59-cbddf4a19660" width="400"/><br>
>>    The following function is also not representable, but would be very useful for [bouncy animations](https://easings.net/#easeOutBack):<br>
>>    <img src="https://github.com/user-attachments/assets/af02b044-ae4c-4250-b181-72178ad9f3f3" width="400"/>
>> 
>> Fortunately, there is no technical reason why JavaFX can't support the full range of animations that can be represented with a cubic Beziér interpolation function.
>> 
>> This PR includes the following changes:
>> 1. The specification of `Interpolatable` is changed to require implementations to accept interpolation factors outside of [0,1].
>> 2. All implementations of `Interpolatable` now correctly return intermediate values outside of [0,1].
>> 3. `SplineInterpolator` now accepts control points with any Y coordinate.
>> 
>> Here's how the result looks like for the previously unrepresentable interpolation function `cubic-bezier(0.34, 2.2, 0.64, 1)`:<br>
>> <img src="https://github.com/user-attachments/assets/72c10d0d-71b4-4bb5-b58c-ae377279b0fd" width="500"/>
>
> Michael Strauß has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Merge branch 'master' into feature/relaxed-interpolation
>  - javadoc
>  - Allow interpolation outside of range [0,1]

looks good; a few minor comments.

modules/javafx.graphics/src/main/java/com/sun/javafx/scene/layout/region/Margins.java line 79:

> 77:         Objects.requireNonNull(endValue, "endValue cannot be null");
> 78: 
> 79:         if (t == 0 || equals(endValue)) {

the class javadoc says


 * If proportional is true, then the values represent fractions or percentages
 * and are in the range 0..1, although this is not enforced.


should this be changed/clarified?

modules/javafx.graphics/src/main/java/com/sun/javafx/scene/layout/region/Margins.java line 92:

> 90: 
> 91:         return new Margins(
> 92:             Math.max(0, InterpolationUtils.interpolate(top, endValue.top, t)),

what is the reason for clamping this at 0, but not 1.0 ?

also, why clamp here but not in `Insets` ?

modules/javafx.graphics/src/main/java/javafx/scene/paint/RadialGradient.java line 251:

> 249:      * @param cycleMethod cycle method applied to the gradient
> 250:      * @param stops the gradient's color specification
> 251:      * @throws IllegalArgumentException if {@code focusDistance} or {@code radius} is negative

can we use the `@since` tag applicable to `@throws` only?
(also below)

modules/javafx.graphics/src/main/java/javafx/scene/paint/RadialGradient.java line 613:

> 611:             if (distance < 0) {
> 612:                 throw new IllegalArgumentException(
> 613:                     "Invalid gradient specification: focus-distance cannot be negative");

why wrap here?
also below

modules/javafx.graphics/src/test/java/test/javafx/scene/paint/RadialGradientTest.java line 291:

> 289:         try {
> 290:             RadialGradient.valueOf("radial-gradient(radius -100, red 0%, blue 30%, black 100%)");
> 291:             fail("IllegalArgument should have been thrown.");

nice: these tests do more than a simple `assertThrows` would do.

modules/javafx.graphics/src/test/java/test/javafx/scene/paint/RadialGradientTest.java line 302:

> 300:         } catch (IllegalArgumentException iae) {
> 301:             assertTrue(iae.getMessage().contains("focus-distance cannot be negative"));
> 302:         }

`RadialGradient` adds `checkInvariants` in three places, but only two tests are added.
Should we add one more?

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

Marked as reviewed by angorya (Reviewer).

PR Review: https://git.openjdk.org/jfx/pull/1822#pullrequestreview-3169216858
PR Review Comment: https://git.openjdk.org/jfx/pull/1822#discussion_r2310509351
PR Review Comment: https://git.openjdk.org/jfx/pull/1822#discussion_r2310519598
PR Review Comment: https://git.openjdk.org/jfx/pull/1822#discussion_r2310582888
PR Review Comment: https://git.openjdk.org/jfx/pull/1822#discussion_r2310586598
PR Review Comment: https://git.openjdk.org/jfx/pull/1822#discussion_r2310595057
PR Review Comment: https://git.openjdk.org/jfx/pull/1822#discussion_r2310611660


More information about the openjfx-dev mailing list