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

2017-12-22 Thread Michael Ennen
I didn't realize "_1" is an escape character in JNI method naming and is
only
needed when the method has a "_" in its' name. Nevermind!

On Fri, Dec 22, 2017 at 2:57 PM, Michael Ennen  wrote:

> I have made quite a bit of good progress I think:
>
> https://github.com/brcolow/openjfx/commit/d92ef24101cf32dfc07b21bdcb5755
> 28d8a58eaf
>
> The public API is starting to look much cleaner:
>
> https://github.com/brcolow/openjfx/blob/master/modules/
> javafx.graphics/src/main/java/javafx/scene/robot/Robot.java
>
> Still a lot to do.
>
> Currently I am running into a JNI UnsatisfiedLinkError for GtkRobot but I
> am unsure why:
>
>   java.lang.UnsatisfiedLinkError: com.sun.glass.ui.gtk.GtkRobot.
> getScreenCapture([I)V
> at 
> javafx.graphics/com.sun.glass.ui.gtk.GtkRobot.getScreenCapture(Native
> Method)
> at javafx.graphics/com.sun.glass.ui.gtk.GtkRobot.getPixelColor(
> GtkRobot.java:93)
>
> I have the following Java method in GtkRobot.java:
>
> @Override native protected void getScreenCapture(int x, int y, int width,
> int height, int[] data);
>
> Which is overriding the abstract method in Robot.java:
>
> protected abstract void getScreenCapture(int x, int y, int width, int
> height, int[] data);
>
> and it is implemented in GlassRobot.cpp thusly:
>
> /*
>  * Class: com_sun_glass_ui_gtk_GtkRobot
>  * Method:getScreenCapture
>  * Signature: ([I)V
>  */
> JNIEXPORT void JNICALL Java_com_sun_glass_ui_gtk_
> GtkRobot_1getScreenCapture
>   (JNIEnv * env, jobject obj, jint x, jint y, jint width, jint height,
> jintArray data)
> {
> (void)obj;
>
> GdkPixbuf *screenshot, *tmp;
> GdkWindow *root_window = gdk_get_default_root_window();
>
> tmp = glass_pixbuf_from_window(root_window, x, y, width, height);
> screenshot = gdk_pixbuf_add_alpha(tmp, FALSE, 0, 0, 0);
> g_object_unref(tmp);
>
> jint *pixels = (jint *)convert_BGRA_to_RGBA((int*)
> gdk_pixbuf_get_pixels(screenshot), width * 4, height);
> env->SetIntArrayRegion(data, 0, height * width, pixels);
> g_free(pixels);
>
> g_object_unref(screenshot);
> }
>
> Have I somehow messed up the signature? As you can see I removed one of
> the "_" prefixes
> to "1getScreenCapture" (which seemed to work for the other Robots) as
> necessary because
> the native method is no longer itself prefixed with "_".
>
> Thanks again.
>
> 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.

Re: Updated OpenJFX build instructions

2017-12-22 Thread Nir Lisker
I did set them, but... if I set them in bash they disappear the next time I
launch it, which made me think they are not being set properly for some
reason even though they appear in `export -p` immediately after. Launching
bash as admin didn't do anything (I thought write permissions issue).

So I went to the /home/user/.bash_profile file and added the export
declarations there. Now they appear on `export -p` properly every launch. I
also did `gradle --stop` and `rm -rf build`, but `gradle clean` gives the
same error.

Otherwise, I also noticed that the folder VS150COMNTOOLS is pointing to
does not exist. I downloaded today VS 2017 Community version 15.5.2 (which
is what I set for MSVC_VER). This is as close as it gets:

---
Nir@Nir-Desktop /cygdrive/c/Program Files (x86)/Microsoft Visual
Studio/2017/Community
$ dir
Common7  Licenses  MSBuild  Team\ Tools  Xml
---

(no VC/Auxiliary/Build)

Is that directory supposed to be created by some process, I needed to
select some installation package for VS, or did something change from
version 14.x to 15.x?

Nir

On Sat, Dec 23, 2017 at 4:14 AM, Kevin Rushforth  wrote:

> Thanks for the feedback...I'll note it.
>
> As for the build failure, did you install Visual Studio 2017 and set the
> two env variables (VS150COMNTOOLS and MSVC_VER) to point to it? If so, then
> it should determine WINSDK_DIR without anything else needed.
>
> -- Kevin
>
>
>
> Nir Lisker wrote:
>
> Thanks Kevin,
>
> I'm going through the process now on Win 10.
>
> A few things to note:
> - In the Platform Prerequisites/cygwin it says to make sure mercurial
> package is installed. Later, under Common Prerequisites/Mercurial it says
> "you can also install Mercurial as a cygwin package". It's not clear then
> if it's needed or optional and if it replaces or complements the other hg
> toolings.
> - The link to Tortoise should be https://tortoisehg.bitbucket.io (not
> .org).
> - Might be trivial but I would say it's worth noting that all commands
> listed there are to be ran in cygwin.
> - The hg clone command probably needs to have a note added about the
> destination folder.
>
> The problems starts after navigating to the /rt directory and executing
> 'gradle tasks' or `gradle projects`:
>
> $ gradle tasks
> :buildSrc:generateGrammarSource
> A problem was found with the configuration of task 
> ':buildSrc:generateGrammarSource'.
> Registering invalid inputs and outputs via TaskInputs and TaskOutputs
> methods has been deprecated and is scheduled to be removed in Gradle 5.0.
>  - Directory 'D:\rt\buildSrc\src\main\antlr' specified for property '$1'
> does not exist.
> :buildSrc:generateGrammarSource UP-TO-DATE
> :buildSrc:compileJava NO-SOURCE
> :buildSrc:compileGroovy UP-TO-DATE
> :buildSrc:processResources NO-SOURCE
> :buildSrc:classes UP-TO-DATE
> :buildSrc:jar UP-TO-DATE
> :buildSrc:assemble UP-TO-DATE
> :buildSrc:compileTestJava NO-SOURCE
> :buildSrc:compileTestGroovy NO-SOURCE
> :buildSrc:processTestResources NO-SOURCE
> :buildSrc:testClasses UP-TO-DATE
> :buildSrc:test NO-SOURCE
> :buildSrc:check UP-TO-DATE
> :buildSrc:build UP-TO-DATE
>
> FAILURE: Build failed with an exception.
>
> * Where:
> Script 'D:\rt\buildSrc\win.gradle' line: 93
>
> * What went wrong:
> A problem occurred evaluating script.
> > FAIL: WINSDK_DIR not defined
>
> * Try:
> Run with --stacktrace option to get the stack trace. Run with --info or
> --debug option to get more log output.
>
> * Get more help at https://help.gradle.org
>
> BUILD FAILED in 2s
>
> ---
>
> I did not install DirectX SDK. Also the output of the above commands in
> the instructions page seems out of date compared to the contents of the
> directory... or I messed up.
>
> Any ideas?
>
>
>
>> I did a first pass over the build instructions on the OpenJFX Wiki [1].
>> I think I cleaned up the worst of the inaccuracies, and added some
>> information that will make it easier to build.
>>
>> I'm not the best person to see whether anything important is missing,
>> though. Someone less familiar with the build should look it over and let
>> me know what I've missed. I won't be able to get back to this until
>> after the holidays and after JDK 10 RDP1 is over, but will take another
>> pass at it then and incorporate the feedback.
>>
>> -- Kevin
>>
>> [1] https://wiki.openjdk.java.net/display/OpenJFX/Building+OpenJFX
>>
>


