I also now favor option 2 and was in the process of writing something up
recommending that we do that. Phil and I (and a couple others) had an
offline discussion and think this is the way to go.
Given the short amount of time to get this into 22, I will file the JBS
issue in the next hour or so.
Nir: if you want to take it, that would be great. Otherwise, I will do
it. We need the PR and CSR filed before the end of this week.
Regarding other methods that choose option 1, many of them are more
complicated (e.g., scene mutation can be done on a background thread as
long as the scene is not "showing") or must be synchronous. Animation
starts something that conceptually happens later on the FX animation
thread anyway, so wrapping it in a runLater (if not already on the right
thread) doesn't really change the semantics in an appreciable way.
-- Kevin
On 1/24/2024 10:26 AM, Nir Lisker 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.
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> <mailto: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
<https://urldefense.com/v3/__https://github.com/openjdk/jfx/pull/1342__;!!ACWV5N9M2RV99hQ!PLtaZAvaobOk-fe_DRSy2AHiPOFUaK683TVXWIvmjnTiUGhmBp3x_GIy_wlMuEV3IHTUMx37HxJHiVuWiFRR$>
-- 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:
publicclassFxTimeLineTest extendsApplication
{
privateBorderPane bp= newBorderPane( newLabel("Loading") );
publicstaticvoidmain( String[] args) {
launch( FxTimeLineTest.class, args);
}
@Override
publicvoidstart( Stage primaryStage) throwsException {
newThread( newLoadScene() ).start();
primaryStage.setScene( newScene( bp, 300, 200 ) );
primaryStage.setTitle( "Memory Usage");
primaryStage.show();
}
privateclassLoadScene extendsTask<Parent> {
@OverrideprotectedParent call() throwsException {
Parent p= FXMLLoader.load( getClass(
).getResource("TestView.fxml") );
Thread.sleep( 1000 );
returnp;
}
@Overrideprotectedvoidsucceeded() {
bp.setCenter( getValue() );
}
@Overrideprotectedvoidfailed() {
getException().printStackTrace();
}
}
}
------------------------------------------------------------------------------------------------------
publicclassTestView
{
@FXMLprivateLabel memory;
privatestaticfinaldoubleMEGABYTE= 1024 * 1024;
@FXMLprivatevoidinitialize()
{
varupdater= newTimeline
(
newK eyFrame( Duration.seconds(2.5), event->
{
varruntime= Runtime.getRuntime();
doublemaxMemory= runtime.maxMemory() / MEGABYTE;
doubleusedMemory= (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
<https://urldefense.com/v3/__http://javafx.com/fxml/1__;!!ACWV5N9M2RV99hQ!PLtaZAvaobOk-fe_DRSy2AHiPOFUaK683TVXWIvmjnTiUGhmBp3x_GIy_wlMuEV3IHTUMx37HxJHia1cBAoi$>"
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/
<https://urldefense.com/v3/__http://www.opera.com/mail/__;!!ACWV5N9M2RV99hQ!PLtaZAvaobOk-fe_DRSy2AHiPOFUaK683TVXWIvmjnTiUGhmBp3x_GIy_wlMuEV3IHTUMx37HxJHifb0G69Z$>
