Re: RFR: 8255015: Inconsistent illumination of 3D shape by PointLight [v12]
On Tue, 26 Oct 2021 19:48:41 GMT, Andreas Heger wrote: >> The inconsistent illumination happens on Macs with retina displays only if >> the 3D shape is placed in a SubScene. The light sources are located with >> wrong coordinates in sub scenes and this causes a different illumination. >> The wrong coordinates for the light sources come from the fact that the >> retina pixel scale factors are not used in a SubScene. >> >> With this pull request, the retina pixel scale factors will be also used in >> SubScenes and this should resolve the bug >> [https://bugs.openjdk.java.net/browse/JDK-8255015](url) > > Andreas Heger has updated the pull request incrementally with one additional > commit since the last revision: > > 8255015: testScene variable must be volatile and new line at the end of the > file added Looks good. I ran the test on all three platforms. On macOS I ran the test on both a retina and non-retina screen. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/531
Re: RFR: 8271090: Missing API docs in scenegraph classes [v3]
On Tue, 26 Oct 2021 09:54:43 GMT, Ajit Ghaisas wrote: >> This PR fixes javadoc warnings primarily in javafx.graphics module along >> with a remaining few in javafx.fxml, javafx.base and javafx.media modules. >> >> Note : >> - The javadoc needs to be generated with the JDK 18 EA build. >> - There are still few remaining warnings in these modules. The root cause is >> different and they will be addressed under >> [JDK-8270996](https://bugs.openjdk.java.net/browse/JDK-8270996) > > Ajit Ghaisas has updated the pull request incrementally with one additional > commit since the last revision: > > fix review comments Marked as reviewed by kcr (Lead). - PR: https://git.openjdk.java.net/jfx/pull/650
Re: RFR: 8255015: Inconsistent illumination of 3D shape by PointLight [v11]
On Mon, 25 Oct 2021 23:50:09 GMT, Kevin Rushforth wrote: >> Andreas Heger has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 10 additional >> commits since the last revision: >> >> - Merge branch 'openjdk:master' into fix-8255015 >> - 8255015: Comments corrected >> - 8255015: Comment about copying pixel scale factors corrected >> - 8255015: Tabs removed from PointLightIllumination.java >> - Merge branch 'openjdk:master' into fix-8255015 >> - 8255015: JUnit Test class added. >> - Merge branch 'openjdk:master' into fix-8255015 >> - Merge branch 'openjdk:master' into fix-8255015 >> - Merge branch 'openjdk:master' into fix-8255015 >> - 8255015: Copy pixel scale factors from graphics object to subscene >> graphics so that the position of lights will be scaled correctly on retina >> displays > > tests/system/src/test/java/test/robot/test3d/PointLightIlluminationTest.java > line 67: > >> 65: private static final intLOWER_CORNER_Y = (int) >> (SCENE_WIDTH_HEIGHT * (1 - CORNER_FACTOR)); >> 66: private static final double COLOR_TOLERANCE= 0.07; >> 67: private static Scene testScene; > > This is created on one thread and tested on another (to see whether it's > already been created), so I recommend making it `volatile` (i.e., `private > static volatile ...`). Also, you might want to explicitly set it to `null` > since you rely on it (yes, I know `null` is the default). @kevinrushforth Thanks for the hint about about making the variable volatile! I've just updated the class accordingly. - PR: https://git.openjdk.java.net/jfx/pull/531
Re: RFR: 8255015: Inconsistent illumination of 3D shape by PointLight [v12]
> The inconsistent illumination happens on Macs with retina displays only if > the 3D shape is placed in a SubScene. The light sources are located with > wrong coordinates in sub scenes and this causes a different illumination. The > wrong coordinates for the light sources come from the fact that the retina > pixel scale factors are not used in a SubScene. > > With this pull request, the retina pixel scale factors will be also used in > SubScenes and this should resolve the bug > [https://bugs.openjdk.java.net/browse/JDK-8255015](url) Andreas Heger has updated the pull request incrementally with one additional commit since the last revision: 8255015: testScene variable must be volatile and new line at the end of the file added - Changes: - all: https://git.openjdk.java.net/jfx/pull/531/files - new: https://git.openjdk.java.net/jfx/pull/531/files/656cb7d8..1598c604 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=531&range=11 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=531&range=10-11 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jfx/pull/531.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/531/head:pull/531 PR: https://git.openjdk.java.net/jfx/pull/531
Re: RFR: 8271091: Missing API docs in UI controls classes [v4]
On Tue, 26 Oct 2021 06:24:35 GMT, Ajit Ghaisas wrote: >> This PR fixes javadoc warnings in javafx.controls and javafx.web modules. >> Note : >> - The javadoc needs to be generated with the JDK 18 EA build. >> - 2 javadoc warnings in javafx.controls TabPane class will be fixed under - >> [JDK-8271085](https://bugs.openjdk.java.net/browse/JDK-8271085) >> - There are still 20 javadoc warnings remaining in javafx.controls module >> and 3 warnings remaining in javafx.web module. The root cause is different >> and they will be addressed under >> [JDK-8270996](https://bugs.openjdk.java.net/browse/JDK-8270996) > > Ajit Ghaisas has updated the pull request incrementally with one additional > commit since the last revision: > > fix review comments Marked as reviewed by kcr (Lead). - PR: https://git.openjdk.java.net/jfx/pull/646
Re: RFR: 8222455: JavaFX error loading glass.dll from cache
On Tue, 26 Oct 2021 12:09:19 GMT, Marius Hanl wrote: > This problem can happen when using multiple instances with different jfx > early access (ea) versions. > > Example: > Instance 1 uses 18-ea+4 and Instance 2 uses 18-ea+1. > Instance 1 is started (and will cache and use libraries), then instance 2. > Now instance 2 detects via a hash comparison that the cached libraries are > not the same as the supplied ones. > With this information instance 2 tries to delete the old libraries but fails > while doing so (as instance 1 uses them currently) and will terminate right > after. > > The problem here is that instance 1 and 2 are using the same cache folder: > **18-ea**. This is because the `NativeLibLoader` uses the `javafx.version` > property for determining the folder name, which in case of an ea version will > always be **18-ea** (for all ea versions starting with 18 obviously). > > Fix as also mentioned in the ticket is to use the `javafx.runtime.version` > property instead. > With this the folders will be named 18-ea+1 and 18-ea+4. This is a simple change, but it has some implications that @johanvos will need to approve. - PR: https://git.openjdk.java.net/jfx/pull/654
RFR: 8222455: JavaFX error loading glass.dll from cache
This problem can happen when using multiple instances with different jfx early access (ea) versions. Example: Instance 1 uses 18-ea+4 and Instance 2 uses 18-ea+1. Instance 1 is started (and will cache and use libraries), then instance 2. Now instance 2 detects via a hash comparison that the cached libraries are not the same as the supplied ones. With this information instance 2 tries to delete the old libraries but fails while doing so (as instance 1 uses them currently) and will terminate right after. The problem here is that instance 1 and 2 are using the same cache folder: **18-ea**. This is because the `NativeLibLoader` uses the `javafx.version` property for determining the folder name, which in case of an ea version will always be **18-ea** (for all ea versions starting with 18 obviously). Fix as also mentioned in the ticket is to use the `javafx.runtime.version` property instead. With this the folders will be named 18-ea+1 and 18-ea+4. - Commit messages: - 8222455: Using javafx.runtime.version as cache directory name so that different ea versions use the same folder Changes: https://git.openjdk.java.net/jfx/pull/654/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=654&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8222455 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/654.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/654/head:pull/654 PR: https://git.openjdk.java.net/jfx/pull/654
Re: RFR: 8274061: Tree-/TableRowSkin: misbehavior on switching skin
On Fri, 24 Sep 2021 16:01:38 GMT, Jeanette Winzenburg wrote: > Cleanup of Tree-/TableRowSkin to support switching skins > > The misbehavior/s > - memory leaks due to manually registered listeners that were not removed > - side-effects due to listeners still active on old skin (like NPEs) > > Fix > - use skin api for all listener registration (for automatic removal in > dispose) > - do not install listeners that are not needed (fixedCellSize, same as in fix > of ListCellSkin > [JDK-8246745](https://bugs.openjdk.java.net/browse/JDK-8246745)) > > Added tests for each listener involved in the fix to guarantee it's still > working and does not have unwanted side-effects after switching skins. > > Note: there are pecularities in row skins (like not updating themselves on > property changes of its control, throwing NPEs when not added to a > VirtualFlow) which are not part of this issue but covered in > [JDK-8274065](https://bugs.openjdk.java.net/browse/JDK-8274065) I have two comments regarding possibility of introducing a regression. Can you please confirm that it does not cause any side effect? Rest of the fix looks ok. modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java line 134: > 132: // that when it changes, we can appropriately add / > remove cells that may or may not > 133: // be required (because we remove all cells that are not > visible). > 134: registerChangeListener(getVirtualFlow().widthProperty(), > e -> tableView.requestLayout()); If this listener is removed, then is there a chance of reintroducing [JDK-8144500](https://bugs.openjdk.java.net/browse/JDK-8144500)? Unfortunately, there is no test added for that bug.. so it is difficult to catch regression, if any. modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java line 154: > 152: // that when it changes, we can appropriately add / > remove cells that may or may not > 153: // be required (because we remove all cells that are not > visible). > 154: registerChangeListener(getVirtualFlow().widthProperty(), > e -> treeTableView.requestLayout()); Same comment as in TableRowSkin regarding this listener. - PR: https://git.openjdk.java.net/jfx/pull/632
[jfx11u] Integrated: 8275835: Change JavaFX release version in jfx11u to 11.0.14
On Mon, 25 Oct 2021 14:34:39 GMT, Kevin Rushforth wrote: > Bump release version in `jfx11u` repo to 11.0.14 for the start of a new > release. This pull request has now been integrated. Changeset: e253fd94 Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/e253fd940710b421a01c85ad3502407d0dea6794 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8275835: Change JavaFX release version in jfx11u to 11.0.14 Reviewed-by: jvos - PR: https://git.openjdk.java.net/jfx11u/pull/55
[jfx17u] Integrated: 8275837: Change JavaFX release version in jfx17u to 17.0.2
On Mon, 25 Oct 2021 14:27:44 GMT, Kevin Rushforth wrote: > Bump release version in `jfx17u` repo to 17.0.2 for the start of a new > release. This pull request has now been integrated. Changeset: 99a629b4 Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx17u/commit/99a629b4006b03d09b8a5d4a9030f9ea963a605f Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8275837: Change JavaFX release version in jfx17u to 17.0.2 Reviewed-by: jvos - PR: https://git.openjdk.java.net/jfx17u/pull/15
RFR: 8275911: Keyboard doesn't show when tapping inside an iOS text input control
After [JDK-8245053](https://bugs.openjdk.java.net/browse/JDK-8245053) for Android, this PR applies the same approach on iOS: tapping on a text input control on iOS shows the keyboard, which hides after the control loses the focus. Now, both platforms have the same behaviour. Tested on an iOS device. - Commit messages: - Show iOS keyboard when user taps on TextInput controls Changes: https://git.openjdk.java.net/jfx/pull/653/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=653&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275911 Stats: 90 lines in 2 files changed: 60 ins; 2 del; 28 mod Patch: https://git.openjdk.java.net/jfx/pull/653.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/653/head:pull/653 PR: https://git.openjdk.java.net/jfx/pull/653
Re: RFR: 8271090: Missing API docs in scenegraph classes [v2]
On Mon, 25 Oct 2021 23:27:19 GMT, Kevin Rushforth wrote: > Looks good with a couple suggestions on `setScene`. We might want to also > file a follow-up javadoc bug so we can get rid of the javadocs for that > method altogether. I have filed - [JDK-8275910](https://bugs.openjdk.java.net/browse/JDK-8275910) for this. - PR: https://git.openjdk.java.net/jfx/pull/650
Re: RFR: 8271090: Missing API docs in scenegraph classes [v3]
> This PR fixes javadoc warnings primarily in javafx.graphics module along with > a remaining few in javafx.fxml, javafx.base and javafx.media modules. > > Note : > - The javadoc needs to be generated with the JDK 18 EA build. > - There are still few remaining warnings in these modules. The root cause is > different and they will be addressed under > [JDK-8270996](https://bugs.openjdk.java.net/browse/JDK-8270996) Ajit Ghaisas has updated the pull request incrementally with one additional commit since the last revision: fix review comments - Changes: - all: https://git.openjdk.java.net/jfx/pull/650/files - new: https://git.openjdk.java.net/jfx/pull/650/files/9ff692db..e4d337c5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=650&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=650&range=01-02 Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jfx/pull/650.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/650/head:pull/650 PR: https://git.openjdk.java.net/jfx/pull/650
