Hi Jurgen, even assuming that it was as easy as you say it is, there are more problems with this proposal.
1. If non-FX thread access to some methods is allowed, this will become a permanent addition to the API and a complication we have to deal with. There are proposals to refactor the animation framework, and this will become harder as a result. This also has direct implications on the implementation (you already pointed out two instances where fields would have to be marked volatile). Multithreaded code is never easy, even in easy cases. Mutation visibility will become a permanent question for future development. 2. We will need to specify that the `onFinished` handler might be invoked on the thread that calls play() or on the FX thread, or we need to make sure that `onFinished` is always invoked on the FX thread. 3. Any change that would expressly allow concurrent access to the FX framework will require lots of testing to ensure that we don't introduce regressions or race conditions. This competes with reviewer time for other features and fixes. 4. That being said, all of the effort might still be a good investment if the added value was significant. But I don't see the significance of it, as you can simply wrap the play() call in Platform.runLater and be done with it. On Tue, Jan 23, 2024 at 4:56 PM Jurgen Doll <jav...@ivoryemr.co.za> wrote: > > Hi John and others, > > I don't think we are entirely on the same page, so here's the objective. > > The Goal: To determine if the FX animation thread and a SINGLE other thread > can access Animation in a safe manner wrt play, stop, and resume. > > Non Goal: Multi-threaded access of Animation play, stop, and resume is NOT a > goal. > > Wrt play() and resume(), it is ALWAYS safe to call them on a background > thread because at this point the Animation isn't being processed by the FX > thread as the "Animation" isn't contained in the AbstractPrimaryTimer > receivers array. > > Wrt stop() from what I can tell there are no shared method calls that need > synchronization (see audit below), however the following two boolean flags > should be marked as volatile in order to ensure that animation is cut short > if executing: > > TimelineClipCore.abort > ClipEnvelope.abort > > This is simple enough to add to my original proposal of replacing the arrays > in AbstractPrimaryTimer with CopyOnWriteArrayList which is thread safe and > replicates the intended behavior in a clear and concise way. > > Here are the methods that are called when stop() is invoked: > > Timeline.stop() > > getStatus() > > clipCore.abort() > > isStopped() > > clipEnvelope.abortCurrentPulse() > > doStop() > > timer.removePulseReceiver(pulseReceiver); > > // After this point it doesn't matter > > setStatus(Status.STOPPED) > > doSetCurrentRate(0.0) > > jumpTo(Duration.ZERO) > > > And for AbstractPrimaryTimer.timePulseImpl: > > AbstractPrimaryTimer.timePulseImpl > > Animation.PulseReceiver.timePulse > > Animation.doTimePulse > > clipEnvelope.timePulse > > animation.doPlayTo > > clipCore.playTo > > visitKeyFrame > > setTime > > animation.setCurrentTicks > > clipInterpolator.interpolate > > animation.doJumpTo > > sync(false) > > setCurrentTicks > > clipCore.jumpTo > > timeline.getStatus() > > clipInterpolator.interpolate > > > Regards > Jurgen