Re: Updated OpenJFX build instructions

2017-12-22 Thread Kevin Rushforth

Thanks for the feedback...I'll note it.

As for the build failure, did you install Visual Studio 2017 and set the 
two env variables (VS150COMNTOOLS and MSVC_VER) to point to it? If so, 
then it should determine WINSDK_DIR without anything else needed.


-- Kevin


Nir Lisker wrote:

Thanks Kevin,

I'm going through the process now on Win 10.

A few things to note:
- In the Platform Prerequisites/cygwin it says to make sure mercurial 
package is installed. Later, under Common Prerequisites/Mercurial it 
says "you can also install Mercurial as a cygwin package". It's not 
clear then if it's needed or optional and if it replaces or 
complements the other hg toolings.
- The link to Tortoise should be https://tortoisehg.bitbucket.io (not 
.org).
- Might be trivial but I would say it's worth noting that all commands 
listed there are to be ran in cygwin.
- The hg clone command probably needs to have a note added about the 
destination folder.


The problems starts after navigating to the /rt directory and 
executing 'gradle tasks' or `gradle projects`:


$ gradle tasks
:buildSrc:generateGrammarSource
A problem was found with the configuration of task 
':buildSrc:generateGrammarSource'. Registering invalid inputs and 
outputs via TaskInputs and TaskOutputs methods has been deprecated and 
is scheduled to be removed in Gradle 5.0.
 - Directory 'D:\rt\buildSrc\src\main\antlr' specified for property 
