Re:

2016-06-21 Thread Anton Litvinov 

Hello Sergey and Semyon,

Sergey, thank you for review of this fix. In the 1st version of the fix 
I decided to return "null", because at least one implementation of 
"javax.swing.plaf.synth.SynthStyle" which is 
"sun.swing.plaf.synth.DefaultSynthStyle" returns "null" from its 
implementation of "getColorForState(SynthContext, ColorType)", but while 
I worked on the fix, I did not read the comment in "NimbusStyle.java" 
which you quoted. Yes, I agree that it is incorrect to return "null" 
from the method "NimbusStyle.getColorForState".


Adding support of "Selected", "Disabled+Selected" states to 
"List.cellRenderer" component in 
"src/java.desktop/share/classes/javax/swing/plaf/nimbus/skin.laf" file 
was considered before the review request of the 1st fix version, but it 
was defined that in "SwingSet2" demo selected items in the list were 
still drawn with wrong colors. However today I defined the reason of 
this issue in "SwingSet2" demo, and it is caused by the fact that in the 
demo a custom extension of "javax.swing.DefaultListCellRenderer" class 
is used instead of the default 
"javax.swing.plaf.synth.SynthListUI.SynthListCellRenderer" which is 
capable of setting "SynthConstants.SELECTED" bit mask to its state 
through the call sequence 
"SynthListCellRenderer.getListCellRendererComponent" --> 
"SynthLookAndFeel.setSelectedUI".


The 2nd version of the fix was created. Could you please review the 2nd 
version of the fix.


Webrev 01: http://cr.openjdk.java.net/~alitvinov/8057791/jdk9/webrev.01

In the new fix version the regression test stayed unchanged. The new fix 
version adds support of "Selected", "Disabled+Selected" states to 
"List.cellRenderer" component in 
"src/java.desktop/share/classes/javax/swing/plaf/nimbus/skin.laf" to 
make "NimbusStyle.getColorForState" method return correct colors for 
"SynthListCellRenderer" instances used in "JList" in Nimbus L&F.


The following 2 sets of regression tests were executed on Windows platform:
1. Automatic regression tests from open and closed sets located in 
"javax/swing/JList", "javax/swing/plaf/nimbus" directories.

2. JCK tests:
api/javax_swing/CellRendererPane
api/javax_swing/DefaultCellEditor
api/javax_swing/DefaultListCellRenderer
api/javax_swing/DefaultListModel
api/javax_swing/DefaultListSelectionModel
api/javax_swing/JList
api/javax_swing/plaf/ListUI
api/javax_swing/plaf/nimbus
api/javax_swing/plaf/synth/SynthListUI

Thank you,
Anton

On 6/20/2016 3:13 PM, Sergey Bylokhov wrote:

Hi, Anton.
I am a little bit worried about the change in 
NimbusStyle.getColorForState(). Before the fix we intentionally never 
return null from this method, but return DEFAULT_COLOR which has this 
javadoc:

/**
 * The Color to return from getColorForState if it would 
otherwise have

 * returned null.
 *
 * Returning null from getColorForState is a very bad thing, as 
it causes
 * the AWT peer for the component to install a SystemColor, which 
is not a

 * UIResource. As a result, if null is returned from
 * getColorForState, then thereafter the color is not updated for 
other

 * states or on LAF changes or updates. This DEFAULT_COLOR is used to
 * ensure that a ColorUIResource is always returned from
 * getColorForState.
 */

And after the fix this method will return null for all ListCellRenderer.

Can you please clarify why the change states of "List.cellRenderer" in 
plaf/nimbus/skin.laf does not solve the problem?



On 6/13/2016 12:12 AM, Anton Litvinov wrote:

Hello,

Could you please review the following fix for the bug.

Bug: https://bugs.openjdk.java.net/browse/JDK-8057791
Webrev: http://cr.openjdk.java.net/~alitvinov/8057791/jdk9/webrev.00

The bug is the fact that selected elements in "javax.swing.JList"
component are drawn with wrong background and foreground colors,
though "JList.getSelectionForeground()",
"JList.getSelectionBackground()" methods return the correct color
values for the tested JList instance.

Wrong colors are returned in run time from the method
"javax.swing.plaf.synth.SynthStyle.getColor" for
"javax.swing.plaf.synth.SynthListUI.SynthListCellRenderer" instance,
because it can be only in 1 state "SynthConstants.ENABLED" and cannot
be in "SynthConstants.SELECTED" state, therefore the call from this
method to
"javax.swing.plaf.nimbus.NimbusStyle.getColorForState(SynthContext,
ColorType)" method always returns some wrong default L&F foreground,
background colors which are observed in this bug.

