<Swing Dev> [9] Review request for JDK-8150176 [hidpi] wrong resolution variant of multi-res. image is used for TrayIcon

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 9/14/2016 12:01 PM, Rajeev Chamyal wrote:
>
> Hello Alexandr,
>
> Thanks for the review.
>
> Please review the webrev updated as per review comments.
>
> http://cr.openjdk.java.net/~rchamyal/8150176/webrev.03/ 
> <http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.03/>
>
> Regards,
>
> Rajeev Chamyal
>
> *From:*Alexandr Scherbatiy
> *Sent:* 12 September 2016 20:07
> *To:* Rajeev Chamyal; swing-dev at openjdk.java.net; Sergey Bylokhov
> *Subject:* Re: <Swing Dev> [9] Review request for JDK-8150176 [hidpi] 
> wrong resolution variant of multi-res. image is used for TrayIcon
>
> On 9/9/2016 3:06 PM, Rajeev Chamyal wrote:
>
>     Hello Alexandr,
>
>     Please review the updated webrev.
>
>     http://cr.openjdk.java.net/~rchamyal/8150176/webrev.02/
>     <http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.02/>
>
> WTrayIconPeer.java
> +                gr.drawImage(image, 0, 0, (autosize ? w : 
> image.getWidth(observer)),
> +                             (autosize ? h : 
> image.getHeight(observer)), observer);
>
> The w and h are scaled. It looks like the image.getWidth(observer) and 
> image.getHeight(observer) also should be scaled in the same way.
>
>  MultiResolutionTrayIconTest.java
> +        latch.await();
>
> It is better to add a timeout here.
>
> Thanks,
> Alexandr.
>
>
>     Update: Updated webrev is fixing the issue in windows only.
>
>     We have a separate bug linux and it will be fixed through a
>     separate webrev.
>
>     https://bugs.openjdk.java.net/browse/JDK-8154551
>
>     Regards,
>
>     Rajeev Chamyal
>
>     *From:*Alexandr Scherbatiy
>     *Sent:* 14 June 2016 15:21
>     *To:* Rajeev Chamyal; swing-dev at openjdk.java.net
>     <mailto:swing-dev at openjdk.java.net>; Sergey Bylokhov
>     *Subject:* Re: <Swing Dev> [9] Review request for JDK-8150176
>     [hidpi] wrong resolution variant of multi-res. image is used for
>     TrayIcon
>
>     On 6/13/2016 3:18 PM, Rajeev Chamyal wrote:
>
>
>         Hello Alexandr,
>
>         Thanks for the review. I have updated the webrev as per review
>         comments.
>
>         http://cr.openjdk.java.net/~rchamyal/8150176/webrev.01/
>         <http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.01/>
>
>         I tried drawing the image directly to paint graphics without
>         buffered image and it was getting cropped.
>
>
>     Did you paint it using non scaled width and height?
>       g.drawImage(image, 0, 0, curW, curH, null);
>     Is the g transform already scaled?
>
>     XTrayIconPeer:
>
>     606                 if (scaleX > 1.0 && scaleY > 1.0) {
>
>     607                     resolutionVariant =
>     ((MultiResolutionImage) image).
>
>
>        It is better to change the condition to "image instanceof
>     MultiResolutionImage". It is necessary to not get CCE for non
>     multi-resolution image and the multi-resolution image should
>     return the best resolution variant for any scale.
>
>     618                         gr.drawImage(resolutionVariant, 0, 0,
>
>     619                                 curW, curH, observer);
>
>       The width and height should be scaled here to draw to whole
>     buffered image.
>
>     WTrayIconPeer:
>
>      133         BufferedImage bufImage = new
>     BufferedImage(TRAY_ICON_WIDTH, TRAY_ICON_HEIGHT,
>      134 BufferedImage.TYPE_INT_ARGB);
>
>      The size for the buffered image should be scaled in the same was
>     as for XTrayIconPeer.
>      It may require to update the native code as well to set proper
>     high-resolution icon.
>
>     Thanks,
>     Alexandr.
>
>
>
>
>         Regards,
>
>         Rajeev Chamyal
>
>         *From:*Alexandr Scherbatiy
>         *Sent:* 09 June 2016 20:43
>         *To:* Rajeev Chamyal; swing-dev at openjdk.java.net
>         <mailto:swing-dev at openjdk.java.net>; Sergey Bylokhov
>         *Subject:* Re: <Swing Dev> [9] Review request for JDK-8150176
>         [hidpi] wrong resolution variant of multi-res. image is used
>         for TrayIcon
>
>         On 6/9/2016 11:55 AM, Rajeev Chamyal wrote:
>
>
>
>             Hello All,
>
>             Please review the following fix.
>
>             Bug: https://bugs.openjdk.java.net/browse/JDK-8150176
>
>             Webrev:
>             http://cr.openjdk.java.net/~rchamyal/8150176/webrev.00/
>             <http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.00/>
>
>             Issue: Wrong resolution variant image is used in Tray Icon.
>
>             Fix : Applying the device transform to graphics object to
>             select the correct image.
>
>             The image could be cropped on Linux because the high
>         resolution icon which size is bigger that the original image
>         is drawn to the buffered image with un-scaled size curW x CurH.
>           It is better to get a resolution variant from the
>         multi-resolution image, draw it to a buffered image with the
>         same scaled size and then draw the buffered image to the paint
>         graphics using original size:
>             -------
>             Image resolutionVariant = ((MultiResolutionImage)
>         image).getResolutionVariant(scaleX * curW, scaleY * curH);
>             BufferedImage bufImage = new BufferedImage(scaleX * curW,
>         scaleY * curH, BufferedImage.TYPE_INT_ARGB);
>             // ...
>             gr.drawImage(image, 0, 0, scaleX * curW, scaleY * curH,
>         observer);
>             // ...
>             g.drawImage(bufImage, 0, 0, curW, curH, observer); // non
>         scaled width and height
>             -------
>
>           By the way, is the buffered image necessary in this case? Is
>         it possible to draw the image directly to the paint graphics?
>             -------
>              g.drawImage(image, 0, 0, curW, curH, null);
>             -------
>
>         Thanks,
>         Alexandr.
>
>             Regards,
>
>             Rajeev Chamyal
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160914/89b320b1/attachment.html>