On Fri, 2 Aug 2024 01:39:11 GMT, Michael Strauß <mstra...@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 two additional > commits since the last revision: > > - fix since tag > - adjust table styling Thank you for this contribution! This is the second batch of code review comments. Will probably have more after testing. modules/javafx.graphics/src/main/java/javafx/scene/paint/Stop.java line 162: > 160: // re-allocating when we run out of space. In general, we expect > the size of stop lists > 161: // to be quite small (a single-digit number of stops at most). > 162: Stop[] stops = new Stop[firstList.size() + secondList.size()]; are there unit tests to cover most of the scenarios, at least using short sequences? modules/javafx.graphics/src/test/java/test/javafx/scene/paint/StopListTest.java line 264: > 262: new Stop(1, color2)), > 263: res); > 264: } Do you think any other combinations should be added, for completeness sake? For example, let's consider two lists with 2 points each. We might want to test scenarios: -- -- --| |-- --- --- ----| ---| ------ -- |----- |-- |---| |---| |-- --| --- -- and also have the two lists swapped maybe? what do you think? ------------- PR Review: https://git.openjdk.org/jfx/pull/1522#pullrequestreview-2215836542 PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1702023646 PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1702046900