'$1' does not exist.

:buildSrc:generateGrammarSource UP-TO-DATE
:buildSrc:compileJava NO-SOURCE
:buildSrc:compileGroovy UP-TO-DATE
:buildSrc:processResources NO-SOURCE
:buildSrc:classes UP-TO-DATE
:buildSrc:jar UP-TO-DATE
:buildSrc:assemble UP-TO-DATE
:buildSrc:compileTestJava NO-SOURCE
:buildSrc:compileTestGroovy NO-SOURCE
:buildSrc:processTestResources NO-SOURCE
:buildSrc:testClasses UP-TO-DATE
:buildSrc:test NO-SOURCE
:buildSrc:check UP-TO-DATE
:buildSrc:build UP-TO-DATE

FAILURE: Build failed with an exception.

* Where:
Script 'D:\rt\buildSrc\win.gradle' line: 93

* What went wrong:
A problem occurred evaluating script.
> FAIL: WINSDK_DIR not defined

* Try:
Run with --stacktrace option to get the stack trace. Run with --info 
or --debug option to get more log output.


* Get more help at https://help.gradle.org

BUILD FAILED in 2s

---

I did not install DirectX SDK. Also the output of the above commands 
in the instructions page seems out of date compared to the contents of 
the directory... or I messed up.


Any ideas?

 


I did a first pass over the build instructions on the OpenJFX Wiki
[1].
I think I cleaned up the worst of the inaccuracies, and added some
information that will make it easier to build.

I'm not the best person to see whether anything important is missing,
though. Someone less familiar with the build should look it over
and let
me know what I've missed. I won't be able to get back to this until
after the holidays and after JDK 10 RDP1 is over, but will take
another
pass at it then and incorporate the feedback.

-- Kevin

[1] https://wiki.openjdk.java.net/display/OpenJFX/Building+OpenJFX




Re: Updated OpenJFX build instructions

2017-12-22 Thread Nir Lisker
Thanks Kevin,

I'm going through the process now on Win 10.

A few things to note:
- In the Platform Prerequisites/cygwin it says to make sure mercurial
package is installed. Later, under Common Prerequisites/Mercurial it says
"you can also install Mercurial as a cygwin package". It's not clear then
if it's needed or optional and if it replaces or complements the other hg
toolings.
- The link to Tortoise should be https://tortoisehg.bitbucket.io (not .org).
- Might be trivial but I would say it's worth noting that all commands
listed there are to be ran in cygwin.
- The hg clone command probably needs to have a note added about the
destination folder.

