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

Alexandr Scherbatiy alexandr.scherbatiy at oracle.com
Thu Sep 15 10:09:59 UTC 2016


The fix looks good to me.

Thanks,
Alexandr.

On 9/15/2016 1:01 PM, Sergey Bylokhov wrote:
> +1
>
> On 15.09.16 9:24, Rajeev Chamyal wrote:
>> 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