Integrated: 8272779: Package docs for javafx.embed.swing are misleading

[Prasanta Sadhukhan]

On Tue, 24 Aug 2021 05:10:40 GMT, Prasanta Sadhukhan  
wrote:

> The API docs for the javafx.embed.swing package are misleading in that they 
> only talk about the JFXPanel capability (embedding a JavaFX Scene in a Swing 
> JComponent) and ignore the SwingNode functionality entirely. 
> Fix the package doc.

This pull request has now been integrated.

Changeset: 91f01709
Author:    Prasanta Sadhukhan 
URL:   
https://git.openjdk.java.net/jfx/commit/91f017091786654a4564a5a054dbd22e6a01c120
Stats: 19 lines in 1 file changed: 12 ins; 0 del; 7 mod

8272779: Package docs for javafx.embed.swing are misleading

Reviewed-by: kcr

-

PR: https://git.openjdk.java.net/jfx/pull/609

Re: RFR: 8272779: Package docs for javafx.embed.swing are misleading [v3]

[Prasanta Sadhukhan]

> The API docs for the javafx.embed.swing package are misleading in that they 
> only talk about the JFXPanel capability (embedding a JavaFX Scene in a Swing 
> JComponent) and ignore the SwingNode functionality entirely. 
> Fix the package doc.

Prasanta Sadhukhan has updated the pull request incrementally with one 
additional commit since the last revision:

  Fix review comment

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/609/files
  - new: https://git.openjdk.java.net/jfx/pull/609/files/dd60daad..eecd971b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=609&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=609&range=01-02

  Stats: 5 lines in 1 file changed: 0 ins; 1 del; 4 mod
  Patch: https://git.openjdk.java.net/jfx/pull/609.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/609/head:pull/609

PR: https://git.openjdk.java.net/jfx/pull/609

Re: RFR: 8272779: Package docs for javafx.embed.swing are misleading [v2]

[Prasanta Sadhukhan]

> The API docs for the javafx.embed.swing package are misleading in that they 
> only talk about the JFXPanel capability (embedding a JavaFX Scene in a Swing 
> JComponent) and ignore the SwingNode functionality entirely. 
> Fix the package doc.

Prasanta Sadhukhan has updated the pull request incrementally with one 
additional commit since the last revision:

  Fix

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/609/files
  - new: https://git.openjdk.java.net/jfx/pull/609/files/88b9b285..dd60daad

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=609&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=609&range=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jfx/pull/609.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/609/head:pull/609

PR: https://git.openjdk.java.net/jfx/pull/609

RFR: 8272779: Package docs for javafx.embed.swing are misleading

[Prasanta Sadhukhan]

The API docs for the javafx.embed.swing package are misleading in that they 
only talk about the JFXPanel capability (embedding a JavaFX Scene in a Swing 
JComponent) and ignore the SwingNode functionality entirely. 
Fix the package doc.

-

Commit messages:
 - Fix
 - Fix

Changes: https://git.openjdk.java.net/jfx/pull/609/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=609&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8272779
  Stats: 18 lines in 1 file changed: 13 ins; 0 del; 5 mod
  Patch: https://git.openjdk.java.net/jfx/pull/609.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/609/head:pull/609

PR: https://git.openjdk.java.net/jfx/pull/609

Re: RFR: 8269638: Property methods, setters, and getters in printing API should be final [v4]

[Prasanta Sadhukhan]

On Thu, 15 Jul 2021 18:06:44 GMT, Phil Race  wrote:

>> - Make various setters and getters and properties final as needed
>> - Move documentation to the property so the setters and getters inherit it, 
>> with an exception for the special case of JobSettings.setPageRanges()
>> - Override toString() on the properties in JobSettings so it doesn't 
>> delegate to the JobSettings class.
>> - Add a manual test program just so you can see what toString() does. No 
>> pass or fail, just informative.
>> 
>> This will need a CSR but I won't create that until the review is done.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8269638: Property methods, setters, and getters in printing API should be 
> final

Marked as reviewed by psadhukhan (Reviewer).

-

PR: https://git.openjdk.java.net/jfx/pull/574

Re: RFR: 8269638: Property methods, setters, and getters in printing API should be final [v4]

[Prasanta Sadhukhan]

On Thu, 15 Jul 2021 18:06:44 GMT, Phil Race  wrote:

>> - Make various setters and getters and properties final as needed
>> - Move documentation to the property so the setters and getters inherit it, 
>> with an exception for the special case of JobSettings.setPageRanges()
>> - Override toString() on the properties in JobSettings so it doesn't 
>> delegate to the JobSettings class.
>> - Add a manual test program just so you can see what toString() does. No 
>> pass or fail, just informative.
>> 
>> This will need a CSR but I won't create that until the review is done.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8269638: Property methods, setters, and getters in printing API should be 
> final

looks good..
However I have a different question...I was looking at printerProperty and I 
saw In l182, we are not checking for getDefaultPrinter() returns null or not 
but in l120, we do...Is it not required in l182?

-

PR: https://git.openjdk.java.net/jfx/pull/574

Re: RFR: 8269638: Property methods, setters, and getters in printing API should be final [v3]

[Prasanta Sadhukhan]

On Wed, 14 Jul 2021 20:44:47 GMT, Phil Race  wrote:

>> - Make various setters and getters and properties final as needed
>> - Move documentation to the property so the setters and getters inherit it, 
>> with an exception for the special case of JobSettings.setPageRanges()
>> - Override toString() on the properties in JobSettings so it doesn't 
>> delegate to the JobSettings class.
>> - Add a manual test program just so you can see what toString() does. No 
>> pass or fail, just informative.
>> 
>> This will need a CSR but I won't create that until the review is done.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8269638: Property methods, setters, and getters in printing API should be 
> final

modules/javafx.graphics/src/main/java/javafx/print/PrinterJob.java line 226:

> 224:  * Setting a null value for printer will install the default printer.
> 225:  * Setting the current printer has no effect.
> 226:  * @return the Printer for this job

If we are using {@code Printer} preferred pattern above for new addition, 
probably we should use here too, no?

tests/manual/printing/JobSettingsInfo.java line 45:

> 43: import javafx.scene.Scene;
> 44: import javafx.scene.control.TextArea;
> 45: import javafx.scene.layout.*;

One more place wildcard overlooked...Since we changed one case above, probably 
it can also be rectified..

-

PR: https://git.openjdk.java.net/jfx/pull/574

Re: RFR: 8269638: Property methods, setters, and getters in printing API should be final [v2]

[Prasanta Sadhukhan]

On Mon, 12 Jul 2021 22:04:25 GMT, Phil Race  wrote:

>> - Make various setters and getters and properties final as needed
>> - Move documentation to the property so the setters and getters inherit it, 
>> with an exception for the special case of JobSettings.setPageRanges()
>> - Override toString() on the properties in JobSettings so it doesn't 
>> delegate to the JobSettings class.
>> - Add a manual test program just so you can see what toString() does. No 
>> pass or fail, just informative.
>> 
>> This will need a CSR but I won't create that until the review is done.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8269638: Property methods, setters, and getters in printing API should be 
> final

ok

-

Marked as reviewed by psadhukhan (Reviewer).

PR: https://git.openjdk.java.net/jfx/pull/574

Re: RFR: 8269638: Property methods, setters, and getters in printing API should be final [v2]

[Prasanta Sadhukhan]

On Mon, 12 Jul 2021 22:04:25 GMT, Phil Race  wrote:

>> - Make various setters and getters and properties final as needed
>> - Move documentation to the property so the setters and getters inherit it, 
>> with an exception for the special case of JobSettings.setPageRanges()
>> - Override toString() on the properties in JobSettings so it doesn't 
>> delegate to the JobSettings class.
>> - Add a manual test program just so you can see what toString() does. No 
>> pass or fail, just informative.
>> 
>> This will need a CSR but I won't create that until the review is done.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8269638: Property methods, setters, and getters in printing API should be 
> final

modules/javafx.graphics/src/main/java/javafx/print/PrinterJob.java line 212:

> 210: 
> 211: /**
> 212:  * Property representing the {@code Printer} for this job.

I guess we normally used  throughout . Probably better to be 
consistent.

tests/manual/printing/JobSettingsInfo.java line 31:

> 29: import javafx.collections.FXCollections;
> 30: import javafx.collections.ObservableList;
> 31: import javafx.print.*;

Maybe we can do away with this wildcard and import explicit class.. Also, down 
below one more place...

-

PR: https://git.openjdk.java.net/jfx/pull/574

Re: RFR: 8223717: javafx printing: Support Specifying Print to File in the API [v6]

[Prasanta Sadhukhan]

On Thu, 1 Jul 2021 00:38:25 GMT, Phil Race  wrote:

>> This enhancement adds the String property outputFileProperty() to the 
>> JobSettings class.
>> The value should be a string that references a local file encoded as a URL.
>> If this is non-null and set to a location that the user has permission to 
>> write to,
>> then the printer output will be spooled there instead of the printer, so 
>> long as the platform printing system supports this.
>> The user can of course also set a print-to-file destination in the platform 
>> printer dialogs which may over-ride what the application set. But now the 
>> application can also see what it was set to, and cancel or alter it if 
>> necessary.
>> 
>> A simple manual test is provided, manual mainly because the few real 
>> printing functional tests are all manual as they are only useful if run with 
>> a printer configured.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8223717: javafx printing: Support Specifying Print to File in the API

Marked as reviewed by psadhukhan (Reviewer).

-

PR: https://git.openjdk.java.net/jfx/pull/543

Re: RFR: 8223717: javafx printing: Support Specifying Print to File in the API [v3]

[Prasanta Sadhukhan]

On Thu, 24 Jun 2021 23:00:42 GMT, Phil Race  wrote:

>> This enhancement adds the String property outputFileProperty() to the 
>> JobSettings class.
>> The value should be a string that references a local file encoded as a URL.
>> If this is non-null and set to a location that the user has permission to 
>> write to,
>> then the printer output will be spooled there instead of the printer, so 
>> long as the platform printing system supports this.
>> The user can of course also set a print-to-file destination in the platform 
>> printer dialogs which may over-ride what the application set. But now the 
>> application can also see what it was set to, and cancel or alter it if 
>> necessary.
>> 
>> A simple manual test is provided, manual mainly because the few real 
>> printing functional tests are all manual as they are only useful if run with 
>> a printer configured.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8223717: javafx printing: Support Specifying Print to File in the API

modules/javafx.graphics/src/main/java/com/sun/prism/j2d/print/J2DPrinterJob.java
 line 352:

> 350: try {
> 351: settings.setOutputFile(dest.getURI().toURL().toString());
> 352: } catch (MalformedURLException e) {

I guess in 2D RasterPrinterJob, if there is any exception, we try to form a 
default file "out.prn"  
[https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/sun/print/RasterPrinterJob.java#L1137]
and try to print there. Can't we do it here too?

modules/javafx.graphics/src/main/java/com/sun/prism/j2d/print/J2DPrinterJob.java
 line 356:

> 354: } else {
> 355: settings.setOutputFile("");
> 356: }

Actually in 2D , it seems if dest is null, we redirect printing to actual 
printer instead of file. Not sure if that is what we need to do here too!!
[https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/sun/print/RasterPrinterJob.java#L1148]

modules/javafx.graphics/src/main/java/com/sun/prism/j2d/print/J2DPrinterJob.java
 line 839:

> 837: security.checkPrintJobAccess();
> 838: String file = settings.getOutputFile();
> 839: if (!file.isEmpty()) {

Don't we need to check for file!= null?

modules/javafx.graphics/src/main/java/javafx/print/JobSettings.java line 485:

> 483:  * 
> 484:  * If the application does not have permission to write to the 
> specified
> 485:  * file, a {@code SecurityException} may be thrown when printing.

As I see in 2D atleast in Win32PrintService, if there is a SecurityException 
for creating a Destination object from a URI, it retries again by creating a 
new URL with default file "file:out.prn"
Is it not needed here?
[https://github.com/openjdk/jdk/blob/master/src/java.desktop/windows/classes/sun/print/Win32PrintService.java#L1181]

-

PR: https://git.openjdk.java.net/jfx/pull/543

Re: RFR: 8231372: JFXPanel fails to render if setScene called on Swing thread [v2]

[Prasanta Sadhukhan]

On Sat, 5 Dec 2020 13:42:21 GMT, Kevin Rushforth  wrote:

>> This fix was originally proposed by @mruzicka in PR #16 which was closed 
>> several months ago without being integrated. At the time we didn't have a 
>> test case that was failing.
>> 
>> While evaluating a bug that I filed, 
>> [JDK-8235843](https://bugs.openjdk.java.net/browse/JDK-8235843), I 
>> discovered that that bug was a duplicate of this one. The original proposed 
>> fix is correct, although I modified it slightly to add a try / finally so 
>> that the secondary event loop will be terminated even if the setScene throws 
>> an exception. I also added a unit test. Since the bulk of the fix is from PR 
>> #16, I will add the contributor of that PR.
>> 
>> NOTE to reviewers: I pushed two commits to my branch. The first is exactly 
>> the commit created for PR #16 and the second is the unit test along with the 
>> fix to the code to add try / finally. As always, they will be squashed into 
>> a single commit by Skara.
>
> Kevin Rushforth has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Dispose JFrame after each test

Marked as reviewed by psadhukhan (Reviewer).

-

PR: https://git.openjdk.java.net/jfx/pull/362

Re: RFR: 8231372: JFXPanel fails to render if setScene called on Swing thread

[Prasanta Sadhukhan]

On Fri, 4 Dec 2020 02:07:37 GMT, Kevin Rushforth  wrote:

> This fix was originally proposed by @mruzicka in PR #16 which was closed 
> several months ago without being integrated. At the time we didn't have a 
> test case that was failing.
> 
> While evaluating a bug that I filed, 
> [JDK-8235843](https://bugs.openjdk.java.net/browse/JDK-8235843), I discovered 
> that that bug was a duplicate of this one. The original proposed fix is 
> correct, although I modified it slightly to add a try / finally so that the 
> secondary event loop will be terminated even if the setScene throws an 
> exception. I also added a unit test. Since the bulk of the fix is from PR 
> #16, I will add the contributor of that PR.
> 
> NOTE to reviewers: I pushed two commits to my branch. The first is exactly 
> the commit created for PR #16 and the second is the unit test along with the 
> fix to the code to add try / finally. As always, they will be squashed into a 
> single commit by Skara.

Marked as reviewed by psadhukhan (Reviewer).

-

PR: https://git.openjdk.java.net/jfx/pull/362

Re: [jfx15] RFR: 8220484: JFXPanel renders a slanted image with a hidpi monitor scale of 125% or 175% [v6]

[Prasanta Sadhukhan]

On Mon, 20 Jul 2020 18:11:34 GMT, Oliver Schmidtmer 
 wrote:

>> In edge cases where monitor scaling of 1.25 or 1.75 is active, Math.ceil and 
>> Math.round produce different results and
>> EmbeddedScene#getPixels in JFXPanel#paintComponent causes an off-by-one 
>> error on the line width and therefore sheared
>> rendering.  The changes were already proposed by the submitter in 
>> JBS-8220484.
>> 
>> OCA is signed and send.
>
> Oliver Schmidtmer has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - run test only on windows
>  - Typo

Marked as reviewed by psadhukhan (Reviewer).

-

PR: https://git.openjdk.java.net/jfx/pull/246

Re: RFR: 8220484: JFXPanel renders a slanted image with a hidpi monitor scale of 125% or 175%

[Prasanta Sadhukhan]

On Wed, 3 Jun 2020 15:46:36 GMT, Oliver Schmidtmer 
 wrote:

> In edge cases where monitor scaling of 1.25 or 1.75 is active, Math.ceil and 
> Math.round produce different results and
> EmbeddedScene#getPixels in JFXPanel#paintComponent causes an off-by-one error 
> on the line width and therefore sheared
> rendering.  The changes were already proposed by the submitter in JBS-8220484.
> 
> OCA is signed and send.

In 2D, we normally use sun.java2d.pipe.Region.clipRound as it also checks for 
-ve/+ve max INTEGER but I guess that is
internal class to FX so it's ok to use Math.ceil.  Approval pending test 
creation.

-

PR: https://git.openjdk.java.net/jfx/pull/246

Re: CFV: New OpenJFX Committer: Arun Joseph

[Prasanta Sadhukhan]

Vote: YES

Regards
Prasanta
On 26-Mar-20 5:14 PM, Kevin Rushforth wrote:

I hereby nominate Arun Joseph [1] to OpenJFX Committer.

Arun is a member of JavaFX team at Oracle, who has contributed 13 
commits [2][3] to OpenJFX.


Votes are due by April 9, 2020.

Only current OpenJFX Committers [4] 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 [5]. Nomination to a 
project Committer is described in [6].


Thanks.

-- Kevin

[1] http://openjdk.java.net/census#ajoseph

[2] https://github.com/openjdk/jfx/commits?author=arun-Joseph

[3] 
https://github.com/openjdk/jfx/commit/b2d85645ffc7edc327b28e12eede6505912d7491


[4] http://openjdk.java.net/census#openjfx

[5] http://openjdk.java.net/bylaws#lazy-consensus

[6] http://openjdk.java.net/projects#project-committer

Re: [Approved] RFR: 8200224: Multiple press event when JFXPanel gains focus

[Prasanta Sadhukhan]

On Tue, 26 Nov 2019 13:06:36 GMT, Florian Kirmaier  
wrote:

> The pull request has been updated with additional changes.
> 
> 
> 
> Added commits:
>  - 24385eb8: JDK-8200224
>  - e0829ad3: JDK-8200224
>  - c190384f: JDK-8200224
>  - 17b458b1: JDK-8200224
> 
> Changes:
>   - all: https://git.openjdk.java.net/jfx/pull/25/files
>   - new: https://git.openjdk.java.net/jfx/pull/25/files/44774dfb..24385eb8
> 
> Webrevs:
>  - full: https://webrevs.openjdk.java.net/jfx/25/webrev.04
>  - incr: https://webrevs.openjdk.java.net/jfx/25/webrev.03-04
> 
>   Issue: https://bugs.openjdk.java.net/browse/JDK-8200224
>   Stats: 141 lines in 2 files changed: 123 ins; 11 del; 7 mod
>   Patch: https://git.openjdk.java.net/jfx/pull/25.diff
>   Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25

Looks good albeit with minor suggestion regarding presence of bugid in comment



Approved by psadhukhan (Reviewer).

PR: https://git.openjdk.java.net/jfx/pull/25

Re: [Rev 04] RFR: 8200224: Multiple press event when JFXPanel gains focus

[Prasanta Sadhukhan]

On Tue, 26 Nov 2019 13:06:36 GMT, Florian Kirmaier  
wrote:

> The pull request has been updated with additional changes.
> 
> 
> 
> Added commits:
>  - 24385eb8: JDK-8200224
>  - e0829ad3: JDK-8200224
>  - c190384f: JDK-8200224
>  - 17b458b1: JDK-8200224
> 
> Changes:
>   - all: https://git.openjdk.java.net/jfx/pull/25/files
>   - new: https://git.openjdk.java.net/jfx/pull/25/files/44774dfb..24385eb8
> 
> Webrevs:
>  - full: https://webrevs.openjdk.java.net/jfx/25/webrev.04
>  - incr: https://webrevs.openjdk.java.net/jfx/25/webrev.03-04
> 
>   Issue: https://bugs.openjdk.java.net/browse/JDK-8200224
>   Stats: 141 lines in 2 files changed: 123 ins; 11 del; 7 mod
>   Patch: https://git.openjdk.java.net/jfx/pull/25.diff
>   Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25

modules/javafx.swing/src/main/java/javafx/embed/swing/JFXPanel.java line 449:

> 448: requestFocus();
> 449: // This fixes JDK-8087914 without causing JDK-8200224
> 450: // It is safe, because in JavaFX only the method 
> "setFocused(true)" is called,

We actually do not clutter source code with bugids, it will be good if it can 
be removed with proper comments maybe something like "extra simulated mouse 
pressed event is removed by making the JavaFX scene focused"

PR: https://git.openjdk.java.net/jfx/pull/25

[13] RFR JDK-8222212:Memory Leak with SwingNode using Drag and Drop function

[Prasanta Sadhukhan]

Hi All,

Please review fix for SwingNode memory leak when SwingNode content has a 
drop target therefore, initiating a DnD. FXDnd holds a strong reference 
to SwingNode thereby preventing it to be disposed.
Proposed fix is to weakreference the swingnode object in FXDnDInteropN 
class.


Bug: https://bugs.openjdk.java.net/browse/JDK-812
webrev: http://cr.openjdk.java.net/~psadhukhan/812/webrev.0/

Regards
Prasanta

[12] RFR JDK-8211248: Create manual drag and drop tests for FX / Swing interop

[Prasanta Sadhukhan]

Hi Kevin, All

Please review few manual tests being added to test swingnode/jfxpanel 
DnD feature.


Bug:https://bugs.openjdk.java.net/browse/JDK-8211248

webrev: http://cr.openjdk.java.net/~psadhukhan/fx/8211248/webrev.0/

Regards
Prasanta

[12] RFR JDK-8211249:Refactor javafx.swing implementation to get rid of unneeded abstraction layer

[Prasanta Sadhukhan]

Hi Kevin,All,

Please review a cleanup of the abstraction layer done during refactoring 
of fx swing interop implementation.


Bug: https://bugs.openjdk.java.net/browse/JDK-8211249
webrev: http://cr.openjdk.java.net/~psadhukhan/fx/8211249/webrev.0/

Regards
Prasanta

Re: [12] RFR: JDK-8210092: Remove old javafx.swing implementation

[Prasanta Sadhukhan]

+1 with the additional refactoring done.

Regards
Prasanta
On 25-Sep-18 6:03 PM, Kevin Rushforth wrote:

Yes, that seems like a good bit of additional cleanup. I'll do that.

-- Kevin


On 9/25/2018 1:41 AM, Prasanta Sadhukhan wrote:


Hi Kevin,

Removal looks ok but one thing, do we still need to keep the 
enhanced-for loop in InterOpFactory.java#getInstance now that we have 
only 1 factory? I guess we can probably remove the whole reflection 
thing also and instantiate InteropFactoryN directly, no?


Regards
Prasanta
On 22-Sep-18 5:23 AM, Kevin Rushforth wrote:

Please review the following on GitHub:

https://bugs.openjdk.java.net/browse/JDK-8210092
https://github.com/javafxports/openjdk-jfx/pull/207
https://github.com/javafxports/openjdk-jfx/pull/207/files

This will remove the old JDK-10-based implementation of FX / Swing 
interop and cleanup the build to remove the legacy qualified exports 
and the logic that optionally excludes the new implementation. It 
also changes the dependency that javafx.swing has on 
jdk.unsupported.desktop from "requires static" (optional) to 
"requires", which will fix an existing bug with jlinked images that 
include javafx.swing.


-- Kevin

Re: [12] RFR: JDK-8210092: Remove old javafx.swing implementation

[Prasanta Sadhukhan]

Hi Kevin,

Removal looks ok but one thing, do we still need to keep the 
enhanced-for loop in InterOpFactory.java#getInstance now that we have 
only 1 factory? I guess we can probably remove the whole reflection 
thing also and instantiate InteropFactoryN directly, no?


Regards
Prasanta
On 22-Sep-18 5:23 AM, Kevin Rushforth wrote:

Please review the following on GitHub:

https://bugs.openjdk.java.net/browse/JDK-8210092
https://github.com/javafxports/openjdk-jfx/pull/207
https://github.com/javafxports/openjdk-jfx/pull/207/files

This will remove the old JDK-10-based implementation of FX / Swing 
interop and cleanup the build to remove the legacy qualified exports 
and the logic that optionally excludes the new implementation. It also 
changes the dependency that javafx.swing has on 
jdk.unsupported.desktop from "requires static" (optional) to 
"requires", which will fix an existing bug with jlinked images that 
include javafx.swing.


-- Kevin

Re: Swing module's module-info file

[Prasanta Sadhukhan]

This is because if fx is compiled with a jdk version which does not have 
jdk.unsupported.desktop module then having module-info.java in its 
original place would cause compilation error as module-info.java contains


requires static jdk.unsupported.desktop;

So, the idea was to copy the file into a directory which is not on the 
module-source-path. Then build.gradle copy it from there to gensrc 
directory optionally filtering the above line
 task copyModuleInfo(type: Copy, description: "copy module-info file to 
gensrc") {

    from "src/main/module-info/module-info.java"
    into "$buildDir/gensrc/java/"
    filter { line->
    !HAS_UNSUPPORTED_DESKTOP && 
line.contains('jdk.unsupported.desktop') ? null : line

    }
    }

Regards
Prasanta
On 8/7/2018 6:42 AM, Nir Lisker wrote:

Hi,

I didn't follow all the latest changes to the Swing module, but I notice
now its module-info.java file is not in the same place where other modules
have theirs:

It's under javafx.swing\src\main\module-info instead of
javafx.\src\main\java.

Is there a reason for this?

- Nir

Re: JavaFX SWT and Swing-Interopt

[Prasanta Sadhukhan]

Hi Tom,

I am not able to see the problem in latest workspace. I believe this 
issue is already fixed by JDK-8185634.


Regards
Prasanta
On 7/31/2018 1:16 PM, Tom Schindl wrote:

Hi,

In one of our customers application we have major problems when
embedding JavaFX into SWT (but from my tests the situation is equal for
Swing).

In the end all windows (most commonly popup-windows) are affected
because they don't have a parent-window assigned and so they can be
hidden by the window embedding them - if the window opened is Modal like
eg the one from ColorPicker the JavaFX UI is also blocked.

To see what I mean just use this snippet below and do the following:
* Bring up the DropDown-List
* Bring up the custom color dialog
* Click somewhere in the JFrame

You notice the following:
* JavaFX Window is moved behind Swing-Window
* JavaFX UI is blocked


package test;

import javax.swing.JFrame;

import javafx.application.Platform;
import javafx.embed.swing.JFXPanel;
import javafx.geometry.Pos;
import javafx.scene.Scene;
import javafx.scene.control.ColorPicker;
import javafx.scene.layout.BorderPane;

public class TestSwingInterop extends JFrame {

public TestSwingInterop() {
setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
final JFXPanel fxPanel = new JFXPanel();
 add(fxPanel);
 
 Platform.runLater(new Runnable() {

 @Override
 public void run() {
 initFX(fxPanel);
 }
 });
}

private static void initFX(JFXPanel fxPanel) {
 // This method is invoked on JavaFX thread
 Scene scene = createScene();
 fxPanel.setScene(scene);
 }

private static Scene createScene() {
ColorPicker picker = new ColorPicker();
BorderPane.setAlignment(picker, Pos.TOP_LEFT);
BorderPane p = new BorderPane(picker);

return new Scene(p);
}

public static void main(String[] args) {
TestSwingInterop s = new TestSwingInterop();
s.setBounds(100, 100, 1000, 800);
s.setVisible(true);
}
}


I've tested this on Java8 and Java9 but didn't had a chance to run it on
the latest master.

The problem is that the newly created JavaFX windows don't have the
Native-Window as their parent but the EmbeddedStage (who is not bound to
a native-window handle). It looks like EmbeddedStage has already been
prepared (see getRawHandle() : long) to support something like that in
future but com.sun.glass.ui.Window and its subclasses have never been
extend to create a window based on a pure long-pointer.

The only exception and the reason I guess the strategy would work was
used for Applets.

Before diving deeper into this I wanted to get a feedback from people
who know things better: Does the strategy of passing along the window
pointer (eg from SWT, don't know about Swing yet) to Glass and using it
as the parent pointer sound like a proper idea?

Tom

[11] RFR JDK-8207923: Disable failing Swing interop tests in SandboxAppTest

[Prasanta Sadhukhan]

Hi Kevin, All

Please review the disabling of the 2 failing tests in SandboxAppTest.

--- a/tests/system/src/test/java/test/sandbox/SandboxAppTest.java Mon 
Jul 16 14:30:19 2018 +0530
+++ b/tests/system/src/test/java/test/sandbox/SandboxAppTest.java Fri 
Jul 20 10:49:46 2018 +0530

@@ -29,6 +29,7 @@
 import java.util.ArrayList;
 import junit.framework.AssertionFailedError;
 import org.junit.Test;
+import org.junit.Ignore;

 import static org.junit.Assert.*;
 import static org.junit.Assume.*;
@@ -109,11 +110,13 @@
 runSandboxedApp("FXNonApp");
 }

+    @Ignore("JDK-8202451")
 @Test (timeout = 15000)
 public void testJFXPanelApp() throws Exception {
 runSandboxedApp("JFXPanelApp");
 }

+    @Ignore("JDK-8202451")
 @Test (timeout = 15000)
 public void testJFXPanelImplicitExitApp() throws Exception {
 runSandboxedApp("JFXPanelImplicitExitApp", 0);

Regards
Prasanta

[11] RFR JDK-8201291:Clicking a JFXPanel having setFocusable(false) causes its processMouseEvent method to loop forever

[Prasanta Sadhukhan]

Hi All,

Please review a fix for JFXPanel focusability issue
Bug: https://bugs.openjdk.java.net/browse/JDK-8201291
webrev: http://cr.openjdk.java.net/~psadhukhan/fx/8201291/webrev.0/

Regards
Prasanta

Re: [11] RFR: Enhancement: JDK-8195811:Support FX Swing interop using public API

[Prasanta Sadhukhan]

http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.9/

with sorting and whitespace fixed, hopefully.

Regards
Prasanta
On 7/13/2018 9:16 PM, Kevin Rushforth wrote:

Hi Prasanta,

> I had done "gradle checkrepo" followed by "checkWhiteSpace -F" to 
get rid of whitespace problem.


It looks like you either didn't refresh your patch, or you 
subsequently introduced additional white space problems. Here is the 
output of applying your .8 patch and running 'gradle checkrepo' :


> Task :checkrepo FAILED
modules/javafx.swing/src/main/java/com/sun/javafx/embed/swing/FXDnD.java 
:tabs:trailingWhitespace:
modules/javafx.swing/src/main/java/com/sun/javafx/embed/swing/FXDnDInterop.java 
:tabs:trailingWhitespace:
modules/javafx.swing/src/main/java/com/sun/javafx/embed/swing/InteropFactory.java 
:tabs:
modules/javafx.swing/src/main/java/com/sun/javafx/embed/swing/JFXPanelInterop.java 
:trailingWhitespace:
modules/javafx.swing/src/main/java/com/sun/javafx/embed/swing/SwingNodeInterop.java 
:tabs:trailingWhitespace:


Found 5 whitespace or executable issues

To correct, use
   bash tools/scripts/checkWhiteSpace -F -x

I'll continue testing anyway, assuming that the only changes at this 
point will be white space changes or similar, which won't invalidate 
testing.


-- Kevin


On 7/13/2018 2:05 AM, Prasanta Sadhukhan wrote:


Hi Kevin,Ambarish,

Please find modified webrev taking care of review comments.
http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.8/
I had done "gradle checkrepo" followed by "checkWhiteSpace -F" to get 
rid of whitespace problem.


Regards
Prasanta
On 7/13/2018 5:29 AM, Kevin Rushforth wrote:

Hi Prasanta,

I confirm that the test failures are gone now. I need to do some 
final testing (e.g., a clean build using both JDK 11 and JDK 10 on 
all three platforms). I also want a second look at a couple of the 
implementation files, but overall it's close I think. Here are my 
review comments so far:


General review comments:

* White space problems
  - trailing whitespace and TABS, which must be fixed
  -  InteropFactoryO constructor missing a space: 'Exception{'

* Several of the files have unused imports -- also can you please 
sort the imports?



build.gradle:

* MINOR: naming suggestion:

> 2344 task checkJDKUnsupportedModule(...)

Maybe a more descrptive name for the task would be "copyModuleInfo"?


javafx.swing/src/main/module-info/module-info.java:

* Ordering of requires directives

> 41 requires static jdk.unsupported.desktop;

Can you move this before the 'requires transitive' statements -- it 
should group with the previous block (the 'requires' statements)



InteropFactory:

* MINOR: maybe use a lambda for the following?


  37 AccessController.doPrivileged(new 
PrivilegedAction() {

  38 public Object run() {
  39 verbose = Boolean.valueOf(
  40 System.getProperty("javafx.embed.swing.verbose"));
  41 return null;
  42 }
  43 });


JFXPanelInterop

  38 public abstract long setMask();

Would ‘getMask’ be a better name? (it’s a getter not a setter)


FOLLOW-ON BUGS:

* We should file a follow-on bug to remove the runtime qualified 
exports (e.g., from testrun.args) when running a JDK that 
HAS_UNSUPPORTED_DESKTOP; this isn't high priority



-- Kevin


On 7/11/2018 8:48 AM, Prasanta Sadhukhan wrote:


Hi Kevin,

Please find modified webrev fixing the other test failures by 
modifying the overrideNativeWindowHandle JNI method

http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.7/

Regards
Prasanta
On 7/10/2018 3:58 PM, Prasanta Sadhukhan wrote:


Hi Kevin,

Please find modified webrev with some more refactoring to move 
more common code to SwingNode

and also this solves SwingNodeMemoryLeakTest failure
http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.6/

Regards
Prasanta
On 7/10/2018 7:13 AM, Kevin Rushforth wrote:

Hi Prasanta,

The public API looks fine now. I sent you an offline note about 
one of the test failures (SwingNodeMemoryLeakTest).


There are still whitespace problems that will cause a failure in 
'gradle checkrepo' (and 'hg jcheck').


I'll do a more thorough review in the next day or so.

-- Kevin


On 7/9/2018 4:12 AM, Prasanta Sadhukhan wrote:


Hi Kevin,

Modified webrev to address the "public" leakage
http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.5/

I am looking into the test failures.

Regards
Prasanta
On 7/7/2018 4:30 AM, Kevin Rushforth wrote:
Most things are working with either mode (JDK 10 with qualified 
exports or JDK 11 without), although I do get a few test failures.


There is a serious issue with leakage into the public API that 
needs to be addressed before worrying about the failures, but 
I'll list the test failures at the end.


SwingNode.java

Re: [11] RFR: Enhancement: JDK-8195811:Support FX Swing interop using public API

[Prasanta Sadhukhan]

Hi Kevin,Ambarish,

Please find modified webrev taking care of review comments.
http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.8/
I had done "gradle checkrepo" followed by "checkWhiteSpace -F" to get 
rid of whitespace problem.


Regards
Prasanta
On 7/13/2018 5:29 AM, Kevin Rushforth wrote:

Hi Prasanta,

I confirm that the test failures are gone now. I need to do some final 
testing (e.g., a clean build using both JDK 11 and JDK 10 on all three 
platforms). I also want a second look at a couple of the 
implementation files, but overall it's close I think. Here are my 
review comments so far:


General review comments:

* White space problems
  - trailing whitespace and TABS, which must be fixed
  -  InteropFactoryO constructor missing a space: 'Exception{'

* Several of the files have unused imports -- also can you please sort 
the imports?



build.gradle:

* MINOR: naming suggestion:

> 2344 task checkJDKUnsupportedModule(...)

Maybe a more descrptive name for the task would be "copyModuleInfo"?


javafx.swing/src/main/module-info/module-info.java:

* Ordering of requires directives

> 41 requires static jdk.unsupported.desktop;

Can you move this before the 'requires transitive' statements -- it 
should group with the previous block (the 'requires' statements)



InteropFactory:

* MINOR: maybe use a lambda for the following?


  37 AccessController.doPrivileged(new 
PrivilegedAction() {

  38 public Object run() {
  39 verbose = Boolean.valueOf(
  40 System.getProperty("javafx.embed.swing.verbose"));
  41 return null;
  42 }
  43 });


JFXPanelInterop

  38 public abstract long setMask();

Would ‘getMask’ be a better name? (it’s a getter not a setter)


FOLLOW-ON BUGS:

* We should file a follow-on bug to remove the runtime qualified 
exports (e.g., from testrun.args) when running a JDK that 
HAS_UNSUPPORTED_DESKTOP; this isn't high priority



-- Kevin


On 7/11/2018 8:48 AM, Prasanta Sadhukhan wrote:


Hi Kevin,

Please find modified webrev fixing the other test failures by 
modifying the overrideNativeWindowHandle JNI method

http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.7/

Regards
Prasanta
On 7/10/2018 3:58 PM, Prasanta Sadhukhan wrote:


Hi Kevin,

Please find modified webrev with some more refactoring to move more 
common code to SwingNode

and also this solves SwingNodeMemoryLeakTest failure
http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.6/

Regards
Prasanta
On 7/10/2018 7:13 AM, Kevin Rushforth wrote:

Hi Prasanta,

The public API looks fine now. I sent you an offline note about one 
of the test failures (SwingNodeMemoryLeakTest).


There are still whitespace problems that will cause a failure in 
'gradle checkrepo' (and 'hg jcheck').


I'll do a more thorough review in the next day or so.

-- Kevin


On 7/9/2018 4:12 AM, Prasanta Sadhukhan wrote:


Hi Kevin,

Modified webrev to address the "public" leakage
http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.5/

I am looking into the test failures.

Regards
Prasanta
On 7/7/2018 4:30 AM, Kevin Rushforth wrote:
Most things are working with either mode (JDK 10 with qualified 
exports or JDK 11 without), although I do get a few test failures.


There is a serious issue with leakage into the public API that 
needs to be addressed before worrying about the failures, but 
I'll list the test failures at the end.


SwingNode.java:

This public class is part of the API. You cannot make any of the 
following fields public as this would leak implementation into 
public API:


+    public int swingPrefWidth;
+    public int swingPrefHeight;
+    public int swingMaxWidth;
+    public int swingMaxHeight;
+    public int swingMinWidth;
+    public int swingMinHeight;

+    public final Object getLightweightFrame() { return lwFrame; }

+    public final ReentrantLock paintLock = new ReentrantLock();

+    public boolean grabbed; // lwframe initiated grab

+    public void setImageBuffer(...)

+   public void setImageBounds(...);

+    public void repaintDirtyRegion(...)

+    public void ungrabFocus(boolean postUngrabEvent)

If you need to access them from other packages, you can either 
use the accessor pattern (this might be easiest) or else refactor 
it further to move more of this down to the implementation. I 
note that even though SwingNodeInterop is abstract, it can still 
have non-abstract methods if that helps in your refactoring.



SwingFXUtils.java

Same problem as SwingNode, although to a lesser extent. The 
following must not be public:


+    public static void runOnFxThread(Runnable runnable)
+    public static void runOnEDT(final Runnable r)
+    public static void runOnEDTAndWait(Object nestedLoopKey, 
Runnable r)

+    public static void leaveFXNestedLoop(Object nestedLoopKey)

Re: [11] RFR: Enhancement: JDK-8195811:Support FX Swing interop using public API

[Prasanta Sadhukhan]

Hi Kevin,

Please find modified webrev fixing the other test failures by modifying 
the overrideNativeWindowHandle JNI method

http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.7/

Regards
Prasanta
On 7/10/2018 3:58 PM, Prasanta Sadhukhan wrote:


Hi Kevin,

Please find modified webrev with some more refactoring to move more 
common code to SwingNode

and also this solves SwingNodeMemoryLeakTest failure
http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.6/

Regards
Prasanta
On 7/10/2018 7:13 AM, Kevin Rushforth wrote:

Hi Prasanta,

The public API looks fine now. I sent you an offline note about one 
of the test failures (SwingNodeMemoryLeakTest).


There are still whitespace problems that will cause a failure in 
'gradle checkrepo' (and 'hg jcheck').


I'll do a more thorough review in the next day or so.

-- Kevin


On 7/9/2018 4:12 AM, Prasanta Sadhukhan wrote:


Hi Kevin,

Modified webrev to address the "public" leakage
http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.5/

I am looking into the test failures.

Regards
Prasanta
On 7/7/2018 4:30 AM, Kevin Rushforth wrote:
Most things are working with either mode (JDK 10 with qualified 
exports or JDK 11 without), although I do get a few test failures.


There is a serious issue with leakage into the public API that 
needs to be addressed before worrying about the failures, but I'll 
list the test failures at the end.


SwingNode.java:

This public class is part of the API. You cannot make any of the 
following fields public as this would leak implementation into 
public API:


+    public int swingPrefWidth;
+    public int swingPrefHeight;
+    public int swingMaxWidth;
+    public int swingMaxHeight;
+    public int swingMinWidth;
+    public int swingMinHeight;

+    public final Object getLightweightFrame() { return lwFrame; }

+    public final ReentrantLock paintLock = new ReentrantLock();

+    public boolean grabbed; // lwframe initiated grab

+    public void setImageBuffer(...)

+   public void setImageBounds(...);

+    public void repaintDirtyRegion(...)

+    public void ungrabFocus(boolean postUngrabEvent)

If you need to access them from other packages, you can either use 
the accessor pattern (this might be easiest) or else refactor it 
further to move more of this down to the implementation. I note 
that even though SwingNodeInterop is abstract, it can still have 
non-abstract methods if that helps in your refactoring.



SwingFXUtils.java

Same problem as SwingNode, although to a lesser extent. The 
following must not be public:


+    public static void runOnFxThread(Runnable runnable)
+    public static void runOnEDT(final Runnable r)
+    public static void runOnEDTAndWait(Object nestedLoopKey, 
Runnable r)

+    public static void leaveFXNestedLoop(Object nestedLoopKey)


JFXPanel is fine.

-

* System tests failures on Linux:

test.robot.javafx.embed.swing.SwingNodeJDialogTest > 
testJDialogAbove FAILED
    java.lang.AssertionError: JDialog is not above JavaFX stage 
expected: but 
was:


test.robot.javafx.embed.swing.SwingNodeJDialogTest > 
testNodeRemovalAfterShow FAILED
    java.lang.AssertionError: JDialog is not above JavaFX stage 
expected: but 
was:


test.robot.javafx.embed.swing.SwingNodeJDialogTest > 
testStageCloseAfterShow FAILED
    java.lang.AssertionError: JDialog is not above JavaFX stage 
expected: but 
was:


test.javafx.embed.swing.SwingNodeMemoryLeakTest > 
testSwingNodeMemoryLeak FAILED

    java.lang.AssertionError: expected:<10> but was:<9>
    at org.junit.Assert.fail(Assert.java:91)
    at org.junit.Assert.failNotEquals(Assert.java:645)
    at org.junit.Assert.assertEquals(Assert.java:126)
    at org.junit.Assert.assertEquals(Assert.java:470)
    at org.junit.Assert.assertEquals(Assert.java:454)
    at 
test.javafx.embed.swing.SwingNodeMemoryLeakTest.testSwingNodeMemoryLeak(SwingNodeMemoryLeakTest.java:97)


Two of these, SwingNodeJDialogTest.testNodeRemovalAfterShow and 
SwingNodeJDialogTest.testStageCloseAfterShow, also fail on Mac


-- Kevin


On 7/5/2018 11:29 PM, Prasanta Sadhukhan wrote:


My Bad. Please find modified webrev restoring the filter

http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.4/

Regards
Prasanta
On 7/6/2018 6:47 AM, Kevin Rushforth wrote:

One quick comment:

This no longer compiles with OpenJDK10. It looks like the logic 
to optionally filter out jdk.unsupported.desktop from 
javafx.swing's module-info.java got lost between the .2 and .3 
versions.


-- Kevin


On 7/4/2018 4:35 AM, Prasanta Sadhukhan wrote:

Hi All,

Please review an enhancement to support openjfx swing 
interoperability once the dependancy of internal jdk classes are 
removed.
JDK-8202199 <https://bugs.openjdk.java.net/browse/JDK-8202199> 
provided a new "jdk.unsupported.desktop" module in JDK 11 that 
exports public API that is intended to be used by 

Re: [11] RFR: Enhancement: JDK-8195811:Support FX Swing interop using public API

[Prasanta Sadhukhan]

Hi Kevin,

Please find modified webrev with some more refactoring to move more 
common code to SwingNode

and also this solves SwingNodeMemoryLeakTest failure
http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.6/

Regards
Prasanta
On 7/10/2018 7:13 AM, Kevin Rushforth wrote:

Hi Prasanta,

The public API looks fine now. I sent you an offline note about one of 
the test failures (SwingNodeMemoryLeakTest).


There are still whitespace problems that will cause a failure in 
'gradle checkrepo' (and 'hg jcheck').


I'll do a more thorough review in the next day or so.

-- Kevin


On 7/9/2018 4:12 AM, Prasanta Sadhukhan wrote:


Hi Kevin,

Modified webrev to address the "public" leakage
http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.5/

I am looking into the test failures.

Regards
Prasanta
On 7/7/2018 4:30 AM, Kevin Rushforth wrote:
Most things are working with either mode (JDK 10 with qualified 
exports or JDK 11 without), although I do get a few test failures.


There is a serious issue with leakage into the public API that needs 
to be addressed before worrying about the failures, but I'll list 
the test failures at the end.


SwingNode.java:

This public class is part of the API. You cannot make any of the 
following fields public as this would leak implementation into 
public API:


+    public int swingPrefWidth;
+    public int swingPrefHeight;
+    public int swingMaxWidth;
+    public int swingMaxHeight;
+    public int swingMinWidth;
+    public int swingMinHeight;

+    public final Object getLightweightFrame() { return lwFrame; }

+    public final ReentrantLock paintLock = new ReentrantLock();

+    public boolean grabbed; // lwframe initiated grab

+    public void setImageBuffer(...)

+   public void setImageBounds(...);

+    public void repaintDirtyRegion(...)

+    public void ungrabFocus(boolean postUngrabEvent)

If you need to access them from other packages, you can either use 
the accessor pattern (this might be easiest) or else refactor it 
further to move more of this down to the implementation. I note that 
even though SwingNodeInterop is abstract, it can still have 
non-abstract methods if that helps in your refactoring.



SwingFXUtils.java

Same problem as SwingNode, although to a lesser extent. The 
following must not be public:


+    public static void runOnFxThread(Runnable runnable)
+    public static void runOnEDT(final Runnable r)
+    public static void runOnEDTAndWait(Object nestedLoopKey, 
Runnable r)

+    public static void leaveFXNestedLoop(Object nestedLoopKey)


JFXPanel is fine.

-

* System tests failures on Linux:

test.robot.javafx.embed.swing.SwingNodeJDialogTest > 
testJDialogAbove FAILED
    java.lang.AssertionError: JDialog is not above JavaFX stage 
expected: but 
was:


test.robot.javafx.embed.swing.SwingNodeJDialogTest > 
testNodeRemovalAfterShow FAILED
    java.lang.AssertionError: JDialog is not above JavaFX stage 
expected: but 
was:


test.robot.javafx.embed.swing.SwingNodeJDialogTest > 
testStageCloseAfterShow FAILED
    java.lang.AssertionError: JDialog is not above JavaFX stage 
expected: but 
was:


test.javafx.embed.swing.SwingNodeMemoryLeakTest > 
testSwingNodeMemoryLeak FAILED

    java.lang.AssertionError: expected:<10> but was:<9>
    at org.junit.Assert.fail(Assert.java:91)
    at org.junit.Assert.failNotEquals(Assert.java:645)
    at org.junit.Assert.assertEquals(Assert.java:126)
    at org.junit.Assert.assertEquals(Assert.java:470)
    at org.junit.Assert.assertEquals(Assert.java:454)
    at 
test.javafx.embed.swing.SwingNodeMemoryLeakTest.testSwingNodeMemoryLeak(SwingNodeMemoryLeakTest.java:97)


Two of these, SwingNodeJDialogTest.testNodeRemovalAfterShow and 
SwingNodeJDialogTest.testStageCloseAfterShow, also fail on Mac


-- Kevin


On 7/5/2018 11:29 PM, Prasanta Sadhukhan wrote:


My Bad. Please find modified webrev restoring the filter

http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.4/

Regards
Prasanta
On 7/6/2018 6:47 AM, Kevin Rushforth wrote:

One quick comment:

This no longer compiles with OpenJDK10. It looks like the logic to 
optionally filter out jdk.unsupported.desktop from javafx.swing's 
module-info.java got lost between the .2 and .3 versions.


-- Kevin


On 7/4/2018 4:35 AM, Prasanta Sadhukhan wrote:

Hi All,

Please review an enhancement to support openjfx swing 
interoperability once the dependancy of internal jdk classes are 
removed.
JDK-8202199 <https://bugs.openjdk.java.net/browse/JDK-8202199> 
provided a new "jdk.unsupported.desktop" module in JDK 11 that 
exports public API that is intended to be used by the 
javafx.swing module
and unbundled OpenJFX is now made to depend on these APIs to 
support interoperation
between Swing and JavaFX components to replace previous use of 
internal APIs when it was part of Oracle JDK.


Bug: https://bugs.openjdk.java.net/b

Re: [11] RFR: Enhancement: JDK-8195811:Support FX Swing interop using public API

[Prasanta Sadhukhan]

Hi Kevin,

Modified webrev to address the "public" leakage
http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.5/

I am looking into the test failures.

Regards
Prasanta
On 7/7/2018 4:30 AM, Kevin Rushforth wrote:
Most things are working with either mode (JDK 10 with qualified 
exports or JDK 11 without), although I do get a few test failures.


There is a serious issue with leakage into the public API that needs 
to be addressed before worrying about the failures, but I'll list the 
test failures at the end.


SwingNode.java:

This public class is part of the API. You cannot make any of the 
following fields public as this would leak implementation into public API:


+    public int swingPrefWidth;
+    public int swingPrefHeight;
+    public int swingMaxWidth;
+    public int swingMaxHeight;
+    public int swingMinWidth;
+    public int swingMinHeight;

+    public final Object getLightweightFrame() { return lwFrame; }

+    public final ReentrantLock paintLock = new ReentrantLock();

+    public boolean grabbed; // lwframe initiated grab

+    public void setImageBuffer(...)

+   public void setImageBounds(...);

+    public void repaintDirtyRegion(...)

+    public void ungrabFocus(boolean postUngrabEvent)

If you need to access them from other packages, you can either use the 
accessor pattern (this might be easiest) or else refactor it further 
to move more of this down to the implementation. I note that even 
though SwingNodeInterop is abstract, it can still have non-abstract 
methods if that helps in your refactoring.



SwingFXUtils.java

Same problem as SwingNode, although to a lesser extent. The following 
must not be public:


+    public static void runOnFxThread(Runnable runnable)
+    public static void runOnEDT(final Runnable r)
+    public static void runOnEDTAndWait(Object nestedLoopKey, Runnable r)
+    public static void leaveFXNestedLoop(Object nestedLoopKey)


JFXPanel is fine.

-

* System tests failures on Linux:

test.robot.javafx.embed.swing.SwingNodeJDialogTest > testJDialogAbove 
FAILED
    java.lang.AssertionError: JDialog is not above JavaFX stage 
expected: but 
was:


test.robot.javafx.embed.swing.SwingNodeJDialogTest > 
testNodeRemovalAfterShow FAILED
    java.lang.AssertionError: JDialog is not above JavaFX stage 
expected: but 
was:


test.robot.javafx.embed.swing.SwingNodeJDialogTest > 
testStageCloseAfterShow FAILED
    java.lang.AssertionError: JDialog is not above JavaFX stage 
expected: but 
was:


test.javafx.embed.swing.SwingNodeMemoryLeakTest > 
testSwingNodeMemoryLeak FAILED

    java.lang.AssertionError: expected:<10> but was:<9>
    at org.junit.Assert.fail(Assert.java:91)
    at org.junit.Assert.failNotEquals(Assert.java:645)
    at org.junit.Assert.assertEquals(Assert.java:126)
    at org.junit.Assert.assertEquals(Assert.java:470)
    at org.junit.Assert.assertEquals(Assert.java:454)
    at 
test.javafx.embed.swing.SwingNodeMemoryLeakTest.testSwingNodeMemoryLeak(SwingNodeMemoryLeakTest.java:97)


Two of these, SwingNodeJDialogTest.testNodeRemovalAfterShow and 
SwingNodeJDialogTest.testStageCloseAfterShow, also fail on Mac


-- Kevin


On 7/5/2018 11:29 PM, Prasanta Sadhukhan wrote:


My Bad. Please find modified webrev restoring the filter

http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.4/

Regards
Prasanta
On 7/6/2018 6:47 AM, Kevin Rushforth wrote:

One quick comment:

This no longer compiles with OpenJDK10. It looks like the logic to 
optionally filter out jdk.unsupported.desktop from javafx.swing's 
module-info.java got lost between the .2 and .3 versions.


-- Kevin


On 7/4/2018 4:35 AM, Prasanta Sadhukhan wrote:

Hi All,

Please review an enhancement to support openjfx swing 
interoperability once the dependancy of internal jdk classes are 
removed.
JDK-8202199 <https://bugs.openjdk.java.net/browse/JDK-8202199> 
provided a new "jdk.unsupported.desktop" module in JDK 11 that 
exports public API that is intended to be used by the javafx.swing 
module
and unbundled OpenJFX is now made to depend on these APIs to 
support interoperation
between Swing and JavaFX components to replace previous use of 
internal APIs when it was part of Oracle JDK.


Bug: https://bugs.openjdk.java.net/browse/JDK-8195811
webrev: http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.3/

Regards
Prasanta

Re: [11] RFR: Enhancement: JDK-8195811:Support FX Swing interop using public API

[Prasanta Sadhukhan]

My Bad. Please find modified webrev restoring the filter

http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.4/

Regards
Prasanta
On 7/6/2018 6:47 AM, Kevin Rushforth wrote:

One quick comment:

This no longer compiles with OpenJDK10. It looks like the logic to 
optionally filter out jdk.unsupported.desktop from javafx.swing's 
module-info.java got lost between the .2 and .3 versions.


-- Kevin


On 7/4/2018 4:35 AM, Prasanta Sadhukhan wrote:

Hi All,

Please review an enhancement to support openjfx swing 
interoperability once the dependancy of internal jdk classes are removed.
JDK-8202199 <https://bugs.openjdk.java.net/browse/JDK-8202199> 
provided a new "jdk.unsupported.desktop" module in JDK 11 that 
exports public API that is intended to be used by the javafx.swing module
and unbundled OpenJFX is now made to depend on these APIs to support 
interoperation
between Swing and JavaFX components to replace previous use of 
internal APIs when it was part of Oracle JDK.


Bug: https://bugs.openjdk.java.net/browse/JDK-8195811
webrev: http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.3/

Regards
Prasanta

[11] RFR: Enhancement: JDK-8195811:Support FX Swing interop using public API

[Prasanta Sadhukhan]

Hi All,

Please review an enhancement to support openjfx swing interoperability 
once the dependancy of internal jdk classes are removed.
JDK-8202199  provided 
a new "jdk.unsupported.desktop" module in JDK 11 that exports public API 
that is intended to be used by the javafx.swing module
and unbundled OpenJFX is now made to depend on these APIs to support 
interoperation
between Swing and JavaFX components to replace previous use of internal 
APIs when it was part of Oracle JDK.


Bug: https://bugs.openjdk.java.net/browse/JDK-8195811
webrev: http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.3/

Regards
Prasanta

Re: [11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

[Prasanta Sadhukhan]

On 6/15/2018 1:01 PM, Prasanta Sadhukhan wrote:
Webrev to add @since 11 to jdk.swing.interop classes (only difference 
from .14)


http://cr.openjdk.java.net/~psadhukhan/fxswing.15/

I also tried submitting mach5 job from linux but it is failing to 
download jib (although I candownload from browser and ping 
java.se.oracle.com)
~/Downloads/mach5-1.0.1865-distribution/bin/mach5 --remote-build 
--email prasanta.sadhuk...@oracle.com

Mach 5 Health State: green

Creating job description... done
Creating build ID... 2018-06-15-0626444.prasanta.sadhukhan.open
Publishing source using JIB... 
[2018-06-15T06:26:42,870Z][INFO][main][c.o.j.s.c.SparkyClient] 
Downloading JIB bootstrap script



I am trying from windows but Phil told windows build does not probably 
built docs.

Are there any easier alternative to verify the doc build?


I confirm jdk.unsupported.desktop is not present in doc build.

Regards
Prasanta

Regards
Prasanta
On 6/15/2018 1:56 AM, mandy chung wrote:
I reviewed the module-info.java change. @since 12 is missing in 
jdk.unsupported.desktop module-info.java


Otherwise it's fine.

Does the docs build exclude jdk.unsupported.desktop?  You might have 
confirmed that that I missed.


Mandy

On 6/13/18 12:48 AM, Prasanta Sadhukhan wrote:

Hi Phil, All

Please find modified webrev taking care of streamlining 
SwingInteropUtils class and handling of native window handle in 
LightweightFrameWrapper class by using JNI call in FX

http://cr.openjdk.java.net/~psadhukhan/fxswing.14/

Corresponding CSR: https://bugs.openjdk.java.net/browse/JDK-8202175

Regards
Prasanta

Re: [11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

[Prasanta Sadhukhan]

Webrev to add @since 11 to jdk.swing.interop classes (only difference 
from .14)


http://cr.openjdk.java.net/~psadhukhan/fxswing.15/

I also tried submitting mach5 job from linux but it is failing to 
download jib (although I candownload from browser and ping 
java.se.oracle.com)
~/Downloads/mach5-1.0.1865-distribution/bin/mach5 --remote-build --email 
prasanta.sadhuk...@oracle.com

Mach 5 Health State: green

Creating job description... done
Creating build ID... 2018-06-15-0626444.prasanta.sadhukhan.open
Publishing source using JIB... 
[2018-06-15T06:26:42,870Z][INFO][main][c.o.j.s.c.SparkyClient] 
Downloading JIB bootstrap script
Failed to download 
https://java.se.oracle.com/artifactory/jdk-virtual/com/oracle/java/jib/jib/3.0-SNAPSHOT/jib-3.0-SNAPSHOT.jib.sh.gz


I am trying from windows but Phil told windows build does not probably 
built docs.

Are there any easier alternative to verify the doc build?

Regards
Prasanta
On 6/15/2018 1:56 AM, mandy chung wrote:
I reviewed the module-info.java change.  @since 12 is missing in 
jdk.unsupported.desktop module-info.java


Otherwise it's fine.

Does the docs build exclude jdk.unsupported.desktop?  You might have 
confirmed that that I missed.


Mandy

On 6/13/18 12:48 AM, Prasanta Sadhukhan wrote:

Hi Phil, All

Please find modified webrev taking care of streamlining 
SwingInteropUtils class and handling of native window handle in 
LightweightFrameWrapper class by using JNI call in FX

http://cr.openjdk.java.net/~psadhukhan/fxswing.14/

Corresponding CSR: https://bugs.openjdk.java.net/browse/JDK-8202175

Regards
Prasanta

[11] RFR JDK-8202277:WebView image capture fails with standalone FX due to dependency on javafx.swing

[Prasanta Sadhukhan]

Hi Kevin, All

Please review the fix to remove the dependancy on javafx.swing module 
from javafx.web


Bug: https://bugs.openjdk.java.net/browse/JDK-8202277
webrev: http://cr.openjdk.java.net/~psadhukhan/fx/8202277/webrev/

Regards
Prasanta

Re: [11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

[Prasanta Sadhukhan]

Hi Phil, All

Please find modified webrev taking care of streamlining 
SwingInteropUtils class and handling of native window handle in 
LightweightFrameWrapper class by using JNI call in FX

http://cr.openjdk.java.net/~psadhukhan/fxswing.14/

Corresponding CSR: https://bugs.openjdk.java.net/browse/JDK-8202175

Regards
Prasanta
On 5/30/2018 4:13 AM, Philip Race wrote:

Some follow up comments.

On 5/10/18, 5:49 PM, Philip Race wrote:
I've looked over the Swing code. No time to actually try it out so I 
can only comment on what I see.


I am not sure what I think about class docs like "This class wraps 
 .."
I know no javadoc is generated but this module is exported and 
probably should say something

less specific.

In general I really have to hold my nose when looking at this whole 
module and I

find it distasteful to endorse it.

I really don't like all the things SwingInterOpUtils.java does.


Specific things we should look at in this file
- getDefaultScaleX/Y .. JDK 9 will set a transform on the graphics that
is passed to JComponent.paintComponent() .. so I wonder if we really
need this internal API. I doubt any other code is updating the transform
in a way that would make it a problem so the information you get 
should be valid.


The code to get the data array for the raster and associated information
can be extracted via standard API like this :

DataBuffer  db = BufferedImage.getRaster().getDataBuffer();
if (db instanceof DataBufferInt) {
 data = (DataBufferInt)db.getData();
}

So all of those can be fixed.

The code that accepts a native window handle maybe should require it 
is accessed via JNI ...

It could still be unsupported, but it would be package private or similar.
If you have a native ID, then you are already using native code.
This sidesteps any questions about the stability of such an API which
accepts a native resource.

The code that finds a component by location is probably harmless ...

I'm less sure about the event posting and focus grabbing support.
I'd like to have Sergey comment on those.

the DnD code is too much for me to examine in detail.

I worry we are creating something we'll come to regret here.
The "unsupportedness" needs to be backed up by deprecated-for-removal 
being included

today so we can get rid of it the next release if we want to.


I've looked at jdk.unsupported and they don't seem to have deprecation 
tags so

maybe that isn't an issue.

-phil


No @since tags anywhere

-phil.

On 5/10/18, 8:32 AM, Kevin Rushforth wrote:
I confirm that this fixes the issue. The interface change to make 
the two static methods e instance methods is a good change in any case.


I am not a Swing "R"eviewer, but the .13 webrev gets a +1 from me.

-- Kevin


On 5/10/2018 8:20 AM, Prasanta Sadhukhan wrote:


Hi Kevin,All,

Please find the modified webrev fixing this #1 issue
http://cr.openjdk.java.net/~psadhukhan/fxswing.13/
via change in 
jdk/swing/interop/DropTargetContextWrapper.java#setDropTargetContext 
and FXDnD.java#postDropTargetEvent


For me, #2 works, #3 doesn't work even now due to JDK-8141391 
<https://bugs.openjdk.java.net/browse/JDK-8141391> and #4 works for me.


Regards
Prasanta
On 5/9/2018 11:29 PM, Kevin Rushforth wrote:

Hi Prasanta,

The API looks good now.

All of our automated tests work (except for the ones with a 
security manager due to JDK-8202451 
<https://bugs.openjdk.java.net/browse/JDK-8202451>).


The only functional problem that I see is that Drag and Drop onto 
a SwingNode doesn't work. We need to make sure that we test the 
following four cases:


1. Drag / drop onto a Swing component in a SwingNode
2. Drag / drop from a Swing component in a SwingNode
3. Drag / drop onto a JavaFX control in a JFXPanel
4. Drag / drop from a JavaFX control in a JFXPanel

So far I only tried the first one; the others still need to be 
validated.


-- Kevin



On 5/9/2018 7:14 AM, Prasanta Sadhukhan wrote:

Modified webrev to cater to this

http://cr.openjdk.java.net/~psadhukhan/fxswing.12/

Regards
Prasanta
On 5/9/2018 5:58 PM, Kevin Rushforth wrote:

The following can also be abstract:

LightweightContentWrapper:
  getComponent, createDragGestureRecognizer, 
createDragSourceContextPeer


DropTargetContextWrapper:
  getTargetActions, getDropTarget, getTransferDataFlavors, 
getTransferable, isTransferableJVMLocal


DispatcherWrapper:
  isDispatchThread, createSecondaryLoop

The rest looks good to me (although I still see two public 
methods with "Peer" in the name, so Phil may want those renamed).


-- Kevin


On 5/9/2018 2:14 AM, Prasanta Sadhukhan wrote:

Modified webrev to cater to these 3 observations
http://cr.openjdk.java.net/~psadhukhan/fxswing.11/

Regards
Prasanta

On 5/9/2018 5:03 AM, Kevin Rushforth wrote:
The module definition for jdk.unsupported.desktop and the 
changes to java.desktop look fine.


In reviewing the jdk.swing.interop A

Re: [11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

[Prasanta Sadhukhan]

Hi Kevin,All,

Please find the modified webrev fixing this #1 issue
http://cr.openjdk.java.net/~psadhukhan/fxswing.13/
via change in 
jdk/swing/interop/DropTargetContextWrapper.java#setDropTargetContext and 
FXDnD.java#postDropTargetEvent


For me, #2 works, #3 doesn't work even now due to JDK-8141391 
<https://bugs.openjdk.java.net/browse/JDK-8141391> and #4 works for me.


Regards
Prasanta
On 5/9/2018 11:29 PM, Kevin Rushforth wrote:

Hi Prasanta,

The API looks good now.

All of our automated tests work (except for the ones with a security 
manager due to JDK-8202451 
<https://bugs.openjdk.java.net/browse/JDK-8202451>).


The only functional problem that I see is that Drag and Drop onto a 
SwingNode doesn't work. We need to make sure that we test the 
following four cases:


1. Drag / drop onto a Swing component in a SwingNode
2. Drag / drop from a Swing component in a SwingNode
3. Drag / drop onto a JavaFX control in a JFXPanel
4. Drag / drop from a JavaFX control in a JFXPanel

So far I only tried the first one; the others still need to be validated.

-- Kevin



On 5/9/2018 7:14 AM, Prasanta Sadhukhan wrote:

Modified webrev to cater to this

http://cr.openjdk.java.net/~psadhukhan/fxswing.12/

Regards
Prasanta
On 5/9/2018 5:58 PM, Kevin Rushforth wrote:

The following can also be abstract:

LightweightContentWrapper:
  getComponent, createDragGestureRecognizer, 
createDragSourceContextPeer


DropTargetContextWrapper:
  getTargetActions, getDropTarget, getTransferDataFlavors, 
getTransferable, isTransferableJVMLocal


DispatcherWrapper:
  isDispatchThread, createSecondaryLoop

The rest looks good to me (although I still see two public methods 
with "Peer" in the name, so Phil may want those renamed).


-- Kevin


On 5/9/2018 2:14 AM, Prasanta Sadhukhan wrote:

Modified webrev to cater to these 3 observations
http://cr.openjdk.java.net/~psadhukhan/fxswing.11/

Regards
Prasanta

On 5/9/2018 5:03 AM, Kevin Rushforth wrote:
The module definition for jdk.unsupported.desktop and the changes 
to java.desktop look fine.


In reviewing the jdk.swing.interop API, I have the following 
suggestions / observations:


1. DispatcherWrapper, DragSourceContextWrapper, 
DropTargetContextWrapper, and LightweightContentWrapper can all be 
abstract, along with most of the methods (rather than having an 
empty body return value that is never used).


2. The addNotify method in LightweightFrameWrapper is unused. 
Should be used somewhere? If not, then it can be removed.


The implementation of the new wrapper classes looks OK to me with 
one observation that might or might not matter:


3. The behavior of getDefaultScaleX/Y (which is now in 
SwingInteropUtils) has changed in the case where the Graphics is 
not an instance of SunGraphics2D. The former behavior was to leave 
the instance variables X and Y unchanged. The new behavior will 
set them back to 1.0. Maybe this can't happen in practice, but it 
is something to consider.


-- Kevin


On 5/8/2018 3:31 AM, Alan Bateman wrote:

On 08/05/2018 06:51, Prasanta Sadhukhan wrote:

Modified webrev to rename to InteropProviderImpl

http://cr.openjdk.java.net/~psadhukhan/fxswing.10/

This looks okay to me.

-Alan

Re: [11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

[Prasanta Sadhukhan]

Modified webrev to cater to this

http://cr.openjdk.java.net/~psadhukhan/fxswing.12/

Regards
Prasanta
On 5/9/2018 5:58 PM, Kevin Rushforth wrote:

The following can also be abstract:

LightweightContentWrapper:
  getComponent, createDragGestureRecognizer, createDragSourceContextPeer

DropTargetContextWrapper:
  getTargetActions, getDropTarget, getTransferDataFlavors, 
getTransferable, isTransferableJVMLocal


DispatcherWrapper:
  isDispatchThread, createSecondaryLoop

The rest looks good to me (although I still see two public methods 
with "Peer" in the name, so Phil may want those renamed).


-- Kevin


On 5/9/2018 2:14 AM, Prasanta Sadhukhan wrote:

Modified webrev to cater to these 3 observations
http://cr.openjdk.java.net/~psadhukhan/fxswing.11/

Regards
Prasanta

On 5/9/2018 5:03 AM, Kevin Rushforth wrote:
The module definition for jdk.unsupported.desktop and the changes to 
java.desktop look fine.


In reviewing the jdk.swing.interop API, I have the following 
suggestions / observations:


1. DispatcherWrapper, DragSourceContextWrapper, 
DropTargetContextWrapper, and LightweightContentWrapper can all be 
abstract, along with most of the methods (rather than having an 
empty body return value that is never used).


2. The addNotify method in LightweightFrameWrapper is unused. Should 
be used somewhere? If not, then it can be removed.


The implementation of the new wrapper classes looks OK to me with 
one observation that might or might not matter:


3. The behavior of getDefaultScaleX/Y (which is now in 
SwingInteropUtils) has changed in the case where the Graphics is not 
an instance of SunGraphics2D. The former behavior was to leave the 
instance variables X and Y unchanged. The new behavior will set them 
back to 1.0. Maybe this can't happen in practice, but it is 
something to consider.


-- Kevin


On 5/8/2018 3:31 AM, Alan Bateman wrote:

On 08/05/2018 06:51, Prasanta Sadhukhan wrote:

Modified webrev to rename to InteropProviderImpl

http://cr.openjdk.java.net/~psadhukhan/fxswing.10/

This looks okay to me.

-Alan

Re: [11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

[Prasanta Sadhukhan]

Modified webrev to cater to these 3 observations
http://cr.openjdk.java.net/~psadhukhan/fxswing.11/

Regards
Prasanta

On 5/9/2018 5:03 AM, Kevin Rushforth wrote:
The module definition for jdk.unsupported.desktop and the changes to 
java.desktop look fine.


In reviewing the jdk.swing.interop API, I have the following 
suggestions / observations:


1. DispatcherWrapper, DragSourceContextWrapper, 
DropTargetContextWrapper, and LightweightContentWrapper can all be 
abstract, along with most of the methods (rather than having an empty 
body return value that is never used).


2. The addNotify method in LightweightFrameWrapper is unused. Should 
be used somewhere? If not, then it can be removed.


The implementation of the new wrapper classes looks OK to me with one 
observation that might or might not matter:


3. The behavior of getDefaultScaleX/Y (which is now in 
SwingInteropUtils) has changed in the case where the Graphics is not 
an instance of SunGraphics2D. The former behavior was to leave the 
instance variables X and Y unchanged. The new behavior will set them 
back to 1.0. Maybe this can't happen in practice, but it is something 
to consider.


-- Kevin


On 5/8/2018 3:31 AM, Alan Bateman wrote:

On 08/05/2018 06:51, Prasanta Sadhukhan wrote:

Modified webrev to rename to InteropProviderImpl

http://cr.openjdk.java.net/~psadhukhan/fxswing.10/

This looks okay to me.

-Alan

Re: [11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

[Prasanta Sadhukhan]

We have discussed this internally and we believe that "unsupportedness" 
is the critical property here so it justifies grouping them in the same 
"unsupported" namespace rather than desktop namespace.


Regards
Prasanta
On 5/8/2018 12:56 PM, Ali Ebrahimi wrote:

Hi,
What about " jdk.desktop.unsupported" for new module name?

On Tue, May 8, 2018 at 10:21 AM, Prasanta Sadhukhan 
mailto:prasanta.sadhuk...@oracle.com>> 
wrote:


Modified webrev to rename to InteropProviderImpl

http://cr.openjdk.java.net/~psadhukhan/fxswing.10/
<http://cr.openjdk.java.net/%7Epsadhukhan/fxswing.10/>

Regards
Prasanta

On 5/7/2018 8:35 PM, Kevin Rushforth wrote:

That name seems good to me.

    -- Kevin


    On 5/7/2018 8:01 AM, Prasanta Sadhukhan wrote:

Would InteropProviderImpl sound good?

Regards
Prasanta
On 5/7/2018 8:27 PM, Alan Bateman wrote:



        On 07/05/2018 10:26, Prasanta Sadhukhan wrote:

:

Modified webrev to use InteropProvider
http://cr.openjdk.java.net/~psadhukhan/fxswing.9/
<http://cr.openjdk.java.net/%7Epsadhukhan/fxswing.9/>

This looks okay although for consistent then
InteropImpl could be renamed too.

-Alan







--

Best Regards,
Ali Ebrahimi

Re: [11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

[Prasanta Sadhukhan]

Modified webrev to rename to InteropProviderImpl

http://cr.openjdk.java.net/~psadhukhan/fxswing.10/

Regards
Prasanta
On 5/7/2018 8:35 PM, Kevin Rushforth wrote:

That name seems good to me.

-- Kevin


On 5/7/2018 8:01 AM, Prasanta Sadhukhan wrote:

Would InteropProviderImpl sound good?

Regards
Prasanta
On 5/7/2018 8:27 PM, Alan Bateman wrote:



On 07/05/2018 10:26, Prasanta Sadhukhan wrote:

:

Modified webrev to use InteropProvider
http://cr.openjdk.java.net/~psadhukhan/fxswing.9/
This looks okay although for consistent then InteropImpl could be 
renamed too.


-Alan

Re: [11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

[Prasanta Sadhukhan]

Would InteropProviderImpl sound good?

Regards
Prasanta
On 5/7/2018 8:27 PM, Alan Bateman wrote:



On 07/05/2018 10:26, Prasanta Sadhukhan wrote:

:

Modified webrev to use InteropProvider
http://cr.openjdk.java.net/~psadhukhan/fxswing.9/
This looks okay although for consistent then InteropImpl could be 
renamed too.


-Alan

Re: [11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

[Prasanta Sadhukhan]

On 5/6/2018 1:15 PM, Alan Bateman wrote:

On 05/05/2018 16:20, Prasanta Sadhukhan wrote:
Updated webrev to modify java.desktop module-info.java (only 
difference between webrev7 and this) to remove the duplicate exports 
of sun.awt so we will have now


exports sun.awt to
    jdk.accessibility,
    jdk.unsupported.desktop;

http://cr.openjdk.java.net/~psadhukhan/fxswing.8/


At a high-level, the module declarations look good and using service 
binding to ensure that the jdk.unsupported.desktop module is resolved 
looks good. The only thing is the name of the service provider 
interface, it might be clearer if you use InteropProvider rather than 
InteropInterface.



Modified webrev to use InteropProvider
http://cr.openjdk.java.net/~psadhukhan/fxswing.9/
I assume dependencies/java.desktop/module-info.java.extra will be 
removed once the remaining dependency on sun.print goes away.



Yes, that's the plan.

Regards
Prasanta

-Alan

Re: [11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

[Prasanta Sadhukhan]

Updated webrev to modify java.desktop module-info.java (only difference 
between webrev7 and this) to remove the duplicate exports of sun.awt so 
we will have now


exports sun.awt to
    jdk.accessibility,
    jdk.unsupported.desktop;

http://cr.openjdk.java.net/~psadhukhan/fxswing.8/

Regards
Prasanta
On 5/5/2018 2:28 PM, Prasanta Sadhukhan wrote:

Hi All,

I have tried to address all comments from Nir, Phil, Mandy and Kevin 
got so far in this webrev

http://cr.openjdk.java.net/~psadhukhan/fxswing.7

>>In SwingNode, lines 870 -883, is there a reason that only the first 
method uncommented @Override? I don't understand the comment that 
wants to skip @Override.
If @override is not added, then it will cause recursion between 
LightweightContent.java:130 and LightweightContent.java:187 thereby 
causing StackOverflowError. I am also not sure about the comment of 
skipping @Override but I have removed it from this webrev.


Regards
Prasanta
On 5/5/2018 4:32 AM, Kevin Rushforth wrote:

My quick comments:

1. The changes to java.desktop module-info.java won't compile when 
applied to jdk-client, since there is already a qualified export of 
the sun.swing package to another internal class. Can you update your 
patch to be based off of jdk-client? I note that it requires a small 
patch to be able to build FX with JDK 11 (it's a known gradle issue), 
but it's not hard to do.


2. As I mentioned in an off-line email, the final version of the 
javafx.swing patch will need to use 'requires static 
jdk.unsupported.desktop' in its module-info.java, so that it will 
still run on JDK 10 EA. This means that you will need to ensure that 
jdk.unsupported.desktop is loaded via a service loader as discussed. 
This will not affect the public API at all, so that can proceed in 
parallel, but this needs to be fixed.


3. As I mentioned in my earlier reply to Mandy's comment, Swing 
interop apps will fail if a security manager is installed, because 
the jdk.unsupported.desktop classes are loaded by the app class 
loader. This can be sorted out later as a follow-on bug, with 
permissions added to the default policy file, etc.


4. As Phil mentioned, some docs would be good. They don't need to be 
all that detailed, since it isn't intended for use by applications, 
and will not be included in the published API docs bundle (i.e., it 
should be excluded from "make docs").


I will do a more detailed review early next week.

-- Kevin

On 5/4/2018 1:53 PM, Phil Race wrote:

Two quick comments.

1) I'd like "Peer" removed from all the exported API.
2) It is probably stable enough now that you can start to add some 
javadoc.


-phil.

On 05/04/2018 05:00 AM, Prasanta Sadhukhan wrote:

Hi All,

Please review an enhancement to remove the tight coupling of JDK 
internal class from FX so that
when javafx.* modules are removed from Openjdk build in jdk11, FX 
in general, and fx swing interop, in particular, can still build 
and function.


Right now, FX uses 6 jdk internal packages
sun.swing, sun.awt, java.awt.dnd.peer, sun.awt.dnd, sun.awt.image 
and sun.java2d.


However, the goal is not to use qualified exports of these internal 
packages once FX is removed to be built along with JDK,


The solution will define a new "jdk.unsupported.desktop" module 
that exports public API
that is intended to be used by the javafx.swing module (but it does 
so with public exports and not qualified exports).
The idea is the same as jdk.unsupported, in that it is documented 
as being unsupported.
Further, since it is only intended to be used by javafx.swing, it 
need not be in the default module graph.


The module-info.java will look like this:

|module jdk.unsupported.desktop { requires transitive java.desktop; 
exports jdk.swing.interop; } |||


Enhancement: https://bugs.openjdk.java.net/browse/JDK-8202199, 
https://bugs.openjdk.java.net/browse/JDK-8195811

webrev: cr.openjdk.java.net/~psadhukhan/fxswing.6/
CSR: https://bugs.openjdk.java.net/browse/JDK-8202175

Regards
Prasanta

||

Re: [11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

[Prasanta Sadhukhan]

Hi All,

I have tried to address all comments from Nir, Phil, Mandy and Kevin got 
so far in this webrev

http://cr.openjdk.java.net/~psadhukhan/fxswing.7

>>In SwingNode, lines 870 -883, is there a reason that only the first 
method uncommented @Override? I don't understand the comment that wants 
to skip @Override.
If @override is not added, then it will cause recursion between 
LightweightContent.java:130 and LightweightContent.java:187 thereby 
causing StackOverflowError. I am also not sure about the comment of 
skipping @Override but I have removed it from this webrev.


Regards
Prasanta
On 5/5/2018 4:32 AM, Kevin Rushforth wrote:

My quick comments:

1. The changes to java.desktop module-info.java won't compile when 
applied to jdk-client, since there is already a qualified export of 
the sun.swing package to another internal class. Can you update your 
patch to be based off of jdk-client? I note that it requires a small 
patch to be able to build FX with JDK 11 (it's a known gradle issue), 
but it's not hard to do.


2. As I mentioned in an off-line email, the final version of the 
javafx.swing patch will need to use 'requires static 
jdk.unsupported.desktop' in its module-info.java, so that it will 
still run on JDK 10 EA. This means that you will need to ensure that 
jdk.unsupported.desktop is loaded via a service loader as discussed. 
This will not affect the public API at all, so that can proceed in 
parallel, but this needs to be fixed.


3. As I mentioned in my earlier reply to Mandy's comment, Swing 
interop apps will fail if a security manager is installed, because the 
jdk.unsupported.desktop classes are loaded by the app class loader. 
This can be sorted out later as a follow-on bug, with permissions 
added to the default policy file, etc.


4. As Phil mentioned, some docs would be good. They don't need to be 
all that detailed, since it isn't intended for use by applications, 
and will not be included in the published API docs bundle (i.e., it 
should be excluded from "make docs").


I will do a more detailed review early next week.

-- Kevin

On 5/4/2018 1:53 PM, Phil Race wrote:

Two quick comments.

1) I'd like "Peer" removed from all the exported API.
2) It is probably stable enough now that you can start to add some 
javadoc.


-phil.

On 05/04/2018 05:00 AM, Prasanta Sadhukhan wrote:

Hi All,

Please review an enhancement to remove the tight coupling of JDK 
internal class from FX so that
when javafx.* modules are removed from Openjdk build in jdk11, FX in 
general, and fx swing interop, in particular, can still build and 
function.


Right now, FX uses 6 jdk internal packages
sun.swing, sun.awt, java.awt.dnd.peer, sun.awt.dnd, sun.awt.image 
and sun.java2d.


However, the goal is not to use qualified exports of these internal 
packages once FX is removed to be built along with JDK,


The solution will define a new "jdk.unsupported.desktop" module that 
exports public API
that is intended to be used by the javafx.swing module (but it does 
so with public exports and not qualified exports).
The idea is the same as jdk.unsupported, in that it is documented as 
being unsupported.
Further, since it is only intended to be used by javafx.swing, it 
need not be in the default module graph.


The module-info.java will look like this:

|module jdk.unsupported.desktop { requires transitive java.desktop; 
exports jdk.swing.interop; } |||


Enhancement: https://bugs.openjdk.java.net/browse/JDK-8202199, 
https://bugs.openjdk.java.net/browse/JDK-8195811

webrev: cr.openjdk.java.net/~psadhukhan/fxswing.6/
CSR: https://bugs.openjdk.java.net/browse/JDK-8202175

Regards
Prasanta

||

[11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

[Prasanta Sadhukhan]

Hi All,

Please review an enhancement to remove the tight coupling of JDK 
internal class from FX so that
when javafx.* modules are removed from Openjdk build in jdk11, FX in 
general, and fx swing interop, in particular, can still build and function.


Right now, FX uses 6 jdk internal packages
sun.swing, sun.awt, java.awt.dnd.peer, sun.awt.dnd, sun.awt.image and 
sun.java2d.


However, the goal is not to use qualified exports of these internal 
packages once FX is removed to be built along with JDK,


The solution will define a new "jdk.unsupported.desktop" module that 
exports public API
that is intended to be used by the javafx.swing module (but it does so 
with public exports and not qualified exports).
The idea is the same as jdk.unsupported, in that it is documented as 
being unsupported.
Further, since it is only intended to be used by javafx.swing, it need 
not be in the default module graph.


The module-info.java will look like this:

|module jdk.unsupported.desktop { requires transitive java.desktop; 
exports jdk.swing.interop; } |||


Enhancement: https://bugs.openjdk.java.net/browse/JDK-8202199, 
https://bugs.openjdk.java.net/browse/JDK-8195811

webrev: cr.openjdk.java.net/~psadhukhan/fxswing.6/
CSR: https://bugs.openjdk.java.net/browse/JDK-8202175

Regards
Prasanta

||

Re: [10] RFR JDK-8189280:Memory leak in SwingNode if Stage is not shown

[Prasanta Sadhukhan]

Removed commented code in-place

Regards
Prasanta
On 11/27/2017 1:20 PM, Jayathirth D V wrote:

Hi Prasanta,

In the test case please add reason why you have kept commented code like :
 //public static void main(String[] args) {
 //Application.launch(args);
 //}

 //@Override
 //public void start(Stage primaryStage) {
 //memoryLeakTest();
 //}

Also please resolve indentation problem in comments like:

 //stage.show();
//tk.firePulse();

//tk.firePulse();

Thanks,
Jay
-Original Message-
From: Prasanta Sadhukhan
Sent: Monday, November 27, 2017 1:08 PM
To: Kevin Rushforth; openjfx-dev@openjdk.java.net Mailing
Subject: [10] RFR JDK-8189280:Memory leak in SwingNode if Stage is not shown

Hi All,

Please review a swingnode memory leak fix for fx 
http://cr.openjdk.java.net/~psadhukhan/fx/8189280/webrev.00/
for issue
https://bugs.openjdk.java.net/browse/JDK-8189280

More info in JBS.

Regards
Prasanta

[10] RFR JDK-8189280:Memory leak in SwingNode if Stage is not shown

[Prasanta Sadhukhan]

Hi All,

Please review a swingnode memory leak fix for fx
http://cr.openjdk.java.net/~psadhukhan/fx/8189280/webrev.00/
for issue
https://bugs.openjdk.java.net/browse/JDK-8189280

More info in JBS.

Regards
Prasanta

Re: CFV: New OpenJFX Committer: Ambarish Rapte

[Prasanta Sadhukhan]

Vote: yes

Regards
Prasanta
On 11/1/2017 1:48 AM, Kevin Rushforth wrote:

I hereby nominate Ambarish Rapte [1] to OpenJFX Committer.

Ambarish is a member of JavaFX team at Oracle, who has contributed 10 
changesets [2][3] to OpenJFX.


Votes are due by November 14, 2017.

Only current OpenJFX Committers [4] 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 [5]. Nomination to a 
project Committer is described in [6].


Thanks.

-- Kevin

[1] http://openjdk.java.net/census#arapte

[2] 
http://hg.openjdk.java.net/openjfx/10-dev/rt/log?revcount=20&rev=author%28arapte%29

[3] http://hg.openjdk.java.net/openjfx/10-dev/rt/rev/f7d38650fa15

[4] http://openjdk.java.net/census#openjfx

[5] http://openjdk.java.net/bylaws#lazy-consensus

[6] http://openjdk.java.net/projects#project-committer

[10] RFR JDK-8090267: JFXPanel Input Problem

[Prasanta Sadhukhan]

Hi All,

Please review a input method fx fix
http://cr.openjdk.java.net/~psadhukhan/fx/8090267/webrev.00/
for issue
https://bugs.openjdk.java.net/browse/JDK-8090267

More info in JBS.

Regards
Prasanta

Re: [10] RFR JDK-8088132:[Swing, singleThread] ClassCastException in nested event loop when showing multiple message dialogs in SwingNode

[Prasanta Sadhukhan]

Hi Sergey,

Please find modified webrev catering to DefaultKeyboardFocusManager also
http://cr.openjdk.java.net/~psadhukhan/8088132/webrev.05/

Regards
Prasanta
On 9/23/2017 7:52 AM, Sergey Bylokhov wrote:

Hi, Prasanta.

On 9/19/17 22:26, Prasanta Sadhukhan wrote:

Hi, Prasanta.
 - In this version it is unclear what is a purpose of the 
"fxCheckSequenceThread.start()". This code will start the thread and 
will continue execution, it will not wait when the thread will stop.
 - The DefaultKeyboardFocusManager also should check for 
"javafx.embed.singleThread"



Please find modified webrev catering to both comments
http://cr.openjdk.java.net/~psadhukhan/8088132/webrev.04/


The new code in DefaultKeyboardFocusManager also will not wait when 
the thread will stop.




Regards
Prasanta


On 9/13/17 03:11, Prasanta Sadhukhan wrote:

Hi Sergey,

I have modified the fix to not use SecondaryLoop and not to create 
one thread per dispatch

http://cr.openjdk.java.net/~psadhukhan/8088132/webrev.03/

Regards
Prasanta
On 8/30/2017 11:41 AM, Sergey Bylokhov wrote:

Hi, Prasanta.
Can you please provide some description on how the SecondaryLoop 
will work when it will run on Appkit thread? Is it possible to get 
a deadlock here, since appkit will be blocked?


sequence, ie if the event is not first in sequence, then it will 
made

to
wait till it is the first event or till it is disposed.
Note that the new code (unlike lines 139-150) also waits 1 second, 
so we can get a situation when only one event will be dispatched 
per second, which is not we want to do.
I am not sure how often we create SequencedEvent but creating one 
thread per dispatch look inefficient.




Modified webrev
http://cr.openjdk.java.net/~psadhukhan/fx/8088132/webrev.02/

Regards
Prasanta
On 8/23/2017 9:31 PM, Sergey Bylokhov wrote:

Hi, Prasanta.

On 16.08.2017 3:33, Prasanta Sadhukhan wrote:
Now, since here FX App thread itself is the dispatch thread, we 
can

be sure the events are dispatched in sequence and dispose is

checked

below after EDT.

Why we can sure about this? If the SequencedEvents were created in

one

order but dispatch() for each were called in other order then the
sequenced will not be preserved?


I have tested with couple of singleThread testcase without any

issue.

Regards
Prasanta
On 8/14/2017 10:07 PM, Sergey Bylokhov wrote:

Hi, Prasanta, Kevin.

I have two notes.
   - Does this option is really supported? If it is supported we
should evaluate all usage of EventDispatchThread because in this
mode the dispatch thread is not EDT. For example I am not sure

that

we can skip the code which expects EventDispatchThread.
   - We have the similar pattern: 
"EventQueue.isDispatchThread() ->

cast(EventDispatchThread)" in some other places like in
DefaultKeyboardFocusManager.

-prasanta.sadhuk...@oracle.com  wrote:


Hi All,

Please review this fix
http://cr.openjdk.java.net/~psadhukhan/fx/8088132/webrev.00/
for an fx issue
https://bugs.openjdk.java.net/browse/JDK-8088132

More info in JBS.

Regards
Prasanta

[10] JDK-8187781: "InvalidDnDOperationException: Drag and drop in progress" while running javafx application with option -Djavafx.embed.singleThread=true

[Prasanta Sadhukhan]

Hi All,

Please review a fix
http://cr.openjdk.java.net/~psadhukhan/fx/8187781/webrev.00/
for fx issue
https://bugs.openjdk.java.net/browse/JDK-8187781

More info in JBS.

Regards
Prasanta

Re: [10] RFR JDK-8088132:[Swing, singleThread] ClassCastException in nested event loop when showing multiple message dialogs in SwingNode

[Prasanta Sadhukhan]

Hi Sergey,


On 9/18/2017 11:57 PM, Sergey Bylokhov wrote:

Hi, Prasanta.
 - In this version it is unclear what is a purpose of the 
"fxCheckSequenceThread.start()". This code will start the thread and 
will continue execution, it will not wait when the thread will stop.
 - The DefaultKeyboardFocusManager also should check for 
"javafx.embed.singleThread"



Please find modified webrev catering to both comments
http://cr.openjdk.java.net/~psadhukhan/8088132/webrev.04/

Regards
Prasanta


On 9/13/17 03:11, Prasanta Sadhukhan wrote:

Hi Sergey,

I have modified the fix to not use SecondaryLoop and not to create 
one thread per dispatch

http://cr.openjdk.java.net/~psadhukhan/8088132/webrev.03/

Regards
Prasanta
On 8/30/2017 11:41 AM, Sergey Bylokhov wrote:

Hi, Prasanta.
Can you please provide some description on how the SecondaryLoop 
will work when it will run on Appkit thread? Is it possible to get a 
deadlock here, since appkit will be blocked?



sequence, ie if the event is not first in sequence, then it will made
to
wait till it is the first event or till it is disposed.
Note that the new code (unlike lines 139-150) also waits 1 second, 
so we can get a situation when only one event will be dispatched per 
second, which is not we want to do.
I am not sure how often we create SequencedEvent but creating one 
thread per dispatch look inefficient.




Modified webrev
http://cr.openjdk.java.net/~psadhukhan/fx/8088132/webrev.02/

Regards
Prasanta
On 8/23/2017 9:31 PM, Sergey Bylokhov wrote:

Hi, Prasanta.

On 16.08.2017 3:33, Prasanta Sadhukhan wrote:

Now, since here FX App thread itself is the dispatch thread, we can
be sure the events are dispatched in sequence and dispose is

checked

below after EDT.

Why we can sure about this? If the SequencedEvents were created in

one

order but dispatch() for each were called in other order then the
sequenced will not be preserved?


I have tested with couple of singleThread testcase without any

issue.

Regards
Prasanta
On 8/14/2017 10:07 PM, Sergey Bylokhov wrote:

Hi, Prasanta, Kevin.

I have two notes.
   - Does this option is really supported? If it is supported we
should evaluate all usage of EventDispatchThread because in this
mode the dispatch thread is not EDT. For example I am not sure

that

we can skip the code which expects EventDispatchThread.
   - We have the similar pattern: "EventQueue.isDispatchThread() ->
cast(EventDispatchThread)" in some other places like in
DefaultKeyboardFocusManager.

-prasanta.sadhuk...@oracle.com  wrote:


Hi All,

Please review this fix
http://cr.openjdk.java.net/~psadhukhan/fx/8088132/webrev.00/
for an fx issue
https://bugs.openjdk.java.net/browse/JDK-8088132

More info in JBS.

Regards
Prasanta

Re: [10] RFR JDK-8088132:[Swing, singleThread] ClassCastException in nested event loop when showing multiple message dialogs in SwingNode

[Prasanta Sadhukhan]

Hi Sergey,

I have modified the fix to not use SecondaryLoop and not to create one 
thread per dispatch

http://cr.openjdk.java.net/~psadhukhan/8088132/webrev.03/

Regards
Prasanta
On 8/30/2017 11:41 AM, Sergey Bylokhov wrote:

Hi, Prasanta.
Can you please provide some description on how the SecondaryLoop will work when 
it will run on Appkit thread? Is it possible to get a deadlock here, since 
appkit will be blocked?


sequence, ie if the event is not first in sequence, then it will made
to
wait till it is the first event or till it is disposed.

Note that the new code (unlike lines 139-150) also waits 1 second, so we can 
get a situation when only one event will be dispatched per second, which is not 
we want to do.
I am not sure how often we create SequencedEvent but creating one thread per 
dispatch look inefficient.



Modified webrev
http://cr.openjdk.java.net/~psadhukhan/fx/8088132/webrev.02/

Regards
Prasanta
On 8/23/2017 9:31 PM, Sergey Bylokhov wrote:

Hi, Prasanta.

On 16.08.2017 3:33, Prasanta Sadhukhan wrote:

Now, since here FX App thread itself is the dispatch thread, we can
be sure the events are dispatched in sequence and dispose is

checked

below after EDT.

Why we can sure about this? If the SequencedEvents were created in

one

order but dispatch() for each were called in other order then the
sequenced will not be preserved?


I have tested with couple of singleThread testcase without any

issue.

Regards
Prasanta
On 8/14/2017 10:07 PM, Sergey Bylokhov wrote:

Hi, Prasanta, Kevin.

I have two notes.
   - Does this option is really supported? If it is supported we
should evaluate all usage of EventDispatchThread because in this
mode the dispatch thread is not EDT. For example I am not sure

that

we can skip the code which expects EventDispatchThread.
   - We have the similar pattern: "EventQueue.isDispatchThread() ->
cast(EventDispatchThread)" in some other places like in
DefaultKeyboardFocusManager.

-prasanta.sadhuk...@oracle.com  wrote:


Hi All,

Please review this fix
http://cr.openjdk.java.net/~psadhukhan/fx/8088132/webrev.00/
for an fx issue
https://bugs.openjdk.java.net/browse/JDK-8088132

More info in JBS.

Regards
Prasanta

Re: [10] Review request: 8187361: Revert non reviewed commit done for the fix to JDK-8087528

[Prasanta Sadhukhan]

8087528 has been reverted ok.

+1.

Regards
Prasanta
On 9/8/2017 11:28 AM, Guru Hb wrote:

Hi Murali & Arun,

Please do the review for
JBS : https://bugs.openjdk.java.net/browse/JDK-8187361
  Webrev : 
http://cr.openjdk.java.net/~ghb/8187361/webrev.00/ 


Thanks,
Guru

Re: [10] RFR JDK-8089774: [SwingNode] getKeyLocation not correctly implemented with JavaFX

[Prasanta Sadhukhan]

Modified webrev with implementation for linux too.

http://cr.openjdk.java.net/~psadhukhan/fx/8089774/webrev.01/

Regards
Prasanta
On 9/5/2017 10:01 PM, Prasanta Sadhukhan wrote:

Hi All,

Please review a fix/enhancement
cr.openjdk.java.net/~psadhukhan/fx/8089774/webrev.00/
for a fx issue
https://bugs.openjdk.java.net/browse/JDK-8089774
and enhancement
https://bugs.openjdk.java.net/browse/JDK-8090524

More info in JBS.

Regards
Prasanta

[10] RFR JDK-8089774: [SwingNode] getKeyLocation not correctly implemented with JavaFX

[Prasanta Sadhukhan]

Hi All,

Please review a fix/enhancement
cr.openjdk.java.net/~psadhukhan/fx/8089774/webrev.00/
for a fx issue
https://bugs.openjdk.java.net/browse/JDK-8089774
and enhancement
https://bugs.openjdk.java.net/browse/JDK-8090524

More info in JBS.

Regards
Prasanta

Re: [10] RFR: JDK-8129747:SwingFXUtils.fromFXImage seems to have a bug inside

[Prasanta Sadhukhan]

Gentle reminder for review .

Regards
Prasanta
On 8/30/2017 1:41 PM, Prasanta Sadhukhan wrote:

Hi All,

Please review a fix
http://cr.openjdk.java.net/~psadhukhan/fx/8129747/webrev.00/
for fx issue
https://bugs.openjdk.java.net/browse/JDK-8129747

More info in JBS.

Regards
Prasanta

[10] RFR JDK-8087528:[SWT] FXCanvas: DnD is implemented incorrectly

[Prasanta Sadhukhan]

Hi All,

Please review a fix
http://cr.openjdk.java.net/~psadhukhan/fx/8087528/webrev.00/
for a fx issue
https://bugs.openjdk.java.net/browse/JDK-8087528

More info in JBS.

Regards
Prasanta

[10] RFR JDK-8156042:Modifiers of swing ActionEvent does not work when "-Djavafx.embed.singleThread=true"

[Prasanta Sadhukhan]

Hi All,

Please review a fix
http://cr.openjdk.java.net/~psadhukhan/fx/8156042/webrev.00/
for fx issue
https://bugs.openjdk.java.net/browse/JDK-8156042

More info in JBS.

Regards
Prasanta

[10] RFR: JDK-8129747:SwingFXUtils.fromFXImage seems to have a bug inside

[Prasanta Sadhukhan]

Hi All,

Please review a fix
http://cr.openjdk.java.net/~psadhukhan/fx/8129747/webrev.00/
for fx issue
https://bugs.openjdk.java.net/browse/JDK-8129747

More info in JBS.

Regards
Prasanta

Re: [10] RFR JDK-8088132:[Swing, singleThread] ClassCastException in nested event loop when showing multiple message dialogs in SwingNode

[Prasanta Sadhukhan]

I have incorporated the logic to make sure the events are dispatched in 
sequence, ie if the event is not first in sequence, then it will made to 
wait till it is the first event or till it is disposed.


Modified webrev
http://cr.openjdk.java.net/~psadhukhan/fx/8088132/webrev.02/

Regards
Prasanta
On 8/23/2017 9:31 PM, Sergey Bylokhov wrote:

Hi, Prasanta.

On 16.08.2017 3:33, Prasanta Sadhukhan wrote:
Now, since here FX App thread itself is the dispatch thread, we can 
be sure the events are dispatched in sequence and dispose is checked 
below after EDT.


Why we can sure about this? If the SequencedEvents were created in one 
order but dispatch() for each were called in other order then the 
sequenced will not be preserved?




I have tested with couple of singleThread testcase without any issue.

Regards
Prasanta
On 8/14/2017 10:07 PM, Sergey Bylokhov wrote:

Hi, Prasanta, Kevin.

I have two notes.
  - Does this option is really supported? If it is supported we 
should evaluate all usage of EventDispatchThread because in this 
mode the dispatch thread is not EDT. For example I am not sure that 
we can skip the code which expects EventDispatchThread.
  - We have the similar pattern: "EventQueue.isDispatchThread() -> 
cast(EventDispatchThread)" in some other places like in 
DefaultKeyboardFocusManager.


-prasanta.sadhuk...@oracle.com  wrote:


Hi All,

Please review this fix
http://cr.openjdk.java.net/~psadhukhan/fx/8088132/webrev.00/
for an fx issue
https://bugs.openjdk.java.net/browse/JDK-8088132

More info in JBS.

Regards
Prasanta

[10] RFR JDK-8144504:Multiple SwingNode in JFXPanel in JFrame Cause Extreme Performance Degradation

[Prasanta Sadhukhan]

Hi All,

Please review a fix
cr.openjdk.java.net/~psadhukhan/fx/8144504/webrev.00/
for an issue
https://bugs.openjdk.java.net/browse/JDK-8144504

More info in JBS.

Regards
Prasanta

Re: [10] RFR JDK-8088132:[Swing, singleThread] ClassCastException in nested event loop when showing multiple message dialogs in SwingNode

[Prasanta Sadhukhan]

Any feedback on this?

Regards
Prasanta
On 8/16/2017 4:03 PM, Prasanta Sadhukhan wrote:


Hi Sergey,

AFAIK, FX singleThread feature is supported but experimental feature.
I have modified webrev to include DefaultKeyboardFocusManager too
http://cr.openjdk.java.net/~psadhukhan/fx/8088132/webrev.01/

I do not think there is any problem in skipping code which works with 
EDT as for example, in SequencedEvent#dispatch() it says
Dispatches the nested event after all previous nested events have 
beendispatched or disposed
Now, since here FX App thread itself is the dispatch thread, we can be 
sure the events are dispatched in sequence and dispose is checked 
below after EDT.


I have tested with couple of singleThread testcase without any issue.

Regards
Prasanta
On 8/14/2017 10:07 PM, Sergey Bylokhov wrote:

Hi, Prasanta, Kevin.

I have two notes.
  - Does this option is really supported? If it is supported we should evaluate 
all usage of EventDispatchThread because in this mode the dispatch thread is 
not EDT. For example I am not sure that we can skip the code which expects 
EventDispatchThread.
  - We have the similar pattern: "EventQueue.isDispatchThread() -> 
cast(EventDispatchThread)" in some other places like in DefaultKeyboardFocusManager.

-prasanta.sadhuk...@oracle.com  wrote:


Hi All,

Please review this fix
http://cr.openjdk.java.net/~psadhukhan/fx/8088132/webrev.00/
for an fx issue
https://bugs.openjdk.java.net/browse/JDK-8088132

More info in JBS.

Regards
Prasanta

[10] RFR JDK-8089962:[FXDnD]: in removeDropTarget the isDropTargetListenerInstalled field isn't reset

[Prasanta Sadhukhan]

Hi All,

Please review a fix
http://cr.openjdk.java.net/~psadhukhan/fx/8089962/webrev.00/
for issue
https://bugs.openjdk.java.net/browse/JDK-8089962

More info in JBS.

Regards
Prasanta

[10] RFR JDK-8089351:JFXPanel: non-serializable value stored into instance field of a serializable class

[Prasanta Sadhukhan]

Hi All,

Please review a fix
http://cr.openjdk.java.net/~psadhukhan/fx/8089351/webrev.00/
for issue
https://bugs.openjdk.java.net/browse/JDK-8089351

More info in JBS.

Regards
Prasanta

[10] RFR JDK-8089005:JFXPanel.setScene(Scene) uses bad synchronization pattern

[Prasanta Sadhukhan]

Hi All,

Please review fx fix
http://cr.openjdk.java.net/~psadhukhan/fx/8089005/webrev.00/
for issue
https://bugs.openjdk.java.net/browse/JDK-8089005

More info in JBS.

Regards
Prasanta

Re: [10] RFR JDK-8088132:[Swing, singleThread] ClassCastException in nested event loop when showing multiple message dialogs in SwingNode

[Prasanta Sadhukhan]

Hi Sergey,

AFAIK, FX singleThread feature is supported but experimental feature.
I have modified webrev to include DefaultKeyboardFocusManager too
http://cr.openjdk.java.net/~psadhukhan/fx/8088132/webrev.01/

I do not think there is any problem in skipping code which works with 
EDT as for example, in SequencedEvent#dispatch() it says


Dispatches the nested event after all previous nested events have 
beendispatched or disposed


Now, since here FX App thread itself is the dispatch thread, we can be 
sure the events are dispatched in sequence and dispose is checked below 
after EDT.


I have tested with couple of singleThread testcase without any issue.

Regards
Prasanta
On 8/14/2017 10:07 PM, Sergey Bylokhov wrote:

Hi, Prasanta, Kevin.

I have two notes.
  - Does this option is really supported? If it is supported we should evaluate 
all usage of EventDispatchThread because in this mode the dispatch thread is 
not EDT. For example I am not sure that we can skip the code which expects 
EventDispatchThread.
  - We have the similar pattern: "EventQueue.isDispatchThread() -> 
cast(EventDispatchThread)" in some other places like in DefaultKeyboardFocusManager.

- prasanta.sadhuk...@oracle.com wrote:


Hi All,

Please review this fix
http://cr.openjdk.java.net/~psadhukhan/fx/8088132/webrev.00/
for an fx issue
https://bugs.openjdk.java.net/browse/JDK-8088132

More info in JBS.

Regards
Prasanta

[10] RFR JDK-8088132:[Swing, singleThread] ClassCastException in nested event loop when showing multiple message dialogs in SwingNode

[Prasanta Sadhukhan]

Hi All,

Please review this fix
http://cr.openjdk.java.net/~psadhukhan/fx/8088132/webrev.00/
for an fx issue
https://bugs.openjdk.java.net/browse/JDK-8088132

More info in JBS.

Regards
Prasanta

[10] RFR JDK-8087914: [JFXPanel] Clicking on Menu in JFXPanel ignored if another swing component has focus

[Prasanta Sadhukhan]

Hi All,

Please review this fx fix
http://cr.openjdk.java.net/~psadhukhan/fx/8087914/webrev.00/
for issue
https://bugs.openjdk.java.net/browse/JDK-8087914

More info in JBS.

Regards
Prasanta

[10] RFR JDK-8156592:JFXPanel should use the new FocusEvent.getCause() API

[Prasanta Sadhukhan]

Hi All,

Please review a fix
http://cr.openjdk.java.net/~psadhukhan/fx/8156592/webrev.00/
for issue
https://bugs.openjdk.java.net/browse/JDK-8156592

More info in JBS.

Regards
Prasanta

[10] RFR JDK-8088874:AssertionError in javafx.embed.swing.SwingDnD.endDnD

[Prasanta Sadhukhan]

Hi All,

Please review a fix
http://cr.openjdk.java.net/~psadhukhan/fx/8088874/webrev.00/
for issue
https://bugs.openjdk.java.net/browse/JDK-8088874

More info in JBS.

Regards
Prasanta

[10] RFR JDK-8185890:Intermittent NPE in JLightweightFrame when updating cursor aceoss multiple graphics devices

[Prasanta Sadhukhan]

Hi All,

Please review this patch
http://cr.openjdk.java.net/~psadhukhan/fx/8185890/webrev.00/
for issue
https://bugs.openjdk.java.net/browse/JDK-8185890

More info in JBS.

Regards
Prasanta

[10] RFR JDK-8185792: Entering accents in a textfield on a JFXPanel produces NPE

[Prasanta Sadhukhan]

Hi All,

Please review a fix
http://cr.openjdk.java.net/~psadhukhan/fx/8185792/webrev.00/
for an fx issue
https://bugs.openjdk.java.net/browse/JDK-8185792

More info in JBS.

Regards
Prasanta

Re: [10] RFR: JDK-8133329: Drag and Drop of files in a SwingNode fails

[Prasanta Sadhukhan]

On 8/4/2017 2:57 AM, Jim Graham wrote:
I noticed a "FIXME" comment in there.  Didn't we do a pass a while 
back and convert all of them into JBS entries?


The new code simply repeats what was done for the existing Exception 
which is what has the "FIXME" comment so I would use this opportunity 
to upgrade that FIXME into an actual bug entry.


Also, perhaps we could use the multi-catch syntax here instead of 
creating a new catch?



Ok. I have put it as a mult-catch in this webrev
http://cr.openjdk.java.net/~psadhukhan/fx/8133329/webrev.01/

Regards
Prasanta

...jim

On 8/3/17 2:01 AM, Prasanta Sadhukhan wrote:

Hi All,

Please review a fix
http://cr.openjdk.java.net/~psadhukhan/fx/8133329/webrev.00/
for issue
https://bugs.openjdk.java.net/browse/JDK-8133329
More information in JBS.

Regards
Prasanta

[10] RFR: JDK-8133329: Drag and Drop of files in a SwingNode fails

[Prasanta Sadhukhan]

Hi All,

Please review a fix
http://cr.openjdk.java.net/~psadhukhan/fx/8133329/webrev.00/
for issue
https://bugs.openjdk.java.net/browse/JDK-8133329
More information in JBS.

Regards
Prasanta

[10] RFR: JDK-8098836:various NullPointerException in JFXPanel

[Prasanta Sadhukhan]

Hi All,

Please review the fix
http://cr.openjdk.java.net/~psadhukhan/fx/8098836/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8098836

More information in JBS.

Regards
Prasanta

[10] RFR JDK-8181786: Extra runLater causes impossible states to be possible using javafx.embed.singleThread=true

[Prasanta Sadhukhan]

Hi All,

Request to review the fix
http://cr.openjdk.java.net/~psadhukhan/8181786/webrev.00/
for issue
https://bugs.openjdk.java.net/browse/JDK-8181786

Regards
Prasanta

Re: [OpenJDK 2D-Dev] RFR: 8088395: Print dialogs are not blocking/modal w.r.t specified owner windows

[Prasanta Sadhukhan]

Looks good to me.

Regards
Prasanta
On 3/14/2017 11:02 PM, Phil Race wrote:

I have updated webrevs described below, and also I answer your questions

FX is updated only to fix trailing white space and one modifier ordering
http://cr.openjdk.java.net/~prr/8088395.1/

The JDK side has a few modifications :
http://cr.openjdk.java.net/~prr/8176350.1/
(a) awt_PrintDialog.cpp and awt_PrintJob.cpp have an added safety check.
We now call ::IsWindowHandle(id) as we do elsewhere to make sure its 
valid.


(b) The Page Setup Dialog on Linux was not seeing the AlwaysOnTop 
property.
In part this was because it wasn't being checked in 
ServiceDialog.initPageDialog() - fixed
In other part it is because the attributes weren't propagated. The 
reason for this is
a bit lengthy to explain but the main thing to say is that we have an 
instance variable

attributes as well as a local variable in some places.
I didn't want to touch any code that even theoretically might affect 
2D printing so
instead I cached the attribute as we do the owner id so it is passed 
on properly

Thanks to Kevin for spotting this problem


On 03/14/2017 09:02 AM, Sergey Bylokhov wrote:

Hi, Phil.
I have only two questions:
 - Does it mean that we do not support "ontop" property via public 
API in idk?


There is no API for the "alwaysOnTop" AWT property for the print dialogs.
I actually don't think it can be implemented for the windows native 
print controls.
There is an internal way to make an AWT window owner for the Swing 
print dialog
but nothing public and it doesn't help here as the owner is not an AWT 
window.



 - Probably the name should contain «owner» instead of «parent»?, But 
since this is not a public API I guess it is not an issue.


I suppose that is more correct. Something to address if it ever makes 
its way into API or docs.


-phil.






This has an FX bug + webrev :
https://bugs.openjdk.java.net/browse/JDK-8088395
http://cr.openjdk.java.net/~prr/8088395/index.html 



and also a JDK-side fix and webrev :-
https://bugs.openjdk.java.net/browse/JDK-8176530
http://cr.openjdk.java.net/~prr/8176350/

The problem is FX modal dialogs are ignoring the Window parameter.
We can fix the problem with disabling the modal parent on the FX side
and that is why most files in FX are updated.

But it does not fix the "on top" issue which requires the JDK fixes and
to pass in the DialogOnTop private attribute.
The JDK code is there solely for FX and won't have any visibility
unless FX passes in the private attribute.

On Linux it uses the standard AWT "always on top" modality
On windows it uses the HWND for the FX window and windows native 
modality

On Mac you won't see anything since Mac does this automatically

-phil.