The problems starts after navigating to the /rt directory and executing
'gradle tasks' or `gradle projects`:

$ gradle tasks
:buildSrc:generateGrammarSource
A problem was found with the configuration of task
':buildSrc:generateGrammarSource'. Registering invalid inputs and outputs
via TaskInputs and TaskOutputs methods has been deprecated and is scheduled
to be removed in Gradle 5.0.
 - Directory 'D:\rt\buildSrc\src\main\antlr' specified for property '$1'
does not exist.
:buildSrc:generateGrammarSource UP-TO-DATE
:buildSrc:compileJava NO-SOURCE
:buildSrc:compileGroovy UP-TO-DATE
:buildSrc:processResources NO-SOURCE
:buildSrc:classes UP-TO-DATE
:buildSrc:jar UP-TO-DATE
:buildSrc:assemble UP-TO-DATE
:buildSrc:compileTestJava NO-SOURCE
:buildSrc:compileTestGroovy NO-SOURCE
:buildSrc:processTestResources NO-SOURCE
:buildSrc:testClasses UP-TO-DATE
:buildSrc:test NO-SOURCE
:buildSrc:check UP-TO-DATE
:buildSrc:build UP-TO-DATE

FAILURE: Build failed with an exception.

* Where:
Script 'D:\rt\buildSrc\win.gradle' line: 93

* What went wrong:
A problem occurred evaluating script.
> FAIL: WINSDK_DIR not defined

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or
--debug option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 2s

---

I did not install DirectX SDK. Also the output of the above commands in the
instructions page seems out of date compared to the contents of the
directory... or I messed up.

Any ideas?



> I did a first pass over the build instructions on the OpenJFX Wiki [1].
> I think I cleaned up the worst of the inaccuracies, and added some
> information that will make it easier to build.
>
> I'm not the best person to see whether anything important is missing,
> though. Someone less familiar with the build should look it over and let
> me know what I've missed. I won't be able to get back to this until
> after the holidays and after JDK 10 RDP1 is over, but will take another
> pass at it then and incorporate the feedback.
>
> -- Kevin
>
> [1] https://wiki.openjdk.java.net/display/OpenJFX/Building+OpenJFX
>


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

2017-12-22 Thread Michael Ennen
I have made quite a bit of good progress I think:

https://github.com/brcolow/openjfx/commit/d92ef24101cf32dfc07b21bdcb575528d8a58eaf

The public API is starting to look much cleaner:

https://github.com/brcolow/openjfx/blob/master/modules/javafx.graphics/src/main/java/javafx/scene/robot/Robot.java

Still a lot to do.

Currently I am running into a JNI UnsatisfiedLinkError for GtkRobot but I
am unsure why:

  java.lang.UnsatisfiedLinkError:
