Sounds great. Much appreciated. 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