Re: [Rev 01] RFR: 8242523: Update the animation and clip envelope classes

2020-06-01 Thread Ambarish Rapte
On Thu, 28 May 2020 16:44:33 GMT, Nir Lisker  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.
>
> Nir Lisker has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now
> contains 10 commits:
>  - Merge branch 'master' into 
> 8242523_Update_the_Animation_and_ClipEnvelope_classes
>  - Synch whitespace fix
>  - Addressed review comments
>  - Removing cycleNumber for now
>  - Fix typo in TicksCalculation & add an explanation for an animation test
>  - Initial push of 8242523
>  - Comment out erroneous test
>  - Fix IndefiniteCycleDuration test
>  - Fixed single loop test
>  - Initial commit

Marked as reviewed by arapte (Reviewer).

-

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


Re: [Rev 01] RFR: 8242523: Update the animation and clip envelope classes

2020-05-28 Thread Kevin Rushforth
On Thu, 28 May 2020 16:44:33 GMT, Nir Lisker  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.
>
> Nir Lisker has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now
> contains 10 commits:
>  - Merge branch 'master' into 
> 8242523_Update_the_Animation_and_ClipEnvelope_classes
>  - Synch whitespace fix
>  - Addressed review comments
>  - Removing cycleNumber for now
>  - Fix typo in TicksCalculation & add an explanation for an animation test
>  - Initial push of 8242523
>  - Comment out erroneous test
>  - Fix IndefiniteCycleDuration test
>  - Fixed single loop test
>  - Initial commit

Marked as reviewed by kcr (Lead).

-

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


Re: [Rev 01] RFR: 8242523: Update the animation and clip envelope classes

2020-05-28 Thread Nir Lisker
On Thu, 28 May 2020 15:51:19 GMT, Kevin Rushforth  wrote:

>> Doesn't the autogenerated JavaDoc for the `getX`, `setX` and `xProperty` 
>> methods work by matching the method name to
>> the property name regardless of they location in the class? 
>> `doSetCurrentRate` shouldn't ever be able to interfere with
>> that. I can add a `@param` anyway.
>
> So it would seem. It still seems a bit fragile, but as long as it works, I 
> won't push the point (we are very
> inconsistent on this).

I moved the method down anyway so it will be separated from the public API.

-

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


Re: [Rev 01] RFR: 8242523: Update the animation and clip envelope classes

2020-05-28 Thread Nir Lisker
> 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.

Nir Lisker has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now
contains 10 commits:

 - Merge branch 'master' into 
8242523_Update_the_Animation_and_ClipEnvelope_classes
 - Synch whitespace fix
 - Addressed review comments
 - Removing cycleNumber for now
 - Fix typo in TicksCalculation & add an explanation for an animation test
 - Initial push of 8242523
 - Comment out erroneous test
 - Fix IndefiniteCycleDuration test
 - Fixed single loop test
 - Initial commit

-

Changes: https://git.openjdk.java.net/jfx/pull/196/files
 Webrev: https://webrevs.openjdk.java.net/jfx/196/webrev.01
  Stats: 660 lines in 9 files changed: 344 ins; 180 del; 136 mod
  Patch: https://git.openjdk.java.net/jfx/pull/196.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/196/head:pull/196

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