Here's another option, which I have implemented as a proof of concept [0]: The play/stop/etc. methods are currently specified to be "asynchronous". This language should be removed, such that the methods will be implied to execute synchronously from the point of view of the caller (like every method that doesn't specify anything to the contrary).
All changes to observable state will remain instantly visible for the calling thread. However, internally, interactions with AbstractPrimaryTimer are posted to the FX thread if necessary. This is not an unsurprising change, since the callback from the FX thread was always occuring at an unspecified time in the future. To make this work, AbstractPrimaryTimer::pause/resume/nanos will have to be synchronized to ensure field visibility across threads. In the Animation class, interactions with AbstractPrimaryTimer will be encapsulated in the new nested class AnimationPulseReceiver, which also deduplicates redundant interactions with AbstractPrimaryTimer. For example, repeatedly calling start() and stop() in quick succession may require just a single interaction with AbstractPrimaryTimer in the future (if we ended up in the running state), or no interaction at all (if we ended up in the stopped state). [0] https://github.com/openjdk/jfx/pull/1349 On Wed, Jan 24, 2024 at 7:27 PM Nir Lisker <nlis...@gmail.com> wrote: > > These are the options I see as reasonable: > > 1. Document that these methods *must* be run on the FX thread and throw an > exception otherwise. This leaves it to the responsibility of the user. > However, this will cause the backwards compatibility problems that Jugen > brought up. As a side note, we do this in other methods already, but I always > questioned why let the developer do something illegal - if there's only one > execution path, force it. > 2. Document that these methods *are* run on the FX thread (the user doesn't > need to do anything) and change the implementation to call runLater(...) > internally unless they are already on the FX thread. This will be backwards > compatible for the most part (see option 3). The downside might be some > surprise when these methods behave differently. > 3. Document that it's *recommended* that these methods be run on the FX > thread and let the user be responsible for the threading. We can explain that > manipulating nodes that are attached to an active scenegraph should be > avoided. > > I prefer option 2 over 1 regardless of the backwards compatibility issue > even, but would like to know if I'm missing something here because in theory > this change could be done to any "must run on the FX thread" method and I > question why the user had the option to get an exception. > Option 3 is risky and I wager a guess that it will be used wrongly more often > than not. It does allow some (what I would call) valid niche uses. I never > did it. >
