Re: BaselineOffset of StackPane is inconsistent
I determined this is caused by the StackPane's child node's layoutY value
sometimes being 0.0 and sometimes being 7.5. Which is strange, since
layout should always be complete on the TreeCells by the time they are
painted.
Which got me thinking.. sure maybe on one pass it wasn't initialized or
something, but why would it keep changing?
Then I learned that updateItem is being called much more often than I had
thought. I had previously thought it was only called when the TreeCell was
meant to render a new or different TreeItem. Apparently it is called when
the TreeItem selection changes. I'm not sure why, as no parameters to
updateItem include the selection state, so a TreeCell doesn't know that is
why it is being called, and the documentation and sample code doesn't
mention it.
In fact the documentation for Cell isItemChanged specifically states:
* This method is called by Cell subclasses so that certain
CPU-intensive
* actions (specifically, calling {@link #updateItem(Object, boolean)})
are
* only performed when necessary
*(that is, they are only performed * when the currently set {@link
#itemProperty() item} is considered to be * different than the proposed
new item that could be set).*
Which implies to me (along with the method name "updateItem") that
assigning a different item to the Cell is the only reason for updateItem to
be called.
This is clearly not the case.
Another observation is that, while changing the selected item such that I
observe the misalignment, the last time getBaselineOffset is called for any
particular cell while processing that event, it always returns the same
value of 7.5, but it seems it doesn't always use this value for when it
paints.
E.g. if I toggle the selection of a cell, updateItem is called once,
creating new Nodes for the Cell's graphic, and getBaselineOffset for the
StackPane is called 4 times. The first 3 times the first managed child of
the StackPane has layoutY = 0.0, the last time it is 7.5.
There's a bug here somewhere...
Scott
On Tue, Jan 24, 2023 at 1:43 PM Scott Palmer wrote:
> I'm seeing something odd with the alignment of content in a TextFlow and
> I'm not sure if I'm doing something wrong, or if there is a bug there.
> The getBaselineOffset() method of the StackPane is returning inconsistent
> values. If I sub-class it to force a constant offset, everything works.
> Since the StackPane content is always the same, I would expect
> getBaselineOffset to always return the same value, but in my sample program
> (below) sometimes it returns 6.5 and other times it returns 14.0. (Tested
> on macOS 13.2 w. OpenJDK 19/OpenJFX19)
> Parent.getBaselineOffset() is documented as: "Calculates the baseline
> offset based on the first managed child." - which should always be the same
> in my example. Sure enough, I checked and
> getChildren().get(0).getBaselineOffset() is always returning the same value
> (13.0) - so where is the discrepancy coming from?
>
> Possibly related to https://bugs.openjdk.org/browse/JDK-8091236
>
> The following program demonstrates the issue. While selecting things in
> the TreeView the TextFlows are repainted with different alignment between
> the StackPane node and the Text nodes.
>
> package bugs;
>
> import javafx.application.Application;
> import javafx.scene.Scene;
> import javafx.scene.control.ContentDisplay;
> import javafx.scene.control.Label;
> import javafx.scene.control.TreeCell;
> import javafx.scene.control.TreeItem;
> import javafx.scene.control.TreeView;
> import javafx.scene.layout.StackPane;
> import javafx.scene.layout.VBox;
> import javafx.scene.paint.Color;
> import javafx.scene.shape.Circle;
> import javafx.scene.shape.Polygon;
> import javafx.scene.text.Text;
> import javafx.scene.text.TextFlow;
> import javafx.stage.Stage;
>
> public class TextFlowWithIcons extends Application {
> public static void main(String[] args) {
> launch(args);
> }
>
> @Override
> public void start(Stage primaryStage) {
> TreeItem node1 = new TreeItem<>("item");
> TreeItem node2 = new TreeItem<>("text");
> TreeItem node3 = new TreeItem<>("tree");
> TreeItem root = new TreeItem<>("ROOT");
> root.setExpanded(true);
> root.getChildren().addAll(node1, node2, node3);
> var treeView = new TreeView(root);
> treeView.setCellFactory(this::cellFactory);
> VBox box = new VBox(8, new Label("Fiddle with the
> TreeView...\nWatch the alignment bounce around."), treeView);
> var scene = new Scene(box);
> primaryStage.setScene(scene);
> primaryStage.setTitle("Graphic Alignment Issue");
> primaryStage.show();
> }
>
> private TreeCell cellFactory(TreeView tv) {
> return new CustomCell();
> }
>
> private static class CustomCell extends TreeCell {
>
> public CustomCell() {
> setContentDisplay(ContentDisplay.GRAPHIC_ONLY);
> }
>
> @Override
>
StageStyle.TOOLKIT_DECORATED
I have begun to implement a StageStyle variation called TOOLKIT_DECORATED (better naming welcome). It would be JavaFX's version of: https://docs.gtk.org/gtk4/class.HeaderBar.html The goal is to allow a Toolkit decorated window that accepts controls inside the decoration such as a "hamburger menu", tabs (like firefox) or anything else. On Linux it would depend on: https://docs.gtk.org/gdk3/method.Window.begin_move_drag.html That's because the Window Manager would block moving it within desktop constraints, unless using this function. It would auto-style (as modena does) buttons and other controls inside it. I don't promise I will ever finish this, because I do it for fun and I have no time for fun :) I know I have to do a feature proposal and discussion and It might not be accepted. Cheers -- Thiago.
Re: Q: missing APIs needed for implementation of a rich text control
Many of the items below (meaning excluding the caret features and app control of line spacing) are on my list of things to work on. phil On 1/26/23 12:33 PM, Scott Palmer wrote: [dupe of private message, now including the mailing list] I've been using RichTextFX to make my own code editor/IDE. I see that you have asked that project specifically on GitHub, excellent. One thing that I wanted to do was to use a font like Fira Code that combines characters such as != or >= into a single glyph, but there are no APIs to request that JavaFX render those optional ligatures. Swing APIs automatically do. In fact just implementing some basic font selection has been tedious. Getting the font family from a Font object and trying to use it in CSS to specify -fx-font-family simply doesn't work sometimes. Trying to filter the list of available fonts to show only those that are fixed-width is tedious. I tried to hack something by measuring a couple different characters from the font that would normally be different widths, but this is an unreliable hack. Allowing the user to choose a preferred font and then configuring components to use it via a CSS stylesheet is simply more difficult than it should be. A few years ago I contributed the changes to make the tab-width configurable, which helped with my project a little bit. Other APIs are needed to better control line spacing and measure font baseline offsets and that sort of thing. I see there is an issue for caretBlinkRate, what about changing the caret shape? E.g. block, underscore, vertical bar, etc. Should the block be solid or an outline? Why is caretShape read-only? Why does it return an array of PathElements instead of simply a Shape which by default would be a Path? More control of kerning and general spacing might be nice. I've wanted that in the past, not for a rich text control, but for doing video titles. I can see how the two needs overlap though. I recently asked here about rendering emojis. That's currently very unreliable and broken. Getting something to work consistently cross-platform is not easy. Sometimes emojis are rendered in color, other times not. Mac renders color emojis in gray and at the wrong size (since the sub-pixel rendering was turned off to match macOS). On Windows the emojis are never in color. Regards, Scott On Wed, Jan 25, 2023 at 10:41 PM Scott Palmer wrote: I've been using RichTextFX to make my own code editor/IDE. I see that you have asked that project specifically on GitHub, excellent. One thing that I wanted to do was to use a font like Fira Code that combines characters such as != or >= into a single glyph, but there are no APIs to request that JavaFX render those optional ligatures. Swing APIs automatically do. In fact just implementing some basic font selection has been tedious. Getting the font family from a Font object and trying to use it in CSS to specify -fx-font-family simply doesn't work sometimes. Trying to filter the list of available fonts to show only those that are fixed-width is tedious. I tried to hack something by measuring a couple different characters from the font that would normally be different widths, but this is an unreliable hack. Allowing the user to choose a preferred font and then configuring components to use it via a CSS stylesheet is simply more difficult than it should be. A few years ago I contributed the changes to make the tab-width configurable, which helped with my project a little bit. Other APIs are needed to better control line spacing and measure font baseline offsets and that sort of thing. I see there is an issue for caretBlinkRate, what about changing the caret shape? E.g. block, underscore, vertical bar, etc. Should the block be solid or an outline? Why is caretShape read-only? Why does it return an array of PathElements instead of simply a Shape which by default would be a Path? More control of kerning and general spacing might be nice. I've wanted that in the past, not for a rich text control, but for doing video title. I can see how the two needs overlap though. I recently asked here about rendering emojis. That's currently very unreliable and broken. Getting something to work consistently cross-platform is not easy. Sometimes emojis are rendered in color, other times not. Mac renders color emojis in gray and at the wrong size (since the sub-pixel rendering was turned off to match macOS). On Windows the emojis are never in color. Regards, Scott On Wed, Jan 25, 2023 at 5:38 PM Andy Goryachev wrote: Dear colleagues: I am trying to identify missing public APIs needed to support a rich text control. There is a number of tickets created already against various parts of JavaFX, collected in https://bugs.openjdk.org/browse
Re: Q: missing APIs needed for implementation of a rich text control
[dupe of private message, now including the mailing list] I've been using RichTextFX to make my own code editor/IDE. I see that you have asked that project specifically on GitHub, excellent. One thing that I wanted to do was to use a font like Fira Code that combines characters such as != or >= into a single glyph, but there are no APIs to request that JavaFX render those optional ligatures. Swing APIs automatically do. In fact just implementing some basic font selection has been tedious. Getting the font family from a Font object and trying to use it in CSS to specify -fx-font-family simply doesn't work sometimes. Trying to filter the list of available fonts to show only those that are fixed-width is tedious. I tried to hack something by measuring a couple different characters from the font that would normally be different widths, but this is an unreliable hack. Allowing the user to choose a preferred font and then configuring components to use it via a CSS stylesheet is simply more difficult than it should be. A few years ago I contributed the changes to make the tab-width configurable, which helped with my project a little bit. Other APIs are needed to better control line spacing and measure font baseline offsets and that sort of thing. I see there is an issue for caretBlinkRate, what about changing the caret shape? E.g. block, underscore, vertical bar, etc. Should the block be solid or an outline? Why is caretShape read-only? Why does it return an array of PathElements instead of simply a Shape which by default would be a Path? More control of kerning and general spacing might be nice. I've wanted that in the past, not for a rich text control, but for doing video titles. I can see how the two needs overlap though. I recently asked here about rendering emojis. That's currently very unreliable and broken. Getting something to work consistently cross-platform is not easy. Sometimes emojis are rendered in color, other times not. Mac renders color emojis in gray and at the wrong size (since the sub-pixel rendering was turned off to match macOS). On Windows the emojis are never in color. Regards, Scott On Wed, Jan 25, 2023 at 10:41 PM Scott Palmer wrote: > I've been using RichTextFX to make my own code editor/IDE. I see that you > have asked that project specifically on GitHub, excellent. > One thing that I wanted to do was to use a font like Fira Code that > combines characters such as != or >= into a single glyph, but there are no > APIs to request that JavaFX render those optional ligatures. Swing APIs > automatically do. In fact just implementing some basic font selection has > been tedious. Getting the font family from a Font object and trying to use > it in CSS to specify -fx-font-family simply doesn't work sometimes. Trying > to filter the list of available fonts to show only those that are > fixed-width is tedious. I tried to hack something by measuring a couple > different characters from the font that would normally be different widths, > but this is an unreliable hack. Allowing the user to choose a preferred > font and then configuring components to use it via a CSS stylesheet is > simply more difficult than it should be. > A few years ago I contributed the changes to make the tab-width > configurable, which helped with my project a little bit. Other APIs are > needed to better control line spacing and measure font baseline offsets and > that sort of thing. I see there is an issue for caretBlinkRate, what about > changing the caret shape? E.g. block, underscore, vertical bar, etc. > Should the block be solid or an outline? Why is caretShape read-only? Why > does it return an array of PathElements instead of simply a Shape which by > default would be a Path? More control of kerning and general spacing might > be nice. I've wanted that in the past, not for a rich text control, but for > doing video title. I can see how the two needs overlap though. I recently > asked here about rendering emojis. That's currently very unreliable and > broken. Getting something to work consistently cross-platform is not > easy. Sometimes emojis are rendered in color, other times not. Mac > renders color emojis in gray and at the wrong size (since the sub-pixel > rendering was turned off to match macOS). On Windows the emojis are never > in color. > > Regards, > > Scott > > On Wed, Jan 25, 2023 at 5:38 PM Andy Goryachev > wrote: > >> Dear colleagues: >> >> >> >> I am trying to identify missing public APIs needed to support a rich text >> control. There is a number of tickets created already against various >> parts of JavaFX, collected in https://bugs.openjdk.org/browse/JDK-8300569 >> , though I suspect this list may not be complete. >> >> >> >> If anyone has any suggestions or requests related to the new APIs, I >> would be very interested to learn the context, the reason these APIs are >> needed, and whether a workaround exists. >> >> >> >> Thank you in advance. >> >> >> >> Che
Re: RFR: 8290765: Remove parent disabled/treeVisible listeners [v2]
On Thu, 25 Aug 2022 22:18:44 GMT, Michael Strauß wrote: >> `Node` adds InvalidationListeners to its parent's `disabled` and >> `treeVisible` properties and calls its own `updateDisabled()` and >> `updateTreeVisible(boolean)` methods when the property values change. >> >> These listeners are not required, since `Node` can easily call the >> `updateDisabled()` and `updateTreeVisible(boolean)` methods on its children, >> saving the memory cost of maintaining listeners and bindings. > > Michael Strauß has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains five commits: > > - Replace null check by explicit underConstruction flag > - Merge branch 'master' into fixes/node-listener-cleanup > - Merge > - added tests > - Remove Node.disabled and Node.treeVisible listeners I've resolved some merge conflicts with the latest `master` branch. @arapte can you re-review? - PR: https://git.openjdk.org/jfx/pull/841
Re: RFR: 8290765: Remove parent disabled/treeVisible listeners [v3]
> `Node` adds InvalidationListeners to its parent's `disabled` and > `treeVisible` properties and calls its own `updateDisabled()` and > `updateTreeVisible(boolean)` methods when the property values change. > > These listeners are not required, since `Node` can easily call the > `updateDisabled()` and `updateTreeVisible(boolean)` methods on its children, > saving the memory cost of maintaining listeners and bindings. Michael Strauß has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits: - added javadoc - Merge branch 'master' into fixes/node-listener-cleanup # Conflicts: #modules/javafx.graphics/src/main/java/javafx/scene/Node.java - Replace null check by explicit underConstruction flag - Merge branch 'master' into fixes/node-listener-cleanup - Merge - added tests - Remove Node.disabled and Node.treeVisible listeners - Changes: https://git.openjdk.org/jfx/pull/841/files Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=841&range=02 Stats: 88 lines in 3 files changed: 62 ins; 10 del; 16 mod Patch: https://git.openjdk.org/jfx/pull/841.diff Fetch: git fetch https://git.openjdk.org/jfx pull/841/head:pull/841 PR: https://git.openjdk.org/jfx/pull/841
Re: [jfx20] RFR: 8300013: Node.focusWithin doesn't account for nested focused nodes
On Thu, 12 Jan 2023 12:38:51 GMT, Kevin Rushforth wrote: >> I think this should go into JavaFX 20. > >> I think this should go into JavaFX 20. > > I agree. In the (very likely) case it doesn't get reviewed before the fork, > I'll ask you to retarget it to the `jfx20` branch. @kevinrushforth @arapte With RDP1 ending soon, would you be willing to review this fix? - PR: https://git.openjdk.org/jfx/pull/993
Re: RFR: 8269907 memory leak - Dirty Nodes / Parent removed [v7]
On Thu, 26 Jan 2023 09:54:25 GMT, Florian Kirmaier
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.
I've left some comments.
modules/javafx.graphics/src/main/java/com/sun/javafx/tk/Toolkit.java line 420:
> 418: new WeakHashMap<>();
> 419: final Map cleanupList =
> 420: new WeakHashMap<>();
I wonder why we need `WeakHashMap` here. These maps are short-lived anyway,
since they're local variables.
modules/javafx.graphics/src/main/java/com/sun/javafx/tk/Toolkit.java line 494:
> 492: public void addCleanupListener(TKPulseListener listener) {
> 493: AccessControlContext acc = AccessController.getContext();
> 494: cleanupListeners.put(listener,acc);
Access to `cleanupListeners` is not synchronized on `this` here, but it is
synchronized in L426.
You should either synchronize each and every access, or none of it.
modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 745:
> 743: private void checkCleanDirtyNodes() {
> 744: if(!cleanupAdded) {
> 745: if((window.get() == null || !window.get().isShowing()) &&
> dirtyNodesSize > 0) {
Minor: space after `if`.
You could also reduce nesting by consolidating the two separate conditions.
modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 2112:
> 2110:
> **/
> 2111:
> 2112: private void windowForSceneChanged(Window oldWindow, Window window)
> {
This change doesn't seem necessary.
modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 2129:
> 2127: }
> 2128:
> 2129: private final ChangeListener sceneWindowShowingListener =
> (p, o, n) -> {checkCleanDirtyNodes(); } ;
Minor: should be no space before `;`.
You could also remove the curly braces.
tests/system/src/test/java/test/javafx/scene/DirtyNodesLeakTest.java line 35:
> 33: import junit.framework.Assert;
> 34: import org.junit.BeforeClass;
> 35: import org.junit.Test;
Since this is a new class, you should use the JUnit5 API.
tests/system/src/test/java/test/javafx/scene/DirtyNodesLeakTest.java line 44:
> 42: import static test.util.Util.TIMEOUT;
> 43:
> 44: public class DirtyNodesLeakTest {
Since this tests dirty nodes of a `Scene`, maybe you could use a name like
`Scene_dirtyNodesLeakTest`?
-
PR: https://git.openjdk.org/jfx/pull/584
Re: RFR: 8277848 Binding and Unbinding to List leads to memory leak [v9]
On Thu, 26 Jan 2023 08:54:44 GMT, Florian Kirmaier wrote: >> Making the initial listener of the ListProperty weak fixes the problem. >> The same is fixed for Set and Map. >> Due to a smart implementation, this is done without any performance drawback. >> (The trick is to have an object, which is both the WeakReference and the >> Changelistener) >> By implying the same trick to the InvalidationListener, this should even >> improve the performance of the collection properties. > > Florian Kirmaier has updated the pull request incrementally with one > additional commit since the last revision: > > JDK-8277848 > Removed print statements Marked as reviewed by mstrauss (Committer). - PR: https://git.openjdk.org/jfx/pull/689
Re: RFR: 8277848 Binding and Unbinding to List leads to memory leak [v9]
On Thu, 26 Jan 2023 08:54:44 GMT, Florian Kirmaier wrote: >> Making the initial listener of the ListProperty weak fixes the problem. >> The same is fixed for Set and Map. >> Due to a smart implementation, this is done without any performance drawback. >> (The trick is to have an object, which is both the WeakReference and the >> Changelistener) >> By implying the same trick to the InvalidationListener, this should even >> improve the performance of the collection properties. > > Florian Kirmaier has updated the pull request incrementally with one > additional commit since the last revision: > > JDK-8277848 > Removed print statements I wonder if this issue might be caused by [JDK-8299986](https://bugs.openjdk.org/browse/JDK-8299986) ? - PR: https://git.openjdk.org/jfx/pull/689
[jfx11u] RFR: 8301010: Change JavaFX release version to 11.0.19 in jfx11u
Bump release version to 11.0.19 in build.properties and .jcheck/conf Fix for JDK-8301010 - Commit messages: - Increase release version for JavaFX 11.0.19 Changes: https://git.openjdk.org/jfx11u/pull/125/files Webrev: https://webrevs.openjdk.org/?repo=jfx11u&pr=125&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8301010 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jfx11u/pull/125.diff Fetch: git fetch https://git.openjdk.org/jfx11u pull/125/head:pull/125 PR: https://git.openjdk.org/jfx11u/pull/125
[jfx17u] RFR: 8301011: Change JavaFX release version to 17.0.7 in jfx17u
increase release version in build.properties and jcheck configuration Fix for JDK-8301011 - Commit messages: - Bump release version to 17.0.7 Changes: https://git.openjdk.org/jfx17u/pull/104/files Webrev: https://webrevs.openjdk.org/?repo=jfx17u&pr=104&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8301011 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jfx17u/pull/104.diff Fetch: git fetch https://git.openjdk.org/jfx17u pull/104/head:pull/104 PR: https://git.openjdk.org/jfx17u/pull/104
Re: RFR: 8269907 memory leak - Dirty Nodes / Parent removed [v7]
On Thu, 26 Jan 2023 09:54:25 GMT, Florian Kirmaier 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. I've just merged with master again. It would be great if this could be reviewed at some time. - PR: https://git.openjdk.org/jfx/pull/584
Re: RFR: 8269907 memory leak - Dirty Nodes / Parent removed [v7]
> 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. - Changes: https://git.openjdk.org/jfx/pull/584/files Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=584&range=06 Stats: 130 lines in 3 files changed: 124 ins; 2 del; 4 mod Patch: https://git.openjdk.org/jfx/pull/584.diff Fetch: git fetch https://git.openjdk.org/jfx pull/584/head:pull/584 PR: https://git.openjdk.org/jfx/pull/584
Re: RFR: 8277848 Binding and Unbinding to List leads to memory leak [v8]
On Sun, 22 Jan 2023 01:28:46 GMT, Michael Strauß wrote:
>> 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:
>>
>> - Merge remote-tracking branch 'origjfx/master' into
>> JDK-8277848-list-binding-leak
>>
>># Conflicts:
>>#
>> modules/javafx.base/src/main/java/javafx/beans/property/ListPropertyBase.java
>>#
>> modules/javafx.base/src/main/java/javafx/beans/property/SetPropertyBase.java
>>#
>> modules/javafx.base/src/test/java/test/javafx/beans/property/SetPropertyBaseTest.java
>> - Merge remote-tracking branch 'origjfx/master' into
>> JDK-8277848-list-binding-leak
>> - JDK-8277848
>>Added tests to ensure no premature garbage collection is happening.
>> - JDK-8277848
>>Added 3 more tests to verify that a bug discussed in the PR does not
>> appear.
>> - JDK-8277848
>>Added the 3 requests whitespaces
>> - JDK-8277848
>>Further optimization based code review.
>>This Bugfix should now event improve the performance
>> - JDK-8277848
>>Added missing change
>> - JDK-8277848
>>Fixed memoryleak, when binding and unbinding a ListProperty. The same was
>> fixed for SetProperty and MapProperty.
>
> modules/javafx.base/src/test/java/test/javafx/beans/property/ListPropertyBaseTest.java
> line 824:
>
>> 822: JMemoryBuddy.memoryTest(checker -> {
>> 823: // given
>> 824: System.out.println("Start collection: " +
>> FXCollections.observableArrayList());
>
> By the way, can these `println` statements in various tests be removed?
Done! I've removed the 3 print statements!
-
PR: https://git.openjdk.org/jfx/pull/689
Re: RFR: 8277848 Binding and Unbinding to List leads to memory leak [v9]
> Making the initial listener of the ListProperty weak fixes the problem. > The same is fixed for Set and Map. > Due to a smart implementation, this is done without any performance drawback. > (The trick is to have an object, which is both the WeakReference and the > Changelistener) > By implying the same trick to the InvalidationListener, this should even > improve the performance of the collection properties. Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision: JDK-8277848 Removed print statements - Changes: - all: https://git.openjdk.org/jfx/pull/689/files - new: https://git.openjdk.org/jfx/pull/689/files/8c69689a..d9fccd22 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=689&range=08 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=689&range=07-08 Stats: 3 lines in 3 files changed: 0 ins; 3 del; 0 mod Patch: https://git.openjdk.org/jfx/pull/689.diff Fetch: git fetch https://git.openjdk.org/jfx pull/689/head:pull/689 PR: https://git.openjdk.org/jfx/pull/689
