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

Rajeev Chamyal rajeev.chamyal at oracle.com
Thu Sep 15 06:24:52 UTC 2016


Hello Sergey,

I have removed the html file.
http://cr.openjdk.java.net/~rchamyal/8150176/webrev.03/ 

Regards,
Rajeev Chamyal

-----Original Message-----
From: Sergey Bylokhov 
Sent: 14 September 2016 23:39
To: Alexandr Scherbatiy; Rajeev Chamyal; swing-dev at openjdk.java.net
Subject: Re: <Swing Dev> [9] Review request for JDK-8150176 [hidpi] wrong resolution variant of multi-res. image is used for TrayIcon

Is "MultiResolutionTrayIconTest.html" should be removed?

On 14.09.16 17:44, Alexandr Scherbatiy wrote:
> 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
>>
>>
>>
>>
>>
>>
>>
>>
>>
>


--
Best regards, Sergey.



More information about the swing-dev mailing list