On Tue, 25 Feb 2025 19:25:25 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> tests/system/src/test/java/test/robot/javafx/scene/NodeInitializationStressTest.java
>> line 227:
>>
>>> 225: public class NodeInitializationStressTest extends RobotTestBase {
>>> 226: /* debugging aid: set this flag to true and comment out
>>> assumeFalse(SKIP_TEST) to run specific test(s). */
>>> 227: private static final boolean SKIP_TEST = false;
>>
>> Maybe it's time to remove this flag? Does Eclipse not provide a way to run a
>> single test?
>
> Yes, the problem in Eclipse is that it creates a launch configuration which
> is always incorrect (the dependencies must be modified for each new
> configuration).
> I'd like to keep it. There is very little overhead.
OK. Given how long-running this test is I can see the value when debugging or
adding / modifying tests.
>> tests/system/src/test/java/test/robot/javafx/scene/NodeInitializationStressTest.java
>> line 656:
>>
>>> 654:
>>> 655: @Test
>>> 656: public void imageView() {
>>
>> You mentioned that there was a problem with modifying a writable image on a
>> background thread. Similarly, there might be a problem loading animated GIF
>> images on a background thread. Is there a bug filed?
>
> I guess it works as designed, see ImageView:254
>
>
> Toolkit.getImageAccessor().getImageProperty(_image).addListener(platformImageChangeListener.getWeakListener());
>
> so an animated GIF would most likely be off limits from a background thread
> as well.
I don't think it's intentional. See
[JDK-8222211](https://bugs.openjdk.org/browse/JDK-8222211) where we fixed a
problem with not being able to construct an image on a background thread. It's
quite possible that the fix for JDK-8222211 is insufficient. So I think a
follow-up bug is in order.
A possible fix would be to defer both the animation and setting up image
ChangeListener until we know that it is on the FX app thread (similar to what
you did for Pagination).
>> tests/system/src/test/java/test/robot/javafx/scene/NodeInitializationStressTest.java
>> line 737:
>>
>>> 735: c.setFitHeight(nextDouble(200));
>>> 736: c.setFitWidth(nextDouble(200));
>>> 737: //c.setMediaPlayer(new MediaPlayer(new
>>> Media("no-data-url-support")));
>>
>> Would it be useful to load a media file? If so, you could use the default
>> media file that Ensemble uses.
>
> I don't think we want to download media from an URL as part of this test, and
> data URLs are not supported by Media (and it will be too long anyway).
>
> I think we can classify this under rubric "one can only create a JavaFX
> object in a background thread, but not utilize it".
That seems reasonable.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1713#discussion_r1970558804
PR Review Comment: https://git.openjdk.org/jfx/pull/1713#discussion_r1970568328
PR Review Comment: https://git.openjdk.org/jfx/pull/1713#discussion_r1970576260
