Re: Proposal For Inclusion of Robot and ParametersImpl in the Public API

[Kevin Rushforth]

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 
> 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
>
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

[11] Review request: 8198654: Switch FX's default GTK version to 3

[Kevin Rushforth]

Phil,

Please review the FX change to use GTK 3 by default, as you recently did 
for AWT:


https://bugs.openjdk.java.net/browse/JDK-8198654
http://cr.openjdk.java.net/~kcr/8198654/webrev.00/

I have run a full test, including all robot tests, on Ubuntu 16.04 using 
two different JDKs -- one with your AWT fix and one without. No new 
issues were discovered.


Thanks.

-- Kevin

Re: Proposal For Inclusion of Robot and ParametersImpl in the Public API

[Kevin Rushforth]

I will do an initial review of the API today and suggest next steps.

-- Kevin


Michael Ennen wrote:

Ping :)

On Mon, Jan 8, 2018 at 4:28 PM, Kevin Rushforth 
> 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
>
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
> 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
> 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



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
> 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
> 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