Proposal For Inclusion of Robot and ParametersImpl in the Public API

[Michael Ennen]

I have completed what I would call a first draft of moving the Robot to
JavaFX
public API.

This commit:
https://github.com/brcolow/openjfx/commit/321f949575a201fe28a2ff79c9590daf8db375a8

Includes all the relevant changes necessary to move the glass robot into
the public
API.

I have some other commits on that repository, but they are unrelated and not
part of the Robot proposal at all.

Except for the most recent commit:

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

which explores adding support for the back/forward mouse buttons
(JDK-8090930),
also known as mouse buttons 4 and 5 to JavaFX. This is essentially
independent from
the Robot proposal except if there is interested in this and it (or
something like it,
or based on it), is merged then I can easily adapt the native robot
implementations
(GtkRobot, MonocleRobot, MacRobot, IOsRobot, and WinRobot) to support
these additional mouse keys.

Happy holidays all,
Michael Ennen

On Sat, Dec 23, 2017 at 6:00 PM, Michael Ennen <mike.ennen at gmail.com> wrote:

> Kevin,
>
> You mentioned in your initial response that Jemmy would need to updated
> with any changes
> that result from moving Robot into the JavaFX public API. However, from
> what I can tell
> from a cursory glance, Jemmy seems to use "java.awt.Robot" and not the
> JavaFX Glass
> Robot API and thus no changes to Jemmy would be required after moving
> Robot to the
> JavaFX public API.
>
> Am I missing something?
>
> Thanks, happy holidays (not expecting a response for a while :) ).
>
>
>
> On Fri, Dec 22, 2017 at 10:51 PM, Michael Ennen <mike.ennen at gmail.com>
> wrote:
>
>> 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 <mike.ennen at gmail.com>
>> wrote:
>>
>>> I have made quite a bit of good progress I think:
>>>
>>> https://github.com/brcolow/openjfx/commit/d92ef24101cf32dfc0
>>> 7b21bdcb575528d8a58eaf
>>>
>>> The public API is starting to look much cleaner:
>>>
>>> https://github.com/brcolow/openjfx/blob/master/modules/javaf
>>> x.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(IIII[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: (IIII[I)V
>>>  */
>>> JNIEXPORT void JNICALL Java_com_sun_glass_ui_gtk_GtkR
>>> obot_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*)g
>>> dk_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.rushforth at 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 <mike.ennen at gmail.com>
>>>> 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.rushforth at 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.rushforth at 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 <mike.ennen at gmail.com>
>>>>>>> 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.rushforth at 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 new
>>>>>>>>> "javafx.scene.robot" module:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Task :systemTests:compileTestJava
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> C:\Users\brcolow\dev\openjfx\tests\system\src\test\java\test\robot\com\sun\glass\ui\monocle\ModalDialogTest.java:34:
>>>>>>>>> error: package javafx.scene.robot is not visible
>>>>>>>>> import javafx.scene.robot.Robot;
>>>>>>>>>                    ^
>>>>>>>>>   (package javafx.scene.robot is declared in module javafx.graphics, which
>>>>>>>>> does not export it)
>>>>>>>>> C:\Users\brcolow\dev\openjfx\tests\system\src\test\java\test\robot\com\sun\glass\ui\monocle\RobotTest.java:33:
>>>>>>>>> error: package javafx.scene.robot is not visible
>>>>>>>>> import javafx.scene.robot.Robot;
>>>>>>>>>
>>>>>>>>> I have added:
>>>>>>>>>
>>>>>>>>>     exports javafx.scene.robot;
>>>>>>>>>
>>>>>>>>> to: modules/javafx.graphics/src/main/java/module-info.java
>>>>>>>>>
>>>>>>>>> But this does not seem to be enough.
>>>>>>>>>
>>>>>>>>> On Sun, Dec 3, 2017 at 4:48 PM, Michael Ennen <mike.ennen at gmail.com> <mike.ennen at gmail.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I am still working on all the necessary changes to actually allow openjfx
>>>>>>>>> to compile.
>>>>>>>>> Tons to learn in that arena and I know the code as it is written won't
>>>>>>>>> totally work.
>>>>>>>>> For example one can no longer:
>>>>>>>>>
>>>>>>>>> #include "com_sun_glass_ui_Robot.h"
>>>>>>>>>
>>>>>>>>> as in openjfx\modules\javafx.graphics\src\main\native-glass\win\Robot.cpp
>>>>>>>>>
>>>>>>>>> But I am not sure how those headers are generated and if I can just simply
>>>>>>>>> change
>>>>>>>>> it to "#include javafx_scene_robot_Robot.h" (which I very much doubt).
>>>>>>>>>
>>>>>>>>> On Sun, Dec 3, 2017 at 2:29 PM, Michael Ennen <mike.ennen at gmail.com> <mike.ennen at gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I have created a (small) proposal (building on the work of Benjamin
>>>>>>>>> Gudehaus) about moving some classes in to the public API so that TestFX (a
>>>>>>>>> JavaFX UI testing framework) can continue to work with future JDK releases.
>>>>>>>>> The somewhat nicely formatted proposal can be found as a Github gist:
>>>>>>>>> https://gist.github.com/brcolow/26370db6cab0355186d4a1d13b30fc19
>>>>>>>>>
>>>>>>>>> All suggested changes can be found by using Github Compare View:
>>>>>>>>> https://github.com/brcolow/openjfx/compare/4ccdbbbce5234e2c5
>>>>>>>>> e1f4f1cb8f20430feaa53b6...master
>>>>>>>>>
>>>>>>>>> But I have copied it to this email for convenience:
>>>>>>>>>
>>>>>>>>> ----------------------- PROPOSAL -----------------------
>>>>>>>>>
>>>>>>>>> TestFX, the JavaFX GUI testing framework currently requires 4 (four)
>>>>>>>>> classes that are part of the JDK's private API. They are:
>>>>>>>>>
>>>>>>>>> [com.sun.glass.ui.Application](http://hg.openjdk.java.net/op
>>>>>>>>> enjfx/10-dev/rt/file/tip/modules/javafx.graphics/src/main/
>>>>>>>>> java/com/sun/glass/ui/Application.java)
>>>>>>>>> [com.sun.glass.ui.Pixels](http://hg.openjdk.java.net/openjfx
>>>>>>>>> /10-dev/rt/file/tip/modules/javafx.graphics/src/main/java/
>>>>>>>>> com/sun/glass/ui/Pixels.java)
>>>>>>>>> [com.sun.glass.ui.Robot](http://hg.openjdk.java.net/openjfx/
>>>>>>>>> 10-dev/rt/file/tip/modules/javafx.graphics/src/main/java/com
>>>>>>>>> /sun/glass/ui/Robot.java)
>>>>>>>>> [com.sun.javafx.application.ParametersImpl](http://hg.openjdk.java.net/openjfx/10-dev/rt/file/tip/modules/javafx.
>>>>>>>>> graphics/src/main/java/com/sun/javafx/application/ParametersImpl.java <http://k.java.net/openjfx/10-dev/rt/file/tip/modules/javafx.graphics/src/main/java/com/sun/javafx/application/ParametersImpl.java>)
>>>>>>>>>
>>>>>>>>> In order to compile the project with Java 9, we use the following flags:
>>>>>>>>>
>>>>>>>>> ```sh
>>>>>>>>> --add-exports javafx.graphics/com.sun.glass.ui=org.testfx
>>>>>>>>> --add-exports javafx.graphics/com.sun.javafx.application=org.testfx
>>>>>>>>> ```
>>>>>>>>>
>>>>>>>>> If the --add-exports flags are disabled in a future Java release TestFX
>>>>>>>>> will require these four classes to be moved into the public API to
>>>>>>>>> continue working.
>>>>>>>>>
>>>>>>>>> While these classes are probably not very useful for applications to use
>>>>>>>>> directly, any JavaFX application wanting to write UI tests will most
>>>>>>>>> likely
>>>>>>>>> use TestFX and thus they will indirectly be using these classes.
>>>>>>>>>
>>>>>>>>> JavaFX internal tests also use these classes for essentially the same
>>>>>>>>> purpose (UI tests).
>>>>>>>>>
>>>>>>>>> ### Details of Usage For Each Private API Class
>>>>>>>>>
>>>>>>>>> #### com.sun.javafx.application.ParametersImpl
>>>>>>>>>
>>>>>>>>> ##### TestFX Usage
>>>>>>>>>
>>>>>>>>> ```java
>>>>>>>>> ParametersImpl parameters = new ParametersImpl(applicationArgs);
>>>>>>>>> ParametersImpl.registerParameters(application, parameters);
>>>>>>>>> ```
>>>>>>>>>
>>>>>>>>> The parameters are set on a constructed Application.
>>>>>>>>>
>>>>>>>>> ##### Suggested Public API Replacement
>>>>>>>>>
>>>>>>>>> `javafx.application.Application`:
>>>>>>>>>
>>>>>>>>> ```java
>>>>>>>>> /**
>>>>>>>>>  * Sets the parameters for this Application.
>>>>>>>>>  *
>>>>>>>>>  * <p>
>>>>>>>>>  * NOTE: this method should not be called from the Application
>>>>>>>>> constructor,
>>>>>>>>>  * as it will return null. It may be called in the init() method or any
>>>>>>>>>  * time after that.
>>>>>>>>>  * </p>
>>>>>>>>>  *
>>>>>>>>>  * @param parameters the parameters to set for this Application
>>>>>>>>>  */
>>>>>>>>> public final Parameters setParameters(String... parameters) {
>>>>>>>>>     ParametersImpl parameters = new ParametersImpl(parameters);
>>>>>>>>>     ParametersImpl.registerParameters(this, parameters);
>>>>>>>>> }
>>>>>>>>> ```
>>>>>>>>>
>>>>>>>>> #### com.sun.glass.ui.Application
>>>>>>>>>
>>>>>>>>> ##### TestFX Usage
>>>>>>>>>
>>>>>>>>> ```java
>>>>>>>>> return Application.GetApplication().createRobot();
>>>>>>>>> ```
>>>>>>>>>
>>>>>>>>> The Application class is used to instantiate a Robot.
>>>>>>>>>
>>>>>>>>> ##### Suggested Public API Replacement
>>>>>>>>>
>>>>>>>>> `javafx.application.Application`:
>>>>>>>>> https://github.com/brcolow/openjfx/blob/master/modules/javaf
>>>>>>>>> x.graphics/src/main/java/javafx/application/Application.java#L527
>>>>>>>>>
>>>>>>>>> #### com.sun.glass.ui.Pixels
>>>>>>>>>
>>>>>>>>> ##### TestFX Usage
>>>>>>>>>
>>>>>>>>> ```java
>>>>>>>>> @Override
>>>>>>>>> public Image getCaptureRegion(Rectangle2D region) {
>>>>>>>>>     return waitForAsyncFx(RETRIEVAL_TIMEOUT_IN_MILLIS, () -> {
>>>>>>>>>         Pixels glassPixels = useRobot().getScreenCapture(
>>>>>>>>>             (int) region.getMinX(), (int) region.getMinY(),
>>>>>>>>>             (int) region.getWidth(), (int) region.getHeight()
>>>>>>>>>         );
>>>>>>>>>         return convertFromGlassPixels(glassPixels);
>>>>>>>>>     });
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> private Image convertFromGlassPixels(Pixels glassPixels) {
>>>>>>>>>     int width = glassPixels.getWidth();
>>>>>>>>>     int height = glassPixels.getHeight();
>>>>>>>>>     WritableImage image = new WritableImage(width, height);
>>>>>>>>>
>>>>>>>>>     int bytesPerComponent = glassPixels.getBytesPerComponent();
>>>>>>>>>     if (bytesPerComponent == INT_BUFFER_BYTES_PER_COMPONENT) {
>>>>>>>>>         IntBuffer intBuffer = (IntBuffer) glassPixels.getPixels();
>>>>>>>>>         writeIntBufferToImage(intBuffer, image);
>>>>>>>>>     }
>>>>>>>>>
>>>>>>>>>     return image;
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> private void writeIntBufferToImage(IntBuffer intBuffer,
>>>>>>>>>                                    WritableImage image) {
>>>>>>>>>     PixelWriter pixelWriter = image.getPixelWriter();
>>>>>>>>>     double width = image.getWidth();
>>>>>>>>>     double height = image.getHeight();
>>>>>>>>>
>>>>>>>>>     for (int y = 0; y < height; y++) {
>>>>>>>>>         for (int x = 0; x < width; x++) {
>>>>>>>>>             int argb = intBuffer.get();
>>>>>>>>>             pixelWriter.setArgb(x, y, argb);
>>>>>>>>>         }
>>>>>>>>>     }
>>>>>>>>> }
>>>>>>>>> ```
>>>>>>>>>
>>>>>>>>> Pixels is used to create a screen capture.
>>>>>>>>>
>>>>>>>>> ##### Suggested Public API Replacement
>>>>>>>>>
>>>>>>>>> Bypass needing to expose the Pixels class to the public API by
>>>>>>>>> changing the getScreenCapture method of Robot - that is, changing:
>>>>>>>>>
>>>>>>>>> `public Pixels getScreenCapture(int x, int y, int width, int height)`
>>>>>>>>>
>>>>>>>>> to:
>>>>>>>>>
>>>>>>>>> `public Image getScreenCapture(int x, int y, int width, int height)`
>>>>>>>>>
>>>>>>>>> #### com.sun.glass.ui.Robot
>>>>>>>>>
>>>>>>>>> ##### TestFX Usage
>>>>>>>>>
>>>>>>>>> Essentially every method of Robot is used:
>>>>>>>>>
>>>>>>>>> ```
>>>>>>>>> public void keyPress(int code)
>>>>>>>>> public void keyRelease(int code)
>>>>>>>>> public int getMouseX()
>>>>>>>>> public int getMouseY()
>>>>>>>>> public void mouseMove(int x, int y)
>>>>>>>>> public void mousePress(int buttons)
>>>>>>>>> public void mouseRelease(int buttons)
>>>>>>>>> public void mouseWheel(int wheelAmt)
>>>>>>>>> public int getPixelColor(int x, int y)
>>>>>>>>> public Pixels getScreenCapture(int x, int y, int width, int height)
>>>>>>>>> ```
>>>>>>>>>
>>>>>>>>> ##### Suggested Public API Replacement
>>>>>>>>> https://github.com/brcolow/openjfx/blob/master/modules/javaf
>>>>>>>>> x.graphics/src/main/java/javafx/scene/robot/Robot.java
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Michael Ennen
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Michael Ennen
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Michael Ennen
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Michael Ennen
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Michael Ennen
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Michael Ennen
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Michael Ennen
>>>>
>>>>
>>>
>>>
>>> --
>>> Michael Ennen
>>>
>>
>>
>>
>> --
>> Michael Ennen
>>
>
>
>
> --
> Michael Ennen
>