Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed [v2]

2024-03-27 Thread Karthik P K
On Wed, 27 Mar 2024 23:21:34 GMT, Nir Lisker wrote: >> `forEach` is void, so we can not return a list afterwards. > > You don't need to return a list, you create it ahead of time like was done in > line 167 > > List indices = new ArrayList<>(); > > and the add the elements in `forEach`. > Why

Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed [v2]

2024-03-27 Thread Karthik P K
On Wed, 27 Mar 2024 23:24:51 GMT, Marius Hanl wrote: >>> In the java.util.stream package >>> [docs](https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/util/stream/package-summary.html#SideEffects) >>> it is mentioned that `forEach()` method operates only via side-effects. So >>>

Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed [v2]

2024-03-27 Thread Marius Hanl
On Wed, 27 Mar 2024 13:41:21 GMT, Nir Lisker wrote: > That's still 2 iterations. Yes, but one advantage here: We currently do `final List removed = new ArrayList<>(c.getRemovedSize());`, where we allocate a list with a size, that is probably too big since we filter the removed items. So with

Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed [v2]

2024-03-27 Thread Nir Lisker
On Wed, 27 Mar 2024 23:18:43 GMT, Marius Hanl wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java >> line 773: >> >>> 771: .collect(Collectors.toList()); >>> 772: >>> 773: sortedNewIndices.forEach(this::se

Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed [v2]

2024-03-27 Thread Marius Hanl
On Wed, 27 Mar 2024 13:51:46 GMT, Nir Lisker wrote: >> drmarmac has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove outdated comment > > modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java > lin

Re: RFR: JDK-8186188: TableColumHeader: initial auto-size broken if has graphic [v7]

2024-03-27 Thread Marius Hanl
> This PR fixes the issue that the initial column autosizing is wrong under > some circumstances. > The following things will break the initial autosizing: > - Bold Column text (that is where I initially found this problem) > - Another font / font size > - Graphic > > The reason is actually quite

Re: RFR: 8242553: IntegerSpinner and DoubleSpinner do not wrap around values correctly in some cases

2024-03-27 Thread Kevin Rushforth
On Wed, 27 Mar 2024 19:19:36 GMT, Andy Goryachev wrote: > Now, when wrapping occurs, should max/min be considered as valid steps? For integer values (including list indices), yes, since each integer (or list index) is a discrete value that can have the value min, max, or any value in between.

Re: RFR: JDK-8186188: TableColumHeader: initial auto-size broken if has graphic [v6]

2024-03-27 Thread Marius Hanl
> This PR fixes the issue that the initial column autosizing is wrong under > some circumstances. > The following things will break the initial autosizing: > - Bold Column text (that is where I initially found this problem) > - Another font / font size > - Graphic > > The reason is actually quite

Re: RFR: JDK-8186188: TableColumHeader: initial auto-size broken if has graphic [v5]

2024-03-27 Thread Marius Hanl
> This PR fixes the issue that the initial column autosizing is wrong under > some circumstances. > The following things will break the initial autosizing: > - Bold Column text (that is where I initially found this problem) > - Another font / font size > - Graphic > > The reason is actually quite

Re: RFR: 8327179: Update the 3D lighting application [v6]

2024-03-27 Thread Kevin Rushforth
On Wed, 27 Mar 2024 21:22:47 GMT, Nir Lisker wrote: >> Update for the 3D lighting test tool as described in the JBS issue. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Whitespace? build.gradle line 4049: > 4047: project(

Integrated: JDK-8328750: [TestBug] Improve Stub Font Support

2024-03-27 Thread Marius Hanl
On Thu, 21 Mar 2024 22:06:42 GMT, Marius Hanl wrote: > In https://github.com/openjdk/jfx/pull/1405, I identified some shortcomings > of the stub font implementation. As I don't want to clutter the PR with that, > I decided to cherrypick the improvements I did to a new ticket and PR. > > The cu

Re: RFR: JDK-8186188: TableColumHeader: initial auto-size broken if has graphic [v4]

2024-03-27 Thread Marius Hanl
On Wed, 20 Mar 2024 19:53:40 GMT, Marius Hanl wrote: >> This PR fixes the issue that the initial column autosizing is wrong under >> some circumstances. >> The following things will break the initial autosizing: >> - Bold Column text (that is where I initially found this problem) >> - Another fo

Re: RFR: JDK-8328750: [TestBug] Improve Stub Font Support [v3]

2024-03-27 Thread Andy Goryachev
On Wed, 27 Mar 2024 22:20:42 GMT, Marius Hanl wrote: >> In https://github.com/openjdk/jfx/pull/1405, I identified some shortcomings >> of the stub font implementation. As I don't want to clutter the PR with >> that, I decided to cherrypick the improvements I did to a new ticket and PR. >> >> T

Re: RFR: JDK-8328750: [TestBug] Improve Stub Font Support [v2]

2024-03-27 Thread Marius Hanl
On Tue, 26 Mar 2024 20:53:17 GMT, Andy Goryachev wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> improve (stub) font tests, fallback and documentation > > modules/javafx.graphics/src/test/java/test/com/sun/javafx/pg

Re: RFR: JDK-8328750: [TestBug] Improve Stub Font Support [v3]

2024-03-27 Thread Marius Hanl
> In https://github.com/openjdk/jfx/pull/1405, I identified some shortcomings > of the stub font implementation. As I don't want to clutter the PR with that, > I decided to cherrypick the improvements I did to a new ticket and PR. > > The current implementation has the following shortcomings: >

Re: RFR: JDK-8328750: [TestBug] Improve Stub Font Support [v2]

2024-03-27 Thread Andy Goryachev
On Tue, 26 Mar 2024 20:47:37 GMT, Marius Hanl wrote: >> In https://github.com/openjdk/jfx/pull/1405, I identified some shortcomings >> of the stub font implementation. As I don't want to clutter the PR with >> that, I decided to cherrypick the improvements I did to a new ticket and PR. >> >> T

Re: RFR: 8092102: Labeled: truncated property [v6]

2024-03-27 Thread Andy Goryachev
On Wed, 27 Mar 2024 21:54:37 GMT, Andy Goryachev wrote: >> Adds **Labeled.textTruncated** property which indicates when the text is >> visually truncated (and the ellipsis string is inserted) in order to fit the >> available width. >> >> The new property reacts to changes in the following prop

Re: RFR: 8092102: Labeled: truncated property [v6]

2024-03-27 Thread Andy Goryachev
> Adds **Labeled.textTruncated** property which indicates when the text is > visually truncated (and the ellipsis string is inserted) in order to fit the > available width. > > The new property reacts to changes in the following properties: > - ellipsisString > - font > - height > - text > - wid

Re: RFR: 8327179: Update the 3D lighting application [v5]

2024-03-27 Thread Nir Lisker
On Wed, 27 Mar 2024 20:38:58 GMT, Nir Lisker wrote: >> Update for the 3D lighting test tool as described in the JBS issue. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Added gradle script Looks like the line specified by

Re: RFR: 8327179: Update the 3D lighting application [v6]

2024-03-27 Thread Nir Lisker
> Update for the 3D lighting test tool as described in the JBS issue. Nir Lisker has updated the pull request incrementally with one additional commit since the last revision: Whitespace? - Changes: - all: https://git.openjdk.org/jfx/pull/1387/files - new: https://git.openjdk

Re: RFR: 8327179: Update the 3D lighting application [v5]

2024-03-27 Thread Nir Lisker
On Wed, 27 Mar 2024 20:38:58 GMT, Nir Lisker wrote: >> Update for the 3D lighting test tool as described in the JBS issue. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Added gradle script I added the application to the gr

Re: RFR: 8327179: Update the 3D lighting application [v5]

2024-03-27 Thread Nir Lisker
> Update for the 3D lighting test tool as described in the JBS issue. Nir Lisker has updated the pull request incrementally with one additional commit since the last revision: Added gradle script - Changes: - all: https://git.openjdk.org/jfx/pull/1387/files - new: https://git

Re: RFR: 8242553: IntegerSpinner and DoubleSpinner do not wrap around values correctly in some cases

2024-03-27 Thread Andy Goryachev
On Wed, 27 Mar 2024 19:05:40 GMT, Kevin Rushforth wrote: > the number of discrete positions is `max - min + 1` you are right! my earlier comment is invalid. please ignore. Now, when wrapping occurs, should max/min be considered as valid steps? Example, try both integer and double spinners wi

Re: RFR: 8242553: IntegerSpinner and DoubleSpinner do not wrap around values correctly in some cases

2024-03-27 Thread Kevin Rushforth
On Wed, 27 Mar 2024 18:30:24 GMT, Andy Goryachev wrote: > the end result of stepping once by `amountToStepBy` must be equivalent of > incrementing by one a total of `amountToStepBy` times, wouldn't you agree? Yes, this is the expectaion. > so the formula for positive `amountToStepBy` values sh

Re: RFR: 8242553: IntegerSpinner and DoubleSpinner do not wrap around values correctly in some cases

2024-03-27 Thread Andy Goryachev
On Sun, 24 Mar 2024 15:11:16 GMT, drmarmac wrote: > This PR should fix the issue and cover all relevant cases with new tests. > > Note: This involves a small behavior change, as can be seen in > dblSpinner_testWrapAround_decrement_twoSteps() in SpinnerTest.java:749. With > this change the wrap

Re: RFR: 8242553: IntegerSpinner and DoubleSpinner do not wrap around values correctly in some cases

2024-03-27 Thread drmarmac
On Sun, 24 Mar 2024 15:11:16 GMT, drmarmac wrote: > This PR should fix the issue and cover all relevant cases with new tests. > > Note: This involves a small behavior change, as can be seen in > dblSpinner_testWrapAround_decrement_twoSteps() in SpinnerTest.java:749. With > this change the wrap

Re: RFR: 8242553: IntegerSpinner and DoubleSpinner do not wrap around values correctly in some cases

2024-03-27 Thread Andy Goryachev
On Sun, 24 Mar 2024 15:11:16 GMT, drmarmac wrote: > This PR should fix the issue and cover all relevant cases with new tests. > > Note: This involves a small behavior change, as can be seen in > dblSpinner_testWrapAround_decrement_twoSteps() in SpinnerTest.java:749. With > this change the wrap

Re: RFR: 8325900: Emit a warning on macOS if AWT has set the NSAppearance [v4]

2024-03-27 Thread Martin Fox
On Fri, 23 Feb 2024 21:58:37 GMT, Michael Strauß wrote: >> Platform preferences detection doesn't pick up effective macOS system >> preferences if AWT owns the NSApplication and has set its NSAppearance to a >> fixed value. >> >> The workaround is to set the system property >> "apple.awt.appl

Re: RFR: 8242553: IntegerSpinner and DoubleSpinner do not wrap around values correctly in some cases

2024-03-27 Thread Jeanette Winzenburg
On Wed, 27 Mar 2024 15:17:29 GMT, Andy Goryachev wrote: > (0 + 101) % (100 - 0) + 0 = 1 > > the code in this PR produces no movement (0). Same for the decrement. > which means that the modulo arithmetic is not yet quite correct (_not_ that the modulo behavior as such is wrong ;) nice test c

Re: RFR: 8242553: IntegerSpinner and DoubleSpinner do not wrap around values correctly in some cases

2024-03-27 Thread Andy Goryachev
On Sun, 24 Mar 2024 15:11:16 GMT, drmarmac wrote: > This PR should fix the issue and cover all relevant cases with new tests. > > Note: This involves a small behavior change, as can be seen in > dblSpinner_testWrapAround_decrement_twoSteps() in SpinnerTest.java:749. With > this change the wrap

Re: RFR: 8328746: Remove unused imports in demo apps

2024-03-27 Thread Andy Goryachev
On Thu, 21 Mar 2024 21:50:37 GMT, Andy Goryachev wrote: > Using Eclipse IDE to remove unused imports in **demo apps** (3D, Ensemble, > etc.) and update the copyright year to 2024. Using wildcard for more than 10 > static imports. > > > -- > > This is a trivial change (though fairly large), 1

Re: RFR: 8328746: Remove unused imports in demo apps

2024-03-27 Thread Nir Lisker
On Thu, 21 Mar 2024 21:50:37 GMT, Andy Goryachev wrote: > Using Eclipse IDE to remove unused imports in **demo apps** (3D, Ensemble, > etc.) and update the copyright year to 2024. Using wildcard for more than 10 > static imports. > > > -- > > This is a trivial change (though fairly large), 1

Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed [v2]

2024-03-27 Thread Nir Lisker
On Wed, 27 Mar 2024 09:11:56 GMT, drmarmac wrote: >> This PR removes potentially incorrect usages of Stream.peek(). >> The changed code should be covered by the tests that are already present. > > drmarmac has updated the pull request incrementally with one additional > commit since the last rev

Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed [v2]

2024-03-27 Thread Nir Lisker
On Wed, 27 Mar 2024 12:23:47 GMT, Marius Hanl wrote: >> I'd say .forEach() is used correctly here, according to docs, it guarantees >> execution of the side-effects (add to removed list & clear index), just not >> in any particular order. This way we avoid multiple iteration. > > Another idea i

Re: RFR: 8242553: IntegerSpinner and DoubleSpinner do not wrap around values correctly in some cases

2024-03-27 Thread Kevin Rushforth
On Sun, 24 Mar 2024 15:11:16 GMT, drmarmac wrote: > This PR should fix the issue and cover all relevant cases with new tests. > > Note: This involves a small behavior change, as can be seen in > dblSpinner_testWrapAround_decrement_twoSteps() in SpinnerTest.java:749. With > this change the wrap

Integrated: 8306322: JDK8130122Test fails intermittently

2024-03-27 Thread Jayathirth D V
On Mon, 25 Mar 2024 12:25:33 GMT, Jayathirth D V wrote: > This test has failed once and we are not seeing its failure after that > instance in our test systems. > > This test verifies whether bounds of GridPane gets updated properly on adding > an invisible node. > Initial test has 8 nodes in

Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed [v2]

2024-03-27 Thread Marius Hanl
On Wed, 27 Mar 2024 09:07:15 GMT, drmarmac wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/ControlUtils.java >> line 176: >> >>> 174: .distinct() >>> 175: .filter(removeRowFilter) >>> 176: .forEach(row -> { >> >>

Integrated: 8267565: Support "@3x" and greater high-density image naming convention

2024-03-27 Thread drmarmac
On Fri, 22 Mar 2024 16:17:29 GMT, drmarmac wrote: > This PR extends the range of hi-res images that are loaded via naming > convention, now including scale factors higher than `@2x`. > Supporting these is already being > [recommended](https://developer.apple.com/design/human-interface-guidelin

Re: RFR: 8242553: IntegerSpinner and DoubleSpinner do not wrap around values correctly in some cases

2024-03-27 Thread Jeanette Winzenburg
On Sun, 24 Mar 2024 15:11:16 GMT, drmarmac wrote: > This PR should fix the issue and cover all relevant cases with new tests. > > Note: This involves a small behavior change, as can be seen in > dblSpinner_testWrapAround_decrement_twoSteps() in SpinnerTest.java:749. With > this change the wrap

Re: RFR: 8267565: Support "@3x" and greater high-density image naming convention [v2]

2024-03-27 Thread Michael Strauß
On Tue, 26 Mar 2024 21:27:49 GMT, drmarmac wrote: >> This PR extends the range of hi-res images that are loaded via naming >> convention, now including scale factors higher than `@2x`. >> Supporting these is already being >> [recommended](https://developer.apple.com/design/human-interface-guid

Re: RFR: 8242553: IntegerSpinner and DoubleSpinner do not wrap around values correctly in some cases

2024-03-27 Thread drmarmac
On Sun, 24 Mar 2024 15:11:16 GMT, drmarmac wrote: > This PR should fix the issue and cover all relevant cases with new tests. > > Note: This involves a small behavior change, as can be seen in > dblSpinner_testWrapAround_decrement_twoSteps() in SpinnerTest.java:749. With > this change the wrap

Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed [v2]

2024-03-27 Thread drmarmac
On Tue, 26 Mar 2024 16:32:53 GMT, Karthik P K wrote: >> drmarmac has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove outdated comment > > modules/javafx.controls/src/main/java/javafx/scene/control/ControlUtils.java > line 176: > >>

Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed [v2]

2024-03-27 Thread drmarmac
> This PR removes potentially incorrect usages of Stream.peek(). > The changed code should be covered by the tests that are already present. drmarmac has updated the pull request incrementally with one additional commit since the last revision: Remove outdated comment - Changes:

Re: RFR: 8273349: Check uses of Stream::peek in controls and replace as needed

2024-03-27 Thread drmarmac
On Sun, 24 Mar 2024 15:10:22 GMT, drmarmac wrote: > This PR removes potentially incorrect usages of Stream.peek(). > The changed code should be covered by the tests that are already present. I also removed a code comment that explained the usage of peek() here, which would be outdated now. ---

Re: RFR: 8328746: Remove unused imports in demo apps

2024-03-27 Thread Marius Hanl
On Tue, 26 Mar 2024 16:32:41 GMT, Kevin Rushforth wrote: > static imports (sorted alphabetically) > one blank line > ordinary imports (sorted alphabetically) > > No wildcard imports (I favor an exception for static imports). +1 for something like this. It is a simple rule and pretty sure that it

Re: RFR: JDK-8328750: [TestBug] Improve Stub Font Support [v2]

2024-03-27 Thread Marius Hanl
On Tue, 26 Mar 2024 20:36:02 GMT, Andy Goryachev wrote: >> I can also use `round` here. Seems to be a bit safer, I agree. > > thank you. > > one more question - is it possible to set fractional scale with the > StubToolkit, and how would that work in the StubFontLoader? Does it mean > we'll b