Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-05 Thread Nir Lisker
On Wed, 30 Nov 2022 12:55:08 GMT, Kevin Rushforth wrote: >> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/UploadingPainter.java >> line 41: >> >>> 39: * The PresentingPainter is used when we are rendering to the main >>> screen. >>> 40: */ >>> 41: final class UploadingPaint

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v2]

2022-12-05 Thread Nir Lisker
On Wed, 30 Nov 2022 12:48:10 GMT, Kevin Rushforth wrote: >> If the casts in the numerator actually matter, then the cast in the >> denominator can be removed. The latter are the ones that Eclipse flags for >> me as unnecessary. > > I still think that any change here would be a very low value ch

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-05 Thread Nir Lisker
On Mon, 5 Dec 2022 22:20:59 GMT, John Hendrikx wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix indentations and merge short lines > >> This is one giant pull request, somewhat difficult on the reviewers, >> e

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-05 Thread Nir Lisker
On Tue, 6 Dec 2022 03:03:13 GMT, Andy Goryachev wrote: >> The code was already broken (since 2014 apparently when the new Base64 >> decoder was used). Had the author turned on these handy warnings, they >> would have noticed that something was amiss. Now they unwittingly changed >> the behav

Re: RFR: 8296409: Multiple copies of accelerator change listeners are added to MenuItems, but only 1 is removed [v2]

2022-12-05 Thread Ambarish Rapte
On Mon, 7 Nov 2022 12:03:38 GMT, Dean Wookey wrote: >> When menu buttons are added and removed from the scene, an accelerator >> change listener is added to each menu item in the menu. There is nothing >> stopping the same change listener being added multiple times. >> >> MenuButtonSkinBase ca

Re: RFR: 8296621: Stage steals focus on scene change [v4]

