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

2016-09-14 Thread Rajeev Chamyal
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:  [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/
>> 
>>
>>
>>
>> Regards,
>>
>> Rajeev Chamyal
>>
>>
>>
>> *From:*Alexandr Scherbatiy
>> *Sent:* 12 September 2016 20:07
>> *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 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/
>> 
>>
>> 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
>> ; Sergey Bylokhov
>> *Subject:* Re:  [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/
>> 
>>
>> 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
>> ; 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:
>> htt

Re: [9] Review request for 8165234 Provide a way to not close toggle menu items on mouse click on component level

2016-09-14 Thread Sergey Bylokhov

Hi, Alexandr.
I think we should double check how we operate on this property in case 
of custom L&F. Our code relies that this property is set to "true" by 
default. And if the custom look&feel, which knows nothing about it will 
be in use, then the code in SwingUtilities2 returns false. Is it expected?


On 05.09.16 13:51, Alexandr Scherbatiy wrote:


Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8165234/webrev.01

On 9/2/2016 9:59 PM, Phil Race wrote:

Seems fine except for a grammatical issue in the javadoc

 {@code true}. Setting the property to {@code false} either to
{@literal L&F}

I think you mean ".. either on the ..."

   The typo "either to" is updated to "either on the".

  Thanks,
  Alexandr.



-phil.

On 09/02/2016 05:30 AM, Alexandr Scherbatiy wrote:


Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8165234
  webrev: http://cr.openjdk.java.net/~alexsch/8165234/webrev.00

  The fix JDK-8158566 adds "CheckBoxMenuItem.closeOnMouseClick" and
"RadioButtonMenuItem.closeOnMouseClick" properties
  that control toogle menu items closing on mouse click on L&F level.

  The current fix allows to control menu items closing on a component
level. A toogle menu item
  is not closed on mouse click if the corresponding property is set
to false either on the L&F or component level.

 Thanks,
 Alexandr.








--
Best regards, Sergey.


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

2016-09-14 Thread Sergey Bylokhov

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/




Regards,

Rajeev Chamyal



*From:*Alexandr Scherbatiy
*Sent:* 12 September 2016 20:07
*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 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/


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
; Sergey Bylokhov
*Subject:* Re:  [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/


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
; 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:
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).getReso

Re: [9] Review request for 8154043: Fields not reachable anymore by tab-key, because of new tabbing behaviour of radio button groups.

2016-09-14 Thread Alexandr Scherbatiy

On 9/14/2016 6:41 PM, Alexandr Scherbatiy wrote:

On 9/14/2016 1:48 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

webrev: http://cr.openjdk.java.net/~ssadetsky/8154043/webrev/

The new RadioButton group focus traversal algorithm introduced by 
JDK-8033699 doesn't take into account that group of radio buttons can 
be lay-outed in several lines in container. In this case the 
LayoutFocusTraversalPolicy may mix radio buttons of the same group 
with other components in its focus traversal order and break the 
assumption used in BasicRadioButtonUI#getFocusTransferBaseComponent() 
that radio buttons are sequenced in the focus policy order.


The fix propose to treat a group of toggle buttons as a single focus 
cycle entry in the LayoutFocusTraversalPolicy which order is 
determined by the first toggle button in the group.
  Should the test for the fix JDK-8033699 
test/javax/swing/JRadioButton/8033699/bug8033699.java be updated as well?
  Is it possible to move the focus from the current radio button to the 
next component which does not belong to the current radio buttons group 
by pressing tab key?
  For example in the test FocusCycleRootTest from the issue 
description, moving the focus from the first and the second radio button 
to the text field 1 and from the third and the fourth radio button to 
the text field 2.


  Thanks,
  Alexandr.


  Thanks,
  Alexandr.


--Semyon








Re: javax.swing.text.html.parser.NPrintWriter

2016-09-14 Thread Alexandr Scherbatiy

On 9/14/2016 7:35 PM, Sergey Bylokhov wrote:

I think that NPrintWriter is not public and is not used in jdk.

  May be it it has sense just to remove it?

  Thanks,
  Alexandr.


On 14.09.16 19:29, Alexandr Scherbatiy wrote:


Hello Manuel,

Thank you for the feedback. I have filled an issue on it [1]:
  JDK-8166050 partialArray is not created in
javax.swing.text.html.parser.NPrintWriter.println(...) method

  Do you have any particular test which runs into this issue?

  [1] https://bugs.openjdk.java.net/browse/JDK-8166050

  Thanks,
  Alexandr.

On 9/14/2016 11:41 AM, Manuel Kassens wrote:


Hi,

i found an error in javax
.swing
.text
.html
.parser
.NPrintWriter 





void javax .swing
.text
.html
.parser
.NPrintWriter 

.println(char[] 


array)



 *public**void*println(*char*[] array) {

  *if*(*this*.numPrinted>= *this*.numLines) {

*return*;

  }

  *char*[] partialArray= *null*;

  *for*(*int*i= 0; i< array.length; i++) {

*if*(array[i] == '\n') {

 *this*.numPrinted++;

}

*if*(*this*.numPrinted== *this*.numLines) {

/* è*/ partialArray= *new**char*[i];  */* missing code,
without there will be a NPE */*

 System./arraycopy/(array, 0, partialArray, 0, i);

}

  }

  *if*(partialArray!= *null*) {

*super*.print(partialArray);

  }

  *if*(*this*.numPrinted== *this*.numLines) {

*return*;

  }

  *super*.println(array);

  *this*.numPrinted++;

 }












Re: [9] Review request for 8165594 Bad rendering of Swing UI controls with Windows Classic L&F on HiDPI display

2016-09-14 Thread Semyon Sadetsky

Hi Alexander,

When I press the arrow button (for example in the "111" combo) several 
times I can see artifacts. They are well seen in scale x4.


--Semyon


On 9/14/2016 5:39 PM, Alexandr Scherbatiy wrote:


Hello,

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8165594/webrev.01

 - HiDPI icons are only drawn for scaled graphics.

  The screenshots [1], [2], and [3] show difference between icons 
drawing before and after the fix for scales 1x, 2x, and 4x.


  [1] 
http://cr.openjdk.java.net/~alexsch/8165594/screenshots/icons-windows-classic-1x_01.png
  [2] 
http://cr.openjdk.java.net/~alexsch/8165594/screenshots/icons-windows-classic-2x_01.png
  [3] 
http://cr.openjdk.java.net/~alexsch/8165594/screenshots/icons-windows-classic-4x_01.png


  Thanks,
  Alexandr.

On 9/8/2016 10:59 AM, Andrej Golovnin wrote:

Hi Alexandr,


   [1]
http://cr.openjdk.java.net/~alexsch/8165594/screenshots/icons-windows-classic-1x.png 


The icons do not look right to me. Take look at the top-left part of
the radio button. There is a white pixel between the shadow lines. And
in the selected state there should be a black circle. But instead it
is a square. The check sign of the checkbox is too thin. And the
arrows of the combobox and the vertical scroll bar should have a
single pixel at the top/bottom side. But now they have two pixels.

It would be also nice to see a screen shot of the native Windows
components for comparison.

Personally when I would make changes like that, then my code would
look like this:

if (isNotHiDPI() || itMakesMoreSenseToUseTheOldCode()) {
// use the old good code.
} else {
// use the new code
}

Best regards,
Andrej Golovnin


   [2]
http://cr.openjdk.java.net/~alexsch/8165594/screenshots/icons-windows-classic-2x.png 


   [3]
http://cr.openjdk.java.net/~alexsch/8165594/screenshots/icons-windows-classic-4x.png 



  Thanks,
  Alexandr.








Re: javax.swing.text.html.parser.NPrintWriter

2016-09-14 Thread Sergey Bylokhov

I think that NPrintWriter is not public and is not used in jdk.

On 14.09.16 19:29, Alexandr Scherbatiy wrote:


Hello Manuel,

Thank you for the feedback. I have filled an issue on it [1]:
  JDK-8166050 partialArray is not created in
javax.swing.text.html.parser.NPrintWriter.println(...) method

  Do you have any particular test which runs into this issue?

  [1] https://bugs.openjdk.java.net/browse/JDK-8166050

  Thanks,
  Alexandr.

On 9/14/2016 11:41 AM, Manuel Kassens wrote:


Hi,

i found an error in javax
.swing
.text
.html
.parser
.NPrintWriter



void javax .swing
.text
.html
.parser
.NPrintWriter
.println(char[]
array)



 *public**void*println(*char*[] array) {

  *if*(*this*.numPrinted>= *this*.numLines) {

*return*;

  }

  *char*[] partialArray= *null*;

  *for*(*int*i= 0; i< array.length; i++) {

*if*(array[i] == '\n') {

 *this*.numPrinted++;

}

*if*(*this*.numPrinted== *this*.numLines) {

/* è*/ partialArray= *new**char*[i];  */* missing code,
without there will be a NPE */*

 System./arraycopy/(array, 0, partialArray, 0, i);

}

  }

  *if*(partialArray!= *null*) {

*super*.print(partialArray);

  }

  *if*(*this*.numPrinted== *this*.numLines) {

*return*;

  }

  *super*.println(array);

  *this*.numPrinted++;

 }








--
Best regards, Sergey.


Re: javax.swing.text.html.parser.NPrintWriter

2016-09-14 Thread Alexandr Scherbatiy


Hello Manuel,

Thank you for the feedback. I have filled an issue on it [1]:
  JDK-8166050 partialArray is not created in 
javax.swing.text.html.parser.NPrintWriter.println(...) method


  Do you have any particular test which runs into this issue?

  [1] https://bugs.openjdk.java.net/browse/JDK-8166050

  Thanks,
  Alexandr.

On 9/14/2016 11:41 AM, Manuel Kassens wrote:


Hi,

i found an error in javax 
.swing 
.text 
.html 
.parser 
.NPrintWriter


void javax .swing 
.text 
.html 
.parser 
.NPrintWriter 
.println(char[] 
array)


*public**void*println(*char*[] array) {

*if*(*this*.numPrinted>= *this*.numLines) {

*return*;

}

*char*[] partialArray= *null*;

*for*(*int*i= 0; i< array.length; i++) {

*if*(array[i] == '\n') {

*this*.numPrinted++;

}

*if*(*this*.numPrinted== *this*.numLines) {

/* è*/ partialArray= *new**char*[i]; */* missing code, without there 
will be a NPE */*


System./arraycopy/(array, 0, partialArray, 0, i);

}

}

*if*(partialArray!= *null*) {

*super*.print(partialArray);

}

*if*(*this*.numPrinted== *this*.numLines) {

*return*;

}

*super*.println(array);

*this*.numPrinted++;

}





Re: [9] Review request for 8154043: Fields not reachable anymore by tab-key, because of new tabbing behaviour of radio button groups.

2016-09-14 Thread Alexandr Scherbatiy

On 9/14/2016 1:48 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

webrev: http://cr.openjdk.java.net/~ssadetsky/8154043/webrev/

The new RadioButton group focus traversal algorithm introduced by 
JDK-8033699 doesn't take into account that group of radio buttons can 
be lay-outed in several lines in container. In this case the 
LayoutFocusTraversalPolicy may mix radio buttons of the same group 
with other components in its focus traversal order and break the 
assumption used in BasicRadioButtonUI#getFocusTransferBaseComponent() 
that radio buttons are sequenced in the focus policy order.


The fix propose to treat a group of toggle buttons as a single focus 
cycle entry in the LayoutFocusTraversalPolicy which order is 
determined by the first toggle button in the group.
  Should the test for the fix JDK-8033699 
test/javax/swing/JRadioButton/8033699/bug8033699.java be updated as well?


  Thanks,
  Alexandr.


--Semyon






javax.swing.text.html.parser.NPrintWriter

2016-09-14 Thread Manuel Kassens
Hi,
i found an error in 
javax.swing.text.html.parser.NPrintWriter

void 
javax.swing.text.html.parser.NPrintWriter.println(char[]
 array)

 public void println(char[] array) {
  if (this.numPrinted >= this.numLines) {
return;
  }
  char[] partialArray = null;
  for (int i = 0; i < array.length; i++) {
if (array[i] == '\n') {
 this.numPrinted++;
}
if (this.numPrinted == this.numLines) {
/* ==> */ partialArray = new char[i];  /* missing code, without 
there will be a NPE */
 System.arraycopy(array, 0, partialArray, 0, i);
}
  }
  if (partialArray != null) {
super.print(partialArray);
  }
  if (this.numPrinted == this.numLines) {
return;
  }
  super.println(array);
  this.numPrinted++;
 }



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

2016-09-14 Thread Alexandr Scherbatiy

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/ 



Regards,

Rajeev Chamyal

*From:*Alexandr Scherbatiy
*Sent:* 12 September 2016 20:07
*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 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/


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
; Sergey Bylokhov
*Subject:* Re:  [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/


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
; 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:
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 * 

Re: [9] Review request for 8165594 Bad rendering of Swing UI controls with Windows Classic L&F on HiDPI display

2016-09-14 Thread Alexandr Scherbatiy


Hello,

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8165594/webrev.01

 - HiDPI icons are only drawn for scaled graphics.

  The screenshots [1], [2], and [3] show difference between icons 
drawing before and after the fix for scales 1x, 2x, and 4x.


  [1] 
http://cr.openjdk.java.net/~alexsch/8165594/screenshots/icons-windows-classic-1x_01.png
  [2] 
http://cr.openjdk.java.net/~alexsch/8165594/screenshots/icons-windows-classic-2x_01.png
  [3] 
http://cr.openjdk.java.net/~alexsch/8165594/screenshots/icons-windows-classic-4x_01.png


  Thanks,
  Alexandr.

On 9/8/2016 10:59 AM, Andrej Golovnin wrote:

Hi Alexandr,


   [1]
http://cr.openjdk.java.net/~alexsch/8165594/screenshots/icons-windows-classic-1x.png

The icons do not look right to me. Take look at the top-left part of
the radio button. There is a white pixel between the shadow lines. And
in the selected state there should be a black circle. But instead it
is a square. The check sign of the checkbox is too thin. And the
arrows of the combobox and the vertical scroll bar should have a
single pixel at the top/bottom side. But now they have two pixels.

It would be also nice to see a screen shot of the native Windows
components for comparison.

Personally when I would make changes like that, then my code would
look like this:

if (isNotHiDPI() || itMakesMoreSenseToUseTheOldCode()) {
// use the old good code.
} else {
// use the new code
}

Best regards,
Andrej Golovnin


   [2]
http://cr.openjdk.java.net/~alexsch/8165594/screenshots/icons-windows-classic-2x.png
   [3]
http://cr.openjdk.java.net/~alexsch/8165594/screenshots/icons-windows-classic-4x.png

  Thanks,
  Alexandr.






[9] Review request for 8154043: Fields not reachable anymore by tab-key, because of new tabbing behaviour of radio button groups.

2016-09-14 Thread Semyon Sadetsky

Hello,

Please review fix for JDK9:

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

webrev: http://cr.openjdk.java.net/~ssadetsky/8154043/webrev/

The new RadioButton group focus traversal algorithm introduced by 
JDK-8033699 doesn't take into account that group of radio buttons can be 
lay-outed in several lines in container. In this case the 
LayoutFocusTraversalPolicy may mix radio buttons of the same group with 
other components in its focus traversal order and break the assumption 
used in BasicRadioButtonUI#getFocusTransferBaseComponent() that radio 
buttons are sequenced in the focus policy order.


The fix propose to treat a group of toggle buttons as a single focus 
cycle entry in the LayoutFocusTraversalPolicy which order is determined 
by the first toggle button in the group.


--Semyon




[9] fix for JDK-8134612 :clipboard.getData(dataFlavor) can throw UnsupportedFlavorException for registered data flavor

2016-09-14 Thread Ajit Ghaisas
Hi,

 

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

 

Issue :

In this test, exportToClipboard() does not export anything to the clipboard due 
to incorrect text passed to TransferHandler.

Obviously, when we do clipboard.getData() - it throws 
UnsupportedFlavorException. This is the root cause.

Also, when text is imported, the text String cannot be assigned to 
MyStringReader class.

 

Fix :

The test is corrected to use custom dataflavor containing String to export and 
import from clipboard.

Also, the test is enhanced to test a custom dataflavor of Color.

I have referred to :

https://docs.oracle.com/javase/tutorial/uiswing/dnd/dataflavor.html

 

It passes consistently on Windows, Linux and Mac.

 

Webrev :

http://cr.openjdk.java.net/~aghaisas/8134612/webrev.00/

 

Request you to review.

 

Regards,

Ajit

 


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

2016-09-14 Thread Rajeev Chamyal
Hello Alexandr,

 

Thanks for the review.

Please review the webrev updated as per review comments.

http://cr.openjdk.java.net/~rchamyal/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:  [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.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.02/"http://cr.openjdk.java.net/~rchamyal/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; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"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/13/2016 3:18 PM, Rajeev Chamyal wrote:




Hello Alexandr,

 

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

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.01/"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.


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; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"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] Review request for 8157065: There is no the focus border on the selected tab.

2016-09-14 Thread Semyon Sadetsky



On 9/13/2016 9:03 PM, Alexandr Scherbatiy wrote:

On 9/13/2016 8:49 PM, Semyon Sadetsky wrote:

On 9/13/2016 8:46 PM, Alexandr Scherbatiy wrote:

On 9/13/2016 8:34 PM, Semyon Sadetsky wrote:


On 9/13/2016 8:25 PM, Alexandr Scherbatiy wrote:

On 9/13/2016 7:38 PM, Semyon Sadetsky wrote:



On 9/13/2016 7:21 PM, Alexandr Scherbatiy wrote:

On 9/12/2016 10:42 PM, Semyon Sadetsky wrote:



On 9/12/2016 9:48 PM, Alexandr Scherbatiy wrote:

On 9/12/2016 7:52 PM, Semyon Sadetsky wrote:

On 9/12/2016 6:50 PM, Alexandr Scherbatiy wrote:


On 9/12/2016 6:42 PM, Semyon Sadetsky wrote:
GTKPainter does not implement a lot of methods which can be 
accessed by public API. Could you, please, explain, why 
this specific method is more important than, for example, 
paintToolBarContentBackground() or 
paintToggleButtonBorder(), or all other unimplemented?


In general, how do you separate public API methods of the 
SynthPainter class into two sets: the first set that 
*should be* over-riden and the second set of methods that 
*should not be* overr-riden? Are there any systematic 
criterium for that differentiation?
  All the same methods with different number of arguments 
which do not fall to overridden implementation should be 
overridden to provide proper implementation.
Where I can read about this rule for SynthPainter? And it 
obviously is not true.
  This is a usual rule for public methods which can be used by 
an external application.
There are a lot of methods that are not over-riden in 
GTKPainter. I even wrote an examples above.
  The SynthPainter.paintToolBarContentBackground(..., 
orientation) calls 
SynthPainter.paintToolBarContentBackground(...) without the 
orientation and the GTKPainter .paintToolBarContentBackground 
overrides the method without the orientation.  So calls to 
gtkPainter.paintToolBarContentBackground(..., orientation) 
falls down to the overriden method in GTKPainter.


 The same is for SynthPainter.paintProgressBarBackground(..., 
orientation) and paintScrollBarBackground(..., orientation) 
methods.


 The SynthPainter has only one paintToggleButtonBorder() method.
Interesting rule... I thought that more specific method version 
may have different implementation.
  It was done for historical reason. For example before the fix 
JDK-5033822 "Synth ScrollBar paintTrack() dosn't support 
orientation"
  there was only paintScrollBarBackground() method without the 
orientation in the SynthPainter class.
  After the fix the paintScrollBarBackground() method with the 
orientation is added which default implementation just calls the 
same method without the orientation because old user's 
subclasses can override the method without the orientation an 
not be aware about new method version.



What would you say about paintSeparatorBackground() ?
It's (..., orientation) version is over-ridden while the 
generic version is not over-ridden.

   I guess that it is just a bug.
Not sure. GTKPainter has never been providing a systematic API 
from the beginning. It is not for an arbitrary external use, 
because the resulting painting will be unpredictable. I explained 
this in this thread many times. And even if I would like to 
provide this useless method implementation 
  It is a part of the public SynthPainter API and can be called by 
an external application.
I can be called without any predictable result.  This is the 
current state. It cannot be changed by the change you are proposing 
to add.

  The result is described in the public method javadoc.
I were not able to do this because the orientation is a required 
parameter to paint the GTK tab border.
  The overridden method without the orientation can just call the 
overridden method with orientation passing a default orientation 
value.

And what the default value is? It is not defined.

https://docs.oracle.com/javase/7/docs/api/javax/swing/JSeparator.html
Creates a new horizontal separator: Constructor JSeparator().
Sorry, I didn't get how the separator orientation is related to 
tabbed pane border.
https://docs.oracle.com/javase/7/docs/api/javax/swing/JTabbedPane.html#setTabPlacement(int) 


The default value, if not set, is SwingConstants.TOP.
It is a default for JTabbedPane but not for the LnF. JTabbedPane always 
has some orientation and it is used in the painter. But I cannot even 
imagine what should be painted if the GTK painter API is called to paint 
TabbedPane without orientation and why. Because orientation is mandatory 
in the corresponding gtk-lib call.


--Semyon


  Thanks,
  Alexandr.


--Semyon


  Thanks,
  Alexandr.


--Semyon


  Thanks,
  Alexandr.


--Semyon


  Thanks,
  Alexandr.


--Semyon


 Thanks,
 Alexandr.




--Semyon


  Thanks,
  Alexandr.


--Semyon

On 9/12/2016 6:20 PM, Alexandr Scherbatiy wrote:
The paintTabbedPaneTabBorder() without orientation should 
be implemented as well because it can be accessed by 
public API.


Thanks,
Alexandr.

On 6/3/2016 10:54 PM, Semyon Sadetsky wrote:



On 6/3/2016 10:34 P