All automatic regression tests from open and closed sets located in
"javax/swing/JList", "javax/swing/plaf/nimbus" directories were run on
MS Windows 7 OS during verification of the fix.

Thank you,
Anton

Re: [9] Review Request JDK-8147648 [hidpi] multiresolution image: wrong resolution variant is used as icon in the Unity panel

2016-06-21 Thread Alexandr Scherbatiy 

On 6/21/2016 12:16 PM, Rajeev Chamyal wrote:


Hello All,

Please review the following webrev.

Webrev: http://cr.openjdk.java.net/~rchamyal/8147648/webrev.00/ 



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

Issue: Wrong resolution variant is used as icon in the Unity panel.

Cause: The screen transforms are not applied to find the correct 
resolution variant image in current implementation.


Fix: Applied the screen transforms to graphics object.



 222 int scaleX = (int)tx.getScaleX();
 223 int scaleY = (int)tx.getScaleY();
 224 DataBufferInt buffer = new DataBufferInt(scaleX * width * 
scaleY * height);


  The fix is in the shared code and the scale factor can have floating 
point value on Windows. (for example 1.5).

  It is better to round the final width and height after scaling them.

  Thanks,
  Alexandr.


Regards,

Rajeev Chamyal

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

2016-06-21 Thread Prasanta Sadhukhan 



On 6/21/2016 5:16 PM, Alexandr Scherbatiy wrote:

On 6/21/2016 1:58 PM, Prasanta Sadhukhan wrote:



On 6/21/2016 4:14 PM, Alexandr Scherbatiy wrote:

On 6/20/2016 8:10 AM, prasanta sadhukhan wrote:


Gentle reminder for review!!

Regards
Prasanta
On 6/13/2016 4:31 PM, prasanta sadhukhan wrote:

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.


  Could you give two samples how this algorithm work. One sample 
where a whole visible portion of a JTable and another where some 
rows are being printed. What are rMax and  rMin values in both cases 
and how are they calculated?



If a JTable is of 50 rows and only 35 are being visible in page 1, then
if whole visible portion of JTable is printed, rMin will be 0 and 
rMax was 35
so 36 rows were getting printed so I decrement rMax by 1 to 34 so 
only 35 will be printed (same as shown on console).
When we select some row of JTable as in the case of LostText 
testcase, rMin will be say 6 and rMax will be 9 in which case also, I 
was decrementing rMax so rMin=6, rMax=8 so next row was not getting 
painted.

   And what are indices of the selected rows?

It will depend on the last selection. At start, rMin = 0 , rMax = last 
indice, say 10 for a JTable of 10 rows
Now, if we select row 5, rMin and rMax both becomes 5 and we decrement 
rMax so rMax becomes less than rMin and paintCell() due to this check

(int row = rMin; row <= rMax; row++) it does not do
paintCell(g, cellRect, row, column)
and nothing gets painted.

Regards
Prasanta

  Thanks,
  Alexandr.



Regards
Prasanta

  Thanks,
  Alexandr.



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: [9] Review request for 7020860: BasicTreeUI contains getters/setters with unclear spec

2016-06-21 Thread Alexandr Scherbatiy 

On 6/14/2016 5:37 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-7020860

webrev: http://cr.openjdk.java.net/~ssadetsky/7020860/webrev.00/

The methods mentioned in JIRA are not getters/setters according to 
JavaBeans specs, nevertheless some of them got wrong javadocs. The 
proposed fix improves the situation.


   I am not sure about the phrase "that is rendering each cell" in

 474  * Returns the current instance of the {@link 
TreeCellRenderer} that is

 475  * rendering each cell.

  Otherwise, the fix looks good to me.

  Thanks,
  Alexandr.




--Semyon

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

2016-06-21 Thread Alexandr Scherbatiy 

On 6/21/2016 1:58 PM, Prasanta Sadhukhan wrote:



On 6/21/2016 4:14 PM, Alexandr Scherbatiy wrote:

On 6/20/2016 8:10 AM, prasanta sadhukhan wrote:


Gentle reminder for review!!

Regards
Prasanta
On 6/13/2016 4:31 PM, prasanta sadhukhan wrote:

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.


  Could you give two samples how this algorithm work. One sample 
where a whole visible portion of a JTable and another where some rows 
are being printed. What are rMax and  rMin values in both cases and 
how are they calculated?



