HEADS-UP: Threading restriction for Animation play, pause, stop now enforced

[Jurgen Doll]

Hi Michael

> starting an animation on a background thread would only be safe if the
> object graph affected by the animation is not accessed on the
> background thread after calling the play() method. Any access may
> potentially corrupt internal state.

Agreed. Actually we should drive this further and say that "No changes  
should be made to properties that are being 'animated' while animation is  
in progress regardless of whether you're on the FX thread or not, or if  
the object is in the scene graph or not".

         updater.play();
         memoryLabel.setStyle( "-fx-text-fill: green" );  // This is OK
         memoryLabel.setText( "This is NOT okay" );  // This is NOT, move  
to before play

Interesting API idea, however I don't think it's really needed for  
sensible developers.

Regards
Jurgen


On Mon, 22 Jan 2024 22:53:48 +0200, Michael Strauß  
<michaelstrau2 at gmail.com> wrote:

> Hi Jurgen,
>
> starting an animation on a background thread would only be safe if the
> object graph affected by the animation is not accessed on the
> background thread after calling the play() method. Any access may
> potentially corrupt internal state.
>
> From what I can see in your example, you're not doing anything with
> TestView after starting the animation, so you should be good here.
>
> From an API perspective, I wonder if that makes it too easy to
> introduce subtle bugs into an application. As a general principle, I
> think an API should make it easy to do the right thing, and hard to do
> the wrong thing. Allowing potentially unsafe method calls without some
> explicit procedure (like wrapping the call in Platform.runLater) might
> not be a good idea.
>
> Maybe we should provide an API that allows developers to safely modify
> an object graph on a background thread and call Platform.runLater
> without worrying about concurrent modifications.
>
> Something like this:
>
> try (var scope = Platform.deferredScope()) {
>     var updater = new Timeline(
>        new KeyFrame(Duration.seconds(2.5), event ->
>        {
>           int maxMemory = ....;
>           int usedMemory = ....;
>           memoryLabel.setText(usedMemory + " MB / "+ maxMemory +" MB");
>        })
>     );
>
>     // This call will only be dispatched after the deferred scope is  
> closed:
>     scope.runLater(updater::play);
>
>     // Still safe to call in the deferred scope, wouldn't be safe if
>     // Platform.runLater had been called before:
>     memoryLabel.setText("N/A");
> }