Rate of embedded animations

[Nir Lisker]

Then I will take approach 1 for now.

Remind me again, which JBS bug you are planning to use to fix this issue?


https://bugs.openjdk.java.net/browse/JDK-8236858

On Wed, Jan 22, 2020 at 1:31 AM Kevin Rushforth <kevin.rushforth at oracle.com>
wrote:

> This got buried in my inbox, and I missed seeing it.
>
> I'll respond inline.
>
>
> On 1/10/2020 6:45 PM, Nir Lisker wrote:
>
> I forgot to bring up https://bugs.openjdk.java.net/browse/JDK-8200206,
> which is also relevant to this discussion. The solution to that one is
> probably allowing only 1 parent, similar to the scenegraph policy. That
> will avoid parent animations fighting over the state of common children
> animations.
>
>
> Good point. Regardless of which approach (1 or 2) we go with, I agree that
> allowing only a single parent transition is best. I note that this would be
> a semantic change, though, so even though we ought to decide what we want
> the behavior to be, actually enforcing a single parent is probably best
> left to JavaFX 15 (I note that
> https://bugs.openjdk.java.net/browse/JDK-8200206 is targeted to 15).
>
> On Sat, Jan 11, 2020 at 3:08 AM Nir Lisker <nlisker at gmail.com> wrote:
>
>> What happens if you set the rate of the parent transition and then set
>>> the rate of a child animation?
>>>
>> What if you bind to the rate of a child animation?
>>
>>
>> There is already some code that touches this in the Animation's rate
>> property:
>>
>> public void invalidated() {
>>     final double newRate = getRate();
>>     if (isRunningEmbedded()) {
>>         if (isBound()) {
>>             unbind();
>>         }
>>         set(oldRate);
>>         throw new IllegalStateException("Cannot set rate of embedded
>> animation while running.");
>>     }
>>
>> isRunningEmbedded checks for a parent animation that is not STOPPED. That
>> means that the rate of a child animation can only change while the parent
>> is stopped, but it is ignored (the child's ClipEnvelope's rate is updated
>> to the parent's rate instead).
>> Binding to the rate property is never an issue, but it does give
>> meaningless results since the rate is ignored. Binding the rate property
>> itself will cause it to be unbinded.
>>
>> If we go with option 1, which is almost the current situation, there is
>> only some cleanup needed when it comes to the relation between the
>> Animation's rate and its ClipEnvelope's rate.
>>
>
> This (option 1) seems easiest, but I can see am argument for either
> approach.
>
> Remind me again, which JBS bug you are planning to use to fix this issue?
>
> If we go with option 2, we can eliminate the duplication of rate in the
>> Animation and the ClipEnvelope (though ClipEvenope to Animation is somewhat
>> like a peer to a Node, which duplicates the values as well, so maybe we
>> want that?). The children's rate will always be the parent's rate. In this
>> case the above code should probably be modified to account for STOPPED
>> parents as well. A bound rate property will be unbinded and setting it will
>> be either ignored or throw an exception.
>>
>
> Yes, I would agree with this.
>
>
>> I'll note that currentRate is eliminated from the ClipEnvelope in the fix
>> I'm working on, but as it's read-only, it's a different story.
>>
>> As long as the implementation
>>> is consistent in ignoring the rate property from the child Animation,
>>> this seems like a fine solution and is consistent with other properties
>>> where one property overrides another without modifying it.
>>
>>
>> Which other properties?
>>
>
> I meant outside animation. Node.opacity for example. You can set it to any
> value and it is clamped on use. I realize that this is a different
> situation.
>
>
> delay, autoReverse, and cycleCount all work separately on the parent and
>> child. For example, if both parent and child have autoReverse = true and
>> cycleCount = 2, the animation will play forwards, backwards, forwards,
>> backwards, so both are taken into account.
>>
>
> OK.
>
> -- Kevin
>
>
> On Wed, Jan 8, 2020 at 1:14 AM Kevin Rushforth <kevin.rushforth at oracle.com>
>> wrote:
>>
>>> A rate in a "parent" transition should probably override the rate in a
>>> child Animation. There are a couple ways to do it, and it sounds like it
>>> doesn't really implement it right.
>>>
>>> 1. The public rate property values of child Animations within a parent
>>> Transition are ignored, but not updated. As long as the implementation
>>> is consistent in ignoring the rate property from the child Animation,
>>> this seems like a fine solution and is consistent with other properties
>>> where one property overrides another without modifying it.
>>>
>>> 2. Setting a rate on the parent Transition actually modifies the child
>>> node. This can work, but has more issues. What happens if you set the
>>> rate of the parent tansition and then set the rate of a child animation?
>>> What if you bind to the rate of a child animation?
>>>
>>> -- Kevin
>>>
>>> On 1/1/2020 1:04 PM, Nir Lisker wrote:
>>> > Hi,
>>> >
>>> > As part of my work in the animations package I've come across an
>>> undefined
>>> > spec. Is the rate of an animation that is embedded in a
>>> Parallel/Sequential
>>> > transition inherited and overridden by its parent?
>>> >
>>> > If I have an animations with a rate of 1 and a parent with a rate of
>>> 2, and
>>> > I add the animation to the parent, does the rate property change from
>>> 1 to
>>> > 2 (generating a change/invalidation event etc.)? Then, should any
>>> > subsequent changes to the parent's rate trigger a change in the child's
>>> > rate?
>>> >
>>> > The implementation is a bit messy in this regard. The rate and
>>> currentRate
>>> > values exist in both Animation and its ClipEnvelope, which is the
>>> source
>>> > for a couple of bugs. For example, changing the rate of a
>>> > ParallelTransition changes the child's ClipEnvelope's rate and
>>> currentRate,
>>> > but only the animation's currentRate (and not rate). Is it supposed to
>>> be
>>> > this way?
>>> >
>>> > What is clear is that it's not possible to change the rate of the
>>> embedded
>>> > animation directly.
>>> >
>>> > The docs do not talk about the rate of an embedded animation (they do
>>> talk
>>> > about the node property though).
>>> >
>>> > - Nir
>>>
>>>
>