Re: Proposal For Inclusion of Robot and ParametersImpl in the Public API

2018-03-19 Thread Michael Ennen
Ping :)

On Mon, Jan 8, 2018 at 4:28 PM, Kevin Rushforth 
wrote:

> I'll take a look some time after RDP2 of JDK 10.
>
>
> -- Kevin
>
>
> Michael Ennen wrote:
>
> Hey Kevin,
>
> Hope you had a good holiday. Hopefully you will get some time in the
> coming weeks
> to review my work.
>
> Thanks!
>
> On Wed, Dec 20, 2017 at 3:05 PM, Kevin Rushforth <
> kevin.rushfo...@oracle.com> wrote:
>
>> Sure, no problem. One quick comment is that a common way to solve this is
>> by delegating to an implementation class, which would then be sub-classes.
>>
>>
>> -- Kevin
>>
>>
>> Michael Ennen wrote:
>>
>> I am not trying to be a burden here. I understand that you may not have
>> time to hand-hold
>> to this degree. I will try and make progress, sorry for the follow up
>> question.
>>
>> On Wed, Dec 20, 2017 at 2:08 PM, Michael Ennen 
>> wrote:
>>
>>> How can Robot call into the implementation when it is a super class of
>>> the implementations?
>>>
>>> On Wed, Dec 20, 2017 at 2:04 PM, Kevin Rushforth <
>>> kevin.rushfo...@oracle.com> wrote:
>>>


 Michael Ennen wrote:

 I have a question about how to proceed with the Robot code.

 The base abstract Robot class is: https://github.com/brcolow
 /openjfx/blob/master/modules/javafx.graphics/src/main/java/j
 avafx/scene/robot/Robot.java

 As you can see for each method, such as "getMouseX()" there is a "_"
 prefixed method
 which is abstract and a non-prefixed method:

 protected abstract int _getMouseX();

 public int getMouseX() {
 Application.checkEventThread();
 return _getMouseX();
 }

 I have copied this from the private Robot API.

 Is there a better way to do this? Would this pass review?


 Yes there are better ways to do this. No it would not pass review,
 since this would be leaking implementation into the public API.

 Rather than copying the public / protected methods from the internal
 package, it probably makes more sense to start with what a Robot API should
 look like and then have that call into the implementation (suitably
 modified so it better matches the public API). For one thing you will then
 leave the implementation, including the per-platform code, where it belongs
 -- in glass. The Robot API can be informed by the current implementation,
 but should not be defined by it.

 -- Kevin



 Thanks very much.


 On Tue, Dec 5, 2017 at 5:29 PM, Kevin Rushforth <
 kevin.rushfo...@oracle.com> wrote:

> Glad you got the build working. You can post back on this thread when
> you are ready.
>
>
> -- Kevin
>
>
> Michael Ennen wrote:
>
> Correction:
>
> Adding ""--add-exports javafx.graphics/javafx.scene.robot=ALL-UNNAMED"
> to buildSrc/addExports.
>
> For posterity :)
>
> On Mon, Dec 4, 2017 at 6:08 PM, Michael Ennen 
> wrote:
>
>> Ah, indeed, missed adding "--add-opens 
>> javafx.graphics/javafx.scene.robot=ALL-UNNAMED"
>> to buildSrc/addExports.
>> Thanks for the guidance on that.
>>
>> I will continue to work on this in the GitHub repo and polish it up
>> (add javadocs, better method signatures, etc.) and
>> even plan on maybe improving the underlying native Robot
>> implementations (for example fixing/improving the
>> way color profiles are handled for MacRobot).
>>
>> I will also take a look at "fixing" JemmyFX to use the new public API
>> (as well as any other place in the JavaFX code
>> base that does).
>>
>> I was expecting that JDK 11 would be the appropriate time frame,
>> especially because it will be the release where
>> private APIs will be totally inaccessible, correct?
>>
>> After I get it in a reasonable state should I post back on this
>> mailing list thread or what would be the appropriate
>> way?
>>
>> Thanks Kevin.
>>
>> On Mon, Dec 4, 2017 at 5:12 PM, Kevin Rushforth <
>> kevin.rushfo...@oracle.com> wrote:
>>
>>> This is a limitation of the the way --patch-modules works. You will
>>> need to add an entry in:
>>>
>>> buildSrc/addExports
>>>
>>> Btw, as for the proposal itself, this might need to be a JEP
>>> depending on the scope. In any case, it could be considered in the JDK 
>>> 11
>>> time frame, but there are several things that need to be worked out 
>>> before
>>> making Robot a public API, including the fact that the JemmyFX 
>>> framework in
>>> the openjfx/jfx/tests directory uses Robot. Once you get a working
>>> prototype, it would be interesting to discuss it in more detail.
>>>
>>> -- Kevin
>>>
>>>
>>>
>>> Michael Ennen wrote:
>>>
>>> Currently I am stuck with tests not being able to see the 

