RFR: 8242523: Update the animation and clip envelope classes

[Ambarish Rapte]

On Fri, 24 Apr 2020 00:58:30 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

> Mostly refactoring in preparation of the upcoming fixes. The changes might look like a lot, but it's mostly rearranging
> of methods. Summery of changes:
> ### Animation
> * Added `isNearZero` and `areNearEqual` methods that deal with `EPSILON` checks.
> * Added `isStopped`, `isRunning` and `isPaused` convenience methods.
> * Added `runHandler` method to deal with running the handler.
> * Moved methods to be grouped closer to where they are used rather than by visibility.
> * Removed the static import for `TickCalculation`.
> * Various small subjective readability changes.
> * Behavioral changes: switching `autoReverse` and `onFinished` properties from "Simple" to "Base" properties; and lazily
>   initializing the `cuePoints` map.
> 
> ### Clip Envelopes
> * Added `MultiLoopClipEnvelope` as an intermediate parent for infinite and finite clip envelopes.
> * Rearranged methods order to be consistent.
> * Replaced the `checkBounds` method with a new overload of `Utils.clamp`.
> * Renamed `pos` to `cyclePos`.
> * Extracted common methods: `changedDirection` and `ticksRateChange`
> * Added internal documentation.
> 
> Also corrected a typo in `TicksCalculation` and added an explanation for an animation test.

Overall looks good to me, Added few minor comments.

modules/javafx.graphics/src/main/java/com/sun/scenario/animation/shared/MultiLoopClipEnvelope.java line 48:

> 47:
> 48:     protected boolean autoReverse() {
> 49:         return autoReverse;

I would suggest the name to be `isAutoReverese`

modules/javafx.graphics/src/main/java/com/sun/scenario/animation/shared/ClipEnvelope.java line 46:

> 45:  */
> 46: public abstract class ClipEnvelope {
> 47:

I think the removal of line 46 was unintended change.

modules/javafx.graphics/src/main/java/javafx/animation/Animation.java line 142:

> 141:     }
> 142:
> 143:     /*

Both these methods seem like they belong to Utils.java. If moved to Utils.java then the all the calls
`(Math.abs(currentRate - rate) < EPSILON)` can be changed to use these methods from Utils.java. If we move these
methods then, Utils.java also needs to declare  `static final double EPSILON = 1e-12;`  and the EPSILON here can be
initialized as `private static final double EPSILON = Utils.EPSILON;`

modules/javafx.graphics/src/main/java/com/sun/scenario/animation/shared/MultiLoopClipEnvelope.java line 61:

> 60:
> 61:     protected boolean changedDirection(double newRate) {
> 62:         return newRate * rate < 0;

I would recommend to name as `isDirectionChanged`

modules/javafx.graphics/src/main/java/com/sun/scenario/animation/shared/SingleLoopClipEnvelope.java line 102:

> 101:             long ticksChange = Math.round(currentTick * currentRate);
> 102:             ticks = Utils.clamp(0, deltaTicks + ticksChange, cycleTicks);
> 103:             AnimationAccessor.getDefault().playTo(animation, ticks, cycleTicks);

This could remain unchanged. The `ticksChange` value is not really used elsewhere here.

modules/javafx.graphics/src/main/java/com/sun/scenario/animation/shared/MultiLoopClipEnvelope.java line 59:

> 58:         return Math.round((ticks - deltaTicks) * Math.abs(newRate / rate));
> 59:      }
> 60:

This is similar to `ClipEnvelope.ticksRateChange()` with a minor difference. Can this be removed from here and
`ClipEnvelope.ticksRateChange()` be reused ?

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

Changes requested by arapte (Reviewer).

PR: https://git.openjdk.java.net/jfx/pull/196