Proposal For Inclusion of Robot and ParametersImpl in the Public API

Kevin Rushforth kevin.rushforth at oracle.com
Mon Jan 8 23:28:31 UTC 2018


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.rushforth at oracle.com <mailto: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 <mailto: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
>>         <mailto: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/javafx/scene/robot/Robot.java
>>>             <https://github.com/brcolow/openjfx/blob/master/modules/javafx.graphics/src/main/java/javafx/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
>>>             <mailto: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
>>>>                 <mailto: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
>>>>                     <mailto: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> <mailto: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> <mailto: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 <https://gist.github.com/brcolow/26370db6cab0355186d4a1d13b30fc19>
>>>>>>>
>>>>>>>                         All suggested changes can be found by using Github Compare View:
>>>>>>>
>>>>>>>                         https://github.com/brcolow/openjfx/compare/4ccdbbbce5234e2c5 <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 <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/ <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.Pa <http://com.sun.javafx.application.Pa>rametersImpl](http://hg.openjd
>>>>>>>                         k.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 <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 <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


More information about the openjfx-dev mailing list