Re: JDK-8130379: Enhance the Bounds class with getCenter method

2018-03-19 Thread Nir Lisker
Hi Kevin,

I didn't see any opinions after a week. Please look at the webrev for:

https://bugs.openjdk.java.net/browse/JDK-8130379
http://cr.openjdk.java.net/~nlisker/8130379/webrev.00/

I'm still slightly in favor of having the 2D and 3D center methods, but the
webrev does not contain them.

Thanks,
Nir

On Mon, Mar 12, 2018 at 11:07 PM, Kevin Rushforth <
kevin.rushfo...@oracle.com> wrote:

> Hi Nir,
>
> This seems like a simple enough RFE, and I am not aware of anyone else
> working on it, nor any barriers that would make it difficult.
>
> As with any RFE that adds API (or implements a new feature) the API
> additions will need to be approved first. Currently we use the CSR process
> [1][2], but we will want to change that at some point. Note that as
> indicated in the "More community participation in JavaFX" thread [3], new
> API / new features will still have a higher bar for acceptance  than bug
> fixes.
>
> Also, any new feature needs to be accompanied by API docs and tests.
>
> Thanks!
>
> -- Kevin
>
> [1] https://wiki.openjdk.java.net/display/csr/Main
> [2] https://wiki.openjdk.java.net/display/csr/CSR+FAQs
> [2] http://mail.openjdk.java.net/pipermail/openjfx-dev/2018-Febr
> uary/021335.html
>
>
>
>
> Nir Lisker wrote:
>
>> Hello,
>>
>> I would like to work on https://bugs.openjdk.java.net/browse/JDK-8130379,
>> which adds convenience API to the Bounds class. Any comments?
>>
>> - Nir
>>
>>
>


Re: [11] RFR : JDK-8195800 : Eliminate dependency on sun.reflect.misc in javafx modules

2018-03-19 Thread Kevin Rushforth

This looks good to me, too, with one comment:

The three new files have DOS line endings. Please convert to UNIX-style 
line endings, else it will fail jcheck (and fail the 
tools/scripts/checkWhiteSpace).


-- Kevin


mandy chung wrote:



On 3/19/18 7:10 AM, Ambarish Rapte wrote:


Hi Kevin, Alan & Mandy,

 


Request you to review this fix:

Webrev: 
http://cr.openjdk.java.net/~arapte/fx/8195800/webrev.00/ 



JBS: https://bugs.openjdk.java.net/browse/JDK-8195800

 



This looks okay.  Nit FieldUtil isn't a trampoline class and you may 
just drop this comment:


  30 /*
  31  * Create a trampoline class.
  32  */

  
Mandy


Re: [11] RFR : JDK-8195800 : Eliminate dependency on sun.reflect.misc in javafx modules

2018-03-19 Thread mandy chung



On 3/19/18 7:10 AM, Ambarish Rapte wrote:


Hi Kevin, Alan & Mandy,

    Request you to review this fix:

    Webrev: 
http://cr.openjdk.java.net/~arapte/fx/8195800/webrev.00/ 



    JBS: https://bugs.openjdk.java.net/browse/JDK-8195800



This looks okay.  Nit FieldUtil isn't a trampoline class and you may 
just drop this comment:


  30 /*
  31  * Create a trampoline class.
  32  */

Mandy


[11] RFR : JDK-8195800 : Eliminate dependency on sun.reflect.misc in javafx modules

2018-03-19 Thread Ambarish Rapte
Hi Kevin, Alan & Mandy,

 

Request you to review this fix:

Webrev: 
http://cr.openjdk.java.net/~arapte/fx/8195800/webrev.00/ 

JBS: https://bugs.openjdk.java.net/browse/JDK-8195800

 

Regards,

Ambarish

 


Re: Gradle wrappers

2018-03-19 Thread Kevin Rushforth

As long as it is optional, this seems like a fine idea.

When we explored it before, there was some concern about checking in the 
gradlew.jar file to the repo -- we would need to get additional 
third-party legal approval -- so I would need to check on that before 
this can be merged to the mainline.


-- Kevin


Johan Vos wrote:

Hi,

Using Gradle wrappers has been discussed a number of times, but there was
never a clear resolution afaik.

Andres opened an issue for using gradle wrappers:
https://github.com/javafxports/openjdk-jfx/issues/23

I personally like gradle wrappers myself, it makes it easier to automate
builds etc. If developers really want to use a specific (other) gradle
version, it is of course very well possible to do so.

Opinions?

- Johan
  


Gradle wrappers

2018-03-19 Thread Johan Vos
Hi,

Using Gradle wrappers has been discussed a number of times, but there was
never a clear resolution afaik.

Andres opened an issue for using gradle wrappers:
https://github.com/javafxports/openjdk-jfx/issues/23

I personally like gradle wrappers myself, it makes it easier to automate
builds etc. If developers really want to use a specific (other) gradle
version, it is of course very well possible to do so.

Opinions?

- Johan