Glass Robot and getSCreenCapture

[Scott Palmer]

It isn't public API now.. but keep in mind that making it public in
some way has been requested for over two years.  See RT-17571
Since those issues have made the internal Robot API known, no doubt
people are using it. (We all know how well people listen to the
warnings about using internal APIs .. *cough* sun.misc.Unsafe *cough*)

Cheers,

Scott

On Wed, Mar 26, 2014 at 10:59 AM, Stephen F Northover
<steve.x.northover at oracle.com> wrote:
> Hi David,
>
> Sorry to be getting to this so late.  An uninitialized pixel is normally
> considered to be black.  If you throw an exception, clients will need to
> either catch the exception or perform the same test that you are performing
> before they call the "API".  Since this is not pubic API, no client will be
> affected so even if we make the change to throw the exception and then
> decide not to do this later, we can change it.
>
> What is happening now?  Who is being affected by this bug?  Is it easy to
> change the implementations to return black?  This seems better to me than
> throwing the exception.
>
> As I say, if we throw the exception, then we will only break ourselves, not
> clients of FX API.  Have we ensured that the exception will not break SQE
> tests.
>
> Again, sorry to be getting to this so late and apologies if all of this has
> been discussed in another thread that I missed,
> Steve
>
>
> On 2014-03-25 2:46 PM, David Hill wrote:
>>
>> 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);
>>
>>
>