Re: 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ß  
<michaelstr...@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");
}