Re: Review Request JDK-8159168 [hidpi] Window.setShape() works incorrectly on HiDPI

[Rajeev Chamyal]

Hello Alexandr,

 

Thanks for the review.

As per windows specification X & Y scale are always equal that's why I have put 
scaleX == scaleY check.

But it may change in future so I have removed this check.

 

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

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 04 July 2016 18:03
To: Rajeev Chamyal; swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  Review Request JDK-8159168 [hidpi] Window.setShape() 
works incorrectly on HiDPI

 

On 7/4/2016 3:09 PM, Rajeev Chamyal wrote:



Hello All,

 

Please review the following webrev.

 

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

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8159168/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8159168/webrev.00/
 

 

Issue: In HiDPI screen shape set through window::setShape API is not scaled 
based on system scale.

Fix:. Updated the WComponentPeer::applyShape to update shape based on system 
scale. 

1131 double scaleX = 
winGraphicsConfig.getDefaultTransform().getScaleX();
1132 double scaleY = 
winGraphicsConfig.getDefaultTransform().getScaleY();

 The getDefaultTransform() is called twice which leads that AffineTransform 
object is created two times
1133 if (scaleX != 1 && scaleY != 1 && scaleX == scaleY) {

   Is the check scaleX == scaleY really necessary here?

   Is it possible to make the test automated? Just run it with option "@run 
main/othervm -Dsun.java2d.uiScale=2 TestName" and check the area where the 
shape is drawn?

  Thanks,
  Alexandr.



 

Regards,

Rajeev Chamyal

 

 

Re: Review Request JDK-8159168 [hidpi] Window.setShape() works incorrectly on HiDPI

[Alexandr Scherbatiy]

On 7/4/2016 3:09 PM, Rajeev Chamyal wrote:


Hello All,

Please review the following webrev.

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

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



Issue: In HiDPI screen shape set through window::setShape API is not 
scaled based on system scale.


Fix:. Updated the WComponentPeer::applyShape to update shape based on 
system scale.


1131 double scaleX = 
winGraphicsConfig.getDefaultTransform().getScaleX();
1132 double scaleY = 
winGraphicsConfig.getDefaultTransform().getScaleY();


 The getDefaultTransform() is called twice which leads that 
AffineTransform object is created two times

1133 if (scaleX != 1 && scaleY != 1 && scaleX == scaleY) {

   Is the check scaleX == scaleY really necessary here?

   Is it possible to make the test automated? Just run it with option 
"@run main/othervm -Dsun.java2d.uiScale=2 TestName" and check the area 
where the shape is drawn?


  Thanks,
  Alexandr.


Regards,

Rajeev Chamyal

Review Request JDK-8159168 [hidpi] Window.setShape() works incorrectly on HiDPI

[Rajeev Chamyal]

Hello All,

 

Please review the following webrev.

 

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

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

 

Issue: In HiDPI screen shape set through window::setShape API is not scaled 
based on system scale.

Fix:. Updated the WComponentPeer::applyShape to update shape based on system 
scale. 

 

Regards,

Rajeev Chamyal

 

Re: RfR JDK-8145207 [macosx] JList, VO can't access non-visible list items

[Alexandr Scherbatiy]

On 6/18/2016 5:31 AM, Pete Brunet wrote:

Please review the following patch.

Bug: https://bugs.openjdk.java.net/browse/JDK-8145207
Patch: http://cr.openjdk.java.net/~ptbrunet/JDK-8145207/webrev.00/

This fixes the following functionality that was not working with the
JList of ListDemo of SwingSet2.
- start VoiceOver
- start SwingSet2
- start the ListDemo
- press Tab until focus is on the list, should hear VO when changing
selections with up/down arrow
- when interacting with list should hear that there are 30 (total)
items, not 26 (visible) items
- when using control+option+up/downarrow should be able to move to and
select (control+option+spacebar) non-visible items past the 26th visible
item
- should be able to multi-select both visible and invisible items using
control+option+command+return and VO should read the item just added
- should be able to shift extend items with shift up or shift down arrow
and VO should announce the item just added or removed


CAccessibility:
 639 childrenAndRoles.clear();
 640 childrenAndRoles.addAll(newArray);

- Is it possible just to assign the newArray to the childrenAndRoles? Is 
it necessary that the childrenAndRoles has final keyword?
- Please, format the code on lines 630-631 to romevo unnessary spaces in 
round brackets.


CAccessible:

- static method getActiveDescendant() is not used in the CAccessible 
class but only in CAccessibility. Is it possible to move it to the 
CAccessibility class?

- Please, split the long lines. You may use static imports for constants.

JavaComponentAccessibility:
 716 if (returnValue == -1) {
 717 return NSNotFound;
 718 } else {
 719 return returnValue;
 720 }

 - This can be written shorter: return (returnValue == -1) ? NSNotFound 
: returnValue;


 998 if ([self isSelectable:[ThreadUtilities getJNIEnv]]) {
 999 return YES;
1000 } else {
1001 return NO;
1002 }

- Is there a macros which can convert jboolean to BOOL?
- Could you also split the modified lines where it is possible?

Thanks,
Alexandr.



Pete