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@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@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@openjdk.java.net >> <mailto:swing-dev@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@openjdk.java.net >> <mailto:swing-dev@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.
