On 3/21/14, Mar 21, 12:53 PM, David Hill wrote:
Having heard a little feedback, here is my proposal in the form of a
review request :-)
My recommendation would be modify the JavaDoc contract to specify
that the x,y and x+width, y+height must be within the screen
bounds, with an IllegalArgumentException if out of bounds. This
would be checked in Robot, prior to calling the native impls.
This code is "internal" API, so I expect the real impact would be
on SQE.
I really like the idea of adding a bounds restriction - so that the
requested bounds must be within the Screen.
It seems the simplest solution to my issue of handling the odd edge
case of out of bound pixels, with the least likely impact.
This means that existing code in the implementations are not affected.
I suspect that there will we little if any impact on SQE tests,
given that most of us would avoid asking for a screen capture with
undefined pixels. I do expect that we will encounter a few
exceptions to this when tests are run on smaller displays (like
embedded).
I also added bounds checking to Robot.getPixelColor() for
consistency, and because Embedded passes this call through to common
code for screen capture.
I did a grep through the JavaFX code base, and don't see any JavaFX
use cases. I expect any "golden image" test code could be affected.
I separated out this internal API changes from my embedded changes
so we have a clear and easy thing to review.
Jira: https://javafx-jira.kenai.com/browse/RT-36382
Patch: is inline in the Jira, but also here:
diff -r bb72bd2fa889
modules/graphics/src/main/java/com/sun/glass/ui/Robot.java
--- a/modules/graphics/src/main/java/com/sun/glass/ui/Robot.java Tue
Mar 25 14:21:26 2014 -0400
+++ b/modules/graphics/src/main/java/com/sun/glass/ui/Robot.java Tue
Mar 25 14:41:37 2014 -0400
@@ -144,9 +144,20 @@
protected abstract int _getPixelColor(int x, int y);
/**
* Returns pixel color at specified screen coordinates in
IntARGB format.
+ *
+ * If the requested pixel is not contained by the actual Screen
+ * bounds an IllegalArgumentException will be thrown.
+ *
+ * @param x The screen X of the requested capture (must be >=0)
+ * @param y The screen Y of the requested capture (must be >=0)
*/
public int getPixelColor(int x, int y) {
Application.checkEventThread();
+ Screen s = Screen.getMainScreen();
+ if (x < 0 || y < 0 ||
+ x >= s.getWidth() || y > s.getHeight()) {
+ throw new IllegalArgumentException("Capture out of
bounds");
+ }
return _getPixelColor(x, y);
}
@@ -162,13 +173,27 @@
* will result in a Pixels object with dimensions (20x20).
Calling code
* should use the returned objects's getWidth() and getHeight()
methods
* to determine the image size.
- *
+ *
* If (@code isHiDPI) is {@code false}, the returned Pixels
object is of
* the requested size. Note that in this case the image may be
scaled in
* order to fit to the requested dimensions if running on a
HiDPI display.
+ *
+ * If the requested capture bounds is not contained by the
actual Screen
+ * bounds an IllegalArgumentException will be thrown.
+ *
+ * @param x The screen X of the requested capture (must be >=0)
+ * @param y The screen Y of the requested capture (must be >=0)
+ * @param width The of width the requested capture (must be >=1
and fit on the screen)
+ * @param height The of width the requested capture (must be
>=1 and fit on the screen)
+ * @param isHiDPI return HiDPI if available
*/
public Pixels getScreenCapture(int x, int y, int width, int
height, boolean isHiDPI) {
Application.checkEventThread();
+ Screen s = Screen.getMainScreen();
+ if (x < 0 || y < 0 ||
+ x + width > s.getWidth() || y + height > s.getHeight()) {
+ throw new IllegalArgumentException("Capture out of
bounds");
+ }
return _getScreenCapture(x, y, width, height, isHiDPI);
}
@@ -176,6 +201,14 @@
* Returns a capture of the specified area of the screen.
* It is equivalent to calling getScreenCapture(x, y, width,
height, false),
* i.e. this method takes a "LowDPI" screen shot.
+ *
+ * If the requested capture bounds is not contained by the
actual Screen
+ * bounds an IllegalArgumentException will be thrown.
+ *
+ * @param x The screen X of the requested capture (must be >=0)
+ * @param y The screen Y of the requested capture (must be >=0)
+ * @param width The of width the requested capture (must be >=1
and fit on the screen)
+ * @param height The of width the requested capture (must be
>=1 and fit on the screen)
*/
public Pixels getScreenCapture(int x, int y, int width, int
height) {
return getScreenCapture(x, y, width, height, false);