com.sun.glass.ui.gtk.GtkRobot.getScreenCapture([I)V
at
javafx.graphics/com.sun.glass.ui.gtk.GtkRobot.getScreenCapture(Native
Method)
at
javafx.graphics/com.sun.glass.ui.gtk.GtkRobot.getPixelColor(GtkRobot.java:93)

I have the following Java method in GtkRobot.java:

@Override native protected void getScreenCapture(int x, int y, int width,
int height, int[] data);

Which is overriding the abstract method in Robot.java:

protected abstract void getScreenCapture(int x, int y, int width, int
height, int[] data);

and it is implemented in GlassRobot.cpp thusly:

/*
 * Class: com_sun_glass_ui_gtk_GtkRobot
 * Method:getScreenCapture
 * Signature: ([I)V
 */
JNIEXPORT void JNICALL Java_com_sun_glass_ui_gtk_GtkRobot_1getScreenCapture
  (JNIEnv * env, jobject obj, jint x, jint y, jint width, jint height,
jintArray data)
{
(void)obj;

GdkPixbuf *screenshot, *tmp;
GdkWindow *root_window = gdk_get_default_root_window();

tmp = glass_pixbuf_from_window(root_window, x, y, width, height);
screenshot = gdk_pixbuf_add_alpha(tmp, FALSE, 0, 0, 0);
g_object_unref(tmp);

jint *pixels = (jint
*)convert_BGRA_to_RGBA((int*)gdk_pixbuf_get_pixels(screenshot), width * 4,
height);
env->SetIntArrayRegion(data, 0, height * width, pixels);
g_free(pixels);

g_object_unref(screenshot);
}

Have I somehow messed up the signature? As you can see I removed one of the
"_" prefixes
to "1getScreenCapture" (which seemed to work for the other Robots) as
necessary because
the native method is no longer itself prefixed with "_".

Thanks again.

On Wed, Dec 20, 2017 at 3:05 PM, Kevin Rushforth  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

Re: Updated OpenJFX build instructions

2017-12-22 Thread Michael Ennen
Thanks for taking the time to do this, Kevin.

On Fri, Dec 22, 2017 at 11:13 AM, Kevin Rushforth <
kevin.rushfo...@oracle.com> wrote:

> [resend with typo fixed in Subject line]
>
> I did a first pass over the build instructions on the OpenJFX Wiki [1]. I
> think I cleaned up the worst of the inaccuracies, and added some
> information that will make it easier to build.
>
> I'm not the best person to see whether anything important is missing,
> though. Someone less familiar with the build should look it over and let me
> know what I've missed. I won't be able to get back to this until after the
> holidays and after JDK 10 RDP1 is over, but will take another pass at it
> then and incorporate the feedback.
>
> -- Kevin
>
> [1] https://wiki.openjdk.java.net/display/OpenJFX/Building+OpenJFX
>
>


-- 
Michael Ennen


Updated OpenJFX build instructions

2017-12-22 Thread Kevin Rushforth

[resend with typo fixed in Subject line]

I did a first pass over the build instructions on the OpenJFX Wiki [1]. 
I think I cleaned up the worst of the inaccuracies, and added some 
information that will make it easier to build.


I'm not the best person to see whether anything important is missing, 
though. Someone less familiar with the build should look it over and let 
me know what I've missed. I won't be able to get back to this until 
after the holidays and after JDK 10 RDP1 is over, but will take another 
pass at it then and incorporate the feedback.


-- Kevin

[1] https://wiki.openjdk.java.net/display/OpenJFX/Building+OpenJFX



Updated OpenJFX buld instructions

2017-12-22 Thread Kevin Rushforth
I did a first pass over the build instructions on the OpenJFX Wiki [1]. 
I think I cleaned up the worst of the inaccuracies, and added some 
information that will make it easier to build.


I'm not the best person to see whether anything important is missing, 
though. Someone less familiar with the build should look it over and let 
me know what I've missed. I won't be able to get back to this until 
after the holidays and after JDK 10 RDP1 is over, but will take another 
pass at it then and incorporate the feedback.


-- Kevin

[1] https://wiki.openjdk.java.net/display/OpenJFX/Building+OpenJFX



RFR : JDK-8192056 : Memory leak when removing javafx.scene.shape.Sphere-objects from a group or container

2017-12-22 Thread Ambarish Rapte
Hi Kevin & Ajit,

 

Request you to review this fix

Issue: https://bugs.openjdk.java.net/browse/JDK-8192056

Webrev: http://cr.openjdk.java.net/~arapte/fx/8192056/webrev.02

 

Regards,

Ambarish

 


[11] Review Request: JDK-8190411 NPE in SliderSkin:140 if Slider.Tooltip.autohide is true

2017-12-22 Thread Prem Balakrishnan
Hi Kevin, Ajit

 

Request you to review following fix:

 

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

 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Epkbalakr/fx/8190411/webrev.00/"http://cr.openjdk.java.net/~pkbalakr/fx/8190411/webrev.00/
 

 

Regards,

Prem