I should have been more clear in my previous message. I am creating the Robot thusly:
javafx.scene.robot.Robot constructor (public API so JavaFX users can simply instantiate one): public Robot() { // Ensure we have proper permission for creating a robot. final SecurityManager sm = System.getSecurityManager(); if (sm != null) { sm.checkPermission(CREATE_ROBOT_PERMISSION); } peer = GlassRobot.getRobot(); } static method com.sun.glass.ui.GlassRobot.getRobot(): public static GlassRobot getRobot() { return Application.GetApplication().createRobot(); } This seems straight-forward. I am not sure how to change this to Toolkit, because the individual application implementations (GtkApplication, WinApplication, etc.) all instantiate their correct, platform-specific Robot instance. On Wed, Mar 21, 2018 at 11:58 PM, Michael Ennen <mike.en...@gmail.com> wrote: > Quick question: > > Currently a Robot is created by calling `Application.createRobot` which > delegates > to the underlying platform-specific application class (GtkApplication, > WinApplication, etc.) > via `com.sun.glass.ui.Application.GetApplication().createRobot();` > > > You suggest moving this to the Toolkit class? If GtkRobot, WinRobot, etc. > all extend > the abstract base class GlassRobot (the peer), how will Toolkit be able to > instantiate > the correct, platform-specific class? Sorry, I got stuck trying to > implement this part > of your review. The rest is easy for me to follow and will be no problem. > > > On Tue, Mar 20, 2018 at 4:28 PM, Kevin Rushforth < > kevin.rushfo...@oracle.com> wrote: > >> Hi Michael, >> >> Here is some quick feedback. >> >> I think what you have is heading in the right direction as far as the >> public API goes. I'd like to get some feedback from other developers as >> well. I would want to make sure that the API meets the needs of multiple >> developers. >> >> I took a look at the public class and as I may have mentioned earlier, it >> will help to split the API and the implementation even further, by creating >> a peer object as we do for Scene, Stage, etc., rather than having the glass >> platform implementation directly subclass the public Robot class. >> >> Then you can easily delegate to the Glass Robot peer without having any >> implementation leak into the public API (e.g., the overridden create and >> dispose methods). >> >> So what you would have in that case is something more like this: >> >> public final class Robot { >> >> private final GlassRobot peer; >> >> public Robot() { >> // Ensure we have proper permission for creating a robot. >> final SecurityManager sm = System.getSecurityManager(); >> if (sm != null) { >> sm.checkPermission(CREATE_ROBOT_PERMISSION); >> } >> >> Application.checkEventThread(); >> >> peer = Toolkit.createRobot(); >> } >> >> // NOTE: for the rest, the peer can do the thread check >> >> public void keyPress(KeyCode keyCode) { >> peer.keyPress(keyCode); >> } >> >> public void keyRelease(KeyCode keyCode) { >> peer.keyRelease(keyCode); >> } >> >> public final void keyType(KeyCode keyCode) { >> keyPress(keyCode); >> keyRelease(keyCode); >> } >> >> ... >> >> >> And so on. The Toolkit class can return a glass robot "peer" which should >> then only need minor changes (e.g., to the signatures of methods that have >> slightly different types) from the current one. The QuantumToolkit >> implementation of createPeer can construct, initialize, and return the >> GlassRobot instance. The StubToolkit and DummyToolkit implementations can >> throw an UnsuppportedOperationException. >> >> As for the public API, the set of methods you have seem mostly what we >> would want. Here are a few quick thoughts: >> >> void keyPress(KeyCode keyCode); >> void keyRelease(KeyCode keyCode); >> void keyType(KeyCode keyCode); >> >> int getMouseX(); // maybe should return double? >> int getMouseY(); >> >> Point2D getMousePosition(); >> >> void mouseMove(int x, int y); // maybe params should be double? >> >> void mouseMove(Point2D location); >> >> void mousePress(MouseButton button); // This one seems >> redundant...covered by the next method >> void mousePress(MouseButton... buttons); >> >> void mouseRelease(MouseButton button); // This one seems >> redundant...covered by the next method >> void mouseRelease(MouseButton... buttons); >> >> // I don't see the need for this method >> void mouseWheel(int wheelAmt, VerticalDirection direction); >> >> void mouseWheel(int wheelAmt); >> >> Color getPixelColor(int x, int y); // maybe params should be double? >> >> Color getPixelColor(Point2D location); >> >> // Probably the following should return WritableImage to match Snapshot. >> maybe also have a variable that takes a WritableImage to allow caller to >> allocate and reuse (that might overkill in this case)? >> >> Image getScreenCapture(int x, int y, int width, int height); // maybe >> params should be double? >> Image getScreenCapture(int x, int y, int width, int height, boolean >> scaleToFit); // maybe params should be double? >> Image getScreenCapture(Rectangle2D region); >> Image getScreenCapture(Rectangle2D region, boolean scaleToFit); >> >> void getScreenCapture(int x, int y, int width, int height, int[] data); >> // Not sure we need / want this one >> >> The biggest question I have is whether we should follow the pattern of >> the other related public APIs in Screen and Stage and use doubles >> everywhere rather than ints. Also, we will need to see whether there are >> any implications for multiple screens (probably not, but the docs will need >> to specify what coordinates the x, and y values are in). >> >> Hopefully this will spark a discussion. >> >> >> -- Kevin >> >> >> Michael Ennen wrote: >> >> Ping :) >> >> On Mon, Jan 8, 2018 at 4:28 PM, Kevin Rushforth < >> kevin.rushfo...@oracle.com> 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 <mike.en...@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.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 <mike.en...@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.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 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.en...@gmail.com> >>>>>>>>> <mike.en...@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.en...@gmail.com> >>>>>>>>> <mike.en...@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 > -- Michael Ennen