On Thu, 26 Jan 2023 09:54:25 GMT, Florian Kirmaier <fkirma...@openjdk.org>
wrote:
>> After thinking about this issue for some time, I've now got a solution.
>> I know put the scene in the state it is, before is was shown, when the
>> dirtyNodes are unset, the whole scene is basically considered dirty.
>> This has the drawback of rerendering, whenever a window is "reshown", but it
>> restores sanity about memory behaviour, which should be considered more
>> important.
>
> Florian Kirmaier has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains eight commits:
>
> - JDK-8269907
> Added missing changes after merge
> - Merge remote-tracking branch 'origjfx/master' into
> JDK-8269907-dirty-and-removed
>
> # Conflicts:
> # modules/javafx.graphics/src/main/java/com/sun/javafx/tk/Toolkit.java
> # modules/javafx.graphics/src/main/java/javafx/scene/Scene.java
> - Merge remote-tracking branch 'origin/master'
> - JDK-8269907
> Removed the sync methods for the scene, because they don't work when peer
> is null, and they are not necessary.
> - JDK-8269907
> Fixed rare bug, causing bounds to be out of sync.
> - JDK-8269907
> We now require the rendering lock when cleaning up dirty nodes. To do so,
> we moved some code required for snapshot into a reusable method.
> - JDK-8269907
> The bug is now fixed in a new way. Toolkit now supports registering
> CleanupListeners, which can clean up the dirty nodes, avoiding memoryleaks.
> - JDK-8269907
> Fixing dirty nodes and parent removed, when a window is no longer showing.
> This typically happens with context menus.
modules/javafx.graphics/src/main/java/com/sun/javafx/tk/Toolkit.java line 492:
> 490: }
> 491:
> 492: public void addCleanupListener(TKPulseListener listener) {
This name is misleading. It adds a listener which gets auto removed when called
(and hence why there is no `removeCleanupListener` counterpart). In effect, it
works a bit like `runLater`, a one time action to be run on the next pulse.
In general, I would like to see more comments/docs to explain what is going on.
modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 746:
> 744: if(!cleanupAdded) {
> 745: if((window.get() == null || !window.get().isShowing()) &&
> dirtyNodesSize > 0) {
> 746: Toolkit.getToolkit().addCleanupListener(cleanupListener);
This adds a listener to be run on the next pulse, yet no pulse is being
requested. Probably something else will request a pulse, but seems like you may
not want to rely on that. Specifically, `addToDirtyList` won't request a pulse
if the stage is already closed (peer is `null`), and only then do you remove
some nodes. It still works though in that case, probably because something
else in JavaFX is requesting a pulse (nothing in `Scene` does though when peer
is `null`).
modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 1304:
> 1302: }
> 1303:
> 1304: private void synchronizeSceneNodesWithLock() {
The name is confusing, it seems to indicate that you shouldn't call this if you
don't have the lock, but instead it does the locking itself. The `WithLock`
part should just be dropped as it can be called at any time, anywhere AFAICS,
and that this may involving locking does not need to expressed in the name.
-------------
PR: https://git.openjdk.org/jfx/pull/584
