I think that we should realize that almost all JavaFX public class
(controls, properties, listeners, etc) are not thread safe. This means
that they must be manipulated by a single thread at all times. This can
be the FX thread, but it can also be another thread, as long as there
never are two threads manipulating these JavaFX components
concurrently. So it is fine to construct controls on one thread, as
long as that thread is finished when they're handed over to the FX
thread by making them visible (or by starting animations!).
The problem with using animations in controls (and starting them
immediately) is that this brings a second thread into play: the FX
thread -- this may not be what you want when you are constructing
controls "offline". The animation may be changing properties, while the
offline thread is changing the same properties, which can lead to all
kinds of issues, like listeners running concurrently for the same property.
I therefore think that a control that is starting animations before it
is showing (attached to a scene, and is visible) is fundamentally
flawed. Controls should only start their animations when showing and
must terminate them when no longer showing. In other words, controls
should be written to prevent the FX thread accessing them (via
animation), unless they're sure that's currently safe (it's safe when
they're currently showing).
So even though we can make a call like "play" safe to call on another
thread, realize that by calling `play`, you are starting a process where
the FX thread may start modifying the control at any time, even while
you are still constructing the control on another thread.
Control authors need to fix this IMHO. Tooltip was fixed by using a
TreeShowingProperty at the time, which is similar to what I'm suggesting
that animations should be started/stopped based on the showing status of
your control.
This can be as easy as:
this.sceneProperty()
.flatMap(Scene::windowProperty)
.flatMap(Window::showingProperty)
.orElse(false)
.subscribe(showing -> {
if (showing) animation.play();
else animation.stop();
});
--John
On 29/08/2023 11:51, Jurgen Doll wrote:
Thanks for the heads-up Kevin,
I gave it a spin and found that generally because I use Task to load
my fxml views I had problems.
Some of these I could resolve by wrapping the offending line with
runlater in the fxml initialise method. This reminded me though of the
days when Tooltips had to be wrapped as well and it felt wrong because
generally a view may be constructed and modified on any thread as long
as it's not yet attached to a Scene in a Window that is showing.
This is highlighted further because I also have some third party
controls and libraries that are being initialized as part of the view,
which now just crash my application. This means that I cannot
instantiate these controls or libraries on any thread I want but have
to make sure its done on the FX thread, even though they're not
attached to a Scene yet.
As a possible solution I was wondering since the Animation API says
that calls to play() and stop() are asynchronous if it wouldn't then
be valid to instead of throwing an exception, if the call to it isn't
being made on the FX thread, that it rather be delegated to the FX
thread with for example something like:
public abstract class Animation {
public void play() {
if ( Platform.isFxApplicationThread() ) playFX();
else Platform.runLater( () -> playFX() );
}
private void playFX() {
// previous play() code
}
}
This would then prevent the NPE errors that sometimes occur but not
put a burden on the existing code in the wild and allow views to be
loaded with Task call() without worries.
Thanks, regards
Jurgen
On 8/18/2023 4:17 PM, Kevin Rushforth wrote:
As a heads-up for app developers who use JavaFX animation (including
Animation, along with any subclasses, and AnimationTimer), a change
went into the JavaFX 22+5 build to enforce that the play, pause, and
stop methods must be called on the JavaFX Application thread.
Applications should have been doing that all along (else they would
have been subject to unpredictable errors), but for those who aren't
sure, you might want to take 22+5 for a spin and see if you have any
problems with your application. Please report them on the list if you do.
See JDK-8159048 [1] and CSR JDK-8313378 [2] for more information on
this change.
Thanks.
-- Kevin
[1] https://bugs.openjdk.org/browse/JDK-8159048
[2] https://bugs.openjdk.org/browse/JDK-8313378
