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. On Wed, Jan 24, 2024 at 4:21 PM Kevin Rushforth <kevin.rushfo...@oracle.com> wrote: > I'd like to hear from the others on this. I don't see any fundamental > problem with having the play/pause/stop methods wrap their implementation > in a runLater (if not on the FX Application thread already), and > documenting that it does so, if we can get general agreement. > > -- Kevin > > > On 1/24/2024 5:29 AM, Jurgen Doll wrote: > > Hi Kevin > > If I may make one more final appeal then to an alternative solution please. > > Could we then instead of throwing an Exception rather invoke runLater if > needed inside play, stop, and resume. > > Putting the onus on the developer is fine if it is the developer that is > invoking the call, but if it's in a library then it's a no go. > > In my application I have two libraries that I know of where this happens. > The major problem is that with FX22 as it now stands my application just > crashes because play() does an FX thread check and throws an Exception > which it never did before. There are bound to be other applications out > there that are going to find themselves in a similar position. > > PLEASE ! > > Regards > Jurgen > > > > On Wed, 24 Jan 2024 15:15:31 +0200, Kevin Rushforth > <kevin.rushfo...@oracle.com> <kevin.rushfo...@oracle.com> wrote: > > Thank you to Jurgen for raising the question and to Nir, John, and Michael > for evaluating it. > > I conclude that there is insufficient motivation to revert the change in > behavior implemented by JDK-8159048 to allow calling the play/pause/stop > methods of Animation on a background thread. Doing so without making it > fully multi-thread-safe would be asking for problems, and making it fully > multi-thread-safe would be a fair bit of work to do it right without a > clear benefit. > > We will proceed with the current approach and let JDK-8159048 stand. > Further, we will proceed with https://bugs.openjdk.org/browse/JDK-8324219 > which is under review in https://github.com/openjdk/jfx/pull/1342 > > -- Kevin > > On 1/24/2024 12:30 AM, Nir Lisker wrote: > > After playing around with the code sample, I think that this is not the > right way to use the animation. The reason is that there is no point in > starting the animation before the control is attached to the scenegraph, or > even made visible. A small refactoring where, e.g., the controller class > exposes a method to start the animation in onSucceeded or just calls it on > the FX thread is enough. I never start an animation as part of the > construction because it's not the right time. John suggested tying the > lifecycle of the animation to the showing of the node, which also solves > the problem. > > There are animations like PauseTransition or other non-interfering > Timelines that could reasonably be run on a background thread. Or maybe > just on an unconnected control. This could be a reason to not limit > animation methods to the FX thread at the expense of possible user errors, > but document the pitfall. > > I don't see a good use case for modifying controls in a background thread > while still interacting with the scenegraph, hence for adding multithread > support. > > - Nir > > On Mon, Jan 22, 2024, 12:59 Jurgen Doll <jav...@ivoryemr.co.za> wrote: > >> Here's an example as requested by Nir: >> >> public class FxTimeLineTest extends Application >> >> { >> >> private BorderPane bp = new BorderPane( new Label("Loading") ); >> >> public static void main( String[] args ) { >> >> launch( FxTimeLineTest.class, args ); >> >> } >> >> @Override >> >> public void start( Stage primaryStage ) throws Exception { >> >> new Thread( new LoadScene() ).start(); >> >> primaryStage.setScene( new Scene( bp, 300, 200 ) ); >> >> primaryStage.setTitle( "Memory Usage" ); >> >> primaryStage.show(); >> >> } >> >> private class LoadScene extends Task<Parent> { >> >> @Override protected Parent call() throws Exception { >> >> Parent p = FXMLLoader.load( getClass( ).getResource("TestView.fxml") ); >> >> Thread.sleep( 1000 ); >> >> return p; >> >> } >> >> @Override protected void succeeded() { >> >> bp.setCenter( getValue() ); >> >> } >> >> @Override protected void failed() { >> >> getException().printStackTrace(); >> >> } >> >> } >> >> } >> >> >> ------------------------------------------------------------------------------------------------------ >> >> public class TestView >> >> { >> >> @FXML private Label memory; >> >> private static final double MEGABYTE = 1024 * 1024; >> >> @FXML private void initialize() >> >> { >> >> var updater = new Timeline >> >> ( >> >> new K eyFrame( Duration.seconds(2.5), event -> >> >> { >> >> var runtime = Runtime.getRuntime(); >> >> double maxMemory = runtime.maxMemory() / MEGABYTE; >> >> double usedMemory = (runtime.totalMemory() - runtime.freeMemory()) / >> MEGABYTE; >> >> memory.setText( (int) usedMemory + " MB / " + (int) maxMemory +" MB" ); >> >> }) >> >> ); >> >> updater.setCycleCount(Animation.INDEFINITE); // This FXML is being >> loaded on a background thread >> >> updater.play(); >> >> } >> >> } >> >> >> ------------------------------------------------------------------------------------------------------ >> >> TestView.fxml >> >> <?xml version="1.0" encoding="UTF-8"?> >> >> <?import javafx.scene.control.Label?> >> >> <?import javafx.scene.layout.StackPane?> >> >> <StackPane xmlns:fx="http://javafx.com/fxml/1"; fx:controller="TestView"> >> >> <children> >> >> <Label fx:id="memory" text="Current / Max MB" > >> >> <properties hashCode="12345" /> >> >> </Label> >> >> </children> >> >> </StackPane> >> >> >> >> On Sat, 20 Jan 2024 17:08:41 +0200, Nir Lisker <nlis...@gmail.com> wrote: >> >> Hi Jurgen, >> >> What I'm confused about the most is what it is you are actually trying to >> do that necessitates the use of animations outside of the FX thread. You >> said that you need to initialize controls on another thread, and that you >> are using Task (both of which are fine), but how does playing animations >> relate? Playing an animation is something that is done explicitly, usually >> in order to manipulate data. Can you give a real use case, like a minimized >> version of what you're doing? >> >> - Nir >> >> > > > > -- > Using Opera's mail client: http://www.opera.com/mail/ > > >
