<AWT Dev> Review Request JDK:-8162959 [HiDPI] screenshot artifacts using AWT Robot

Alexandr Scherbatiy alexandr.scherbatiy at oracle.com
Tue Feb 7 17:23:47 UTC 2017


On 2/6/2017 1:19 PM, Prem Balakrishnan wrote:
>
> Hi Phil  and  Alex,
>
> Please review updated patch as per review comments.
>
> http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.08/ 
> <http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.08/>
>

   Just as a small optimization. The private method 
Robot.createCompatibleImage(...) can return an array of BufferedImage-s.
   The createScreenCapture(...) can return just an image from the zero 
index and the createMultiResolutionScreenCapture(...) returns 
BaseMultiResolutionImage(array of images).

   It is better if the public javadoc part is checked by a native speaker.

   Thanks,
   Alexandr.
>
> Multi-monitor scenario will be handled under JDK-8173972.
>
> https://bugs.openjdk.java.net/browse/JDK-8173972
>
> Regards,
>
> Prem
>
> *From:*Phil Race
> *Sent:* Tuesday, January 31, 2017 5:24 AM
> *To:* Prem Balakrishnan; Alexander Scherbatiy
> *Cc:* Sergey Bylokhov; awt-dev at openjdk.java.net
> *Subject:* Re: Review Request JDK:-8162959 [HiDPI] screenshot 
> artifacts using AWT Robot
>
> I like
>
> 3. createMultiresolutionScreenCapture(Rectangle rect);
>
>
> -phil.
>
> On 01/30/2017 01:11 AM, Prem Balakrishnan wrote:
>
>     Hi Phil,
>
>     Thank you for the review.
>
>     >Is Robot only capable of a screen capture from the default screen
>     ? I hope not.
>
>     Robot.createScreenCapture() can be used to capture screenshots on
>     Multi-Monitor as well.
>
>     To capture screen on Multi-Monitor user should calculate the total
>     width of multiple
>
>      screens, based on the position of secondary monitor(i.e.,
>     Right/Left) with respect to
>
>     main monitor and pass it  to createScreenCapture() method.
>
>     >So I would expect that where there are two screens with different
>     DPI settings/transforms
>     >the logic in here will be sensitive to the target screen.
>
>     I tried with MacBookPro (macOS Sierra version 10.12 , Retina ) by
>     connecting a secondary monitor non-Retina display.
>
>     keeping secondary monitor to the right/left of main display and
>     targeted rectangle on each sides, expected results are obtained.
>
>     The current patch addresses most common scenario of single HiDPI
>     monitor setup and dual monitor (Hi-DPI, non-HiDPI) setup.
>
>     As “Handling multi monitor setup with different DPI settings”
>     requires additional effort to make it work on all platforms, I
>     propose to fix it separately.
>
>     >And what happens if the target rectangle overlaps the edge
>     between two screens ?
>
>     I tried this scenario with Mac connecting non-retina secondary
>     monitor,  OS doesn’t allow user to place a window in between two
>     screens.
>
>     >The more significant API change is that the isHiDPI parameter is
>     a misnomer.
>     >It really means something else and I think what we should do is
>     delete it
>     >and change the method name to something else.
>
>     Thank you for the Draft.
>
>     From my point of view, I think we should maintain the method name
>     “createScreenCapture()”, because two functions doing the same task
>
>     should have the same name so that  method will have maximum
>     proximal visibility i.e., developer will easily know that there
>     exists two method variants to take screenshots.
>
>     (At least the method name should start with create….);
>
>     Some of the signatures  we can choose from:
>     1. createScreenCapture(int x, int y, int width, int height);
>
>     2. createScreenCapture(Point pt, Dimension d);
>
>     3. createMultiresolutionScreenCapture(Rectangle rect);
>
>     4. createMultiImageScreenCapture(Rectangle rect);
>
>     Request your feedback on the same.
>
>     Once we finalize the signature and behavior, I  shall update the
>     patch and send it for review.
>
>     Regards,
>
>     Prem
>
>     *From:*Philip Race
>     *Sent:* Wednesday, January 25, 2017 11:44 PM
>     *To:* Alexandr Scherbatiy
>     *Cc:* Prem Balakrishnan; Sergey Bylokhov; awt-dev at openjdk.java.net
>     <mailto:awt-dev at openjdk.java.net>
>     *Subject:* Re: Review Request JDK:-8162959 [HiDPI] screenshot
>     artifacts using AWT Robot
>
>     I'd like to propose some revision to the API and also have a
>     question about the implementation.
>
>     The question about the implementation concerns this code :-
>
>     465         AffineTransform tx = GraphicsEnvironment.
>
>     466                
>     getLocalGraphicsEnvironment().getDefaultScreenDevice().
>
>     467                 getDefaultConfiguration().getDefaultTransform();
>
>     468         double uiScaleX = tx.getScaleX();
>
>     469         double uiScaleY = tx.getScaleY();
>
>
>     Is Robot only capable of a screen capture from the default screen
>     ? I hope not.
>     So I would expect that where there are two screens with different
>     DPI settings/transforms
>     the logic in here will be sensitive to the target screen.
>     And what happens if the target rectangle overlaps the edge between
>     two screens ?
>
>     Regarding the API, I think that it needs to explain that it is
>     "not 'hi dpi" per se,
>     rather that it is the non-default transform that matters.
>
>     Additionally the doc should properly link to the class names, not
>     just write them
>     as words.
>
>     The more significant API change is that the isHiDPI parameter is a
>     misnomer.
>     It really means something else and I think what we should do is
>     delete it
>     and change the method name to something else.
>
>     Here is my draft :
>      /**
>       * Creates an image containing pixels read from the screen.
>       * This image does not include the mouse cursor.
>       * This method should be used in case there is a scaling
>     transform from
>       * user space to screen (device) space.
>       * Typically this means that the display is a high resolution screen,
>       * although strictly it means any case in which there is such a
>     transform.
>       * Returns a {@link java.awt.image.BufferedImage} for a
>     non-scaled display and
>       * a {@link java.awt.image.MultiResolutionImage} where there is a
>     scaling
>       * transform.
>       * <p>
>       * The {@code MultiResolutionImage} will have two image variants:
>       * <ul>
>       * <li> Base Image with user specified size. This is scaled from
>     the screen.
>       * <li> Native device resolution image with device size pixels.
>       * </ul>
>       * @param   screenRect     Rect to capture in screen coordinates
>       * @return  The captured image
>       * @throws  IllegalArgumentException if {@code screenRect} width
>     and height
>       * are not greater than zero
>       * @throws  SecurityException if {@code readDisplayPixels} permission
>       * is not granted
>       * @see     SecurityManager#checkPermission
>       * @see     AWTPermission
>       */
>
>      public synchronized Image multiImageeScreenCapture(Rectangle
>     screenRect);
>
>     -phil.
>
>     On 1/20/17, 3:20 AM, Alexandr Scherbatiy wrote:
>
>     On 1/19/2017 3:57 PM, Prem Balakrishnan wrote:
>
>         Hi Alex,
>
>         Please review updated patch as per review comments.
>
>         http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.06/
>         <http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.06/>
>
>         Automated test validates both Robot.getPixelColor() and output
>         of Robot.createScreenCapture() API's.
>
>         Jtreg and JCK tests passed on build with fix across all the
>         platforms(Mac, Windows and Linux) without causing any regression.
>
>     src/java.desktop/share/classes/java/awt/Robot.java
>       > 421      * @param   isHiDPI     Indicates if HiDPI Display
>
>       There should be mention that this argument value is ignored when
>     a screenshot is taken on non-HiDPI display
>
>       Thanks,
>       Alexandr.
>
>
>
>     Regards,
>
>     Prem
>
>     *From:*Alexandr Scherbatiy
>     *Sent:* Thursday, January 12, 2017 2:39 PM
>     *To:* Prem Balakrishnan; Sergey Bylokhov; awt-dev at openjdk.java.net
>     <mailto:awt-dev at openjdk.java.net>; Philip Race
>     *Subject:* Re: Review Request JDK:-8162959 [HiDPI] screenshot
>     artifacts using AWT Robot
>
>
>     > 421      * @param   isHiDPI     Indicates if HiDPI Display
>
>       There should be mention that this argument value is ignored when
>     a screenshot is taken on non-HiDPI display
>
>     Please also check the the method Robot.getPixelColor(int x, int y)
>     also correctly works on Mac OS X, Windows and Linux after the fix.
>
>     Thanks,
>     Alexandr.
>
>     On 1/10/2017 11:54 AM, Prem Balakrishnan wrote:
>
>         Hi Alex,
>
>         Please review updated patch,
>
>         http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.05/
>         <http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.05/>
>
>         updated imageOption to kCGWindowImageBestResolution  which
>         returns Best Resolution Image while capturing screen using
>         CGWindowListCreateImage() method.
>
>         Regards,
>
>         Prem
>
>         *From:*Alexandr Scherbatiy
>         *Sent:* Tuesday, January 10, 2017 1:08 AM
>         *To:* Prem Balakrishnan; Sergey Bylokhov;
>         awt-dev at openjdk.java.net <mailto:awt-dev at openjdk.java.net>;
>         Philip Race
>         *Subject:* Re: Review Request JDK:-8162959 [HiDPI] screenshot
>         artifacts using AWT Robot
>
>
>           CGWindowListCreateImage() method which is used to capture a
>         screenshot on Mac OS X by Robot has imageOption argument which
>         can have value: kCGWindowImageBestResolution
>         https://developer.apple.com/reference/coregraphics/1666230-quartz_window_services/window_image_types?language=objc
>            "When capturing the window, return the best image
>         resolution. The returned image size may be different than the
>         screen size."
>
>           Could this option help to get a high-resolution CGImageRef
>         on HiDPI display?
>
>           Thanks,
>           Alexandr.
>
>         On 12/27/2016 12:14 PM, Prem Balakrishnan wrote:
>
>             Hi Alexander,
>
>             Please review updated patch,
>
>             http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.04/
>             <http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.04/>
>
>             I used  wrong bounds while creating images, correct it in
>             this patch.
>
>             Regards,
>
>             Prem
>
>             *From:*Alexander Scherbatiy
>             *Sent:* Wednesday, December 14, 2016 5:18 PM
>             *To:* Prem Balakrishnan; Sergey Bylokhov;
>             awt-dev at openjdk.java.net
>             <mailto:awt-dev at openjdk.java.net>; Philip Race
>             *Subject:* Re: Review Request JDK:-8162959 [HiDPI]
>             screenshot artifacts using AWT Robot
>
>             On 07/12/16 10:24, Prem Balakrishnan wrote:
>
>
>
>
>
>             Hi Alexander,
>
>             Please review updated patch as per review comments.
>
>             http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.03/
>             <http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.03/>
>
>               I used a simple app [1] to create a screenshot of a
>             frame on Mac OS X:
>                     Rectangle rect = frame.getBounds();
>                     rect.setLocation(frame.getLocationOnScreen());
>                     BufferedImage img = robot.createScreenCapture(rect);
>
>               It takes only bottom right quarter of the frame [2].
>
>               Is it possible to use some API that makes a screenshot
>             and returns an NSImage on Mac OS X?
>               In this case it would be possible to get a high
>             resolution representation from the NSImage and return it
>             in the same way as it is updated on Windows and Linux.
>
>               [1]
>             http://cr.openjdk.java.net/~alexsch/8162959/screenshot/RobotScreenshot.java
>             <http://cr.openjdk.java.net/%7Ealexsch/8162959/screenshot/RobotScreenshot.java>
>               [2]
>             http://cr.openjdk.java.net/~alexsch/8162959/screenshot/screenshot.png
>             <http://cr.openjdk.java.net/%7Ealexsch/8162959/screenshot/screenshot.png>
>
>               Thanks,
>               Alexandr.
>
>
>
>
>
>             Regards,
>
>             Prem
>
>             *From:*Alexandr Scherbatiy
>             *Sent:* Monday, November 21, 2016 8:10 PM
>             *To:* Prem Balakrishnan; Sergey Bylokhov;
>             awt-dev at openjdk.java.net
>             <mailto:awt-dev at openjdk.java.net>; Phil Race
>             *Subject:* Re: Review Request JDK:-8162959 [HiDPI]
>             screenshot artifacts using AWT Robot
>
>             On 11/16/2016 8:46 AM, Prem Balakrishnan wrote:
>
>
>
>
>
>
>             Hi Alexander,
>
>             Please review update patch as per review comments.
>
>             http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.02/
>             <http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.02/>
>
>             With this patch, Robot.createScreenCapture(Rectangle)
>             method returns a scaled down image on the HiDPI display.
>
>
>             - The native part on Mac OS X and Linux should be updated
>             as well.
>
>             - 416      * Returns BufferedImage for Non-HiDPI display
>             and MultiResolutionImage
>               417      * for HiDPI display with two resolution variants.
>
>             There should be added an explanation what is the content
>             of the first and the second resolution variant.
>
>             Thanks,
>             Alexandr.
>
>
>
>
>
>
>
>             Regards,
>
>             Prem
>
>             *From:*Alexandr Scherbatiy
>             *Sent:* Thursday, November 03, 2016 4:05 PM
>             *To:* Prem Balakrishnan; Sergey Bylokhov;
>             awt-dev at openjdk.java.net <mailto:awt-dev at openjdk.java.net>
>             *Subject:* Re: Review Request JDK:-8162959 [HiDPI]
>             screenshot artifacts using AWT Robot
>
>             On 11/2/2016 1:57 PM, Prem Balakrishnan wrote:
>
>
>
>
>
>
>
>             Hi Alexander,
>
>             Please review updated patch.
>
>             http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.01/
>             <http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.01/>
>
>             Added a new public API “*Image
>             createHiDPIScreenCapture(Rectangle screenRect)”.*
>
>             Returns an ordinary screenshot(BufferedImage) if the UI
>             scale is 1.
>
>             Returns MultiResolutionImage for HiDPI display with two
>             resolution variants,
>
>             1.Low Resolution/base image with user input width and
>             height,  I have used interpolation algorithm for scaling ,
>             adapted from JavaFX Robot
>             (http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/89a5de54b7dc),
>
>             2.High Resolution image with scaled width and height .
>
>               - Please, check that the
>             Robot.createScreenCapture(Rectangle) method returns a
>             scaled down image on the HiDPI display
>               - Probably existing Java methods can be used for an
>             image scaling.
>               For example, there is the Image.getScaledInstance(int
>             width, int height, int hints) method.
>               Or may be it is better just to create an image with
>             required size and draw the high-resolution image into it
>             using a specific scale and rendering hints.
>
>               Thanks,
>               Alexandr.
>
>
>
>
>
>
>
>
>             Regards,
>
>             Prem
>
>             *From:*Alexander Scherbatiy
>             *Sent:* Thursday, October 13, 2016 3:21 PM
>             *To:* Prem Balakrishnan; Sergey Bylokhov;
>             awt-dev at openjdk.java.net <mailto:awt-dev at openjdk.java.net>
>             *Subject:* Re: Review Request JDK:-8162959 [HiDPI]
>             screenshot artifacts using AWT Robot
>
>             On 06/10/16 15:28, Prem Balakrishnan wrote:
>
>                 Hi*,*
>
>                 **
>
>                 Please review fix for JDK 9,
>
>                 *Bug:*https://bugs.openjdk.java.net/browse/JDK-8162959
>
>                 *Webrev:*http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.00/
>                 <http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.00/>
>
>
>                 I have adapted the same fix as used for JavaFX Robot
>
>                 *Bug: *JDK-8162783
>                 <https://bugs.openjdk.java.net/browse/JDK-8162783>.
>
>                 *Patch:
>                 *http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/89a5de54b7dc
>
>                 New Public API ” BufferedImage
>                 createScreenCapture(Rectangle screenRect, boolean
>                 isHiDPI)”is added,
>
>                 Which will have a parameter to specify if HiDPI.
>
>               It is better to a add public method which returns
>             MultiResolution image on HiDPI display and  consists of
>             two resoltion variants
>                - base image which has size requested by a user
>                - high-resolution image which creates an original
>             screen capture
>
>               The proposed by your algorithm can be applied to the
>             base image.
>               For more details see JDK-8020618 [macosx] java.awt.Robot
>             makes blurry screen captures on Retina
>
>               Thanks,
>               Alexandr.
>
>
>
>
>
>
>
>
>               
>
>             Regards,
>
>             Prem
>
>               
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20170207/2123d848/attachment-0001.html>


More information about the awt-dev mailing list