On Sun, 1 Sep 2024 12:31:06 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 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 48 additional 
> commits since the last revision:
> 
>  - Merge branch 'master' into feature/interpolatable
>  - remove StyleConverter.WithReconstructionSupport
>  - fix line separators
>  - StyleableStringProperty should be transitionable
>  - non-interpolatable values should always transition discretely
>  - only call get() when necessary
>  - add more documentation
>  - replace reconstruction annotation with interface
>  - interpolate integers in real number space
>  - replace StyleConverter.SupportsDeconstruction interface with annotation
>  - ... and 38 more: https://git.openjdk.org/jfx/compare/8080d15f...2337ca98

I've been looking at `TransitionMediator`s and `TransitionTimer`s -- these two 
classes seem to always go together.  There is some nasty bookkeeping going on 
that requires clearing a reference of the mediator.  I've had some success just 
eliminating the Mediator code and storing the timer directly.  It works 
something like this:

In the styleable property implementations, create and store a `TransitionTimer` 
directly.  Provide it with a lambda that would do the `onUpdate` code.  Also 
store the `endValue`.  So for `StyleableDoubleProperty` it be something like:

    private TransitionTimer tt;
    private double endValue;

Then in `set` clear the timer if needed:

        if (tt != null && v == endValue) {
            tt = null;
            endValue = 0;  // important for reference types
        }

I'd also do this in `bind` instead of relying on the `onStop` callback.

You could return just a `Future` from `TransitionTimer.run` as all that is 
needed is the ability to cancel.

To make reversal detection work, you may need to pass something more to 
`TransitionTimer.run` -- however, see Q1 -- is this something FX has decided to 
provide that is not part of CSS?  Should we consider making this smarter (Q2)?

I could imagine the reversal detection should be working with a start value 
that only resets on animation completion (ie. when `tt` is set back to `null`). 
 The reversal should then be calculated based on how much time we've been 
animating already since `tt` became non-null (transition time minus time spent 
animating already).  If this is negative, then animate normally.  Is it 
positive, then shorten the transition.

If we're still interested in doing reversals, storing a start time + start 
value would also be needed.  These values should only reset when an animation 
ends (ie. whenever we clear `tt` to `null`).  All this can be wrapped together 
in a helper class managed by the property, something like:

      record AnimationInfo(Future<?> transitionTimer, T startValue, T endValue, 
long startTime) {}

In a field:

      AnimationInfo info;

If that field is non-null, it should "derive" a new AnimationInfo without 
altering the startValue and startTime:

      AnimationInfo derive(Future<?> newTimer, T newEndValue);

I don't think TransitionTimer needs to be aware of any of this, as long as it 
has the `onUpdate` lambda provided.

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

PR Comment: https://git.openjdk.org/jfx/pull/1522#issuecomment-2325076593

Reply via email to