Thanks!
I just filed https://bugs.openjdk.org/browse/JDK-8324658
It should mostly revert JDK-8159048 (although the sentence added to the
class docs about animations running on the JavaFX app thread is still
valid), and some of the unit tests might still be valid.
-- Kevin
On 1/24/2024 10:50 AM, Nir Lisker wrote:
If there's an agreement, I can do it tomorrow. It will effectively
revert JDK-8159048 and supersede JDK-8324219.
On Wed, Jan 24, 2024 at 8:42 PM Kevin Rushforth
<kevin.rushfo...@oracle.com> wrote:
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$>
