<Swing Dev> [9] Review request for JDK-8150176 [hidpi] wrong resolution variant of multi-res. image is used for TrayIcon
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Thu Sep 15 10:01:24 UTC 2016
+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.
>
--
Best regards, Sergey.
More information about the swing-dev
mailing list