Re: RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX [v4]
> The following primitive constructors were deprecated in JDK 9 and are
> deprecated for removal in JDK 16.
>
> java.lang.Byte
> java.lang.Short
> java.lang.Integer
> java.lang.Long
> java.lang.Float
> java.lang.Double
> java.lang.Character
> java.lang.Boolean
>
> This change removes call to the primitive constructors with the `valueOf()`
> factory method of respective class.
> Calls like the following to create array get autoboxed so it does not require
> a change.
>
> `Double dArr[] = new Double[] {10.1, 20.2};`
Ambarish Rapte has updated the pull request incrementally with one additional
commit since the last revision:
some more autoboxing
-
Changes:
- all: https://git.openjdk.java.net/jfx/pull/423/files
- new: https://git.openjdk.java.net/jfx/pull/423/files/e34c6a8f..b104b373
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jfx=423=03
- incr: https://webrevs.openjdk.java.net/?repo=jfx=423=02-03
Stats: 5 lines in 4 files changed: 0 ins; 0 del; 5 mod
Patch: https://git.openjdk.java.net/jfx/pull/423.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/423/head:pull/423
PR: https://git.openjdk.java.net/jfx/pull/423
Re: RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX [v3]
On Thu, 11 Mar 2021 00:29:41 GMT, Nir Lisker wrote:
>> Ambarish Rapte has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> correct Float.valueOf()
>
> modules/javafx.graphics/src/main/java/com/sun/prism/es2/X11GLFactory.java
> line 171:
>
>> 169: deviceDetails.put("XVisualID",
>> Long.valueOf(nGetVisualID(nativeCtxInfo)));
>> 170: deviceDetails.put("XDisplay",
>> Long.valueOf(nGetDisplay(nativeCtxInfo)));
>> 171: deviceDetails.put("XScreenID",
>> Integer.valueOf(nGetDefaultScreen(nativeCtxInfo)));
>
> Autobox?
I think the statements look more readable the way they are.
The type of LHS of the expression is Object, so a reader will have to find
return type of those methods to understand what type of Object gets created. I
think such calls which use a return value from a method should use explicit
calls.
But then on other side in case if return type of the method is changed, then we
need to also change these explicit calls.
Sounds like a decision to make for other time. What do you think ?
-
PR: https://git.openjdk.java.net/jfx/pull/423
Re: RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX [v3]
On Wed, 10 Mar 2021 23:59:05 GMT, Nir Lisker wrote: >> Ambarish Rapte has updated the pull request incrementally with one >> additional commit since the last revision: >> >> correct Float.valueOf() > > apps/toys/LayoutDemo/src/layout/CustomPane.java line 92: > >> 90: Collections.sort(sortedManagedChidlren, (c1, c2) >> 91: -> Double.valueOf(c2.prefHeight(-1)).compareTo( >> 92: Double.valueOf(c1.prefHeight(-1; > > This can be replaced with > > Collections.sort(sortedManagedChidlren, (c1, c2) -> > Double.compare(c2.prefHeight(-1), c1.prefHeight(-1))); > > or > > Collections.sort(sortedManagedChidlren, > Comparator.comparingDouble(n -> n.prefHeight(-1)).reversed()); > > I think. > Same for the other places that do comparing. As this change is a cleanup and intended to replace deprecated constructor calls with autoboxing or `valueOf()` method, I think we should not make other changes as part of this PR. - PR: https://git.openjdk.java.net/jfx/pull/423
Re: RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX [v3]
On Thu, 11 Mar 2021 00:18:17 GMT, Nir Lisker wrote:
>> Ambarish Rapte has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> correct Float.valueOf()
>
> modules/javafx.base/src/test/java/test/javafx/util/DurationTest.java line 289:
>
>> 287: @Test public void add_ZERO_and_INDEFINITE_ResultsInIndefinite() {
>> 288: //assertTrue(0.0 + Double.POSITIVE_INFINITY ==
>> Double.POSITIVE_INFINITY); // sanity check
>> 289: assertEquals(Double.valueOf(Double.POSITIVE_INFINITY),
>> Double.valueOf(0.0 + Double.POSITIVE_INFINITY)); // sanity check
>
> I don't understand why convert to `Double` for the assertion test, but more
> than that, I don't understand why this test is needed for `Duration`. Isn't
> this is a guaranteed behavior of fp arithmetic?
>
> Same for all other such asserts.
>From the look of these tests, many look unnecessary to be here. But As these
>tests exist from very beginning, there might have been a good reason that they
>were added. As long as they don't harm, and if you don't have any hard call on
>them, let's keep them as is.(with just the `valueOf` change)
-
PR: https://git.openjdk.java.net/jfx/pull/423
Re: RFR: 8257895: Allow building of JavaFX media libs for Apple Silicon [v2]
> - Added support to compile media on arm. > - libffi is based on 3.3. Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision: 8257895: Allow building of JavaFX media libs for Apple Silicon [v2] - Changes: - all: https://git.openjdk.java.net/jfx/pull/412/files - new: https://git.openjdk.java.net/jfx/pull/412/files/99f8788f..ca11a752 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=412=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx=412=00-01 Stats: 4 lines in 2 files changed: 2 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jfx/pull/412.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/412/head:pull/412 PR: https://git.openjdk.java.net/jfx/pull/412
Re: RFR: 8257895: Allow building of JavaFX media libs for Apple Silicon [v2]
On Wed, 10 Mar 2021 23:46:39 GMT, Kevin Rushforth wrote: >> Alexander Matveev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8257895: Allow building of JavaFX media libs for Apple Silicon [v2] > > This breaks the build on Windows. > > cl.exe -DFFI_BUILDING -DGSTREAMER_LITE -I../../../3rd_party/libffi/include > -I../../../3rd_party/libffi/include/win/x64 -nologo -W3 -WX- -EHsc -GS > -fp:precise -Gm- -Zc:wchar_t -Zc:forScope -Gd -wd"4430" -analyze- > -errorReport:queue -O1 -Oy -MD -Gy -GF -DX86_WIN64 -TC -c > -Fo.../jfx/modules/javafx.media/build/native/win/Release/obj/3rd_party/libffi/src/java_raw_api.obj > ../../../3rd_party/libffi/src/java_raw_api.c > ../../../3rd_party/libffi/include/win/x64\ffi.h(58): fatal error C1083: > Cannot open include file: 'ffitarget.h': No such file or directory > make[1]: *** [Makefile.ffi:79: > .../jfx/modules/javafx.media/build/native/win/Release/obj/3rd_party/libffi/src/closures.obj] > Error 2 > make[1]: *** Waiting for unfinished jobs > java_raw_api.c > ../../../3rd_party/libffi/include/win/x64\ffi.h(58): fatal error C1083: > Cannot open include file: 'ffitarget.h': No such file or directory > make[1]: *** [Makefile.ffi:79: > .../jfx/modules/javafx.media/build/native/win/Release/obj/3rd_party/libffi/src/java_raw_api.obj] > Error 2 > make[1]: Leaving directory > '.../jfx/modules/javafx.media/src/main/native/gstreamer/projects/win/glib-lite' > make: *** [Makefile:60: > .../jfx/modules/javafx.media/build/native/win/Release/libffi.lib] Error 2 > make: Leaving directory > '.../jfx/modules/javafx.media/src/main/native/gstreamer/projects/win/glib-lite' > > FAILURE: Build failed with an exception. > > * Where: > Build file '...\jfx\build.gradle' line: 3284 > > * What went wrong: > Execution failed for task ':media:buildWinGlib'. >> Process 'command 'make'' finished with non-zero exit value 2 Windows build should be fixed. - PR: https://git.openjdk.java.net/jfx/pull/412
Re: RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX [v3]
On Thu, 11 Mar 2021 00:23:48 GMT, Nir Lisker wrote: >> Ambarish Rapte has updated the pull request incrementally with one >> additional commit since the last revision: >> >> correct Float.valueOf() > > modules/javafx.controls/src/test/java/test/javafx/scene/chart/XYChartTest.java > line 85: > >> 83: Font f = yaxis.getTickLabelFont(); >> 84: // default caspian value for font size = 10 >> 85: assertEquals(10, Double.valueOf(f.getSize()).intValue()); > > Any reason to convert `f.getSize()` to `int` and then to `Double`? Is it for > flooring the `double`? > > Same for the other places. `f.getSize()` returns a double value, the statement can also be written as `assertEquals(10, f.getSize(), 0f);` But, as this change is a cleanup and is intended to remove the calls to deprecated constructors, I think it should be limited to changing calls to autobox or to use `valueOf()` method. - PR: https://git.openjdk.java.net/jfx/pull/423
Re: RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX [v3]
On Thu, 11 Mar 2021 00:12:25 GMT, Nir Lisker wrote:
>> Ambarish Rapte has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> correct Float.valueOf()
>
> modules/javafx.base/src/test/java/test/com/sun/javafx/collections/MappingChangeTest.java
> line 52:
>
>> 50: @Test
>> 51: public void testAddRemove() {
>> 52: Change change = new
>> NonIterableChange.SimpleRemovedChange(0, 1, Integer.valueOf(5),
>> originalList);
>
> Autobox?
First two parameters are primitive type integer and the third parameter is
template type.
Changing it to autobox would make it less readable.
-
PR: https://git.openjdk.java.net/jfx/pull/423
Re: RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX [v3]
On Thu, 11 Mar 2021 00:26:38 GMT, Nir Lisker wrote:
>> Ambarish Rapte has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> correct Float.valueOf()
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/ComboBoxTest.java
> line 643:
>
>> 641: ComboBox cb = new ComboBox();
>> 642: StringConverter sc = cb.getConverter();
>> 643: assertEquals("42", sc.toString(Integer.valueOf(42)));
>
> Autobox?
>From this test code the type of parameter of `sc.toString()` is not obvious.
`StringConverter` is a template class. `toString()` accepts a template type
parameter.
Using autoboxing at such instances would hamper the readability.
So I think autoboxing should be used only when the type is very obvious from
just a glance of code.
> apps/samples/Ensemble8/src/samples/java/ensemble/samples/language/swing/SampleTableModel.java
> line 54:
>
>> 52: {Double.valueOf(567), Double.valueOf(956), Double.valueOf(1154)},
>> 53: {Double.valueOf(1292), Double.valueOf(1665),
>> Double.valueOf(1927)},
>> 54: {Double.valueOf(1292), Double.valueOf(2559),
>> Double.valueOf(2774)}
>
> Autobox?
The array is of type Object. When LHS is Object then autobox chooses a suitable
type by itself.
So here, if we autobox then the values get autoboxed to `Integer`. We do not
intend to change the test behavior as part of this fix so let's keep this as
`Double.valueOf`.
-
PR: https://git.openjdk.java.net/jfx/pull/423
Re: RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX [v3]
On Thu, 11 Mar 2021 00:41:48 GMT, Nir Lisker wrote:
>> Ambarish Rapte has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> correct Float.valueOf()
>
> modules/javafx.web/src/ios/java/javafx/scene/web/JSONDecoder.java line 101:
>
>> 99: long val = Long.parseLong(sNum);
>> 100: if ((val <= Integer.MAX_VALUE) && (Integer.MIN_VALUE <=
>> val)) {
>> 101: return Integer.valueOf(int) val);
>
> Autobox?
I think, when LHS is of type `Object`, it is good to be explicit on creating
Object, it improves readability.
In this case, type of val is `long`, so autobox would create an object of
`Long` not `Integer`.
-
PR: https://git.openjdk.java.net/jfx/pull/423
Re: RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX [v3]
On Wed, 10 Mar 2021 19:49:25 GMT, Ambarish Rapte wrote:
>> The following primitive constructors were deprecated in JDK 9 and are
>> deprecated for removal in JDK 16.
>>
>> java.lang.Byte
>> java.lang.Short
>> java.lang.Integer
>> java.lang.Long
>> java.lang.Float
>> java.lang.Double
>> java.lang.Character
>> java.lang.Boolean
>>
>> This change removes call to the primitive constructors with the `valueOf()`
>> factory method of respective class.
>> Calls like the following to create array get autoboxed so it does not
>> require a change.
>>
>> `Double dArr[] = new Double[] {10.1, 20.2};`
>
> Ambarish Rapte has updated the pull request incrementally with one additional
> commit since the last revision:
>
> correct Float.valueOf()
I left some comments where I think autoboxing does the same work explicit
boxing does. Unless I'm missing something, all these places can be simplified.
apps/samples/Ensemble8/src/app/java/ensemble/samplepage/PieChartDataVisualizer.java
line 92:
> 90: }
> 91: try {
> 92: return (Double) Double.valueOf(string);
Looks like an unnecessary cast.
apps/toys/LayoutDemo/src/layout/CustomPane.java line 92:
> 90: Collections.sort(sortedManagedChidlren, (c1, c2)
> 91: -> Double.valueOf(c2.prefHeight(-1)).compareTo(
> 92: Double.valueOf(c1.prefHeight(-1;
This can be replaced with
Collections.sort(sortedManagedChidlren, (c1, c2) ->
Double.compare(c2.prefHeight(-1), c1.prefHeight(-1)));
or
Collections.sort(sortedManagedChidlren, Comparator.comparingDouble(n
-> n.prefHeight(-1)).reversed());
I think.
Same for the other places that do comparing.
modules/javafx.controls/src/test/java/test/javafx/scene/control/MenuBarTest.java
line 582:
> 580:
> 581: boolean click = true;
> 582: final Boolean firstClick = Boolean.valueOf(click);
Autobox?
apps/samples/3DViewer/src/main/java/com/javafx/experiments/shape3d/SkinningMesh.java
line 110:
> 108: for (int i = 0; i < nPoints; i++) {
> 109: if (weights[j][i] != 0.0f) {
> 110: weightIndices[j].add(Integer.valueOf(i));
Autobox?
apps/samples/Ensemble8/src/samples/java/ensemble/samples/language/swing/SampleTableModel.java
line 54:
> 52: {Double.valueOf(567), Double.valueOf(956), Double.valueOf(1154)},
> 53: {Double.valueOf(1292), Double.valueOf(1665),
> Double.valueOf(1927)},
> 54: {Double.valueOf(1292), Double.valueOf(2559), Double.valueOf(2774)}
Autobox?
modules/javafx.base/src/test/java/test/com/sun/javafx/collections/MappingChangeTest.java
line 52:
> 50: @Test
> 51: public void testAddRemove() {
> 52: Change change = new
> NonIterableChange.SimpleRemovedChange(0, 1, Integer.valueOf(5),
> originalList);
Autobox?
modules/javafx.base/src/test/java/test/javafx/util/DurationTest.java line 289:
> 287: @Test public void add_ZERO_and_INDEFINITE_ResultsInIndefinite() {
> 288: //assertTrue(0.0 + Double.POSITIVE_INFINITY ==
> Double.POSITIVE_INFINITY); // sanity check
> 289: assertEquals(Double.valueOf(Double.POSITIVE_INFINITY),
> Double.valueOf(0.0 + Double.POSITIVE_INFINITY)); // sanity check
I don't understand why convert to `Double` for the assertion test, but more
than that, I don't understand why this test is needed for `Duration`. Isn't
this is a guaranteed behavior of fp arithmetic?
Same for all other such asserts.
modules/javafx.controls/src/test/java/test/javafx/scene/chart/XYChartTest.java
line 85:
> 83: Font f = yaxis.getTickLabelFont();
> 84: // default caspian value for font size = 10
> 85: assertEquals(10, Double.valueOf(f.getSize()).intValue());
Any reason to convert `f.getSize()` to `int` and then to `Double`? Is it for
flooring the `double`?
Same for the other places.
modules/javafx.controls/src/test/java/test/javafx/scene/control/ComboBoxTest.java
line 643:
> 641: ComboBox cb = new ComboBox();
> 642: StringConverter sc = cb.getConverter();
> 643: assertEquals("42", sc.toString(Integer.valueOf(42)));
Autobox?
modules/javafx.graphics/src/main/java/com/sun/prism/es2/X11GLFactory.java line
171:
> 169: deviceDetails.put("XVisualID",
> Long.valueOf(nGetVisualID(nativeCtxInfo)));
> 170: deviceDetails.put("XDisplay",
> Long.valueOf(nGetDisplay(nativeCtxInfo)));
> 171: deviceDetails.put("XScreenID",
> Integer.valueOf(nGetDefaultScreen(nativeCtxInfo)));
Autobox?
modules/javafx.graphics/src/main/java/com/sun/prism/j2d/J2DPipeline.java line
64:
> 62: @Override
> 63: public ResourceFactory getResourceFactory(Screen screen) {
> 64: Integer index = Integer.valueOf(screen.getAdapterOrdinal());
Autobox?
Same for the other similar places.
modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java line 477:
> 475: v.set(value);
> 476:
Re: RFR: 8257895: Allow building of JavaFX media libs for Apple Silicon
On Fri, 26 Feb 2021 03:58:17 GMT, Alexander Matveev wrote: > - Added support to compile media on arm. > - libffi is based on 3.3. This breaks the build on Windows. cl.exe -DFFI_BUILDING -DGSTREAMER_LITE -I../../../3rd_party/libffi/include -I../../../3rd_party/libffi/include/win/x64 -nologo -W3 -WX- -EHsc -GS -fp:precise -Gm- -Zc:wchar_t -Zc:forScope -Gd -wd"4430" -analyze- -errorReport:queue -O1 -Oy -MD -Gy -GF -DX86_WIN64 -TC -c -Fo.../jfx/modules/javafx.media/build/native/win/Release/obj/3rd_party/libffi/src/java_raw_api.obj ../../../3rd_party/libffi/src/java_raw_api.c ../../../3rd_party/libffi/include/win/x64\ffi.h(58): fatal error C1083: Cannot open include file: 'ffitarget.h': No such file or directory make[1]: *** [Makefile.ffi:79: .../jfx/modules/javafx.media/build/native/win/Release/obj/3rd_party/libffi/src/closures.obj] Error 2 make[1]: *** Waiting for unfinished jobs java_raw_api.c ../../../3rd_party/libffi/include/win/x64\ffi.h(58): fatal error C1083: Cannot open include file: 'ffitarget.h': No such file or directory make[1]: *** [Makefile.ffi:79: .../jfx/modules/javafx.media/build/native/win/Release/obj/3rd_party/libffi/src/java_raw_api.obj] Error 2 make[1]: Leaving directory '.../jfx/modules/javafx.media/src/main/native/gstreamer/projects/win/glib-lite' make: *** [Makefile:60: .../jfx/modules/javafx.media/build/native/win/Release/libffi.lib] Error 2 make: Leaving directory '.../jfx/modules/javafx.media/src/main/native/gstreamer/projects/win/glib-lite' FAILURE: Build failed with an exception. * Where: Build file '...\jfx\build.gradle' line: 3284 * What went wrong: Execution failed for task ':media:buildWinGlib'. > Process 'command 'make'' finished with non-zero exit value 2 - Changes requested by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/412
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v4]
On Wed, 10 Mar 2021 22:35:31 GMT, Florian Kirmaier
wrote:
>> Fixing deadlock when calling Application.launch in the FXThread after
>> Platform.startup
>
> Florian Kirmaier has updated the pull request incrementally with one
> additional commit since the last revision:
>
> JDK-8263322
> updated the javadoc to mention the new case.
The fix looks good. I have a couple comments on the tests, mainly about making
them more robust by having only a single `@Test` method in each file.
tests/system/src/test/java/test/com/sun/javafx/application/InitializeJavaFXStartupTest.java
line 46:
> 44:
> 45: @Test (timeout = 15000)
> 46: public void testStartupThenLaunch() throws Exception {
It will be more robust for these two test methods to be in separate files.
Otherwise they might interfere with each other. If `testStartupThenLaunchInFX`
runs first, then an error in that test (e.g., if the fix isn't applied and it
times out) will cause the `testStartupThenLaunch` to fail. Conversely, if
`testStartupThenLaunch` runs first, then the system is in a state where the
Application has already been started by the time `testStartupThenLaunchInFX`
runs. It will still pass, but won't be a solid test of the fix, since that case
would fail today.
tests/system/src/test/java/test/com/sun/javafx/application/InitializeJavaFXLaunchTest.java
line 46:
> 44:
> 45: @Test (timeout = 15000)
> 46: public void testStartupThenLaunch() throws Exception {
It will be more robust for these two test methods to be in separate files.
Otherwise they might interfere with each other.
tests/system/src/test/java/test/com/sun/javafx/application/InitializeJavaFXBase.java
line 80:
> 78: System.out.println("Finished launch!");
> 79: Assert.fail("Error: No Exception was thrown - expected
> IllegalStateException");
> 80: } catch (IllegalStateException e) {
Maybe add a comment that this exception is expected?
-
PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v4]
On Wed, 10 Mar 2021 22:40:26 GMT, Kevin Rushforth wrote: >> I've now added the new Exception to the JavaDoc of the Application. > > The API spec changes look good. You can add that to the Specification section > of the CSR. You can either paste the diffs for the javadoc comments, or else > just add the one new line for each method. Either way, the name of the class > (javafx.application.Application) should be listed prior to the diffs / > additions. Also, make it clear which methods are getting the new `@exception` > tags. Great, I've updated the CSR! - PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v4]
On Wed, 10 Mar 2021 22:32:26 GMT, Florian Kirmaier wrote: >> Regarding the CSR, it is only concerned with the public API, including the >> specification (javadoc-generated API documentation), and any behavioral >> changes. >> >> You can leave the CSR in the Draft state until there is something to >> document in the Specification section, and the review of the PR is far >> enough along so that those changes are likely to be final. For this PR, the >> changes that need to be documented in the CSR haven't been made yet. The >> changes needed will be in the javadoc comments of public >> `Application::launch` methods. > > I've now added the new Exception to the JavaDoc of the Application. The API spec changes look good. You can add that to the Specification section of the CSR. You can either paste the diffs for the javadoc comments, or else just add the one new line for each method. Either way, the name of the class (javafx.application.Application) should be listed prior to the diffs / additions. Also, make it clear which methods are getting the new `@exception` tags. - PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v4]
On Wed, 10 Mar 2021 22:10:04 GMT, Kevin Rushforth wrote: >>> About the CSR: >>> This commit actually restores original behavior, when JavaFX was called >>> with Application.launch, so it's not really a change in the API, it's more >>> like a fix for a regression. >> >> Not quite. See my reply above. >> >>> How would I create a CSR? Just by creating a ticket at >>> https://bugs.openjdk.java.net/issues/?jql=issuetype%20%3D%20CSR with the >>> type CSR? >> >> Almost. Go to your bug in JBS, and use the "More" pull-down menu. There is a >> "Create CSR" option. > > Regarding the CSR, it is only concerned with the public API, including the > specification (javadoc-generated API documentation), and any behavioral > changes. > > You can leave the CSR in the Draft state until there is something to document > in the Specification section, and the review of the PR is far enough along so > that those changes are likely to be final. For this PR, the changes that need > to be documented in the CSR haven't been made yet. The changes needed will be > in the javadoc comments of public `Application::launch` methods. I've now added the new Exception to the JavaDoc of the Application. - PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v3]
On Wed, 10 Mar 2021 22:03:52 GMT, Kevin Rushforth wrote: >> Florian Kirmaier has updated the pull request incrementally with three >> additional commits since the last revision: >> >> - JDK-8263322 >>removed unused imports, added missing change >> - JDK-8263322 >>Updated the unit-test so they match the wanted behavior discussed in the >> PR >> - JDK-8263322 >>Added the tests for both cases, when JavaFX was initialized with >> Application.launch and Platform.startup > > modules/javafx.graphics/src/main/java/com/sun/javafx/application/LauncherImpl.java > line 661: > >> 659: >> 660: // Note, this method is called on the FX Application Thread >> 661: PlatformImpl.startup(() -> startupLatch.countDown()); > > Glad to see this reverted. I was going to ask you why it was needed (it > shouldn't be). It was added, because I thought it shouldn't be allowed to call Application.launch after Platform.startup. - PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v4]
> Fixing deadlock when calling Application.launch in the FXThread after > Platform.startup Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision: JDK-8263322 updated the javadoc to mention the new case. - Changes: - all: https://git.openjdk.java.net/jfx/pull/421/files - new: https://git.openjdk.java.net/jfx/pull/421/files/0abc71e0..c16b6067 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=421=03 - incr: https://webrevs.openjdk.java.net/?repo=jfx=421=02-03 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jfx/pull/421.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/421/head:pull/421 PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263402: MemoryLeak: Node hardreferences it's previous Parent after csslayout and getting removed from the scene
On Wed, 10 Mar 2021 22:25:32 GMT, Florian Kirmaier wrote: > Fixing a memory leak. > A node hard references its old parent after CSS layout and getting removed. > This shouldn't be the case, this is very counterintuitive. > > The fix uses a WeakReference in CSSStyleHelper for firstStyleableAncestor. > This should be fine because the CSS should only depend on it if it's still > the real parent. > In that case, it doesn't get collected. Since this touches CSS, it needs a second reviewer. - PR: https://git.openjdk.java.net/jfx/pull/424
RFR: 8263402: MemoryLeak: Node hardreferences it's previous Parent after csslayout and getting removed from the scene
Fixing a memory leak. A node hard references its old parent after CSS layout and getting removed. This shouldn't be the case, this is very counterintuitive. The fix uses a WeakReference in CSSStyleHelper for firstStyleableAncestor. This should be fine because the CSS should only depend on it if it's still the real parent. In that case, it doesn't get collected. - Commit messages: - 8263402 Changes: https://git.openjdk.java.net/jfx/pull/424/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx=424=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263402 Stats: 116 lines in 2 files changed: 107 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jfx/pull/424.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/424/head:pull/424 PR: https://git.openjdk.java.net/jfx/pull/424
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v3]
On Wed, 10 Mar 2021 17:30:21 GMT, Kevin Rushforth wrote: >> About the CSR: >> This commit actually restores original behavior, when JavaFX was called with >> Application.launch, so it's not really a change in the API, it's more like a >> fix for a regression. >> How would I create a CSR? Just by creating a ticket at >> https://bugs.openjdk.java.net/issues/?jql=issuetype%20%3D%20CSR with the >> type CSR? > >> About the CSR: >> This commit actually restores original behavior, when JavaFX was called with >> Application.launch, so it's not really a change in the API, it's more like a >> fix for a regression. > > Not quite. See my reply above. > >> How would I create a CSR? Just by creating a ticket at >> https://bugs.openjdk.java.net/issues/?jql=issuetype%20%3D%20CSR with the >> type CSR? > > Almost. Go to your bug in JBS, and use the "More" pull-down menu. There is a > "Create CSR" option. Regarding the CSR, it is only concerned with the public API, including the specification (javadoc-generated API documentation), and any behavioral changes. You can leave the CSR in the Draft state until there is something to document in the Specification section, and the review of the PR is far enough along so that those changes are likely to be final. For this PR, the changes that need to be documented in the CSR haven't been made yet. The changes needed will be in the javadoc comments of public `Application::launch` methods. - PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v3]
On Wed, 10 Mar 2021 21:54:32 GMT, Florian Kirmaier wrote: >> Fixing deadlock when calling Application.launch in the FXThread after >> Platform.startup > > Florian Kirmaier has updated the pull request incrementally with three > additional commits since the last revision: > > - JDK-8263322 >removed unused imports, added missing change > - JDK-8263322 >Updated the unit-test so they match the wanted behavior discussed in the PR > - JDK-8263322 >Added the tests for both cases, when JavaFX was initialized with > Application.launch and Platform.startup modules/javafx.graphics/src/main/java/com/sun/javafx/application/LauncherImpl.java line 661: > 659: > 660: // Note, this method is called on the FX Application Thread > 661: PlatformImpl.startup(() -> startupLatch.countDown()); Glad to see this reverted. I was going to ask you why it was needed (it shouldn't be). - PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v3]
> Fixing deadlock when calling Application.launch in the FXThread after > Platform.startup Florian Kirmaier has updated the pull request incrementally with three additional commits since the last revision: - JDK-8263322 removed unused imports, added missing change - JDK-8263322 Updated the unit-test so they match the wanted behavior discussed in the PR - JDK-8263322 Added the tests for both cases, when JavaFX was initialized with Application.launch and Platform.startup - Changes: - all: https://git.openjdk.java.net/jfx/pull/421/files - new: https://git.openjdk.java.net/jfx/pull/421/files/6abe618e..0abc71e0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=421=02 - incr: https://webrevs.openjdk.java.net/?repo=jfx=421=01-02 Stats: 307 lines in 5 files changed: 200 ins; 106 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/421.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/421/head:pull/421 PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX [v3]
On Wed, 10 Mar 2021 19:49:25 GMT, Ambarish Rapte wrote:
>> The following primitive constructors were deprecated in JDK 9 and are
>> deprecated for removal in JDK 16.
>>
>> java.lang.Byte
>> java.lang.Short
>> java.lang.Integer
>> java.lang.Long
>> java.lang.Float
>> java.lang.Double
>> java.lang.Character
>> java.lang.Boolean
>>
>> This change removes call to the primitive constructors with the `valueOf()`
>> factory method of respective class.
>> Calls like the following to create array get autoboxed so it does not
>> require a change.
>>
>> `Double dArr[] = new Double[] {10.1, 20.2};`
>
> Ambarish Rapte has updated the pull request incrementally with one additional
> commit since the last revision:
>
> correct Float.valueOf()
Marked as reviewed by kcr (Lead).
-
PR: https://git.openjdk.java.net/jfx/pull/423
Re: RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX [v3]
On Wed, 10 Mar 2021 19:37:51 GMT, Kevin Rushforth wrote: >> Ambarish Rapte has updated the pull request incrementally with one >> additional commit since the last revision: >> >> correct Float.valueOf() > > Looks good with a couple problems (in code that we must not be building) > noted below. This is a simple change, but it would be good to have a second pair of eyes on it. - PR: https://git.openjdk.java.net/jfx/pull/423
Re: RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX [v3]
> The following primitive constructors were deprecated in JDK 9 and are
> deprecated for removal in JDK 16.
>
> java.lang.Byte
> java.lang.Short
> java.lang.Integer
> java.lang.Long
> java.lang.Float
> java.lang.Double
> java.lang.Character
> java.lang.Boolean
>
> This change removes call to the primitive constructors with the `valueOf()`
> factory method of respective class.
> Calls like the following to create array get autoboxed so it does not require
> a change.
>
> `Double dArr[] = new Double[] {10.1, 20.2};`
Ambarish Rapte has updated the pull request incrementally with one additional
commit since the last revision:
correct Float.valueOf()
-
Changes:
- all: https://git.openjdk.java.net/jfx/pull/423/files
- new: https://git.openjdk.java.net/jfx/pull/423/files/588ec4ce..e34c6a8f
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jfx=423=02
- incr: https://webrevs.openjdk.java.net/?repo=jfx=423=01-02
Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod
Patch: https://git.openjdk.java.net/jfx/pull/423.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/423/head:pull/423
PR: https://git.openjdk.java.net/jfx/pull/423
Re: RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX [v2]
On Wed, 10 Mar 2021 19:24:22 GMT, Ambarish Rapte wrote:
>> The following primitive constructors were deprecated in JDK 9 and are
>> deprecated for removal in JDK 16.
>>
>> java.lang.Byte
>> java.lang.Short
>> java.lang.Integer
>> java.lang.Long
>> java.lang.Float
>> java.lang.Double
>> java.lang.Character
>> java.lang.Boolean
>>
>> This change removes call to the primitive constructors with the `valueOf()`
>> factory method of respective class.
>> Calls like the following to create array get autoboxed so it does not
>> require a change.
>>
>> `Double dArr[] = new Double[] {10.1, 20.2};`
>
> Ambarish Rapte has updated the pull request incrementally with one additional
> commit since the last revision:
>
> use autoboxing when obvious
Looks good with a couple problems (in code that we must not be building) noted
below.
apps/samples/Ensemble8/src/samples/java/ensemble/samples/language/swing/SwingInterop.java
line 222:
> 220: private Pane createBrowser() {
> 221: Double widthDouble = Integer.valueOf(PANEL_WIDTH).doubleValue();
> 222: Double heightDouble =
> Integer.valueOf(PANEL_HEIGHT).doubleValue();
I think `Double.valueof(PANEL_WIDTH)` would be simpler? Or, you can use
auto-boxing and just say:
Double widthDouble = PANEL_WIDTH;
Double heightDouble = PANEL_HEIGHT;
modules/javafx.graphics/src/test/jslc/com/sun/scenario/effect/compiler/parser/PrimaryExprTest.java
line 68:
> 66: Expr tree = parseTreeFor("1.234");
> 67: assertTrue(tree instanceof LiteralExpr);
> 68: assertEquals(((LiteralExpr)tree).getValue(),
> Float.valueOf(1.234));
We must not be building or running these tests. This won't compile without
casting to `float` or using a float constant, `1.234f`.
modules/javafx.graphics/src/test/jslc/com/sun/scenario/effect/compiler/parser/UnaryExprTest.java
line 62:
> 60: UnaryExpr tree = parseTreeFor("+72.4");
> 61: assertEquals(tree.getOp(), UnaryOpType.PLUS);
> 62: assertEquals(((LiteralExpr)tree.getExpr()).getValue(),
> Float.valueOf(72.4));
Same problem here (and in the next method). This needs to be `72.4f`
-
PR: https://git.openjdk.java.net/jfx/pull/423
Re: RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX [v2]
> The following primitive constructors were deprecated in JDK 9 and are
> deprecated for removal in JDK 16.
>
> java.lang.Byte
> java.lang.Short
> java.lang.Integer
> java.lang.Long
> java.lang.Float
> java.lang.Double
> java.lang.Character
> java.lang.Boolean
>
> This change removes call to the primitive constructors with the `valueOf()`
> factory method of respective class.
> Calls like the following to create array get autoboxed so it does not require
> a change.
>
> `Double dArr[] = new Double[] {10.1, 20.2};`
Ambarish Rapte has updated the pull request incrementally with one additional
commit since the last revision:
use autoboxing when obvious
-
Changes:
- all: https://git.openjdk.java.net/jfx/pull/423/files
- new: https://git.openjdk.java.net/jfx/pull/423/files/17e6ea28..588ec4ce
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jfx=423=01
- incr: https://webrevs.openjdk.java.net/?repo=jfx=423=00-01
Stats: 7 lines in 4 files changed: 0 ins; 0 del; 7 mod
Patch: https://git.openjdk.java.net/jfx/pull/423.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/423/head:pull/423
PR: https://git.openjdk.java.net/jfx/pull/423
Re: RFR: 8260528: Clean glass-gtk sizing and positioning code [v25]
> This is a new approach to rewrite parts of gtk glass backend to be more clean. > > I will provide small "manageable" PR to incrementally make the backend better. > > This PR adresses cleanup of the Size and Positioning code. It makes code more > "straightforward" and easier to maintain. > > Current status (Ubuntu 20.04): >  > > (*) Some of the iconify tests are also failing on the main branch. > > `gradlew -PFULL_TEST=true -PUSE_ROBOT=true :systemTests:test --tests > test.robot.javafx.stage.IconifyTest` on a second run produces 4 tests, 2 > failures. Thiago Milczarek Sayao has updated the pull request incrementally with one additional commit since the last revision: Fix bug in content oriented child windows - Changes: - all: https://git.openjdk.java.net/jfx/pull/367/files - new: https://git.openjdk.java.net/jfx/pull/367/files/6a48f2a6..0dd478a8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=367=24 - incr: https://webrevs.openjdk.java.net/?repo=jfx=367=23-24 Stats: 6 lines in 1 file changed: 6 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jfx/pull/367.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/367/head:pull/367 PR: https://git.openjdk.java.net/jfx/pull/367
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v2]
On Wed, 10 Mar 2021 16:42:41 GMT, Florian Kirmaier wrote: > About the CSR: > This commit actually restores original behavior, when JavaFX was called with > Application.launch, so it's not really a change in the API, it's more like a > fix for a regression. Not quite. See my reply above. > How would I create a CSR? Just by creating a ticket at > https://bugs.openjdk.java.net/issues/?jql=issuetype%20%3D%20CSR with the type > CSR? Almost. Go to your bug in JBS, and use the "More" pull-down menu. There is a "Create CSR" option. - PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v2]
On Wed, 10 Mar 2021 17:24:47 GMT, Kevin Rushforth wrote: >> `Initalize the FX runtime via Platform.startup and then launch an >> Application on another thread (should succeed)` >> I don't think that should succeed. I would expect it to throw an exception. >> If that would be the case, then both startup-methods would result in >> different states, which wouldn't be so good. >> >> I've now restructured the tests. >> It's a total of 4 tests. 2 after Application.launch and 2 after >> Platform.startup. >> In both cases I check what happens, a Application is launched from another >> thread, or from the javafx thread. In all cases an exception is expected. > >> Initalize the FX runtime via Platform.startup and then launch an Application >> on another thread (should succeed) > I don't think that should succeed. I would expect it to throw an exception. > If that would be the case, then both startup-methods would result in > different states, which wouldn't be so good. > > No, this really should succeed. Internally, it is a similar case to what the > special Java launcher method does when launching a Java class that extends > `Application` and which may have a main method that calls > `Application.launch`. We check the various cases of launching an application > with/without it extending `Application` in the tests under > [tests/system/src/test/java/test/launchertest/](https://github.com/openjdk/jfx/tree/master/tests/system/src/test/java/test/launchertest/). To further clarify, `Application.launch` will start the FX platform only if it is not already started by some other means. Then it will (in all cases) run the applicaiton life-cycle. Note this from the `Application` class docs: Life-cycle The entry point for JavaFX applications is the Application class. The JavaFX runtime does the following, in order, whenever an application is launched: 1. Starts the JavaFX runtime, if not already started (see Platform.startup(Runnable) for more information) ... - PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v2]
On Wed, 10 Mar 2021 17:00:41 GMT, Florian Kirmaier wrote: > Initalize the FX runtime via Platform.startup and then launch an Application > on another thread (should succeed) I don't think that should succeed. I would expect it to throw an exception. If that would be the case, then both startup-methods would result in different states, which wouldn't be so good. No, this really should succeed. Internally, it is a similar case to what the special Java launcher method does when launching a Java class that extends `Application` and which may have a main method that calls `Application.launch`. We check the various cases of launching an application with/without it extending `Application` in the tests under [tests/system/src/test/java/test/launchertest/](https://github.com/openjdk/jfx/tree/master/tests/system/src/test/java/test/launchertest/). - PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v2]
On Wed, 10 Mar 2021 15:29:37 GMT, Florian Kirmaier wrote: > Contribution.MD states, that it's automatically checked for whitespace > errors? Is this not true? Yes, Skara's jcheck does basic sanity checking (tabs, trailing white space, line endings). Why do you ask? I didn't raise this as a question in this PR. Do you have reason to believe that this isn't working? > As a clarification, this PR restores the previous behavior before > Platform.startup. With this PR Application.launch and Platform.startup > behaves the same. Not quite. `Platform.startup` is a new method that was previously available only via an internal implementation class, which simply starts up the FX runtime without running any of the application life cycle. Once this was added to public API, it became possible (but is seldom necessary) to call `Platform.startup` and then later call `Application.launch`, which should work fine as long as the latter call is _not_ on the FX Application Thread. What was missed is a check to make sure that `Application.launch` is not called on the FX Application thread. This should have been documented to throw an exception. It is somewhat similar to, but not the same as, the case of calling `Application.launch` more than once. - PR: https://git.openjdk.java.net/jfx/pull/421
RFR: 8257512: Remove use of deprecated primitive constructors in JavaFX
The following primitive constructors were deprecated in JDK 9 and are
deprecated for removal in JDK 16.
java.lang.Byte
java.lang.Short
java.lang.Integer
java.lang.Long
java.lang.Float
java.lang.Double
java.lang.Character
java.lang.Boolean
This change removes call to the primitive constructors with the `valueOf()`
factory method of respective class.
Calls like the following to create array get autoboxed so it does not require a
change.
`Double dArr[] = new Double[] {10.1, 20.2};`
-
Commit messages:
- replace depr cotrs with valueOf()
Changes: https://git.openjdk.java.net/jfx/pull/423/files
Webrev: https://webrevs.openjdk.java.net/?repo=jfx=423=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8257512
Stats: 75 lines in 27 files changed: 0 ins; 0 del; 75 mod
Patch: https://git.openjdk.java.net/jfx/pull/423.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/423/head:pull/423
PR: https://git.openjdk.java.net/jfx/pull/423
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v2]
On Wed, 10 Mar 2021 16:01:42 GMT, Florian Kirmaier
wrote:
>> tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line
>> 65:
>>
>>> 63: } catch (Exception e) {
>>> 64: System.out.println("got exception: " + e);
>>> 65: e.printStackTrace();
>>
>> You should rethrow the exception in this case, since it indicates an
>> unexpected error. Better still, you can remove this block and not catch the
>> Exception in the first place.
>
> If I would rethrow it, it would just print it out, it's currently doing. I
> could save the Exception in a variable, to rethrow it from the other thread,
> but I think that would be unnecessarily complicated.
Not relevant anymore, because of Util.runAndWait()
>> tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line
>> 24:
>>
>>> 22: }
>>> 23:
>>> 24: public static void initializeApplication() throws Exception {
>>
>> This method is unused, along with the `InitializeApp` class. Did you plan to
>> use it?
>
> It's useful to compare it with the behavior of the two methods to start
> JavaFX.
> If it would be my codebase, I would keep it, so if someone investigates it
> later, it's easier to investigate for differences. But I can also delete it,
> what would you prefer?
It's now used because i added more test cases.
-
PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v2]
On Wed, 10 Mar 2021 16:42:41 GMT, Florian Kirmaier wrote: >> I forgot to commit a part of the fix, which is why the second test hang. >> It's now part of the PR. > > About the CSR: > This commit actually restores original behavior, when JavaFX was called with > Application.launch, so it's not really a change in the API, it's more like a > fix for a regression. > How would I create a CSR? Just by creating a ticket at > https://bugs.openjdk.java.net/issues/?jql=issuetype%20%3D%20CSR with the type > CSR? `Initalize the FX runtime via Platform.startup and then launch an Application on another thread (should succeed)` I don't think that should succeed. I would expect it to throw an exception. If that would be the case, then both startup-methods would result in different states, which wouldn't be so good. I've now restructured the tests. It's a total of 4 tests. 2 after Application.launch and 2 after Platform.startup. In both cases I check what happens, a Application is launched from another thread, or from the javafx thread. In all cases an exception is expected. - PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v2]
On Wed, 10 Mar 2021 16:23:53 GMT, Florian Kirmaier wrote: >> Contribution.MD states, that it's automatically checked for whitespace >> errors? Is this not true? >> >> As a clarification, this PR restores the previous behavior before >> Platform.startup. With this PR Application.launch and Platform.startup >> behaves the same. > > I forgot to commit a part of the fix, which is why the second test hang. It's > now part of the PR. About the CSR: This commit actually restores original behavior, when JavaFX was called with Application.launch, so it's not really a change in the API, it's more like a fix for a regression. How would I create a CSR? Just by creating a ticket at https://bugs.openjdk.java.net/issues/?jql=issuetype%20%3D%20CSR with the type CSR? - PR: https://git.openjdk.java.net/jfx/pull/421
Re: CFV: New OpenJFX Committer: John Neffenger
Vote: YES — Arun Joseph > On 10-Mar-2021, at 5:27 PM, Kevin Rushforth > wrote: > > I hereby nominate John Neffenger [1] to OpenJFX Committer. > > John is an OpenJFX community member, who has contributed 10 commits [2] to > OpenJFX. > > Votes are due by March 24, 2021 at 12:00 UTC. > > Only current OpenJFX Committers [3] are eligible to vote on this nomination. > Votes must be cast in the open by replying to this mailing list. > > For Lazy Consensus voting instructions, see [4]. Nomination to a project > Committer is described in [6]. > > Thanks. > > -- Kevin > > > [1] https://openjdk.java.net/census#jgneff > > [2] > https://github.com/openjdk/jfx/search?q=author-email%3Ajohn%40status6.com=author-date=Commits > > [3] https://openjdk.java.net/census#openjfx > > [4] https://openjdk.java.net/bylaws#lazy-consensus > > [6] https://openjdk.java.net/projects#project-committer > >
Re: CFV: New OpenJFX Committer: John Neffenger
Vote: YES Jose On Wed, Mar 10, 2021 at 12:57 PM Kevin Rushforth wrote: > I hereby nominate John Neffenger [1] to OpenJFX Committer. > > John is an OpenJFX community member, who has contributed 10 commits [2] > to OpenJFX. > > Votes are due by March 24, 2021 at 12:00 UTC. > > Only current OpenJFX Committers [3] are eligible to vote on this > nomination. Votes must be cast in the open by replying to this mailing > list. > > For Lazy Consensus voting instructions, see [4]. Nomination to a project > Committer is described in [6]. > > Thanks. > > -- Kevin > > > [1] https://openjdk.java.net/census#jgneff > > [2] > > https://github.com/openjdk/jfx/search?q=author-email%3Ajohn%40status6.com=author-date=Commits > > [3] https://openjdk.java.net/census#openjfx > > [4] https://openjdk.java.net/bylaws#lazy-consensus > > [6] https://openjdk.java.net/projects#project-committer > > > --
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v2]
On Wed, 10 Mar 2021 13:15:43 GMT, Kevin Rushforth wrote: >> Florian Kirmaier has updated the pull request incrementally with three >> additional commits since the last revision: >> >> - JDK-8263322 >>Added missing change >> - JDK-8263322 >>Small changes based on code review >> - JDK-8263322 >>Small changes based on code review > > tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line > 36: > >> 34: latch.countDown(); >> 35: }); >> 36: latch.await(); > > This needs to be changed to a flavor of await with a timeout (you can assert > that it doesn't timeout). Also, I don't think this needs to be its own > method, since the only thing the `initialize` method does is call this. I actually prefer it as a separate method. It makes it more reusable. And it makes it also easier to compare the behavior, between the two possible ways to initialize JavaFX - PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v2]
On Wed, 10 Mar 2021 15:29:37 GMT, Florian Kirmaier wrote: >>> * Initalize the FX runtime via Platform.startup and then launch an >>> Application on another thread (should throw ISE) >> >> That should be: >> >> * Initalize the FX runtime via Platform.startup and then launch an >> Application on the FX Application thread (should throw ISE) > > Contribution.MD states, that it's automatically checked for whitespace > errors? Is this not true? > > As a clarification, this PR restores the previous behavior before > Platform.startup. With this PR Application.launch and Platform.startup > behaves the same. I forgot to commit a part of the fix, which is why the second test hang. It's now part of the PR. - PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v2]
On Wed, 10 Mar 2021 13:50:02 GMT, Kevin Rushforth wrote:
>> Florian Kirmaier has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - JDK-8263322
>>Added missing change
>> - JDK-8263322
>>Small changes based on code review
>> - JDK-8263322
>>Small changes based on code review
>
> tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line
> 68:
>
>> 66: }
>> 67: });
>> 68: Assert.assertTrue("Timeout", latch.await(10, TimeUnit.SECONDS));
>
> If you use `runAndWait` you won't need this.
Done
> tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line
> 77:
>
>> 75: Application.launch(TestApp.class);
>> 76: System.out.println("Finished launch!");
>> 77: throw new Exception("We excpect an error!");
>
> Usual practice in this case would be to use the junit `Assert.fail` method
> (or else `throw new AssertionError(...)`).
done
> tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line
> 28:
>
>> 26: Application.launch(InitializeApp.class);
>> 27: }).start();
>> 28: appLatch.await();
>
> Presuming you intend to use this (as opposed to removing it), this should be
> changed to an await with a timeout.
done
-
PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v2]
> Fixing deadlock when calling Application.launch in the FXThread after > Platform.startup Florian Kirmaier has updated the pull request incrementally with three additional commits since the last revision: - JDK-8263322 Added missing change - JDK-8263322 Small changes based on code review - JDK-8263322 Small changes based on code review - Changes: - all: https://git.openjdk.java.net/jfx/pull/421/files - new: https://git.openjdk.java.net/jfx/pull/421/files/9a70d349..6abe618e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=421=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx=421=00-01 Stats: 195 lines in 3 files changed: 109 ins; 84 del; 2 mod Patch: https://git.openjdk.java.net/jfx/pull/421.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/421/head:pull/421 PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup
On Wed, 10 Mar 2021 13:19:03 GMT, Kevin Rushforth wrote:
>> Fixing deadlock when calling Application.launch in the FXThread after
>> Platform.startup
>
> tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line
> 65:
>
>> 63: } catch (Exception e) {
>> 64: System.out.println("got exception: " + e);
>> 65: e.printStackTrace();
>
> You should rethrow the exception in this case, since it indicates an
> unexpected error. Better still, you can remove this block and not catch the
> Exception in the first place.
If I would rethrow it, it would just print it out, it's currently doing. I
could save the Exception in a variable, to rethrow it from the other thread,
but I think that would be unnecessarily complicated.
> tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line
> 53:
>
>> 51: }
>> 52:
>> 53: @Test
>
> All of the test methods should take a timeout parameter: `@Test (timeout =
> 15000)`
done
> tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line
> 56:
>
>> 54: public void testStartupThenLaunchInFX() throws Exception {
>> 55: CountDownLatch latch = new CountDownLatch(1);
>> 56: Platform.runLater(() -> {
>
> I recommend using `Util.runAndWait` since it will propagate exceptions from
> the `runLater` lambda to the caller.
This significantly simplifies the test!
-
PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup
On Wed, 10 Mar 2021 13:14:15 GMT, Kevin Rushforth wrote:
>> Fixing deadlock when calling Application.launch in the FXThread after
>> Platform.startup
>
> tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line
> 24:
>
>> 22: }
>> 23:
>> 24: public static void initializeApplication() throws Exception {
>
> This method is unused, along with the `InitializeApp` class. Did you plan to
> use it?
It's useful to compare it with the behavior of the two methods to start JavaFX.
If it would be my codebase, I would keep it, so if someone investigates it
later, it's easier to investigate for differences. But I can also delete it,
what would you prefer?
> modules/javafx.graphics/src/main/java/com/sun/javafx/application/LauncherImpl.java
> line 174:
>
>> 172: final String[] args) {
>> 173:
>> 174: if (com.sun.glass.ui.Application.isEventThread() ||
>> launchCalled.getAndSet(true)) {
>
> Can you do this as two separate `if` checks, with the `isEventThread` check
> first? The error message for this case should be something like "Application
> launch must not be called on the JavaFX Application Thread".
done
> tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line 1:
>
>> 1: package test.javafx.scene;
>
> 1. Missing Copyright header block
> 2. Other platform startup tests are in `test.com.sun.javafx.application`; can
> you move this test there as well?
done
-
PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup
On Wed, 10 Mar 2021 14:17:57 GMT, Kevin Rushforth wrote: >> The idea behind the fix is good. A few changes are needed: >> >> 1. The check should be split into two separate `if` statements, each with >> their own error message (see inline). >> >> 2. This should be documented in the API docs for the Application::launch >> method as an additional case that will throw an IllegalStateException. It >> will need a reasonably trivial CSR since we are specifying a new case that >> will cause an exception to be thrown. >> >> 3. The system test need to be broken up into separate files, one for each >> `@Test` method, since application launching tests need to run in their own >> JVM. If you want to share any code, you could refactor it into a common >> class that has methods (not annotated with `@Test`) to perform the testing, >> and separate classes that will subclass the common class, each with a single >> `@Test` method that simply calls into the method in the common class to do >> the testing. See any number of examples in >> `tests/system/src/test/java/test/com/sun/javafx/application/` (which is also >> a better location for your new test). You will want to add a cleanup method >> annotated with `@AfterClass` that calls `Platform.exit()`. I think three >> tests would be good: >> * Initalize the FX runtime via Platform.startup and then launch an >> Application on another thread (should succeed) >> * Initalize the FX runtime via Platform.startup and then launch an >> Application ~~on another thread~~ the FX Application Thread (should throw >> ISE) >> * Initalize the FX runtime via Application.launch and then launch a >> second Application on another thread (should throw ISE). >> >> >> I note that when I run your test, it hangs (on Windows...I didn't try it on >> other platforms). >> >> gradle --info -PFULL_TEST=true cleanTest :systemTests:test --test >> InitializeJavaFXTest >> >> This might be resolved by ensuring each test is in its own JVM as requested >> above. >> >> >> Additional comments are inline. > >> * Initalize the FX runtime via Platform.startup and then launch an >> Application on another thread (should throw ISE) > > That should be: > > * Initalize the FX runtime via Platform.startup and then launch an > Application on the FX Application thread (should throw ISE) Contribution.MD states, that it's automatically checked for whitespace errors? Is this not true? As a clarification, this PR restores the previous behavior before Platform.startup. With this PR Application.launch and Platform.startup behaves the same. - PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup
On Wed, 10 Mar 2021 13:54:14 GMT, Kevin Rushforth wrote: > * Initalize the FX runtime via Platform.startup and then launch an > Application on another thread (should throw ISE) That should be: * Initalize the FX runtime via Platform.startup and then launch an Application on the FX Application thread (should throw ISE) - PR: https://git.openjdk.java.net/jfx/pull/421
Re: RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup
On Tue, 9 Mar 2021 21:49:36 GMT, Florian Kirmaier wrote:
> Fixing deadlock when calling Application.launch in the FXThread after
> Platform.startup
The idea behind the fix is good. A few changes are needed:
1. The check should be split into two separate `if` statements, each with their
own error message (see inline).
2. This should be documented in the API docs for the Application::launch method
as an additional case that will throw an IllegalStateException. It will need a
reasonably trivial CSR since we are specifying a new case that will cause an
exception to be thrown.
3. The system test need to be broken up into separate files, one for each
`@Test` method, since application launching tests need to run in their own JVM.
If you want to share any code, you could refactor it into a common class that
has methods (not annotated with `@Test`) to perform the testing, and separate
classes that will subclass the common class, each with a single `@Test` method
that simply calls into the method in the common class to do the testing. See
any number of examples in
`tests/system/src/test/java/test/com/sun/javafx/application/` (which is also a
better location for your new test). You will want to add a cleanup method
annotated with `@AfterClass` that calls `Platform.exit()`. I think three tests
would be good:
* Initalize the FX runtime via Platform.startup and then launch an
Application on another thread (should succeed)
* Initalize the FX runtime via Platform.startup and then launch an
Application on another thread (should throw ISE)
* Initalize the FX runtime via Application.launch and then launch a second
Application on another thread (should throw ISE).
I note that when I run your test, it hangs (on Windows...I didn't try it on
other platforms).
gradle --info -PFULL_TEST=true cleanTest :systemTests:test --test
InitializeJavaFXTest
This might be resolved by ensuring each test is in its own JVM as requested
above.
Additional comments are inline.
modules/javafx.graphics/src/main/java/com/sun/javafx/application/LauncherImpl.java
line 174:
> 172: final String[] args) {
> 173:
> 174: if (com.sun.glass.ui.Application.isEventThread() ||
> launchCalled.getAndSet(true)) {
Can you do this as two separate `if` checks, with the `isEventThread` check
first? The error message for this case should be something like "Application
launch must not be called on the JavaFX Application Thread".
tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line 1:
> 1: package test.javafx.scene;
1. Missing Copyright header block
2. Other platform startup tests are in `test.com.sun.javafx.application`; can
you move this test there as well?
tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line 24:
> 22: }
> 23:
> 24: public static void initializeApplication() throws Exception {
This method is unused, along with the `InitializeApp` class. Did you plan to
use it?
tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line 28:
> 26: Application.launch(InitializeApp.class);
> 27: }).start();
> 28: appLatch.await();
Presuming you intend to use this (as opposed to removing it), this should be
changed to an await with a timeout.
tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line 36:
> 34: latch.countDown();
> 35: });
> 36: latch.await();
This needs to be changed to a flavor of await with a timeout (you can assert
that it doesn't timeout). Also, I don't think this needs to be its own method,
since the only thing the `initialize` method does is call this.
tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line 65:
> 63: } catch (Exception e) {
> 64: System.out.println("got exception: " + e);
> 65: e.printStackTrace();
You should rethrow the exception in this case, since it indicates an unexpected
error. Better still, you can remove this block and not catch the Exception in
the first place.
tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line 53:
> 51: }
> 52:
> 53: @Test
All of the test methods should take a timeout parameter: `@Test (timeout =
15000)`
tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line 77:
> 75: Application.launch(TestApp.class);
> 76: System.out.println("Finished launch!");
> 77: throw new Exception("We excpect an error!");
Usual practice in this case would be to use the junit `Assert.fail` method (or
else `throw new AssertionError(...)`).
tests/system/src/test/java/test/javafx/scene/InitializeJavaFXTest.java line 56:
> 54: public void testStartupThenLaunchInFX() throws Exception {
> 55: CountDownLatch latch = new CountDownLatch(1);
> 56: Platform.runLater(() -> {
I recommend using `Util.runAndWait` since it will
Re: CFV: New OpenJFX Committer: John Neffenger
Vote: YES Zitat von Kevin Rushforth : I hereby nominate John Neffenger [1] to OpenJFX Committer. John is an OpenJFX community member, who has contributed 10 commits [2] to OpenJFX. Votes are due by March 24, 2021 at 12:00 UTC. Only current OpenJFX Committers [3] are eligible to vote on this nomination. Votes must be cast in the open by replying to this mailing list. For Lazy Consensus voting instructions, see [4]. Nomination to a project Committer is described in [6]. Thanks. -- Kevin [1] https://openjdk.java.net/census#jgneff [2] https://github.com/openjdk/jfx/search?q=author-email%3Ajohn%40status6.com=author-date=Commits [3] https://openjdk.java.net/census#openjfx [4] https://openjdk.java.net/bylaws#lazy-consensus [6] https://openjdk.java.net/projects#project-committer
Re: CFV: New OpenJFX Committer: John Neffenger
Vote: yes Thanks, Guru > On 10-Mar-2021, at 5:27 PM, Kevin Rushforth > wrote: > > I hereby nominate John Neffenger [1] to OpenJFX Committer. > > John is an OpenJFX community member, who has contributed 10 commits [2] to > OpenJFX. > > Votes are due by March 24, 2021 at 12:00 UTC. > > Only current OpenJFX Committers [3] are eligible to vote on this nomination. > Votes must be cast in the open by replying to this mailing list. > > For Lazy Consensus voting instructions, see [4]. Nomination to a project > Committer is described in [6]. > > Thanks. > > -- Kevin > > > [1] https://openjdk.java.net/census#jgneff > > [2] > https://github.com/openjdk/jfx/search?q=author-email%3Ajohn%40status6.com=author-date=Commits > > [3] https://openjdk.java.net/census#openjfx > > [4] https://openjdk.java.net/bylaws#lazy-consensus > > [6] https://openjdk.java.net/projects#project-committer > >
Integrated: 8206253: No/Wrong scroll events from touch input in window mode
On Mon, 8 Mar 2021 23:43:10 GMT, Jose Pereda wrote: > This PR changes the parameter names to accommodate class calculations related > to screen event coordinates (AbsX, AbsY). > > As > [discussed](https://bugs.openjdk.java.net/browse/JDK-8206253?focusedCommentId=14405707=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14405707), > the sendScrollXXXEvent methods are currently passing the screen coordinates > (AbsX, AbsY) to the local ones, but they shouldn't modify those, but the > screen ones. > > Tested successfully on Android with ComboBox controls in different positions. This pull request has now been integrated. Changeset: 1473ea96 Author:Jose Pereda URL: https://git.openjdk.java.net/jfx/commit/1473ea96 Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 mod 8206253: No/Wrong scroll events from touch input in window mode Reviewed-by: kcr, jvos - PR: https://git.openjdk.java.net/jfx/pull/420
Re: RFR: 8206253: No/Wrong scroll events from touch input in window mode [v2]
On Tue, 9 Mar 2021 18:51:22 GMT, Jose Pereda wrote: >> This PR changes the parameter names to accommodate class calculations >> related to screen event coordinates (AbsX, AbsY). >> >> As >> [discussed](https://bugs.openjdk.java.net/browse/JDK-8206253?focusedCommentId=14405707=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14405707), >> the sendScrollXXXEvent methods are currently passing the screen coordinates >> (AbsX, AbsY) to the local ones, but they shouldn't modify those, but the >> screen ones. >> >> Tested successfully on Android with ComboBox controls in different positions. > > Jose Pereda has updated the pull request incrementally with one additional > commit since the last revision: > > Refactor parameter names Marked as reviewed by jvos (Reviewer). - PR: https://git.openjdk.java.net/jfx/pull/420
Re: RFR: 8206253: No/Wrong scroll events from touch input in window mode [v2]
On Tue, 9 Mar 2021 18:48:49 GMT, Jose Pereda wrote: >> Yes, that makes sense. >> >> We could refactor the three `sendScrollXXXEvent` methods to something like: >> >> sendScrollXXXEvent(double xAbs, double yAbs, int touchCount) >> or to: >> >> sendScrollXXXEvent(double x, double y, double xAbs, double yAbs, int >> touchCount) >> >> Any preference? > > For now, I've done the first approach, given that there is no conflict with > the `centerX, centerY` variables. I am ok with that. With this rename, the conflict is resolved. - PR: https://git.openjdk.java.net/jfx/pull/420
Re: RFR: 8206253: No/Wrong scroll events from touch input in window mode [v2]
On Tue, 9 Mar 2021 18:51:22 GMT, Jose Pereda wrote: >> This PR changes the parameter names to accommodate class calculations >> related to screen event coordinates (AbsX, AbsY). >> >> As >> [discussed](https://bugs.openjdk.java.net/browse/JDK-8206253?focusedCommentId=14405707=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14405707), >> the sendScrollXXXEvent methods are currently passing the screen coordinates >> (AbsX, AbsY) to the local ones, but they shouldn't modify those, but the >> screen ones. >> >> Tested successfully on Android with ComboBox controls in different positions. > > Jose Pereda has updated the pull request incrementally with one additional > commit since the last revision: > > Refactor parameter names Marked as reviewed by kcr (Lead). - PR: https://git.openjdk.java.net/jfx/pull/420
Re: CFV: New OpenJFX Committer: John Neffenger
Vote: YES On Wed, Mar 10, 2021 at 2:04 PM Kevin Rushforth wrote: > Vote: YES > > -- Kevin > > > On 3/10/2021 3:57 AM, Kevin Rushforth wrote: > > I hereby nominate John Neffenger [1] to OpenJFX Committer. > >
Re: CFV: New OpenJFX Committer: John Neffenger
Vote: YES -- Kevin On 3/10/2021 3:57 AM, Kevin Rushforth wrote: I hereby nominate John Neffenger [1] to OpenJFX Committer.
Re: CFV: New OpenJFX Committer: John Neffenger
Vote: yes On Wed, Mar 10, 2021 at 12:58 PM Kevin Rushforth wrote: > I hereby nominate John Neffenger [1] to OpenJFX Committer. > > John is an OpenJFX community member, who has contributed 10 commits [2] > to OpenJFX. > > Votes are due by March 24, 2021 at 12:00 UTC. > > Only current OpenJFX Committers [3] are eligible to vote on this > nomination. Votes must be cast in the open by replying to this mailing > list. > > For Lazy Consensus voting instructions, see [4]. Nomination to a project > Committer is described in [6]. > > Thanks. > > -- Kevin > > > [1] https://openjdk.java.net/census#jgneff > > [2] > > https://github.com/openjdk/jfx/search?q=author-email%3Ajohn%40status6.com=author-date=Commits > > [3] https://openjdk.java.net/census#openjfx > > [4] https://openjdk.java.net/bylaws#lazy-consensus > > [6] https://openjdk.java.net/projects#project-committer > > >
CFV: New OpenJFX Committer: John Neffenger
I hereby nominate John Neffenger [1] to OpenJFX Committer. John is an OpenJFX community member, who has contributed 10 commits [2] to OpenJFX. Votes are due by March 24, 2021 at 12:00 UTC. Only current OpenJFX Committers [3] are eligible to vote on this nomination. Votes must be cast in the open by replying to this mailing list. For Lazy Consensus voting instructions, see [4]. Nomination to a project Committer is described in [6]. Thanks. -- Kevin [1] https://openjdk.java.net/census#jgneff [2] https://github.com/openjdk/jfx/search?q=author-email%3Ajohn%40status6.com=author-date=Commits [3] https://openjdk.java.net/census#openjfx [4] https://openjdk.java.net/bylaws#lazy-consensus [6] https://openjdk.java.net/projects#project-committer
Re: RFR: 8262276: Debug build of WebKit fails
On Tue, 9 Mar 2021 15:46:06 GMT, Arun Joseph wrote: >> I think this is actually a lack of physical memory. I was doing tests inside >> Windows Sandbox which has 4GB of memory by default. >> >> After repeating the build procedure inside VM with 16GB of memory the build >> succeeded. >> >> Side question - I see there was: >> `C:\jfx\build\modular-sdk\modules_libs\javafx.web\jfxwebkit.dll` >> produced but I don't see any PDB files (debugging and project state >> information) despite `CONF = DebugNative` is present. Am I missing something? > > The PDB files are located in > `modules\javafx.web\build\win\Debug\bin\jfxwebkit.pdb`. At present, they are > not copied to sdk. This bug > (https://bugs.openjdk.java.net/browse/JDK-8263259) is created to fix the same. @arun-Joseph , thanks. This is exactly where PDB files were located. To give something back to the community I've prepared a script ([openjfx-build-env.zip](https://github.com/openjdk/jfx/files/6114811/openjfx-build-env.zip)) which installs [Chocolatey ](https://chocolatey.org/) and all required dependencies needed to came up with `JavaFX` & `WebKit` Windows build environment quickly - _please use at your own risk_). I suggest using dedicated/clean Windows 10 Virtual machine or Windows Sandbox ([with 8GB memory](https://docs.microsoft.com/en-us/windows/security/threat-protection/windows-sandbox/windows-sandbox-configure-using-wsb-file#memory-in-mb)). Steps needed to create JavaFX build from JFX Github repository (with included pull/417): 1. Run `PowerShell` as `Administrator` 2. Run `openjfx-build-env.bat` (extracted from [openjfx-build-env.zip](https://github.com/openjdk/jfx/files/6114811/openjfx-build-env.zip)) 3. Run `Cygwin` 4. Run (last two commands won't be needed anymore after this pull request will be merged to the master): `git clone https://github.com/openjdk/jfx.git` (from `C:`) `git fetch https://git.openjdk.java.net/jfx pull/417/head:pull/417`(from `C:/jfx`) `git checkout pull/417` (from `C:/jfx`) 5. OPTIONAL: Only if `WebKit` and native debug is needed: Edit `gradle.properties` (renamed from `gradle.properties.template`) inside `C:/jfx` and set: `COMPILE_WEBKIT = true` `CONF = DebugNative` 6. Run `chmod +x gradlew` (from `C:/jfx`) 7. Run `./gradlew --console=plain` (from `C:/jfx`) Side notes: - Due to [JDK-8263259](https://bugs.openjdk.java.net/browse/JDK-8263259), the PDB files will be placed inside `c:/jfx/modules/javafx.web/build/win/Debug/bin/jfxwebkit.pdb`. - PR: https://git.openjdk.java.net/jfx/pull/417
