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

2018-05-04 Thread Kevin Rushforth
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

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

2018-05-04 Thread Kevin Rushforth
jdk.unsupported.desktop is defined to the application class loader which I think it's fine as FX modules are defined to the same class loader. I noticed this, too, during my testing this morning. It still fails when the security manager is enabled. We already have a bug filed for this:

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

2018-05-04 Thread mandy chung
I skimmed through the sources.  It's good to see that this patch is straight forward.  A couple of comments: jdk.unsupported.desktop is defined to the application class loader which I think it's fine as FX modules are defined to the same class loader. I expect

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

2018-05-04 Thread Phil Race
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

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

2018-05-04 Thread Nir Lisker
Hi Prasanta, 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. Some minor code comments: * LightweightFrameWrapper#isCompEqual: you can skip the 'if' and just 'return c !=

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

2018-05-04 Thread Kevin Rushforth
Thanks, Prasanta. As an additional note to reviewers: The JDK portion of this change (JDK-8202199) is the subject for this review. The FX webrev is enough to be able to test the JDK side, but will need additional refactoring (to allow it to continue to build / run with JDK 10) before being

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

2018-05-04 Thread 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

Re: Paint Phase Debugging / Performance

2018-05-04 Thread Matthew Elliot
Hi Pedro, The first link I have read through many times, it is very useful for ideas but doesn't really flesh out or go into much detail on each topic. It also comments a few times on the problems we've encountered, 'what costs what' is difficult to understand / measure. The second link I hadn't