2022-12-05 Thread Ambarish Rapte
On Mon, 5 Dec 2022 21:03:09 GMT, Thiago Milczarek Sayao wrote: >> Simple fix to not requestFocus() on scene change. >> >> The attached bug sample shows that the TextField focused on the scene >> remains focused when the scene comes back. > > Thiago Milczarek Sayao has updated the pull request

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-05 Thread Andy Goryachev
On Mon, 5 Dec 2022 22:00:01 GMT, John Hendrikx wrote: >> modules/javafx.graphics/src/main/java/com/sun/javafx/application/LauncherImpl.java >> line 601: >> >>> 599: } >>> 600: args.add("--" + k + "=" + (v != null ? v : "")); >>> 601: idx++; >> >> I suspect t

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-05 Thread Andy Goryachev
On Sat, 3 Dec 2022 20:54:04 GMT, John Hendrikx wrote: >> - Remove unsupported/unnecessary SuppressWarning annotations >> - Remove reduntant type specifications (use diamond operator) >> - Remove unused or duplicate imports >> - Remove unnecessary casts (type is already correct type or can be auto

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-05 Thread John Hendrikx
On Sat, 3 Dec 2022 20:54:04 GMT, John Hendrikx wrote: >> - Remove unsupported/unnecessary SuppressWarning annotations >> - Remove reduntant type specifications (use diamond operator) >> - Remove unused or duplicate imports >> - Remove unnecessary casts (type is already correct type or can be auto

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-05 Thread John Hendrikx
On Mon, 5 Dec 2022 19:59:45 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix indentations and merge short lines > > modules/javafx.graphics/src/main/java/com/sun/javafx/application/LauncherI

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-05 Thread Nir Lisker
On Mon, 5 Dec 2022 20:17:04 GMT, Andy Goryachev wrote: >> I do agree with @kevinrushforth here. >> >> It might be borderline useful to enable the warning once just to see if >> there are any bugs or perhaps to clean up the most obvious cases, but it >> probably makes little sense to enable the

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-05 Thread Nir Lisker
On Mon, 5 Dec 2022 21:40:40 GMT, John Hendrikx wrote: >> modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFile.java >> line 586: >> >>> 584: ascent = -(float)hhea.getShort(4); >>> 585: descent = -(float)hhea.getShort(6); >>> 586:

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-05 Thread Nir Lisker
On Mon, 5 Dec 2022 21:46:12 GMT, John Hendrikx wrote: >> modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 2579: >> >>> 2577: term = nextLayer(lastTerm); >>> 2578: } >>> 2579: return new ParsedValueImpl<>(layers, >>> CornerRadiiConverter.getInstanc

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-05 Thread John Hendrikx
On Mon, 5 Dec 2022 20:37:09 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix indentations and merge short lines > > modules/javafx.graphics/src/main/java/com/sun/prism/d3d/D3DContext.java li

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-05 Thread John Hendrikx
On Mon, 5 Dec 2022 20:11:53 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix indentations and merge short lines > > modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFile.ja

RFR: 8296854: NULL check of CTFontCopyAvailableTables return value is required

2022-12-05 Thread Phil Race
Guard against de-referencing null, although it is currently theoretical as far as I can see (more info in the bug eval) - Commit messages: - 8296854 Changes: https://git.openjdk.org/jfx/pull/968/files Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=968&range=00 Issue: https://b

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-05 Thread Andy Goryachev
On Tue, 22 Nov 2022 22:34:42 GMT, Andy Goryachev wrote: >> I don't see it as a useful warning. The code isn't necessarily better >> without, and can be less clear. I think this might argue for asking Eclipse >> users to disable this warning in the IDE. > > I do agree with @kevinrushforth here.

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-05 Thread Andy Goryachev
On Sat, 3 Dec 2022 20:54:04 GMT, John Hendrikx wrote: >> - Remove unsupported/unnecessary SuppressWarning annotations >> - Remove reduntant type specifications (use diamond operator) >> - Remove unused or duplicate imports >> - Remove unnecessary casts (type is already correct type or can be auto

Re: RFR: 8295324: JavaFX: Blank pages when printing [v4]

2022-12-05 Thread Kevin Rushforth
On Thu, 1 Dec 2022 10:12:26 GMT, eduardsdv wrote: >> This fixes a race condition between application and 'Print Job Thread' >> threads when printing. >> >> The race condition occurs when application thread calls `endJob()`, which in >> effect sets the `jobDone` flag to true, >> and when the 'P

Re: RFR: 8296621: Stage steals focus on scene change [v4]

2022-12-05 Thread Thiago Milczarek Sayao
> Simple fix to not requestFocus() on scene change. > > The attached bug sample shows that the TextField focused on the scene remains > focused when the scene comes back. Thiago Milczarek Sayao has updated the pull request incrementally with one additional commit since the last revision: Add

Re: RFR: 8296621: Stage steals focus on scene change [v2]

2022-12-05 Thread Thiago Milczarek Sayao
On Mon, 5 Dec 2022 16:03:25 GMT, Michael Strauß wrote: >> Thiago Milczarek Sayao has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Add test > > tests/system/src/test/java/test/robot/javafx/scene/SceneChangeShouldNotFocusStageTest.java > l

Re: RFR: 8296621: Stage steals focus on scene change [v3]

2022-12-05 Thread Thiago Milczarek Sayao
> Simple fix to not requestFocus() on scene change. > > The attached bug sample shows that the TextField focused on the scene remains > focused when the scene comes back. Thiago Milczarek Sayao has updated the pull request incrementally with one additional commit since the last revision: Use

Re: RFR: 8187145: (Tree)TableView with null selectionModel: throws NPE on sorting [v13]

2022-12-05 Thread Andy Goryachev
> Setting a null selection model in TableView and TreeTableView produce NPE on > sorting (and probably in some other situations) because the check for null is > missing in several places. > > Setting a null selection model is a valid way to disable selection in a > (tree)table. > > There is a

Re: RFR: 8190411: NPE in SliderSkin:140 if Slider.Tooltip.autohide is true [v4]

2022-12-05 Thread Andy Goryachev
On Mon, 5 Dec 2022 12:48:14 GMT, Karthik P K wrote: >> Cause: When slider is dragged for first time after tooltip appears, >> setOnMousePressed event was not invoked, hence dragStart was null in the >> subsequently invoked event handler (setOnMouseDragged). >> >> Fix: Initialized dragStart in

Re: RFR: 8190411: NPE in SliderSkin:140 if Slider.Tooltip.autohide is true [v4]

2022-12-05 Thread Andy Goryachev
On Mon, 5 Dec 2022 16:57:05 GMT, Andy Goryachev wrote: >> Karthik P K has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update in mouse enter and exit events. Update in unit test > > modules/javafx.controls/src/main/java/javafx/scene/contr

Re: RFR: 8190411: NPE in SliderSkin:140 if Slider.Tooltip.autohide is true [v4]

2022-12-05 Thread Andy Goryachev
On Mon, 5 Dec 2022 12:48:14 GMT, Karthik P K wrote: >> Cause: When slider is dragged for first time after tooltip appears, >> setOnMousePressed event was not invoked, hence dragStart was null in the >> subsequently invoked event handler (setOnMouseDragged). >> >> Fix: Initialized dragStart in

RFR: 8295809: TreeTableViewSkin: memory leak when changing skin

2022-12-05 Thread Andy Goryachev
as determined by SkinMemoryLeakTest (remove line 180) and a leak tester https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java caused by: - adding and not removing listeners - adding and not removing event handlers/filters - adding and not removing cell factory -

Re: RFR: 8295809: TreeTableViewSkin: memory leak when changing skin

2022-12-05 Thread Andy Goryachev
On Mon, 24 Oct 2022 19:06:26 GMT, Andy Goryachev wrote: > as determined by SkinMemoryLeakTest (remove line 180) and a leak tester > https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java > > caused by: > - adding and not removing listeners > - adding and not rem

Re: RFR: 8296621: Stage steals focus on scene change [v2]

2022-12-05 Thread Kevin Rushforth
On Sun, 13 Nov 2022 01:02:36 GMT, Thiago Milczarek Sayao wrote: >> Simple fix to not requestFocus() on scene change. >> >> The attached bug sample shows that the TextField focused on the scene >> remains focused when the scene comes back. > > Thiago Milczarek Sayao has updated the pull request

Re: RFR: 8295531: ComboBoxBaseSkin: memory leak when changing skin [v5]

2022-12-05 Thread Andy Goryachev
On Fri, 2 Dec 2022 17:35:05 GMT, Andy Goryachev wrote: >> as determined by SkinMemoryLeakTest (remove line 166) and a leak tester >> https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java >> >> Affected skins: >> - ColorPicker >> - DatePicker >> - ComboBox >> >

Re: RFR: 8245145: Spinner: throws IllegalArgumentException when replacing skin [v3]

2022-12-05 Thread Andy Goryachev
On Fri, 2 Dec 2022 17:42:00 GMT, Andy Goryachev wrote: >> Fixed SpinnerSkin initialization using new Skin.install(). Also in this PR >> - fixing memory leaks in SpinnerSkin by properly removing all listeners and >> event filters added in the constructor, as well as any child Nodes. >> >> NOTE

Re: RFR: 8295175: SplitPaneSkin: memory leak when changing skin [v5]

2022-12-05 Thread Andy Goryachev
> Fixed memory leak by removing all the listeners in dispose(); > > This PR depends on a new internal class ListenerHelper, a replacement for > LambdaMultiplePropertyChangeListenerHandler. > See https://github.com/openjdk/jfx/pull/908 Andy Goryachev has updated the pull request with a new target

Re: RFR: 8295175: SplitPaneSkin: memory leak when changing skin [v4]

2022-12-05 Thread Andy Goryachev
On Mon, 5 Dec 2022 12:05:16 GMT, Ajit Ghaisas wrote: >> Andy Goryachev has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 29 commits: >> >> - Merge remote-tracking branch 'origin/master' into >>8295175.splitpaneskin.with.helper

Integrated: 8295806: TableViewSkin: memory leak when changing skin

2022-12-05 Thread Andy Goryachev
On Fri, 21 Oct 2022 23:21:52 GMT, Andy Goryachev wrote: > as determined by SkinMemoryLeakTest (remove line 179) and a leak tester > https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java > > caused by: > - adding and not removing listeners > - adding and not rem

Re: RFR: 8296621: Stage steals focus on scene change [v2]

2022-12-05 Thread Michael Strauß
On Mon, 5 Dec 2022 08:19:59 GMT, Ambarish Rapte wrote: > After the fix, the behavior looks similar on all platform, but this way the > user does not get any notification of the Scene change in Application's > content. i.e. the behavior change on Windows(icon not flashing) and Mac(not > bringin

Integrated: 8294589: MenuBarSkin: memory leak when changing skin

2022-12-05 Thread Andy Goryachev
On Thu, 29 Sep 2022 23:00:17 GMT, Andy Goryachev wrote: > Fixed memory leak by using weak listeners and making sure to undo everything > that has been done to MenuBar/Skin in dispose(). > > This PR depends on a new internal class ListenerHelper, a replacement for > LambdaMultiplePropertyChange

Re: RFR: 8296621: Stage steals focus on scene change [v2]

2022-12-05 Thread Michael Strauß
On Sun, 13 Nov 2022 01:02:36 GMT, Thiago Milczarek Sayao wrote: >> Simple fix to not requestFocus() on scene change. >> >> The attached bug sample shows that the TextField focused on the scene >> remains focused when the scene comes back. > > Thiago Milczarek Sayao has updated the pull request

Re: RFR: 8296621: Stage steals focus on scene change [v2]

2022-12-05 Thread Thiago Milczarek Sayao
On Sun, 13 Nov 2022 01:02:36 GMT, Thiago Milczarek Sayao wrote: >> Simple fix to not requestFocus() on scene change. >> >> The attached bug sample shows that the TextField focused on the scene >> remains focused when the scene comes back. > > Thiago Milczarek Sayao has updated the pull request

Re: RFR: 8190411: NPE in SliderSkin:140 if Slider.Tooltip.autohide is true [v4]

2022-12-05 Thread Karthik P K
> Cause: When slider is dragged for first time after tooltip appears, > setOnMousePressed event was not invoked, hence dragStart was null in the > subsequently invoked event handler (setOnMouseDragged). > > Fix: Initialized dragStart in initialize method. > > Test: Added system test to validate

RFR: JDK-8298060: Fix precision bug in gesture recognizer classes

2022-12-05 Thread John Hendrikx
This includes a fix for the precision problem we've found as part of the graphics warnings clean ups. I've included two commits, one with just the minimal fix, and one with the clean ups. I can drop off the 2nd commit if it is deemed to be of no added value. - Commit messages: -

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin [v22]

2022-12-05 Thread John Hendrikx
On Fri, 2 Dec 2022 17:25:16 GMT, Andy Goryachev wrote: >> Fixed memory leak by using weak listeners and making sure to undo everything >> that has been done to MenuBar/Skin in dispose(). >> >> This PR depends on a new internal class ListenerHelper, a replacement for >> LambdaMultiplePropertyCh

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin [v22]

2022-12-05 Thread Ajit Ghaisas
On Fri, 2 Dec 2022 17:25:16 GMT, Andy Goryachev wrote: >> Fixed memory leak by using weak listeners and making sure to undo everything >> that has been done to MenuBar/Skin in dispose(). >> >> This PR depends on a new internal class ListenerHelper, a replacement for >> LambdaMultiplePropertyCh

Re: RFR: 8296621: Stage steals focus on scene change [v2]

2022-12-05 Thread Thiago Milczarek Sayao
On Sun, 13 Nov 2022 01:02:36 GMT, Thiago Milczarek Sayao wrote: >> Simple fix to not requestFocus() on scene change. >> >> The attached bug sample shows that the TextField focused on the scene >> remains focused when the scene comes back. > > Thiago Milczarek Sayao has updated the pull request

Integrated: 8238968: Inconsisent formatting of Rectangle2D toString method

2022-12-05 Thread Lukasz Kostyra
On Mon, 5 Dec 2022 10:50:56 GMT, Lukasz Kostyra wrote: > The string was fixed to match other parameters' formatting. > > There was a PR submitted for this change a long time ago, but there were some > issue with its previous author. Today I found the issue being assigned to me, > so I figured

Re: Warnings

2022-12-05 Thread Nir Lisker
I have been working with John on the effort to remove warnings, so obviously I agree with these. I'd like to add a few points: * There is flexibility in how to tell the comp[iler to mark these possible problems. Some warnings can be turned into errors for stricter standards, and I think that this

Re: RFR: 8295175: SplitPaneSkin: memory leak when changing skin [v4]

2022-12-05 Thread Ajit Ghaisas
On Fri, 2 Dec 2022 17:28:13 GMT, Andy Goryachev wrote: >> Fixed memory leak by removing all the listeners in dispose(); >> >> This PR depends on a new internal class ListenerHelper, a replacement for >> LambdaMultiplePropertyChangeListenerHandler. >> See https://github.com/openjdk/jfx/pull/908

Re: RFR: 8238968: Inconsisent formatting of Rectangle2D toString method

2022-12-05 Thread Nir Lisker
On Mon, 5 Dec 2022 10:50:56 GMT, Lukasz Kostyra wrote: > The string was fixed to match other parameters' formatting. > > There was a PR submitted for this change a long time ago, but there were some > issue with its previous author. Today I found the issue being assigned to me, > so I figured

Re: RFR: 8295806: TableViewSkin: memory leak when changing skin [v3]

2022-12-05 Thread Ajit Ghaisas
On Fri, 2 Dec 2022 17:45:23 GMT, Andy Goryachev wrote: >> as determined by SkinMemoryLeakTest (remove line 179) and a leak tester >> https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java >> >> caused by: >> - adding and not removing listeners >> - adding and no

RFR: 8238968: Inconsisent formatting of Rectangle2D toString method

2022-12-05 Thread Lukasz Kostyra
The string was fixed to match other parameters' formatting. There was a PR submitted for this change a long time ago, but there were some issue with its previous author. Today I found the issue being assigned to me, so I figured I might as well submit a patch for it as it was a quick & easy fix.

Re: RFR: 8190411: NPE in SliderSkin:140 if Slider.Tooltip.autohide is true [v3]

2022-12-05 Thread Karthik P K
> Cause: When slider is dragged for first time after tooltip appears, > setOnMousePressed event was not invoked, hence dragStart was null in the > subsequently invoked event handler (setOnMouseDragged). > > Fix: Initialized dragStart in initialize method. > > Test: Added system test to validate

Re: RFR: 8296621: Stage steals focus on scene change [v2]

2022-12-05 Thread Ambarish Rapte
On Sun, 13 Nov 2022 01:02:36 GMT, Thiago Milczarek Sayao wrote: >> Simple fix to not requestFocus() on scene change. >> >> The attached bug sample shows that the TextField focused on the scene >> remains focused when the scene comes back. > > Thiago Milczarek Sayao has updated the pull request