Michael Ennen 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();`
I just meant that the Toolkit class was a convenient place to call
com.sun.glass.ui.Application.GetApplication().createRobot() -- something
like this:
Toolkit:
public GlassRobot createRobot() { // or could be abstract
throw new UnsupportedOperationException("not implemented");
}
QuantumToolkit:
public GlassRobot createRobot() {
return com.sun.glass.ui.Application.GetApplication().createRobot();
}
The reason for doing it there is because it already has the mechanism
for handling QuantumToolkit versus StubToolkit. Otherwise you wouldn't
need the extra level of indirection.
-- Kevin
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 <mailto: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 <mailto: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
<mailto: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 <mailto: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
<mailto: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/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.rushfo...@oracle.com
<mailto: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
<mailto: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
<mailto: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> <mailto: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> <mailto: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
<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
--
Michael Ennen
--
Michael Ennen
