Re: [9] Review request for JDK-8150176 [hidpi] wrong resolution variant of multi-res. image is used for TrayIcon

2016-06-13 Thread Rajeev Chamyal
Hello Alexandr,

 

Thanks for the review. I have updated the webrev as per review comments.

http://cr.openjdk.java.net/~rchamyal/8150176/webrev.01/

I tried drawing the image directly to paint graphics without buffered image and 
it was getting cropped.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 09 June 2016 20:43
To: Rajeev Chamyal; swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  [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: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.00/"http://cr.openjdk.java.net/~rchamyal/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

 

 


Re: [9] RFR JDK-8159068:The rendering of JTable is broken

2016-06-13 Thread prasanta sadhukhan



On 6/13/2016 12:51 PM, prasanta sadhukhan wrote:

Hi All,

Please review a fix for jdk9 where it was seen that if we try to 
select some rows in a JTable, the text painted in the rows goes missing.


Bug: https://bugs.openjdk.java.net/browse/JDK-8159068

webrev: http://cr.openjdk.java.net/~psadhukhan/8159068/webrev.00/

The issue was rMax value was decremented wrongly so when paintCells() 
is called with wrong rMax, some rows were not printed correctly.


Fix is to make sure rmax is decremented properly, only when we are 
trying to print whole visible portion of JTable and NOT when some rows 
are being painted.


Regarding the regression testcase, I could not make it automated as 
the failure happens on random iteration.
and also, getting selection background/foreground was giving same 
values with and without the missing text.


Also, since it is a regression of 8081491 
, it's testcase are 
working fine with this fix and so did SwingSet2 JTable demo.

Regards
Prasanat




Re: Fix for JDK-8065861 : Pressing Esc does not set 'canceled' property of ProgressMonitor

2016-06-13 Thread Rajeev Chamyal
Looks good to me.

Regards,
Rajeev Chamyal

-Original Message-
From: Alexandr Scherbatiy 
Sent: 10 June 2016 13:06
To: Ajit Ghaisas; Sergey Bylokhov; Rajeev Chamyal; swing-dev@openjdk.java.net
Subject: Re: Fix for JDK-8065861 : Pressing Esc does not set 'canceled' 
property of ProgressMonitor


The fix looks good to me.

Thanks,
Alexandr.

On 6/10/2016 8:50 AM, Ajit Ghaisas wrote:
> Hi,
>
>  Thanks Alex for spotting probable exceptions in code changes.
>   I have corrected the code to address them.
>
>   Here is the updated webrev. Request you to review.
>   http://cr.openjdk.java.net/~aghaisas/8065861/webrev.01/
>
> Regards,
> Ajit
> 
>
> -Original Message-
> From: Alexandr Scherbatiy
> Sent: Thursday, June 09, 2016 8:56 PM
> To: Ajit Ghaisas; Sergey Bylokhov; Rajeev Chamyal; swing-dev@openjdk.java.net
> Subject: Re: Fix for JDK-8065861 : Pressing Esc does not set 'canceled' 
> property of ProgressMonitor
>
> On 6/8/2016 5:35 PM, Ajit Ghaisas wrote:
>> Hi,
>>
>> Bug :
>> https://bugs.openjdk.java.net/browse/JDK-8065861
>>
>> Issue :
>> Pressing Esc does not set 'canceled' property of ProgressMonitor
>>
>> Analysis :
>> ProgressMonitor option pane only gets hidden on pressing Escape key.
>> It is not truly canceled as isCanceled() method continues to return false.
>>
>> Fix :
>> On Pressing Escape key JOptionPane.CLOSED_OPTION value is set in JOptionPane 
>> from BasicOptionPaneUI.java.
>> This value is used as a condition in isCanceled() to identify 
>> ProgressMonitor is canceled by pressing Escape key.
>>
>> Please review the webrev:
>> http://cr.openjdk.java.net/~aghaisas/8065861/webrev.00/
>The updated code calls methods from 'v' object before it is checked to 
> the null which can lead to the NPE. The cancelOption[0] also can throw the 
> IOBE before the arrays length checking.
>
> Thanks,
> Alexandr.
>> On review completion, I will raise a CCC request for documentation change.
>>
>> Regards,
>> Ajit



[9] RFR JDK-8159068:The rendering of JTable is broken

2016-06-13 Thread prasanta sadhukhan

Hi All,

Please review a fix for jdk9 where it was seen that if we try to select 
some rows in a JTable, the text painted in the rows goes missing.


Bug: https://bugs.openjdk.java.net/browse/JDK-8159068

webrev: http://cr.openjdk.java.net/~psadhukhan/8159068/webrev.00/

The issue was rMax value was decremented wrongly so when paintCells() is 
called with wrong rMax, some rows were not printed correctly.


Fix is to make sure rmax is decremented properly, only when we are 
trying to print whole visible portion of JTable and NOT when some rows 
are being painted.


Regarding the regression testcase, I could not make it automated as the 
failure happens on random iteration.
and also, getting selection background/foreground was giving same values 
with and without the missing text.


Regards
Prasanat