<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