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@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@openjdk.java.net
    <mailto:awt-dev@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@openjdk.java.net
    <mailto:awt-dev@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@openjdk.java.net <mailto:awt-dev@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@openjdk.java.net
            <mailto:awt-dev@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@openjdk.java.net
            <mailto:awt-dev@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@openjdk.java.net <mailto:awt-dev@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@openjdk.java.net <mailto:awt-dev@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


Reply via email to