If a JTable is of 50 rows and only 35 are being visible in page 1, then
if whole visible portion of JTable is printed, rMin will be 0 and rMax 
was 35
so 36 rows were getting printed so I decrement rMax by 1 to 34 so only 
35 will be printed (same as shown on console).
When we select some row of JTable as in the case of LostText testcase, 
rMin will be say 6 and rMax will be 9 in which case also, I was 
decrementing rMax so rMin=6, rMax=8 so next row was not getting painted.

   And what are indices of the selected rows?

  Thanks,
  Alexandr.



Regards
Prasanta

  Thanks,
  Alexandr.



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: Review Request JDK-8159152 Ctrl+F6, Ctrl+F5 doesn't work for iconified InternalFrame

2016-06-21 Thread Alexandr Scherbatiy 

On 6/20/2016 5:59 AM, Rajeev Chamyal wrote:


Hello Alexandr,

TestJInternalFrameMinimize test passes after this fix.

  Could you also check that the initial issue where the call 
setComponentOrderCheckingEnabled(true/false) has been added to the 
DefaultDesktopManager.iconifyFrame(JInternalFrame) method?
JDK-6325652 Iconified JInternalFrame does not restore when Ctrl+F5 
is used

  https://bugs.openjdk.java.net/browse/JDK-6325652

  Thanks,
  Alexandr.


Regards,

Rajeev Chamyal

*From:*Alexandr Scherbatiy
*Sent:* 17 June 2016 18:35
*To:* Rajeev Chamyal; Sergey Bylokhov; swing-dev@openjdk.java.net
*Subject:* Re:  Review Request JDK-8159152 Ctrl+F6, Ctrl+F5 
doesn't work for iconified InternalFrame


On 6/17/2016 10:09 AM, Rajeev Chamyal wrote:

Hello All,

Please review the following webrev.

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

Webrev: http://cr.openjdk.java.net/~rchamyal/8159152/webrev.00/


Issue: Internal frame cache is not getting updated properly on
iconifyFrame if there are less than 3 frames on the desktop.

Fix: Updated the iconifyFrame method so that frame cache updates
properly on internal frame remove and icon addition to desktop.


  Could you check that the test 
test/javax/swing/JInternalFrame/8145060/TestJInternalFrameMinimize.java 
passes after the fix?


  Thanks,
  Alexandr.


Regards,

Rajeev Chamyal

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

2016-06-21 Thread Prasanta Sadhukhan 



On 6/21/2016 4:14 PM, Alexandr Scherbatiy wrote:

On 6/20/2016 8:10 AM, prasanta sadhukhan wrote:


Gentle reminder for review!!

Regards
Prasanta
On 6/13/2016 4:31 PM, prasanta sadhukhan wrote:

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.


  Could you give two samples how this algorithm work. One sample where 
a whole visible portion of a JTable and another where some rows are 
being printed. What are rMax and  rMin values in both cases and how 
are they calculated?



If a JTable is of 50 rows and only 35 are being visible in page 1, then
if whole visible portion of JTable is printed, rMin will be 0 and rMax 
was 35
so 36 rows were getting printed so I decrement rMax by 1 to 34 so only 
35 will be printed (same as shown on console).
When we select some row of JTable as in the case of LostText testcase, 
rMin will be say 6 and rMax will be 9 in which case also, I was 
decrementing rMax so rMin=6, rMax=8 so next row was not getting painted.


Regards
Prasanta

  Thanks,
  Alexandr.



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: [9] RFR JDK-8159068:The rendering of JTable is broken

2016-06-21 Thread Alexandr Scherbatiy 

On 6/20/2016 8:10 AM, prasanta sadhukhan wrote:


Gentle reminder for review!!

Regards
Prasanta
On 6/13/2016 4:31 PM, prasanta sadhukhan wrote:

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.


  Could you give two samples how this algorithm work. One sample where 
a whole visible portion of a JTable and another where some rows are 
being printed. What are rMax and  rMin values in both cases and how are 
they calculated?


  Thanks,
  Alexandr.



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

[9] Review Request JDK-8147648 [hidpi] multiresolution image: wrong resolution variant is used as icon in the Unity panel

2016-06-21 Thread Rajeev Chamyal 
Hello All,

 

Please review the following webrev.

 

Webrev: http://cr.openjdk.java.net/~rchamyal/8147648/webrev.00/ 

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

 

Issue: Wrong resolution variant is used as icon in the Unity panel.

Cause: The screen transforms are not applied to find the correct resolution 
variant image in current implementation.

Fix: Applied the screen transforms to graphics object.

 

Regards,

Rajeev Chamyal