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