Re: Nimbus class extending.

[Alexandr Scherbatiy]

On 5/5/2017 2:08 PM, Ashutosh Pandey wrote:


Hello,

I was wondering if someone can offer an advice on how can I modify 
Nimbus L& F classes.


We have used Nimbus L in a sizable desktop product. But lately our 
customers have been reporting class cast exceptions which upon some 
digging appears to be threading issues.




  Could you provide the code snippet that reproduces the exception?

  You can also file an issue on  http://bugreport.java.com/bugreport

  Thanks,
  Alexandr.


I am looking to modify some of your classes and will benefit greatly 
from an example if you have one.


Thank you.

Kind Regards,

Ashutosh Pandey

Senior Principal Engineer

cid:image001.png@01D1FA38.4B2FEE40
Causeway Technologies Limited | Frensham House, Farnham Business Park, 
Weydon Lane, GU9 8QT

T: +44 (0) 1628 552000


Connect: Twitter   | Linkedin 
 | Facebook 
  | YouTube 
 | www.causeway.com 






Go green!  - Consider the environment. Please don't print this e-mail 
unless you really need to





Disclaimer Notice :-

The message and any attachments contained in this e-mail are intended 
for the named recipient(s) only. It may contain privileged or 
confidential information or information which is exempt from 
disclosure under the applicable laws. If you are not the intended 
recipient(s), you must not read, print, retain, copy distribute, 
forward or take any or refrain from taking any action in reliance on 
it or any of its attachments. If you have received or have been 
forwarded this e-mail in error, please notify us immediately by return 
e-mail or telephone (+44 (0)1628 552000) and delete this message from 
the computer or any other data-reading device in its entirety.


Please advise us immediately if you do not or your employer does not 
consent to Internet e-mail for messages of this nature.


Internet communications cannot be guaranteed to be secure and 
error-free as the information could be intercepted, corrupted, lost, 
arrive late or contain viruses. The sender and this Company therefore 
do not and shall not accept any liability or responsibility of 
whatsoever nature in the context of this message and its attachment(s) 
which arises as a result of Internet transmission. Opinions, 
conclusion, representations, views and such other information in this 
message that do not relate to the official business of this Company 
shall be understood as neither given nor endorsed by it.


Registered Office: Comino House, Furlong Road, Bourne End, 
Buckinghamshire, SL8 5AQ

Registered in England No: 3921897 www.causeway.com

Re: [9] RFR Backport of 8160893: [macosx] JMenuItems in JPopupMenu are not accessible

[Alexandr Scherbatiy]

On 4/27/2017 3:34 PM, Mikhail Cherkasov wrote:

Hi all,

Could you please review a backport of 8160893 to jdk8?


   The backport looks good to me.

  Should the email subject be [8] RFR Backport...?

  Thanks,
  Alexandr.

The fix is the same, the only difference in CAccessibility.java
http://cr.openjdk.java.net/~mcherkas/8160893/webrev/src/macosx/classes/sun/lwawt/macosx/CAccessibility.java.frames.html 



in jdk9 fix two imports are removed:
-import java.lang.reflect.Field;
-import sun.awt.AWTAccessor;

but in jdk8 "import sun.awt.AWTAccessor;" has already been removed
and java.lang.reflect.Field is used in other code, so I left it.

jbs: https://bugs.openjdk.java.net/browse/JDK-8160893
jdk9 changeset: 
http://hg.openjdk.java.net/jdk9/client/jdk/rev/e5be7a186fcc

webrev: http://cr.openjdk.java.net/~mcherkas/8160893/webrev/

Thanks,
Mikhail.

Re: [9] Review request for 8179027: JComboBox too small under Windows LAF

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 4/25/2017 6:55 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

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

This a regression introduced by JDK-6490753. The updated JComboBox 
look and feel affected JComboBox preferred size calculation. In the 
fix the size calculation was updated to fit the new L


--Semyon

Re: [9] Review request for 8140237: [TEST_BUG]Test javax/swing/plaf/nimbus/8041642/bug8041642.java fails for OEL 7

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 4/21/2017 8:29 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

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

Delay was fixed to improve test stability on OEL7.

--Semyon

Re: [9] Review Request: 8076249: NPE in AccessBridge while editing JList model

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 4/13/2017 6:57 PM, Mikhail Cherkasov wrote:



On 4/13/2017 6:31 PM, Phil Race wrote:

   31 import javax.swing.*;
Fixed: 
http://cr.openjdk.java.net/~mcherkas/8076249/9/webrev.02/test/javax/accessibility/JList/AccessibleJListChildNPETest.java.html

Re: JDK-8178091 : Bug I will workin on

[Alexandr Scherbatiy]

Hi Patrick,

Please, sign the OCA [1], use the JDK 10 client repository [2] and 
prepare the webrev [3] of the fix with the test.


[1] http://www.oracle.com/technetwork/community/oca-486395.html
[2] http://hg.openjdk.java.net/jdk10/client
[3] http://openjdk.java.net/guide/webrevHelp.html

Thanks,
Alexandr.

On 4/10/2017 1:22 PM, Patrick Chen wrote:
(edit : for example this game coded in java : 
https://github.com/cloudStrif/GoldenSunD will work with java 7

but clearly not with java8 (linux 64 bits) because of lags)

2017-04-10 12:19 GMT+02:00 Patrick Chen >:


Hi every one ,
just wanted to inform that I am working to fix this bug.

it is when we devellop animations thanks to repaint() method ,
for java 7 it works well
but with java8 not ,
(linux 64 bits it doesn't really work )

so after watching the source code it seem that it is not a swing
problem
but AWT : Component.java .

thank you

Re: [9] Review Request: 8177450: javax.swing.text.html.parser.Parser parseScript ignores a character after commend end

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexndr.

On 4/9/2017 10:33 PM, Mikhail Cherkasov wrote:

Ok, thank you!

On 4/9/2017 9:41 PM, Philip Race wrote:

No more comments. That's fine.

-phil.

On 4/9/17, 11:24 AM, Mikhail Cherkasov wrote:

Hi Phil,

Do you have any other comments about the fix?

Thanks,
Mikhail.

On 4/6/2017 10:59 AM, Mikhail Cherkasov wrote:
I re-named the test: 
http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev.02


On 4/5/2017 11:46 PM, Phil Race wrote:

Ok. Sorry there is one more thing.
I know its a sustaining habit but we've previously advised to stop
using the bug id as the test name. Instead use a short name which
is relevant to what is being tested. eg 'HtmlCommentParseTest.java'
The bug ID is in the @bug tag for anyone who needs to know it.

-phil.

On 04/05/2017 03:36 AM, Mikhail Cherkasov wrote:

Hi Phil,

On 4/4/2017 9:13 PM, Phil Race wrote:

2122 if(i > 0) { should be "if (i > 0) {"

fixed: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev.01/
Can you comment on what tests have been run to ensure this does 
not regress anything else ?
I run tests from " test/javax/swing/text/html/parser/" and from 
"test/closed/javax/swing/text/html/parser/"
no new failures, also I run swing JCK tests and again no new 
failures.

-phil.


On 03/31/2017 04:28 AM, Mikhail Cherkasov wrote:

Hello,

Could you please review the fix for jdk9?

The problem is that Parser reads extra symbol after comment or script tag and 
missed the symbol.
I added 'continue' after those tags, so it goes to beginning of Parser while 
loop and process the symbol.
   
Bug:https://bugs.openjdk.java.net/browse/JDK-8177450

Webrev:http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev/

Thanks,
Mikhail.

Re: [9] Review Request: 8177450: javax.swing.text.html.parser.Parser parseScript ignores a character after commend end

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 3/31/2017 2:28 PM, Mikhail Cherkasov wrote:

Hello,

Could you please review the fix for jdk9?

The problem is that Parser reads extra symbol after comment or script tag and 
missed the symbol.
I added 'continue' after those tags, so it goes to beginning of Parser while 
loop and process the symbol.
   
Bug:https://bugs.openjdk.java.net/browse/JDK-8177450

Webrev:http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev/

Thanks,
Mikhail.

Re: RFR: 8043773: Deprecate JComponent.AccessibleJComponent.AccessibleFocusHandler

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 3/28/2017 1:41 AM, Phil Race wrote:

webrev: http://cr.openjdk.java.net/~prr/8043773/
bug : https://bugs.openjdk.java.net/browse/JDK-8043773

This just deprecates the un-used, un-needed inner classs.

A CCC was approved ages ago however the fix was never pushed and I 
can't find

any evidence a code review was even submitted.

Since this is really just a doc change we should be OK to still fix this

I've done a full build including docs + it looks fine.

-phil.

Re: [9] Review request for 8176883 Enable antialiasing for Metal L icons on HiDPI display

[Alexandr Scherbatiy]

On 3/16/2017 9:16 PM, Phil Race wrote:

Hi,

What I was referring to regarding "draws using G2D calls" was that 
paintIcon(..) looks like this



1335 Object aaHint = getAndSetAntialisingHintForScaledGraphics(g);
1336
1337 if (MetalLookAndFeel.usingOcean()) {
1338 paintOceanIcon(c, g, x, y);
1339 setAntialiasingHintForScaledGraphics(g, aaHint);
1340 return;
1341 }
...
..
1386 g.setColor(dotColor);
1387 g.fillOval(2, 2, 7, 7);  << Why does 
Graphics.fillOval (etc) need to be A-Aed here ?

   The graphics has floating point scale on HiDPI display with 150% scale.
   The given oval is the center circle in the radio button. It is not 
painted smooth with the UI scale 1.5.
   I prepared the  screenshot with a radio button (top is before the 
fix and bottom is after the fix with AA rendering hint enabled)

http://cr.openjdk.java.net/~alexsch/8176883/screenshots/00/metal-radio-button-150pcs.png

  Thanks,
  Alexandr.

1388 }
1389
1390 g.translate(-x, -y);
1391 setAntialiasingHintForScaledGraphics(g, aaHint);
1392 }

-phil.

On 03/16/2017 11:04 AM, Alexandr Scherbatiy wrote:


  Hello,

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

  - The SwingUtilities2.getAndSetAntialisingHintForScaledGraphics() 
sets the AA rendering hint only for local display.
  - The SwingUtilities2.setAntialiasingHintForScaledGraphics() method 
javadoc is updated to mention that the null AA rendering hint means 
that the passed graphics is not instance of Graphics2D.


On 3/16/2017 7:47 PM, Phil Race wrote:


Hi,

It looks to me as if this applies AA rendering even when you
are drawing using G2D calls. Why ?
  I updated the methods javadoc. The passed aaHint to the method 
setAntialiasingHintForScaledGraphics() must be null for the non 
Graphics2D.


Also this clearly only helps Metal. Is that because only Metal has 
such problems ?

I would doubt that.

FWIW Motif L probably should not get this treatment.

   The The Windows L already uses the AA hints by the fix JDK-8165594.
   The GTK L uses only integer UI scale factor.
Also if the desktop is "remote" we should avoid AA. I believe we do 
that

for text with Metal so there should be example of how to detect that
somewhere.

  I updated the fix.

  Thanks,
  Alexandr.


-phil.

On 03/16/2017 09:03 AM, Alexandr Scherbatiy wrote:


Hello,

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

  Metal RadioButton and RadioButtonMenuItem icons are not drawn 
smoothly on HiDPI displays especially with floating point scales 
like 1.25 and 1.5.
  The fix enables the antialiasing rendering hint for the radio 
button and radio button menu item if the passed graphics is scaled.


  The [1] folder contains screenshots how icons are drawn before 
the fix (on the left side) and after the fix (on the right side).


  [1] http://cr.openjdk.java.net/~alexsch/8176883/screenshots/00

Thanks,
Alexandr.

Re: [9] Review request for 8176883 Enable antialiasing for Metal L icons on HiDPI display

[Alexandr Scherbatiy]

  Hello,

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

  - The SwingUtilities2.getAndSetAntialisingHintForScaledGraphics() 
sets the AA rendering hint only for local display.
  - The SwingUtilities2.setAntialiasingHintForScaledGraphics() method 
javadoc is updated to mention that the null AA rendering hint means that 
the passed graphics is not instance of Graphics2D.


On 3/16/2017 7:47 PM, Phil Race wrote:


Hi,

It looks to me as if this applies AA rendering even when you
are drawing using G2D calls. Why ?
  I updated the methods javadoc. The passed aaHint to the method 
setAntialiasingHintForScaledGraphics() must be null for the non Graphics2D.


Also this clearly only helps Metal. Is that because only Metal has 
such problems ?

I would doubt that.

FWIW Motif L probably should not get this treatment.

   The The Windows L already uses the AA hints by the fix JDK-8165594.
   The GTK L uses only integer UI scale factor.

Also if the desktop is "remote" we should avoid AA. I believe we do that
for text with Metal so there should be example of how to detect that
somewhere.

  I updated the fix.

  Thanks,
  Alexandr.


-phil.

On 03/16/2017 09:03 AM, Alexandr Scherbatiy wrote:


Hello,

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

  Metal RadioButton and RadioButtonMenuItem icons are not drawn 
smoothly on HiDPI displays especially with floating point scales like 
1.25 and 1.5.
  The fix enables the antialiasing rendering hint for the radio 
button and radio button menu item if the passed graphics is scaled.


  The [1] folder contains screenshots how icons are drawn before the 
fix (on the left side) and after the fix (on the right side).


  [1] http://cr.openjdk.java.net/~alexsch/8176883/screenshots/00

Thanks,
Alexandr.

[9] Review request for 8176883 Enable antialiasing for Metal L icons on HiDPI display

[Alexandr Scherbatiy]

Hello,

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

  Metal RadioButton and RadioButtonMenuItem icons are not drawn 
smoothly on HiDPI displays especially with floating point scales like 
1.25 and 1.5.
  The fix enables the antialiasing rendering hint for the radio button 
and radio button menu item if the passed graphics is scaled.


  The [1] folder contains screenshots how icons are drawn before the 
fix (on the left side) and after the fix (on the right side).


  [1] http://cr.openjdk.java.net/~alexsch/8176883/screenshots/00

Thanks,
Alexandr.

Re: [9] Review request for 8174845 Bad scaling on Windows with large fonts with Java 9ea

[Alexandr Scherbatiy]

I prepared the image [1] which contains the middle.gif icon from the 
MenuSelectionManagerDemo drawn with scales from the top to the bottom 1, 
1.25, 1.5, 2 and
the interpolation rendering hint from the left to the right: not set, 
NEAREST_NEIGHBOR, BILINEAR, BICUBIC.


If it may sense to add some rendering hints for the icons drawing for 
scale higher that one I can file a separate issue on it.


[1] 
http://cr.openjdk.java.net/~alexsch/8174845/image-icons/00/icon-image.png


Thanks,
Alexandr.

On 3/15/2017 8:48 PM, Philip Race wrote:

I see the fix was just pushed. But my comments are clearly not addressed.

-phil.

On 3/15/17, 10:11 AM, Philip Race wrote:

Does this actually try to address everything they complained about here

http://stackoverflow.com/questions/41117190/java-9-on-windows-with-large-fonts 


which points to an image : https://i.stack.imgur.com/fgpBu.png

Check boxes are some of it but not all of it.

There is what appears to be a custom icon there - the black outlined
yellow zig-zg shape - and I don't see how we can draw that ourselves.

Which is why I asked in the bug report if we had looked at how we are 
doing image scaling.
It may be that we are already doing the best we can but I don't know 
that ..


-phil.

On 3/15/17, 8:35 AM, Sergey Bylokhov wrote:

On 3/14/2017 7:31 PM, Sergey Bylokhov wrote:
The initial image at [1] also show some issues in the submenu 
pointer and the menu corners, did we solve them already?
  The submenu icon is updated in the current fix and the issue with 
a menu corner is covered by the fix
JDK-8162350 RepaintManager shifts repainted region when the 
floating point UI scale is used

Thanks, Looks fine.


  Thanks,
  Alexandr.
[1] 
http://stackoverflow.com/questions/41117190/java-9-on-windows-with-large-fonts



Hello,

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


Metal JCheckBox and JMenuItem icons are updated to be drawn by 
ovals, arcs and polygons.


The [1] folder contains screenshots how icons are drawn before 
the fix (on the left side) and after the fix (on the right side) 
with Default and Ocean Metal themes.


[1] 
http://cr.openjdk.java.net/~alexsch/8174845/screenshots/00


Thanks,
Alexandr.

Re: [9] Review Request: 8176448 [macos] Popups in JCombobox and Choice have incorrect location in multiscreen systems

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 3/14/2017 6:12 PM, Sergey Bylokhov wrote:

Hello,
Please review the fix for jdk9.
In the fixes for JDK-7072653 [1] and JDK-8129838 [2] and JDK-8144161[3]

[1] http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/7606d0af7b80

Notes about JDK-7072653
 - AquaComboBoxPopup.java the code "if (py + ph > scrBounds.y + 
scrBounds.height)» compares the vars in a different coordinate 
spaces(combobox vs screen). It is ok in BasicComboPopup.java because 
we translate scrBounds properly.
 - In Aqua the code was changed for the case when 
"JComboBox.isPopDown» property is false, I will create a separate CR 
to fix it when the property is true.


[2] http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/5ea7cbf6a7bd

Notes about JDK-8129838
 - AquaComboBoxPopup.java the second loop was changed to use 
"intersects" and "contains" at the same time, but if the point is 
located on the screen it will be found in the first loop, but if it 
was not found the second loop will be noop.
 - Some code was extracted to the new method getAvailableScreenArea(), 
but it was copied from the place where the only one screen was 
available, so it uses «0» as a left point and does not take into 
account that the screen can have non-zero offset.


[3] hg.openjdk.java.net/jdk9/jdk9/jdk/rev/e8e9c65def6d 



Notes about JDK-8144161:
 - After JDK-7072653 [1] and JDK-8129838 were pushed the test which 
was pushed started to fail on OS X, and this was considered as a 
testbug, even if the code in Aqua l was changed to support this 
functionality.
 - The test was changed to use a different look and feels, but since 
it run the test code a few times it start to work incorrectly, because 
 JFrame.getWindows() can return popup window from the different 
L(dispose was called on it but it was not garbage collected).


Fix description:
  - An additional check in the second loop in AquaComboBoxPopup.java 
was removed.
  - AquaComboBoxPopup.getAvailableScreenArea now takes into account 
that we can have a few screens.
  - AquaComboBoxPopup.computePopupBounds() was changed to take into 
account that scrBounds is in the screen coordinates space.

  - The test bug7072653.java now check all screen in the system.
  - Note that the new test ChoicePopupLocation.java will fail on linux 
in multiscreen because of JDK-8160270. I will fix it after this one.



Bug: https://bugs.openjdk.java.net/browse/JDK-8176448
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8176448/webrev.04/ 

Re: [9] Review request for 8174845 Bad scaling on Windows with large fonts with Java 9ea

[Alexandr Scherbatiy]

On 3/14/2017 7:31 PM, Sergey Bylokhov wrote:
The initial image at [1] also show some issues in the submenu pointer 
and the menu corners, did we solve them already?
  The submenu icon is updated in the current fix and the issue with a 
menu corner is covered by the fix
JDK-8162350 RepaintManager shifts repainted region when the 
floating point UI scale is used


  Thanks,
  Alexandr.


[1] 
http://stackoverflow.com/questions/41117190/java-9-on-windows-with-large-fonts




Hello,

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



 Metal JCheckBox and JMenuItem icons are updated to be drawn by 
ovals, arcs and polygons.


 The [1] folder contains screenshots how icons are drawn before the 
fix (on the left side) and after the fix (on the right side) with 
Default and Ocean Metal themes.


 [1] http://cr.openjdk.java.net/~alexsch/8174845/screenshots/00 



Thanks,
Alexandr.

Re: [9] JDK-8169897: [PIT] javax/swing/plaf/basic/BasicGraphicsUtils/8132119/bug8132119.java fails

[Alexandr Scherbatiy]

On 3/14/2017 3:37 PM, Philip Race wrote:

I am not sure why the test went to the trouble of looking for Arial.
If there was a good reason (Alexander ??) an alternative is to initialise
  The test tries to calculate number of intersection with letter O and 
its underline. It is sensitive to the position of the letter.


  Thanks,
  Alexandr.

String fontName = "Serif".

although swapping out Arial for Serif is a very odd choice.
Arial is a Sans Serif font and Serif fonts are not usually used in UIs.

So "SansSerif" would be better
-phil.

On 3/14/17, 4:46 AM, Prasanta Sadhukhan wrote:

Hi All,

Please review a testbug fix where the testcase is failing in linux 
because it is not able to find "Arial" font and tries to use the font 
found in 0th index of getAvailableFontFamilyNames()

which is "Abyssinica SIL".

Bug: https://bugs.openjdk.java.net/browse/JDK-8169897
webrev: http://cr.openjdk.java.net/~psadhukhan/8169897/webrev.00/

Modified the testcode to use "Serif" which is present in all 
platforms. Tested in windows,linux,mac.


Regards
Prasanta

[9] Review request for 8174845 Bad scaling on Windows with large fonts with Java 9ea

[Alexandr Scherbatiy]

Hello,

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

  Metal JCheckBox and JMenuItem icons are updated to be drawn by ovals, 
arcs and polygons.


  The [1] folder contains screenshots how icons are drawn before the 
fix (on the left side) and after the fix (on the right side) with 
Default and Ocean Metal themes.


  [1]  http://cr.openjdk.java.net/~alexsch/8174845/screenshots/00

Thanks,
Alexandr.

Re: Java_Swings/AWT Native Interface give error in 32-bit window OS

[Alexandr Scherbatiy]

Forwarded to the awt-dev alias.

Thanks,
Alexandr.

On 3/7/2017 8:50 AM, mahendrag gajera wrote:

Hello All,

I follow step as mention in below link and create .dll and java 
application.


https://en.wikibooks.org/wiki/Java_Swings/AWT. Every things work fine 
in window 7/8 64-bit OS.


In window7 32-bit OS, It work after below change in code.

*public class JavaSideCanvas extends Canvas {*
*
*
*static {*
*System.loadLibrary("jawt");*
*System.loadLibrary("NativeSideCanvas");*
*}*
*
*
*public native void paint(Graphics g);*
*
*
*public static void main(String[] args) {*
*Frame frame = new Frame();*
*frame.setBounds(0, 0, 500, 500);*
*JavaSideCanvas jsc = new JavaSideCanvas();*
*frame.add(jsc);*
*frame.addWindowListener(new WindowAdapter() {*
*public void windowClosing(WindowEvent ev) {*
*System.exit(0);*
*}*
*});*
*frame.show();*
*}*
*}*
But it is not working in window8 32-bit OS.

I found the cause,*jawt.lib* is giving error. If i remove from linker 
setting of my project,then it works fine


Can any one help me why it is not working in window8 32-OS only?.
or
Can i write own *JAWT_GetAWT *method for window? So no need to link 
*jawt.lib.*


*Environment *
Project compiled in *jdk-7u55-windows-x86*.
Using 32 bit JRE 6/7/8 in window8 32-bit OS

Please look the attached code for the same.


Thanks in Advance

Re: [9] Review Request: 8176177 The new SwingContainer annotation can be removed from javax.accessibility.AccessibleContext

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 3/6/2017 5:00 PM, Sergey Bylokhov wrote:

Hello,
Please review the fix for jdk9.
When the new annotation SwingContainer was added to the Swing library, we 
applied it to all classes where the old compile time bean-info has an 
isContainer=false attribute.
The goal of this annotation is to point that some class which extends Container 
in the Swing library is not actually a container. So it is an error to mark the 
class if is not a container.

In the fix this annotation was removed from 
javax.accessibility.AccessibleContext.

Bug: https://bugs.openjdk.java.net/browse/JDK-8176177
Webrev can be found at: http://cr.openjdk.java.net/~serb/8176177/webrev.00

CCC will be filed after technical review.

Re: [9] Review Request: 8158209 Editing in TableView breaks the layout, when the document is I18n

[Alexandr Scherbatiy]

documents asked by Alexandr about my fix.
Yours faithfully.
---
Hello,

Could you apply the fix to JDK 9 client repository ...
I applied the fix to JDK 9 client repository 
http://hg.openjdk.java.net/jdk9/client :

openjdk version "9-internal"
OpenJDK Runtime Environment (build 
9-internal+0-2016-05-28-142420.scientificware2016.9devClient)
OpenJDK 64-Bit Server VM (build 
9-internal+0-2016-05-28-142420.scientificware2016.9devClient, 
mixed mode)
The webrev and JTreport/JTWork are in attachment. All works fine, 
except what is mentioned below.
You mentioned that there are artifacts with a table displaying 
in JDK 9.
I can't say when it appeared because before your fix, the table 
size didn't changed when we insert text and all the text stay on 
the same line.

But, I have just notice that if, in the I18nLayoutTest.java, you
- change the line setUndecorated(true) to setUndecorated(false),
- run the program and enlarge the window 2 times larger than the 
text size,

- all the text come back on the same line.
There no such artifacts when i18n is false.
Thanks for your help.
Guy.
----- 
Le 2016-05-27 16:53, Alexandr Scherbatiy a écrit :
The fix should be prepared for the JDK 9 first and only after 
that be

backported to JDK 8u.
Could you apply the fix to JDK 9 client repository
http://hg.openjdk.java.net/jdk9/client
and send the webrev of the fix  plus the test (see
http://openjdk.java.net/guide/webrevHelp.html).
The fix shouldn't contain comments with the old code lines. I 
believe
that references to the bug number are also unnecessary for this 
case.
You mentioned that there are artifacts with a table displaying 
in JDK
9. Do they appear only after fix of they also present even 
without the

fix?
Thanks,
Alexandr.
On 5/27/2016 9:23 AM, Abossolo Foh Guy wrote:

Hello,
Please, please ... could someone examine the patch for 
JDK-8133864 that I suggested ? I really need it to implement 
matrix display in my application. I've spent long time in 2012 
to find what was wrong in tableview. I know Victor's comment 
was not good but you could compare with the tableview 
implementation in the swing/text/html package which works fine.
Beware, the display (with java 9 119) for I18nLayoutTest.java 
is not the same with i18n true (bad) or i18n false (good). With 
i18n true the table go down.

Best regards.
-- 
Hello,
I built the recent OpenJDK8u and OpenJDK9 sources 
(openjdk_versions.txt) with the patch shown in the 
output_diff.txt attachment.
I applied your patch for JDK-8133864 : Wrong display, when the 
document I18n properties is true.
And I applied my patch for JDK-7169915 : Swing Table Layout bad 
repaint of the row.
All works fine in OpenJDK8u and the document is well displayed. 
The table appears as I expected with all its lines and columns 
(JDK-8133864 solved), size and borders included even if we 
write some text inside (JDK-7169915 solved). I can modify it as 
I want.
But with OpenJDK9, the display of the document is wrong. The 
table appears as I expected with all its lines and columns 
(JDK-8133864 solved), size and borders included (JDK-7169915 
solved) but the entire document display is modified, the table 
is displayed on a new line. And the document became unuseable, 
I can't modify it as I want.
This morning, I ran the same test programme (CodeBug.java in 
attachment) with the Oracle JDK9 b114 (i.e. OpenJDK9 only with 
your patch  for JDK-8133864) : The table appears as I expected 
with all its lines and columns (JDK-8133864 solved) but the 
size and borders are bad repainted when we write some text 
inside (JDK-7169915 solved). And the document became unuseable 
too, I can't modify it as I want.

My conclusion :
1- JDK-8133864 is solved with your PATCH.
2- JDK-7169915 could be solved with my PATCH.
3- There is regression between JDK8 and JDK9 may be in the 
Views arrangement.

Best regards.
-- 
Hello,
Could you apply the modification I suggested 4 years ago about 
the Bug : JDK-7169915 : Swing Table Layout bad repaint of the row.
The test case I had sent in 2012 is the same I used in : 
JDK-8133864 Wrong display, when the document I18n properties is 
true.
A version of this test (called I18nLayoutTest.java) is now 
candidate as part of tests for JDK 9 
(/test/javax/swing/text/TableView/)

Yours faithfully.


--
Abossolo Foh Guy
71 rue Guy de Maupassant
69500 Bron

06 87 01 82 27
04 72 81 89 46

Re: Review request for JDK-6490753 JComboBox doesn't look as native combobox in different states of component

[Alexandr Scherbatiy]

I have uploaded the webrev to the http://cr.openjdk.java.net:
  http://cr.openjdk.java.net/~alexsch/mraz.martin/6490753/webrev.01

Thanks,
Alexandr.

On 2/17/2017 2:41 PM, Martin M wrote:

AnimationController.java
 386 if (skin.haveToSwitchStates()) {
 387 skin.paintSkinRaw(g, dx, dy, dw, dh, state);
 388 g.setComposite(AlphaComposite.SrcOver.derive(1 - progress));
 389 skin.paintSkinRaw(g, dx, dy, dw, dh, startState);
 390 } else {

- Could the '1 - progress' value in AlphaComposite.SrcOver.derive(1 - 
progress) be out of range?


Value 'progress' is checked in method 'private void updateProgress()' 
so it always is between 0 and 1.



WindowsComboBoxUI.java
 221 if (this.borderChecked == false) // check border of 
component

 222 replaceBorder();

It is better to use 'if (!this.borderChecked) {replaceBorder();}'.
There are Java Style Guidelines [1] which we need to follow to.

I removed this part and replaced it with one line into method 'public 
void installUI( JComponent c )'. It has the same effect.

WindowsComboBoxUI.java
159// set empty border as default to see vista animated border
160comboBox.setBorder(new EmptyBorder(0,0,0,0));


454 if (comboBox.isPopupVisible())
455 getModel().setPressed(true);
456 else
457 getModel().setPressed(false);

This can be simplified to 
getModel().setPressed(comboBox.isPopupVisible()).


Corrected. this was lame :(


508 @Deprecated
 509 @SuppressWarnings("serial") // Superclass is not serializable 
across versions

 510 protected class WindowsComboPopup extends BasicComboPopup {
 511
 512 private class comboPopUpBorder extends EmptyBorder {

The class WindowsComboPopup is marked as deprecated with comments '* 
This class is now obsolete and doesn't do anything.'
Is it possible to move the popup border class outside and do not use 
the WindowsComboPopup at all?

The comboPopUpBorder class name should start from capital char.

I removed comboPopUpBorder class because painting of this border was 
hardcoded and it looked the same in win7 and win10.
The border is now painted with system skin and looks properly in both 
windows versions.
And I am not sure if it is ok, but I created new class WinComboPopUp 
which extends BasicComboPopup
 to avoid using deprecated WindowsComboPopup. But deprecated class 
also extends BasicComboPopup. So



566 Border border = (Border)UIManager.get("ComboBox.editorBorder");
 567
 568 // correct space on the left side of text (for 
editable combobox)

 569 Insets i = border.getBorderInsets(editor);
 570 border = BorderFactory.createEmptyBorder(i.top, 4, 
i.bottom, i.right);

 571
 572 if (border != null) {
 573 editor.setBorder(border);
 574 }

- The border.getBorderInsets(editor) is called before checking the 
border to null.
- It seems that if a user sets a custom "ComboBox.editorBorder" it 
should not be changed.
Is it possible just to update the dafult "ComboBox.editorBorder" in 
the com.sun.java.swing.plaf.windows.WindowsLookAndFeel class?


I updated default ComboBox.editorBorder.


XPStyle.java
 517 public boolean haveToSwitchStates() {
 518 return switchStates;
 519 }
 520
 521 public void switchStates(boolean b) {
 522 switchStates = b;
 523 }

Could you change the methods access from the public to package? Making 
some API public, even in com.sun.* package may require some additional 
process.


I changed access modifier from public to package.

And I also decreased shifting of list items to right from 3px to 2px.
If I see correctly native comboBox items have 2 px intendation from 
left side of the border, when system font is used.

WindowsComboBoxUI.java
 600 return new Insets(0,2,0,0);  (previous value was 3)
609 setBorder(new EmptyBorder(0, 2, 0, i.right)); (previous value was 
3 as well)


br
Martin



2017-02-03 13:31 GMT+01:00 Alexandr Scherbatiy 
<alexandr.scherba...@oracle.com <mailto:alexandr.scherba...@oracle.com>>:


On 2/1/2017 3:41 PM, Martin M wrote:

bug: https://bugs.openjdk.java.net/browse/JDK-6490753
<https://bugs.openjdk.java.net/browse/JDK-6490753>
webrev:
http://cr.openjdk.java.net/~alexsch/mraz.martin/6490753/webrev.00
<http://cr.openjdk.java.net/%7Ealexsch/mraz.martin/6490753/webrev.00>

Problem description:
Swing JComboBox looks different than native one in states like
e.g. focused state or mouse rollover state and so on.

The fix solves following issues:
- Editable combobox border is regular dark rectangle in all
states now. After the fix border will have light gray color in
normal state, light blue in rollover or hot state and dark blue
in pressed or focused state.
- Editable combobox button is painted almost properly, but not
animated as it should be. The fix will corre

Re: [9] Review Request: 8176046 Replace package.html files with package-info.java in the java.desktop module

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 3/1/2017 9:37 PM, Sergey Bylokhov wrote:

Hello,
Please review the fix for jdk9. This is the last request which covers all 
packages in java.desktop(except javax.swing and java.awt which were covered 
earlier)
  - All "package.html" files were replaced by "package-info.java", even for 
internal packages like eawt.
  - All  were replaced by {@tag }.
  - All files were manually reformatted - 80 chars per line.
  - TM was replaced by  This is why 
the specdiff contains some diffs.

Bug: https://bugs.openjdk.java.net/browse/JDK-8176046
Webrev can be found at: http://cr.openjdk.java.net/~serb/8176046/webrev.00/
Specdiff: 
http://cr.openjdk.java.net/~serb/8176046/specdiff/overview-summary.html

Re: [9] Review request for 8171808: Performance problems in dialogs with large tables when JAB activated

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 3/1/2017 9:20 PM, Sergey Bylokhov wrote:

Looks fine.


28 февр. 2017 г., в 17:40, Mikhail Cherkasov  
написал(а):

Hi Sergey, Alexander,

I have to revert the fix back to first version: 
http://cr.openjdk.java.net/~mcherkas/8171808/9/webrev.03/
because invokeAndWait has side affect and needs to be called on all components 
separately, otherwise runtime exceptions can be occurred.

Thanks,
Mikhail.


On 2/27/2017 8:13 PM, Sergey Bylokhov wrote:

Ok, Looks fine.


Hi again,
with this implementation I see a runtime exception, I see it in AccessBridge. 
getAccessibleNameFromContext.
InvocationUtils.invokeAndWait can't obtain AppContext. I don't see how it 
relates to my changes, but without
my changes it works fine, also it works fine with first version of the fix.

Thanks,
Mikhail.

On 2/27/2017 3:49 PM, Mikhail Cherkasov wrote:

Hi Sergey,

http://cr.openjdk.java.net/~mcherkas/8171808/9/webrev.02/
I wrapped the very fist invocations of  _getVisibleChildrenCount and 
_getVisibleChild into InvocationUtils.invokeAndWait
and removed all InvocationUtils.invokeAndWait inside those two methods.

Thanks,
Mikhail.

On 2/24/2017 1:08 AM, Sergey Bylokhov wrote:

Hi, Mikhail
Why we call invokeAndWait() so many times in the new method?
I guess we can do some work on EDT in one step then we will speedup the code 
when the size of the table is huge and it has lots of visible items.


Hi all,

Could you please review the fix:
http://cr.openjdk.java.net/~mcherkas/8171808/9/webrev.01/
for the following issue:
https://bugs.openjdk.java.net/browse/JDK-8171808

When JAWS asks java how many visible elements in the frame are,
java goes through the whole tree of component and asks each whether it visible 
or not.
During this java creates accessContext for each element, so this requires to 
get data from model.
So if user uses lazy loading or model is large, this counting makes app to 
freeze.

I reduced the number of components that should be checked for visibility,
if we get to a row that is invisible, there's no sense to check next rows, the 
same for columns.

Thanks,
Mikhail.

Re: [9] Review request for 8171808: Performance problems in dialogs with large tables when JAB activated

[Alexandr Scherbatiy]

  The fix looks good to me.

  The second version also contains the extra semicolon on the line 4542.

  Thanks,
  Alexandr.

On 2/27/2017 5:07 PM, Mikhail Cherkasov wrote:

Hi again,
with this implementation I see a runtime exception, I see it in 
AccessBridge. getAccessibleNameFromContext.
InvocationUtils.invokeAndWait can't obtain AppContext. I don't see how 
it relates to my changes, but without
my changes it works fine, also it works fine with first version of the 
fix.


Thanks,
Mikhail.

On 2/27/2017 3:49 PM, Mikhail Cherkasov wrote:

Hi Sergey,

http://cr.openjdk.java.net/~mcherkas/8171808/9/webrev.02/
I wrapped the very fist invocations of  _getVisibleChildrenCount and 
_getVisibleChild into InvocationUtils.invokeAndWait

and removed all InvocationUtils.invokeAndWait inside those two methods.

Thanks,
Mikhail.

On 2/24/2017 1:08 AM, Sergey Bylokhov wrote:

Hi, Mikhail
Why we call invokeAndWait() so many times in the new method?
I guess we can do some work on EDT in one step then we will speedup 
the code when the size of the table is huge and it has lots of 
visible items.



Hi all,

Could you please review the fix:
http://cr.openjdk.java.net/~mcherkas/8171808/9/webrev.01/
for the following issue:
https://bugs.openjdk.java.net/browse/JDK-8171808

When JAWS asks java how many visible elements in the frame are,
java goes through the whole tree of component and asks each whether 
it visible or not.
During this java creates accessContext for each element, so this 
requires to get data from model.
So if user uses lazy loading or model is large, this counting makes 
app to freeze.


I reduced the number of components that should be checked for 
visibility,
if we get to a row that is invisible, there's no sense to check 
next rows, the same for columns.


Thanks,
Mikhail.

Re: [9] Review Request: 8175266 Use package-info.java instead of package.html within swing packages

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 2/20/2017 5:05 PM, Sergey Bylokhov wrote:

Hello,
Please review the fix for jdk9.
 - All "package.html" files were replaced by "package-info.java" in 
javax.swing package and its subpackages.

 - All  were replaced by {@tag }.
 - Some unnecessary  tags were removed.
 - I also wraped some references to the classes in the {@link}
 - All files were manually reformatted - 80 chars per line.

Later I will do the same for all other packages in java client.

Bug: https://bugs.openjdk.java.net/browse/JDK-8175266
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8175266/webrev.00/ 

Specdiff: 
http://cr.openjdk.java.net/~serb/8175266/specdiff/overview-summary.html 



The same bug was fixed in awn already:
https://bugs.openjdk.java.net/browse/JDK-6622944
http://mail.openjdk.java.net/pipermail/awt-dev/2017-February/012610.html

Re: Testing Latest OpenJDK Swing Build

[Alexandr Scherbatiy]

On 2/20/2017 9:49 PM, Landon Blake wrote:
I'd like to help with the development of Swing in OpenJDK. Is there a 
recent build of the Swing code that I can test? Or do I need to build 
the source code myself?


   The  JDK 9 Early Access Releases is available to download from [1].  
To build JDK 9 from the source code use build instructions [2] and the 
JDK 9 client repository [3].


  [1] https://jdk9.java.net/download
  [2] 
http://hg.openjdk.java.net/jdk9/client/raw-file/ecb22730b4df/README-builds.html

  [3] hg.openjdk.java.net/jdk9/client

  Thanks,
  Alexandr.



I've been using Java for many years, but I'm not a C or C++ programmer. :]

Thanks!

Landon

--
Landon
"Homemade cookies. Hmm!"

Web: www.landonblake.com 

LinkedIn: https://www.linkedin.com/in/landonblake

Google+: https://plus.google.com/u/0/+LandonBlake

Facebook: https://www.facebook.com/scott.l.blake.3

Pinterest: https://www.pinterest.com/landon_blake/

Re: Review request for JDK-6490753 JComboBox doesn't look as native combobox in different states of component

[Alexandr Scherbatiy]

On 2/1/2017 3:41 PM, Martin M wrote:
bug: https://bugs.openjdk.java.net/browse/JDK-6490753 

webrev: 
http://cr.openjdk.java.net/~alexsch/mraz.martin/6490753/webrev.00 



Problem description:
Swing JComboBox looks different than native one in states like e.g. 
focused state or mouse rollover state and so on.


The fix solves following issues:
- Editable combobox border is regular dark rectangle in all states 
now. After the fix border will have light gray color in normal state, 
light blue in rollover or hot state and dark blue in pressed or 
focused state.
- Editable combobox button is painted almost properly, but not 
animated as it should be. The fix will correct animation of the button.
- popup with list of choices has black border. The fix will correct 
the border to proper colors.
- All texts were rendered very close to left side of borders. The fix 
shifts texts few screen points to the left.


AnimationController.java
 386 if (skin.haveToSwitchStates()) {
 387 skin.paintSkinRaw(g, dx, dy, dw, dh, state);
 388 g.setComposite(AlphaComposite.SrcOver.derive(1 - progress));
 389 skin.paintSkinRaw(g, dx, dy, dw, dh, startState);
 390 } else {

- Could the '1 - progress' value in AlphaComposite.SrcOver.derive(1 - 
progress) be out of range?


WindowsComboBoxUI.java
 221 if (this.borderChecked == false) // check border of 
component

 222 replaceBorder();

It is better to use 'if (!this.borderChecked) {replaceBorder();}'.
There are Java Style Guidelines [1] which we need to follow to.


 454 if (comboBox.isPopupVisible())
 455 getModel().setPressed(true);
 456 else
 457 getModel().setPressed(false);

This can be simplified to getModel().setPressed(comboBox.isPopupVisible()).


 508 @Deprecated
 509 @SuppressWarnings("serial") // Superclass is not serializable 
across versions

 510 protected class WindowsComboPopup extends BasicComboPopup {
 511
 512 private class comboPopUpBorder extends EmptyBorder {

The class WindowsComboPopup is marked as deprecated with comments '* 
This class is now obsolete and doesn't do anything.'
Is it possible to move the popup border class outside and do not use the 
WindowsComboPopup at all?

The comboPopUpBorder class name should start from capital char.


 566 Border border = 
(Border)UIManager.get("ComboBox.editorBorder");

 567
 568 // correct space on the left side of text (for 
editable combobox)

 569 Insets i = border.getBorderInsets(editor);
 570 border = BorderFactory.createEmptyBorder(i.top, 4, 
i.bottom, i.right);

 571
 572 if (border != null) {
 573 editor.setBorder(border);
 574 }

- The border.getBorderInsets(editor) is called before checking the 
border to null.
- It seems that if a user sets a custom "ComboBox.editorBorder" it 
should not be changed.
Is it possible just to update the dafult "ComboBox.editorBorder" in the 
com.sun.java.swing.plaf.windows.WindowsLookAndFeel class?



XPStyle.java
 517 public boolean haveToSwitchStates() {
 518 return switchStates;
 519 }
 520
 521 public void switchStates(boolean b) {
 522 switchStates = b;
 523 }

Could you change the methods access from the public to package? Making 
some API public, even in com.sun.* package may require some additional 
process.


[1] http://cr.openjdk.java.net/~alundblad/styleguide/index-v6.html

Thanks,
Alexandr.

Re: RFR: 8173409: make setMixingCutoutShape public and remove jdk.desktop

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 1/31/2017 11:28 AM, Sergey Bylokhov wrote:

Looks fine.



Bug: https://bugs.openjdk.java.net/browse/JDK-8173409
Webrev: http://cr.openjdk.java.net/~prr/8173409/

In OpenJDK the jdk.desktop module (not java.desktop) has only one method.
It is the vestige of com.sun.awt.AWTUtilities added in JDK 6 updates.

It provides a way to access to Component.setMixingCutoutShape

the jdk.desktop module was created so that apps could still access it 
without

using --add-exports - although at the expense of a code update.

We'd prefer to eliminate this non-standard API and module before JDK 9
goes final and can do so by making the above method public and standard

As a result jdk.desktop becomes empty and can go away ..

Swing's own internal uses have been updated and should be fully 
compatible

since the implementation is unchanged.

The existing tests which verify the API are updated to use the public 
API.

They should pass as well as they ever did ..

I did leave the AWTAccessor code in there for apps that absolutely 
have no way

to recompile to the new API and need to use --add-exports for now.

A CCC will be submitted for this change.

-phil.

Re: [9] RFR JDK-8172558: [PIT][TEST_BUG] Bad filename for javax/swing/JTable/8133919/DrawGridLinesTest.java

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 1/12/2017 10:04 AM, Prasanta Sadhukhan wrote:

Hi All,

Please review the name change for this regression test which was 
causing jtreg to fail as it was not finding the test.


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

Filename was javax/swing/JTable/8133919/DrawGridLInesTest.java [whith 
capital I]


but test has jtreg tag @run main DrawGridLinesTest

Fix is to rename the test to correct name.

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

Regards
Prasanta

Re: 8172509: [TEST_BUG] [macosx] Failure of the new test java/awt/Focus/FocusTraversalPolicy/ButtonGroupLayoutTraversal/ButtonGroupLayoutTraversalTest.java

[Alexandr Scherbatiy]

On 1/11/2017 2:01 PM, Avik Niyogi wrote:

Hi All,
I have addressed the inputs received in the following update. Please 
review the same for JDK 9.
http://cr.openjdk.java.net/~aniyogi/8172509/webrev.01/ 



With Regards,
Avik Niyogi
On 11-Jan-2017, at 4:22 pm, Prasanta Sadhukhan 
> wrote:


I guess we need to throw the exception if changing LaF fails instead 
of silently consuming it!!


Regards
Prasanta
On 1/11/2017 2:16 PM, Avik Niyogi wrote:

Hi All,

Kindly review the proposed fix for JDK9.

*Bug: https://bugs.openjdk.java.net/browse/JDK-8172509*
*
*
*Webrev: http://cr.openjdk.java.net/~aniyogi/8172509/webrev.00/ 
*

*
*
*Issue: *The focus traversal policy being tested was incorrect

*Cause:* For Aqua and Nimbus, the focus traversal policy used is 
different and as this is tested on default LAF, it fails on Mac OS X
  Could you give more details what are the differences between using 
the focus traversal policy in Metal, Nimbus and Aqua L?


  Thanks,
  Alexandr.


*Fix: *Cross-platform (Metal) LAF is used in case the default LAF is 
either Aqua or Nimbus and the focus traversal works in those cases.


With Regards,
Avik Niyogi

Re: [9] Review request for 8169922 SwingMark/TextArea: 2-7% regression on Linux, Mac, Windows in 9-b143

[Alexandr Scherbatiy]

On 12/16/2016 2:36 AM, Phil Race wrote:
I just noticed that you spell the new methods with a single d :eg : 
getFPMethodOverriden


Like the variables that were there before it should be 
getFPMethodOverridden

and checkFPMethodOverridden

   Here is the updated fix where the typo is updated:
http://cr.openjdk.java.net/~alexsch/8169922/webrev.02



Also, although rather unlikely to be a problem in practice, since there
is nothing in the swing rules to prevent constructing a Swing component on
a random thread, before adding it to the hierarchy, and the shared Map 
is populated

at construction time, it seems like we need a ConcurrentHashMap ..
  It seems that it could be possible that a constructor can schedule to 
run some actions calling SwingUtilities.invokeLater().
  In this case both threads (the thread where a Swing component is 
constructor and EDT) can simultaneously start the object fields updating.
  There is  still no guarantee that all works as expected if a Swing 
component is constructed in non-EDT.


  Thanks,
  Alexandr.



-phil.

On 12/15/2016 09:06 AM, Alexandr Scherbatiy wrote:


Hello,

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

- the HashMap is stored in SoftRefence
- the typo in getMethodArguments() is fixed
- classes FPMethodItem and FPMethodArgs are defined in the PlainView 
class.


Thanks,
Alexandr.

On 12/14/2016 8:30 PM, Sergey Bylokhov wrote:

Hi, Alexander.
Should not the storage be based on soft references? In the current 
solution we will store the references to the checked classes forever 
in the static map.


13 дек. 2016 г., в 18:41, Alexandr Scherbatiy 
<alexandr.scherba...@oracle.com> написал(а):



Hello,

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

  The fix JDK-8156217 checks presence of the overridden methods 
with floating point arguments.
  The proposed fix caches results of found overridden methods with 
floating point arguments.


  I run the SwingSet2 with the custom test which intensively 
creates JTextField, JPasswordField, JTextArea, and JEditorPane.


  The results of the test running with the following JDK are:
  1. Methods with int arguments are always called without the 
methods overridden check: 45822 // before the fix JDK-8156217
  2. Methods with floating point arguments are always called 
without the methods overridden check: 46175

performance decreasing: 100 * (46175 - 45822) / 45822 = 0.77%
  3. Methods with floating point arguments are always called with 
the methods overridden check: 48836 // fix JDK-8156217

performance decreasing: 100 * (48836 - 45822) / 45822 = 6.58%
  4. Methods with floating point arguments are always called and 
the methods overridden checks are cached: 46592 // current fix

performance decreasing: 100 * (46592 - 45822) / 45822 = 1.68%

Thanks,
Alexandr.

Re: [9] Review request for 8133919 [macosx] JTable grid lines are incorrectly positioned on HiDPI display

[Alexandr Scherbatiy]

Hello,

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

The test is converted to the automated.

Thanks,
Alexandr.

On 12/15/2016 1:24 PM, Sergey Bylokhov wrote:

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

  Draw lines calls are replaced by SwingUtilities2.drawHLine()/drawVLine in 
BasicTableUI.
  The added test is manual because the Robot takes low-resolution screen-shot 
so necessary artifacts can be hidden.
  It will  be possible to make the test automated after the fix JDK-8162959 
[HiDPI] screenshot artifacts using AWT Robot (see [1])

Probably it Is possible to automate it via drawing to the image in the paint() 
method?

Re: [9] Review request for 8169922 SwingMark/TextArea: 2-7% regression on Linux, Mac, Windows in 9-b143

[Alexandr Scherbatiy]

Hello,

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

- the HashMap is stored in SoftRefence
- the typo in getMethodArguments() is fixed
- classes FPMethodItem and FPMethodArgs are defined in the PlainView class.

Thanks,
Alexandr.

On 12/14/2016 8:30 PM, Sergey Bylokhov wrote:

Hi, Alexander.
Should not the storage be based on soft references? In the current solution we 
will store the references to the checked classes forever in the static map.


13 дек. 2016 г., в 18:41, Alexandr Scherbatiy <alexandr.scherba...@oracle.com> 
написал(а):


Hello,

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

  The fix JDK-8156217 checks presence of the overridden methods with floating 
point arguments.
  The proposed fix caches results of found overridden methods with floating 
point arguments.

  I run the SwingSet2 with the custom test which intensively creates 
JTextField, JPasswordField, JTextArea, and JEditorPane.

  The results of the test running with the following JDK are:
  1. Methods with int arguments are always called without the methods 
overridden check: 45822 // before the fix JDK-8156217
  2. Methods with floating point arguments are always called without the 
methods overridden check: 46175
performance decreasing: 100 * (46175 - 45822) / 45822 = 0.77%
  3. Methods with floating point arguments are always called with the methods 
overridden check: 48836 // fix JDK-8156217
performance decreasing: 100 * (48836 - 45822) / 45822 = 6.58%
  4. Methods with floating point arguments are always called and the methods 
overridden checks are cached: 46592 // current fix
performance decreasing: 100 * (46592 - 45822) / 45822 = 1.68%

Thanks,
Alexandr.

[9] Review request for 8133919 [macosx] JTable grid lines are incorrectly positioned on HiDPI display

[Alexandr Scherbatiy]

Hello,

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

  Draw lines calls are replaced by 
SwingUtilities2.drawHLine()/drawVLine in BasicTableUI.
  The added test is manual because the Robot takes low-resolution 
screen-shot so necessary artifacts can be hidden.
  It will  be possible to make the test automated after the fix 
JDK-8162959 [HiDPI] screenshot artifacts using AWT Robot (see [1])


Thanks,
Alexandr.

[1] http://mail.openjdk.java.net/pipermail/awt-dev/2016-December/012448.html

[9] Review request for 8169922 SwingMark/TextArea: 2-7% regression on Linux, Mac, Windows in 9-b143

[Alexandr Scherbatiy]

Hello,

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

  The fix JDK-8156217 checks presence of the overridden methods with 
floating point arguments.
  The proposed fix caches results of found overridden methods with 
floating point arguments.


  I run the SwingSet2 with the custom test which intensively creates 
JTextField, JPasswordField, JTextArea, and JEditorPane.


  The results of the test running with the following JDK are:
  1. Methods with int arguments are always called without the methods 
overridden check: 45822 // before the fix JDK-8156217
  2. Methods with floating point arguments are always called without 
the methods overridden check: 46175

performance decreasing: 100 * (46175 - 45822) / 45822 = 0.77%
  3. Methods with floating point arguments are always called with the 
methods overridden check: 48836 // fix JDK-8156217

performance decreasing: 100 * (48836 - 45822) / 45822 = 6.58%
  4. Methods with floating point arguments are always called and the 
methods overridden checks are cached: 46592 // current fix

performance decreasing: 100 * (46592 - 45822) / 45822 = 1.68%

 Thanks,
 Alexandr.

Re: RFR: 8171074 : Test api/javax_swing/UIManager/index.html\#Methods is failing

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 12/13/2016 12:42 AM, Phil Race wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8171074
Webrev: http://cr.openjdk.java.net/~prr/8171074/

Other than the @SuppressWarnings,
UIManager.setLookAndFeel(String className) is now reverted
to how it was before 8155874

-phil.

Re: [9] RFR: JDK-8170349: The printed content is beyond the borders.

[Alexandr Scherbatiy]

On 12/1/2016 9:16 AM, Prasanta Sadhukhan wrote:

On 11/30/2016 2:53 PM, Prasanta Sadhukhan wrote:


Hi All,

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

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

Please review a fix for a continuation/regression of JDK-8081491 
 in which we made 
sure that we print only the JTable rows/columns that is visible on 
console.


This bug manifests itself as, despite marking JTable PrintMode to 
FIT_WIDTH, we are printing only those columns that are visible on 
console.


However, if JTable PrintMode is FIT_WIDTH, then as per spec
https://docs.oracle.com/javase/8/docs/api/javax/swing/JTable.PrintMode.html#FIT_WIDTH
we should print all columns on each page (apparently irrespective of 
what is visible)


The fix takes care of that by making sure the table bounds is 
adjusted to clipwidth [which is already adjusted to total column 
width here 
]
so that all columns are printed and also rectangle border is drawn 
encompassing all columns.


The reason of adjusting the table bounds was because table.print() 
calls BasicTableUI.paint() where we compute visibleBounds

http://hg.openjdk.java.net/jdk9/client/jdk/file/a5e270f2c97d/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTableUI.java#l1817
therefore we need to adjust the visibleBounds to entire columns 
instead of just the visible columns.
  Did the test Swing_JTable/Manual/PrintManualTest_FitWidthMultiple 
pass before the fix for the JDK-8081491?
  It looks like the table bounds never have been changed in the 
TablePrintable.


  Thanks,
  Alexandr.


Regards
Prasanta

The 8081491 testcases passed with this fix as well.

Regards
Prasanta

Re: [9] Review request for 8074883: Tab key should move to focused button in a button group

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 11/29/2016 7:42 PM, Semyon Sadetsky wrote:
Please review the updated webrev: 
http://cr.openjdk.java.net/~ssadetsky/8074883/webrev.03/


changes:
 - according to Sergey's suggestion in the specs term "focusable" is 
replaced with "can be the focus owner".


--Semyon


On 11/29/2016 6:34 PM, Sergey Bylokhov wrote:

On 29.11.16 17:52, Semyon Sadetsky wrote:

On 11/29/2016 5:41 PM, Sergey Bylokhov wrote:

On 24.11.16 18:34, Semyon Sadetsky wrote:

Yes they are different and can results the different results, but
currently spec says the opposite: "(1) method execution is the 
same as
calling (2)". It will be good to rephrase it somehow that we will 
try

to move focus to the selected component.

I still do not understand what inconsistency between the code and the
spec did you find. I think it would be easier if you just write your
vision of the spec.


Inconsistency is that the "focusable" property can be true but the
component can be disabled. So from the specification point of view the
"selected" button should be focused, but we filter it out in
getGroupSelection() and move the focus to the current button. See
comment below.
The specification did not mean the focusable property it "can be a 
focus

owner" (see below) which includes enabled criteria.


And this is what I suggested below:


take a look to the parent method where the
next states are covered "displayable, focusable, visible"(enable state
is missing??). So you can use the same text here or use "can be a
focus owner".

Re: [9] Review request for 8162350 RepaintManager shifts repainted region when the floating point UI scale is used

[Alexandr Scherbatiy]

On 11/30/2016 2:11 AM, Jim Graham wrote:
I agree that the two versions of the paintDoubleBuffered functions 
look like they may not have any noticeable performance difference.  It 
would be nice to avoid the duplication if they don't, I had assumed 
that such a performance test had already been done...?

There could be 2 problems
  - There were checks that Graphics g is instanceof Grpahics2D. If it 
fails it is necessary to fall down to the scale 1.
  - The BufferedImage is used if the volatile image buffer is disabled. 
It seems it is necessary to keep both pixels coordinates and sizes for 
both graphics the passed and the buffer.


   I would prefer to keep the current fix as is and  investigate the 
problems with the non volatile offscreen image and the 
paintDoubleBufferedImpl method elimination in a separate issue:
JDK-8170593 Non volatile OffscreenBuffer should be scaled in 
RepaintManager for HiDPI display

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

  Thanks,
  Alexandr.


clipRound() has appropriate logic for matching the results of filling 
a path.  clipScale() is a more basic "just scale this value and clip 
it" type of operation that has uses - not everything needs to match 
the rasterization logic of fills.  I did a quick grep to see where 
clipScale() is used:


- SG2D.constrain(, ... region) - this method needs to be overhauled as 
it currently relies on scaling a region which is not a valid operation 
- only paths can be scaled, once they are rasterized into a region it 
is too late to adjust their coordinate transform, but I think fixing 
this involves fixing shaped components to remember their outline shape 
rather than an outline region.  :(


- SG2D.drawHiDPI() - this fix changes one of the instances of using 
clipScale() in that method, but not the other one. They should 
probably both match.  I don't have a good answer for which is the best 
rounding method to use here in any case since the original concept was 
that we'd extract the integer pixels and pretend that they were an 
isolated image, but if we end up with a fractional scale then that 
isn't really possible.  It may be better to just leave this alone for 
now.  Does anything about the rest of this fix depend on that change?  
Hopefully if RepaintManager does its job right we end up painting the 
back buffer to the destination all on integer pixel boundaries on both 
the source and dest and so rounding isn't an issue?


- Region.getScaledRegion() - this method should be deleted.  I think 
it is only used in SG2D.constrain() which needs to be rewritten as you 
cannot scale a region and expect any amount of accuracy, but we live 
with it for now until a better fix comes along.  I don't think 
changing the rounding method used by this method will have any useful 
impact, it just needs to be eliminated by fixing the callers to no 
longer need it.


There are a couple of more uses in the AWT that I didn't look too 
closely into at this point, but those are the uses within the Java2D 
area...


...jim

On 11/24/16 8:40 AM, Sergey Bylokhov wrote:

I have a few questions which probably discussed already, then ignore it:

 - SunGraphics2D.java: As far as I understand the clipScale() was 
replaced by clipRound(), because they have different
round logic? It seems that when I wrote the clipScale() I was not 
aware about round logic, and looks like we can change
the clipScale implementation to use clipRound internally instead of 
Math.round(newv), can be fixed by othe fix.
 - Did you check the difference in performance between 
paintDoubleBufferedImpl vs paintDoubleBufferedFPScales? At least
in terms of heavyweight operations it looks similar, and probably we 
can have only one of them? It has an additional

benefits that the new code will be tested on the usual system as well.

On 21.11.16 16:59, Alexandr Scherbatiy wrote:


Hello,

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

 - isFloatingPointScale(AffineTransform) is moved from the 
SunGraphics2D

to the SwingUtilities2 class.

  Thanks,
  Alexandr.

On 11/18/2016 11:23 PM, Jim Graham wrote:

Hi ALexandr,

This looks great.

BTW, when I suggested moving the FPscale test into SG2D I was
suggesting that to avoid having to copy the transform out of it via
getTransform(), but you've found a different solution to that issue
(i.e. the new getTransform(g) method) so it no longer matters where
that utility static function is located.  You can move it back to one
of the Swing classes.

In terms of the logic of choosing which repaint function to use, it
looks like you use the old-style function if the scales don't match,
but won't that cause rendering anomalies?  The new code is still an
improvement for the standard HiDPI case, and I'm guessing that
mismatched scales probably never tends to happen, but we might want to
flag it for further investigation.

+1 relative to whether you want to move the FPscale

Re: [9] Review request for 8162350 RepaintManager shifts repainted region when the floating point UI scale is used

[Alexandr Scherbatiy]

On 11/24/2016 7:40 PM, Sergey Bylokhov wrote:

I have a few questions which probably discussed already, then ignore it:

 - SunGraphics2D.java: As far as I understand the clipScale() was 
replaced by clipRound(), because they have different round logic? It 
seems that when I wrote the clipScale() I was not aware about round 
logic, and looks like we can change the clipScale implementation to 
use clipRound internally instead of Math.round(newv), can be fixed by 
othe fix.

  The clipRound() is used to be consisted with the fillRect(...) rounding.


 - Did you check the difference in performance between 
paintDoubleBufferedImpl vs paintDoubleBufferedFPScales? At least in 
terms of heavyweight operations it looks similar, and probably we can 
have only one of them? It has an additional benefits that the new code 
will be tested on the usual system as well.


  The result of running SwingMark 2 with the following JDK is:
  1. without the fix
1st test run [1]: 92373
2nd test run [2]: 92156

   average: (92373 + 92156) / 2 = 92264.5

  2. paintDoubleBufferedImp() method is always used
1st test run [3]: 92476// (92476 - 92264.5) / 92264.5 / 100 = 
0.23%
2nd test run [4]: 90800  // (90800 - 92264.5) / 92264.5 / 100 = 
-0.000159%


  3.paintDoubleBufferedFPScales () method is always used
1st test run [5]: 91053// (91053 - 92264.5) / 92264.5 / 100 = 
-0.000131%
2nd test run [6]: 92900// (92900 - 92264.5) / 92264.5 / 100 = 
0.69%


Thanks,
Alexandr.

[1] 
http://cr.openjdk.java.net/~alexsch/8162350/swingmark2/repaint-manager-fp-scale-base_00.txt
[2] 
http://cr.openjdk.java.net/~alexsch/8162350/swingmark2/repaint-manager-fp-scale-base_01.txt
[3] 
http://cr.openjdk.java.net/~alexsch/8162350/swingmark2/repaint-manager-fp-scale-int_00.txt

[4]http://cr.openjdk.java.net/~alexsch/8162350/swingmark2/repaint-manager-fp-scale-int_01.txt
[5] 
http://cr.openjdk.java.net/~alexsch/8162350/swingmark2/repaint-manager-fp-scale-fp_00.txt
[6] 
http://cr.openjdk.java.net/~alexsch/8162350/swingmark2/repaint-manager-fp-scale-fp_01.txt


On 21.11.16 16:59, Alexandr Scherbatiy wrote:


Hello,

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

 - isFloatingPointScale(AffineTransform) is moved from the SunGraphics2D
to the SwingUtilities2 class.

  Thanks,
  Alexandr.

On 11/18/2016 11:23 PM, Jim Graham wrote:

Hi ALexandr,

This looks great.

BTW, when I suggested moving the FPscale test into SG2D I was
suggesting that to avoid having to copy the transform out of it via
getTransform(), but you've found a different solution to that issue
(i.e. the new getTransform(g) method) so it no longer matters where
that utility static function is located.  You can move it back to one
of the Swing classes.

In terms of the logic of choosing which repaint function to use, it
looks like you use the old-style function if the scales don't match,
but won't that cause rendering anomalies?  The new code is still an
improvement for the standard HiDPI case, and I'm guessing that
mismatched scales probably never tends to happen, but we might want to
flag it for further investigation.

+1 relative to whether you want to move the FPscale test back out of
SG2D or not...

...jim

On 11/18/16 1:44 AM, Alexandr Scherbatiy wrote:


Thank you. I see that using the integer device-pixel translations
preserves the component painting in the same way for
floating point scales.

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

  - translation adjustment is removed
  - Region.clipRound() is used for pixels coordinates rounding.

  Thanks,
  Alexandr.

On 11/16/2016 1:52 AM, Jim Graham wrote:

Let me clarify something...

On 11/15/16 2:49 AM, Alexandr Scherbatiy wrote:

  Let's consider the following use case:
  scale = 1.5
  A component calls fillRect(1, 1, 1, 1).
  This is (1.5, 1.5, 3.0, 3.0) in the device space
  which fills  (1, 1, 3, 3) and covers 2x2 pixels


Agreed.


  Now the area (1, 1, 1, 1) needs to be repainted
create a backbuffer
translate(-1, -1) // move the top left corner of the area to
the zero point
draw the component into the backbuffer:
  fillRect(1, 1, 1, 1) -> after translation fillRect(0, 0, 1,
1) -> after scaling  (0.0, 0.0, 1.5, 1.5 ) in the
device space
  which fills (0, 0, 1, 1) and covers 1x1 pixels


If you did g.setTransform(identity), g.translate(-1, -1), (then
restore the scale) then the analysis is as follows:

g.setTransform(identity) => [1 0 0] [0 1 0]
g.translate(-1, -1) => [1 0 -1] [0 1 -1]
g.scale(1.5, 1.5) => [1.5 0 -1] [0 1.5 -1]
g.fillRect(1, 1, 1, 1)
=> coordinates are (1.5-1, 1.5-1, 3-1, 3-1)
=> (.5, .5, 2, 2)
=> fills (0, 0, 2, 2)
=> which covers 2x2 pixels

If you did g.translate(-1, -1) on the scaled transform then the
analysis is as follows:

g.transform is [1.5 0 0] [0 1.5 0]
g.translate(-1, -1) is [1.5 0 -1.5] [0 1.5 

Re: [9] Review request for 8160087: Change IOOBE to warning in the scenarios when it had not being thrown before the JDK-8078514

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 11/11/2016 12:36 PM, Alexander Zvegintsev wrote:

+1

--
Thanks,
Alexander.

On 10.11.2016 10:33, Semyon Sadetsky wrote:
please review the updated webrev: 
http://cr.openjdk.java.net/~ssadetsky/8160087/webrev.02/


system property was removed.

--Semyon


On 7/21/2016 4:40 PM, Semyon Sadetsky wrote:



On 7/21/2016 2:51 PM, Alexandr Scherbatiy wrote:

On 7/20/2016 4:46 PM, Semyon Sadetsky wrote:



On 7/20/2016 2:29 PM, Alexandr Scherbatiy wrote:

On 7/20/2016 10:18 AM, Semyon Sadetsky wrote:

On 7/19/2016 4:06 PM, Alexandr Scherbatiy wrote:


On 7/19/2016 2:56 PM, Semyon Sadetsky wrote:

On 19.07.2016 14:20, Alexandr Scherbatiy wrote:


The fix prints the warning method in case of wrong row sorter 
usage. How often this can happen? Could the large number of 
the messages overflow a user output?
In the FilePane this happened only once after the initial file 
list loading.
   I am just worrying that in a user application which does not 
properly use the row sorter there can be a lot of such 
warnings. And it could be some library which he can't be able 
to update. Is it possible to show the warning only once?
Yes. See the updated webrev: 
http://cr.openjdk.java.net/~ssadetsky/8160087/webrev.01/
   A property which should be used by users needs to have the CCC 
request.
It is added on the off-chance. It doesn't merit to be a documented 
property.

  Who should use the proposed property?
If it will not be possible to correct the code (for example an old 
application that is not supported) and seeing the warning is 
displeased for some reasons.


--Semyon


  Thanks,
  Alexandr.



--Semyon

   I believe that printing the warning message only once is enough.

  Thanks,
  Alexandr.


--Semyon


  Thanks,
  Alexandr.



--Semyon


Thanks,
Alexandr.

On 7/19/2016 12:30 PM, Semyon Sadetsky wrote:



On 19.07.2016 12:18, Alexandr Scherbatiy wrote:

On 7/18/2016 11:46 AM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

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


A warning is added to avoid issues in user code to throw 
exceptions which were masked before. See bug descriptions 
for details.
  Should this behavior (which exists for long time) be 
specified in the 
DefaultRowSorter.convertRowIndexToView()/convertRowIndexToModel() 
javadoc?
This was not a 
DefaultRowSorter.convertRowIndexToView()/convertRowIndexToModel() 
issue. It was a mistake in the FilePane class.

RowSorter's javadoc mentions the correct way to use it:

The view invokes a model change method when the underlying 
model has changed. There may be order dependencies in how 
the events are delivered, so a RowSorter should not update 
its mapping until one of these methods is invoked.


--Semyon


  Thanks,
  Alexandr.


--Semyon

Re: [9] Review Request: 8149879 Examine UIDefaults::addResourceBundle(String bundleName) with resource encapsulation

[Alexandr Scherbatiy]

On 11/28/2016 7:41 PM, Sergey Bylokhov wrote:

Hello.

Please review the fix for jdk9.

This fix improve encapsulation of java.desktop module. After the fix 
the method "UIDefaults::addResourceBundle()" will not be able to 
register resource bundles which are located in the java.desktop 
module. Only the java.desktop module itself will be able to use such 
bundles.

  - Will it break existed applications?
  - Is it a common case that some applications register resource 
bundles from JDK?
  - What is the reason that resource bundles from JDK are not supposed 
to be registered by external applications?
  - Is it applicable for all JDK bundles or it should be allowed to 
load bundles for public L like Metal and not for internal (Windows)?
  - Was there the similar issue before the modularization? For example 
an application is not supposed to load internal Java resource bundles if 
the security manager is set.


  Thanks,
  Alexandr.



Bug: https://bugs.openjdk.java.net/browse/JDK-8149879
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8149879/webrev.01

Re: RFR: 8169887: javax/swing/JEditorPane/8080972/TestJEditor.java, javax/swing/text/View/8080972/TestObjectView.java are failing

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 11/18/2016 9:21 PM, Sergey Bylokhov wrote:

Looks fine.

On 18.11.16 21:08, Phil Race wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8169887
webrev : http://cr.openjdk.java.net/~prr/8169887/

8155874 changed many calls from the deprecated Class.newInstance()
to Class.getDeclaredConstructor().newInstance().

However in the cases where the called code may be loaded from
a different class loader (ie app code) these are not equivalent
and require a doPrivileged.

The fix is to revert to Class.newInstance() for all cases where it
is not obvious that we will only call code from within the same
desktop module. This should be the lowest risk fix.

Later (JDK10) we can revisit these.


note that the change at line 473/474 in 
java/beans/PropertyDescriptor.java
was actually a mistake ! I just noticed it and that is reverted to 
what it

was before the first fix :-
http://cr.openjdk.java.net/~prr/8155874/src/java.desktop/share/classes/java/beans/PropertyDescriptor.java.sdiff.html 




-phil

Re: [9] Review request for 8162350 RepaintManager shifts repainted region when the floating point UI scale is used

[Alexandr Scherbatiy]

Hello,

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

 - isFloatingPointScale(AffineTransform) is moved from the 
SunGraphics2D to the SwingUtilities2 class.


  Thanks,
  Alexandr.

On 11/18/2016 11:23 PM, Jim Graham wrote:

Hi ALexandr,

This looks great.

BTW, when I suggested moving the FPscale test into SG2D I was 
suggesting that to avoid having to copy the transform out of it via 
getTransform(), but you've found a different solution to that issue 
(i.e. the new getTransform(g) method) so it no longer matters where 
that utility static function is located.  You can move it back to one 
of the Swing classes.


In terms of the logic of choosing which repaint function to use, it 
looks like you use the old-style function if the scales don't match, 
but won't that cause rendering anomalies?  The new code is still an 
improvement for the standard HiDPI case, and I'm guessing that 
mismatched scales probably never tends to happen, but we might want to 
flag it for further investigation.


+1 relative to whether you want to move the FPscale test back out of 
SG2D or not...


...jim

On 11/18/16 1:44 AM, Alexandr Scherbatiy wrote:


Thank you. I see that using the integer device-pixel translations 
preserves the component painting in the same way for

floating point scales.

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

  - translation adjustment is removed
  - Region.clipRound() is used for pixels coordinates rounding.

  Thanks,
  Alexandr.

On 11/16/2016 1:52 AM, Jim Graham wrote:

Let me clarify something...

On 11/15/16 2:49 AM, Alexandr Scherbatiy wrote:

  Let's consider the following use case:
  scale = 1.5
  A component calls fillRect(1, 1, 1, 1).
  This is (1.5, 1.5, 3.0, 3.0) in the device space
  which fills  (1, 1, 3, 3) and covers 2x2 pixels


Agreed.


  Now the area (1, 1, 1, 1) needs to be repainted
create a backbuffer
translate(-1, -1) // move the top left corner of the area to 
the zero point

draw the component into the backbuffer:
  fillRect(1, 1, 1, 1) -> after translation fillRect(0, 0, 1, 
1) -> after scaling  (0.0, 0.0, 1.5, 1.5 ) in the

device space
  which fills (0, 0, 1, 1) and covers 1x1 pixels


If you did g.setTransform(identity), g.translate(-1, -1), (then 
restore the scale) then the analysis is as follows:


g.setTransform(identity) => [1 0 0] [0 1 0]
g.translate(-1, -1) => [1 0 -1] [0 1 -1]
g.scale(1.5, 1.5) => [1.5 0 -1] [0 1.5 -1]
g.fillRect(1, 1, 1, 1)
=> coordinates are (1.5-1, 1.5-1, 3-1, 3-1)
=> (.5, .5, 2, 2)
=> fills (0, 0, 2, 2)
=> which covers 2x2 pixels

If you did g.translate(-1, -1) on the scaled transform then the 
analysis is as follows:


g.transform is [1.5 0 0] [0 1.5 0]
g.translate(-1, -1) is [1.5 0 -1.5] [0 1.5 -1.5]
g.fillRect(1, 1, 1, 1)
=> coordinates are (1.5-1.5, 1.5-1.5, 3-1.5, 3-1.5)
=> (0, 0, 1.5, 1.5)
=> fill (0, 0, 1, 1)
=> covers 1x1 pixels

The second operation is what you are describing above and that would 
be an inappropriate way to perform damage repair
because you used a scaled translation which did not result in an 
integer coordinate translation.


Please re-read my previous analysis that shows what happens when you 
use integer device-pixel translations which are
translations that happen using integers on a non-scaled transform.  
Note that you can add a scale *AFTER* you apply
the integer device pixel translation and it will not affect the 
integer-ness of the translation.  You can see above
that the difference in how the translate command is issues affects 
where the translation components of the matrix end

up being -1,-1 or -1.5,-1.5...

...jim

Re: [9] Review request for 8162350 RepaintManager shifts repainted region when the floating point UI scale is used

[Alexandr Scherbatiy]

Thank you. I see that using the integer device-pixel translations 
preserves the component painting in the same way for floating point scales.


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

  - translation adjustment is removed
  - Region.clipRound() is used for pixels coordinates rounding.

  Thanks,
  Alexandr.

On 11/16/2016 1:52 AM, Jim Graham wrote:

Let me clarify something...

On 11/15/16 2:49 AM, Alexandr Scherbatiy wrote:

  Let's consider the following use case:
  scale = 1.5
  A component calls fillRect(1, 1, 1, 1).
  This is (1.5, 1.5, 3.0, 3.0) in the device space
  which fills  (1, 1, 3, 3) and covers 2x2 pixels


Agreed.


  Now the area (1, 1, 1, 1) needs to be repainted
create a backbuffer
translate(-1, -1) // move the top left corner of the area to the 
zero point

draw the component into the backbuffer:
  fillRect(1, 1, 1, 1) -> after translation fillRect(0, 0, 1, 1) 
-> after scaling  (0.0, 0.0, 1.5, 1.5 ) in the

device space
  which fills (0, 0, 1, 1) and covers 1x1 pixels


If you did g.setTransform(identity), g.translate(-1, -1), (then 
restore the scale) then the analysis is as follows:


g.setTransform(identity) => [1 0 0] [0 1 0]
g.translate(-1, -1) => [1 0 -1] [0 1 -1]
g.scale(1.5, 1.5) => [1.5 0 -1] [0 1.5 -1]
g.fillRect(1, 1, 1, 1)
=> coordinates are (1.5-1, 1.5-1, 3-1, 3-1)
=> (.5, .5, 2, 2)
=> fills (0, 0, 2, 2)
=> which covers 2x2 pixels

If you did g.translate(-1, -1) on the scaled transform then the 
analysis is as follows:


g.transform is [1.5 0 0] [0 1.5 0]
g.translate(-1, -1) is [1.5 0 -1.5] [0 1.5 -1.5]
g.fillRect(1, 1, 1, 1)
=> coordinates are (1.5-1.5, 1.5-1.5, 3-1.5, 3-1.5)
=> (0, 0, 1.5, 1.5)
=> fill (0, 0, 1, 1)
=> covers 1x1 pixels

The second operation is what you are describing above and that would 
be an inappropriate way to perform damage repair because you used a 
scaled translation which did not result in an integer coordinate 
translation.


Please re-read my previous analysis that shows what happens when you 
use integer device-pixel translations which are translations that 
happen using integers on a non-scaled transform.  Note that you can 
add a scale *AFTER* you apply the integer device pixel translation and 
it will not affect the integer-ness of the translation.  You can see 
above that the difference in how the translate command is issues 
affects where the translation components of the matrix end up being 
-1,-1 or -1.5,-1.5...


...jim

Re: [9] Review request for 8169719 WrappedPlainView.modelToView() should return Rectangle2D

[Alexandr Scherbatiy]

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

  - The test is removed.

  Floating point values mean that the test passes but only integral 
values do not mean that the test fail.


  Thanks,
  Alexandr.

On 11/15/2016 9:52 PM, Phil Race wrote:

The assumption that just because values *can* be non-integral that
they *will* be non-integral is not valid. Even if you know that the
probability is small, it is not zero.

This seems like it could lead to spurious failures of this test.

-phil.


On 11/15/2016 09:18 AM, Alexandr Scherbatiy wrote:

On 11/15/2016 5:08 PM, Sergey Bylokhov wrote:

Should "@SuppressWarnings("deprecation")" be removed from this method?
Why the test is linux and windows specific?

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8169719/webrev.01/
  - "@SuppressWarnings("deprecation")"is removed

  The test is not suitable for Mac OS X because chars advances on Mac 
OS X have integer values.


  Thanks,
  Alexandr.



On 15.11.16 16:49, Alexandr Scherbatiy wrote:


Hello,

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

  WrappedPlainView.modelToView() method is updated to use
Utilities.getTabbedTextWidth() which returns floating point text 
width.


 Thanks,
 Alexandr.

Re: [9] Review request for 8169719 WrappedPlainView.modelToView() should return Rectangle2D

[Alexandr Scherbatiy]

On 11/15/2016 5:08 PM, Sergey Bylokhov wrote:

Should "@SuppressWarnings("deprecation")" be removed from this method?
Why the test is linux and windows specific?

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8169719/webrev.01/
  - "@SuppressWarnings("deprecation")"is removed

  The test is not suitable for Mac OS X because chars advances on Mac 
OS X have integer values.


  Thanks,
  Alexandr.



On 15.11.16 16:49, Alexandr Scherbatiy wrote:


Hello,

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

  WrappedPlainView.modelToView() method is updated to use
Utilities.getTabbedTextWidth() which returns floating point text width.

 Thanks,
 Alexandr.

[9] Review request for 8169719 WrappedPlainView.modelToView() should return Rectangle2D

[Alexandr Scherbatiy]

Hello,

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

  WrappedPlainView.modelToView() method is updated to use 
Utilities.getTabbedTextWidth() which returns floating point text width.


 Thanks,
 Alexandr.

Re: [9] Review request for 8162350 RepaintManager shifts repainted region when the floating point UI scale is used

[Alexandr Scherbatiy]

On 11/15/2016 2:44 AM, Jim Graham wrote:

I want to clarify the following issue...

On 10/24/2016 9:11 AM, Alexandr Scherbatiy wrote:

  Using floating point scale leads that drawing the same thing from
different coordinates gives different results. For example filling a
rectangle with size (1, 1) from location (0, 0) and UI scale 1.5 gives
scaled region (0, 0, 1.5, 1.5) which is rounded to (0, 0, 2, 2). The
same rectangle filled from the location (1, 1) gives the scaled region
(1.5, 1.5, 3, 3) which is rounded to (2, 2, 3, 3). The first rectangle
has size 2 in the device space and the second one has 1.


First, while those primitives would be drawn in 2 different sizes, the 
first should be 1x1 and the second should be 2x2 if they follow our 
fill rules.  Were you seeing something different? Still, the point you 
bring up about the two being 2 different sizes is acknowledged.  Fill 
rule rounding for these rectangles should be "ceil(coordinate - 0.5)".


Drawn with an integer translation of 0,0 (i.e. before scrolling 
operations or damage repair):


scale = 1.5
translate = 0,0 in device pixels
fillRect(0, 0, 1, 1)
transforms to 0.0, 0.0, 1.5, 1.5
fills (0,0,1,1)
covers 1x1 pixels
fillRect(1, 1, 1, 1)
transforms to 1.5, 1.5, 3.0, 3.0
fills (1,1,3,3)
covers 2x2 pixels

scale = 1.5
translate = 1,1 in device pixels
fillRect(0, 0, 1, 1)
transforms to 1.0, 1.0, 2.5, 2.5
fills (1,1,2,2)
covers 1x1 pixels
fillRect(1, 1, 1, 1)
transforms to 2.5, 2.5, 4.0, 4.0
fills (2,2,4,4)
covers 2x2 pixels

No change in the relative sizes is observed.  Now, if you are talking 
about an integer translation in user space, then there is a 
difference, as in:


scale = 1.5
translate = 1,1 in user space
  = 1.5,1.5 in device space
fillRect(0, 0, 1, 1)
transforms to 1.5, 1.5, 3.0, 3.0
fills (1,1,3,3)
covers 2x2 pixels
fillRect(1, 1, 1, 1)
transforms to 3.0, 3.0, 4.5, 4.5
fills (3,3,4,4)
covers 1x1 pixels

That can create a problem, but only if the translation is an integer 
distance in user space.  If RepaintManager is applying integer 
distances in device space, then it should not affect the relative 
sizes of the rendered primitives.


Were you seeing that happen?  Because that would be a rendering bug in 
fillRect...

  Let's consider the following use case:
  scale = 1.5
  A component calls fillRect(1, 1, 1, 1).
  This is (1.5, 1.5, 3.0, 3.0) in the device space
  which fills  (1, 1, 3, 3) and covers 2x2 pixels

  Now the area (1, 1, 1, 1) needs to be repainted
create a backbuffer
translate(-1, -1) // move the top left corner of the area to the 
zero point

draw the component into the backbuffer:
  fillRect(1, 1, 1, 1) -> after translation fillRect(0, 0, 1, 1) -> 
after scaling  (0.0, 0.0, 1.5, 1.5 ) in the device space

  which fills (0, 0, 1, 1) and covers 1x1 pixels

  Draw the backbuffer to the window at coordinates (1, 1)
  The rectangle with size  1x1 pixels is drawn instead of the 2x2

The result of translation, painting to the backbuffer and painting the 
backbuffer from the requested point can differ from just drawing from 
this point just because drawing from some point and drawing the same 
thing from the zero point can differ for the floating point scale.


  Thanks,
  Alexandr.



...jim


  As a result drawing a component from some coordinates and using
Graphics.copyArea() to translate an image to a new location could have a
differ results than just drawing the same component from the new 
location.

  The fix suggests to disable the JScrollPane area copying for floating
point scales.

Thanks,
Alexandr.

On 10/7/2016 4:30 PM, Alexandr Scherbatiy wrote:

On 10/6/2016 11:42 PM, Sergey Bylokhov wrote:

Hi, Alexandr.
Can you please provide some standalone small example, which emulates
this artifacts via java2d API. (The pattern which we use in
RepainManager). It will help to understand the problem.


The code sample [1]  draws two the same shapes (with different colors)
one after another into areas (x, y, w, h) and (x+w, y, w, h)
accordingly in different ways.

The shape is constructed from the following parts:
1. Fill clip area
 - set clip (x, y, w, h)
 - fill the whole image
As a result only clipped area is filled.

2. Fill rect
- fill rect(x, y, w, h) // big rect
- fill rect(x+1, y+1, w-2, h-2) // small rect

3. Draw center lines
- draw line (x, cy, x + w, cy)
- draw line (cx, y, cx, y + h)

The program has the following options:
  RECT - draw two shapes one after another from point (0, 0)
  SHIFTED_RECT - draw two shapes one after another from point (x, y)
  BACKBUFFER - draw the shape into a backbuffer with size (w, h) and
draw the backbuffer from the point (x, y)
  SCALED_BACKBUFFER - draw the shape into a scaled backbuffer with
size (ceil(w*scale), ceil(h*scale)) with scaled graphics from point
(0, 0) and draw it into the rectangle (x, y, w, h)
  ENLARGED_SCALED_BACKBUFFER 

Re: RFR: 8169518: Suppress Deprecation warnings for deprecated Swing APIs

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 11/10/2016 8:50 PM, Phil Race wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8169518
Webrev: http://cr.openjdk.java.net/~prr/8169518

The last set of deprecation warnings to be suppressed or fixed in 
desktop - so

with this fix we can re-enable javac lint checking of deprecation.

I have used JPRT to verify this builds on all platforms.

-phil.

Re: [9] Review request for 8162350 RepaintManager shifts repainted region when the floating point UI scale is used

[Alexandr Scherbatiy]

Hello,

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

  The previous fix renders the dirty region to the  backbuffer from the 
same (x, y) coordinates where it should be repainted.
  The JScrollPane can have large (x, y) values which can exceed the 
double buffer maximum size and some regions can be painted outside the 
backbuffer.


  The current fix tries to adjust the component translation to a value 
which allows to draw a component in the same way when floating point 
scale is used.
  The scale is converted to the irreducible fraction n / m where m is 
the step under which the component is drawn in the same way.
  The translation to the zero point is adjusted to the value: 
-translation + translation % m.

  The backbuffer is enlarged to the value: size + m.

  The floating point scale and general transform are handled 
differently because general transform requires calculation pixel values 
in general way, not just scale * x + translation.
  The integer and floating point scales are handed differently because 
the floating point scale handling now use % operator which can consume 
some time.


  Thanks,
  Alexandr.

On 11/1/2016 11:23 PM, Jim Graham wrote:
Is SunGraphics2D accessible from Swing?  If so, then I'd recommend 
putting the isFPScale() method right on that class so we don't have to 
clone the transform by calling g.getTransform().  Also note that 
g.getTransform() does more than clone the transform as it has to 
factor out the constrainXY translation - which are always integers so 
it won't have any effect on the results of "isFPScale()" and is 
additional wasted work.  Eventually we could tie this into the 
transformState variable in SG2D to differentiate integer and fp scale, 
but that would take a little more work as those flags are used in a 
lot of places - for now we can at least get rid of the transform clone 
and the constraint translation processing.


In the implementation of isFPScale(tx), do you really want to return 
false for non-scale transforms?  It seems to me that if the transform 
has rotations or shears in it then we might need to punt and just 
repaint the whole viewport.  Also, you could simplify it a little to 
avoid an extra "getter" and extra bit math:


isFPScale(AffineTransform tx) {
int type = tx.getType() & ~(FLIP | TRANSLATE);
if (type == 0) {
return false;
} else if (type == SCALE) {
// check for integers
} else {
return true or false?;
}
}

The changes to SG2D.drawHiDPIImage point out that we should probably 
allow fp subimage paramters in the image pipeline for better accuracy, 
but that's a much bigger change.  Until then sub-image blits are not 
going to be accurate on scaled images. Won't this inaccuracy affect 
our back buffer blits in Swing?


The changes to RepaintManager took me a couple of tries to figure out. 
It looks like you are now rendering the dirty region at the 
appropriate X,Y location in the back buffer (rather than at 0,0) in 
all cases to adjust for the fact that rendering the same primitive at 
different locations doesn't always match.  First, you expand the back 
buffer even for the unscaled case which wasn't affected by HiDPI.  
Second, as long as the translate is in device pixels, it shouldn't 
matter where in the buffer you render it, so it should be enough to 
just ensure integer translations - did you try using an integer origin 
for the rendering instead?


...jim

On 10/24/2016 9:11 AM, Alexandr Scherbatiy wrote:


  Hello,

  Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8162350/webrev.01
  - JScrollPane copy area functionality is disabled for floating 
point scale

  - VolatileImage buffer size is recalculates using transform translate

  Using floating point scale leads that drawing the same thing from
different coordinates gives different results. For example filling a
rectangle with size (1, 1) from location (0, 0) and UI scale 1.5 gives
scaled region (0, 0, 1.5, 1.5) which is rounded to (0, 0, 2, 2). The
same rectangle filled from the location (1, 1) gives the scaled region
(1.5, 1.5, 3, 3) which is rounded to (2, 2, 3, 3). The first rectangle
has size 2 in the device space and the second one has 1.
  As a result drawing a component from some coordinates and using
Graphics.copyArea() to translate an image to a new location could have a
differ results than just drawing the same component from the new 
location.

  The fix suggests to disable the JScrollPane area copying for floating
point scales.

Thanks,
Alexandr.

On 10/7/2016 4:30 PM, Alexandr Scherbatiy wrote:

On 10/6/2016 11:42 PM, Sergey Bylokhov wrote:

Hi, Alexandr.
Can you please provide some standalone small example, which emulates
this artifacts via java2d API. (The pattern which we use in
RepainManager). It will help to understand the problem.


The code sample [1]  draws two the same shapes (with dif

Re: [9] Review request for 8075084: https://bugs.openjdk.java.net/browse/JDK-8075084

[Alexandr Scherbatiy]

On 10/25/2016 4:24 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

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

When a modal dialog is shown in scroll bar button click listener it 
blocks all events targeted to the scroll bar owner window.


At the same time clicking on scroll bar button triggers a timer which 
should adjust scroll bar value automatically until mouse button is 
released, i.e. until mouse release event comes to the pushed scroll 
button. So, this timer is not stopped by the mouse release event 
because it is rejected by the modal filter.  When modal dialog is 
closed the timer continues to produce scroll adjustment events and the 
dialog is shown again. Also the scroll button remains pushed and 
hovered all the time (because it didn't get mouse release event).


In the suggested fix the timer is started only if the scroll bar owner 
window keeps input focus after the adjustment callback and the scroll 
button is reset otherwise.


  Is it possible that a scroll bar lost the focus not because a new 
modal dialog opened by some other valid reason and the scroll timer 
should be started for this case?


  Thanks,
  Alexandr.



--Semyon

Re: RFR: 8155874: Fix java.desktop deprecation warnings about Class.newInstance

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 11/7/2016 10:12 PM, Sergey Bylokhov wrote:

Looks fine.

On 07.11.16 21:49, Phil Race wrote:

bug: https://bugs.openjdk.java.net/browse/JDK-8155874
Webrev: http://cr.openjdk.java.net/~prr/8155874/

This hits all across the desktop module, hence the cross-post.

The Class.newInstance() has been deprecated since it
may throw checked exceptions that are not declared.

Class.getConstructor().newInstance() was recommended as a
replacement but it will return only public constructors.

So if you have package access to a package private constructor it will
fail where
as the previous pattern succeeded

So the recommendation now is to use
Class.getDeclaredConstructor().newInstance()
and this fix uses that except for some cases where we have a limited and
known
set of internal "service providers" which are known to use public
classes and constructors.

Also some exception catching has been cleaned up as appropriate for the
new method call and taking advantage of the JDK 1.7
ReflectiveOperationException

-phil.

Re: [9] RFR JDK-8048702: Deprecate obsolete classes in javax/swing/plaf/metal/MetalFileChooserUI.java

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 10/31/2016 3:03 PM, Sergey Bylokhov wrote:

+1
Thanks.

On 31.10.16 8:10, Prasanta Sadhukhan wrote:

Ok. Added javadoc tag.

http://cr.openjdk.java.net/~psadhukhan/8048702/webrev.01/

Regards
Prasanta
On 10/29/2016 1:05 AM, Sergey Bylokhov wrote:

It seems that most of our deprecated api have an annotation and a
javadoc tag. I guess we should add javadoc tag here as well.

On 28.10.16 14:04, Alexandr Scherbatiy wrote:


The fix looks good to me.

Thanks,
Alexandr.

On 10/28/2016 7:19 AM, Prasanta Sadhukhan wrote:

Please find the webrev with the proposed changes.

http://cr.openjdk.java.net/~psadhukhan/8048702/

Regards
Prasanta
On 10/27/2016 8:03 PM, Alexandr Scherbatiy wrote:

On 10/26/2016 10:28 AM, Prasanta Sadhukhan wrote:

Hi All,

Please review a fix for the issue where this obsolete class 
needs to

be deprecated:

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

diff -r aae3690e53e3
src/java.desktop/share/classes/javax/swing/plaf/metal/MetalFileChooserUI.java 




---
a/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalFileChooserUI.java 



Thu Oct 20 14:21:46 2016 +0300
+++
b/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalFileChooserUI.java 



Wed Oct 26 12:56:48 2016 +0530
@@ -570,8 +570,9 @@
 }

 /**
- * Obsolete class, not used in this version.
+ * Obsolete class, not used in this version. deprecated as of
JDK version 9.

  - The deprecated word should start with the capital letter after
the full stop.

  */
+@Deprecated

- since="1.9" can be added.
- Could you provide the webrev for the fix?

  Thanks,
  Alexandr.


 protected class SingleClickListener extends MouseAdapter {
 /**
  * Constructs an instance of {@code SingleClickListener}.
@@ -583,8 +584,9 @@
 }

 /**
- * Obsolete class, not used in this version.
+ * Obsolete class, not used in this version. deprecated as of
JDK version 9.
  */
+@Deprecated
 @SuppressWarnings("serial") // Superclass is not serializable
across versions
 protected class FileRenderer extends DefaultListCellRenderer {
 }

Regards
Prasanta

Re: 8138771: java.awt.image.AbstractMultiResolutionImage needs customized spec for methods of Image which it implements

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 10/31/2016 9:31 PM, Jim Graham wrote:

Looks good.  +1

...jim

On 10/30/16 11:53 PM, Avik Niyogi wrote:

Hi All,
Please review the proposed specification for JDK9 including inputs 
from reviewer reviews.
*cr.openjdk.java.net/~aniyogi/8138771/webrev.05/* 


Thank you in advance.

With Regards,
Avik Niyogi
On 28-Oct-2016, at 1:18 am, Jim Graham > wrote:


Hi Avik,

My suggestion about adding a word "the" was not taken and a couple 
of other changes were made to the @return
statements which are not optimal.  Let's reset and use the following 
@return statements for each of the methods (to

mirror the way these are described in the Image base class):

getWidth() - @return the width of the base image, or -1 if the width 
is not yet known
getHeight() - @return the height of the base image, or -1 if the 
height is not yet known

getGraphics() - @return throws {@code UnsupportedOperationException}
getSource() - @return the image producer that produces the pixels 
for the base image
getProperty() - @return the value of the named property in the base 
image


(It would also be nice if the blank lines were the same in all of 
the doc comments.  Some comments have a couple of
blank lines to separate the javadoc sections and others have no 
blank lines.  But, that doesn't affect correctness, it

is just an easthetic issue...)

...jim

On 10/26/16 11:51 PM, Avik Niyogi wrote:

Hi All,

Please review the proposed specification for JDK9 including inputs 
from reviewer reviews.

*http://cr.openjdk.java.net/~aniyogi/8138771/webrev.04/*
Thank you in advance.

With Regards,
Avik Niyogi

On 27-Oct-2016, at 2:33 am, Jim Graham 

> wrote:

The "@return" tags should not start with "returns" in the text.

Also, in the @return for getProperty(), insert a word "the" as 
"the property of the base image"...


...jim

On 10/26/16 12:36 AM, Avik Niyogi wrote:

Hi All,

Please review the proposed specification for JDK9 including 
inputs from reviver reviews.


*cr.openjdk.java.net/~aniyogi/8138771/webrev.03/* 






Thank you in advance.

With Regards,
Avik Niyogi

Re: [9] Review request for 8153522: Update JLightweightFrame to allow non-integer (and X/Y) scales

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 10/28/2016 9:59 AM, Semyon Sadetsky wrote:

On 10/25/2016 8:17 PM, Alexandr Scherbatiy wrote:



On 9/29/2016 10:55 AM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

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

LightweightFrame Swing embedding API is updated for capability with 
floating point scale to make JavaFX SwingNode correctly shown on 
Windows platform.


Backward API compatibility is preserved but methods with old 
signatures are marked as deprecated.


JLightweightFrame

 155 new Rectangle(0, 0,
 156 (int)Math.round(bbImage.getWidth() / scaleFactorX),
 157 (int)Math.round(bbImage.getHeight() / scaleFactorY)));

 303 int startX = (int)(x * scaleX);
 304 int startY = (int)(y * scaleY);
 305 int width  = (int)((x + w) * scaleX + 0.5) - startX;
 306 int height = (int)((y + h) * scaleY + 0.5) - startY;

The usual rule is to use Math.floor() for the top left corner of a 
region and Math.ceil() for for the right bottom corner.


171 imageBufferReset(data, x, y, width, height, linestride, 
(int)scaleX);


May be it is better to use Math.round() here for the scale rounding.
Please review the updated webrev: 
http://cr.openjdk.java.net/~ssadetsky/8153522/webrev.01/


Thanks,
Alexandr.



--Semyon

[9] Review request for 8168992 Add floating point implementation for new BasicGraphicsUtils text related methods use floating point API

[Alexandr Scherbatiy]

Hello,

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

  The fix JDK-8132119 added stubs for the following methods:
  BasicGraphicsUtils.drawString(JComponent c, Graphics2D g, String 
string, float x, float y)
  BasicGraphicsUtils.drawStringUnderlineCharAt(JComponent c, Graphics2D 
g, String string, int underlinedIndex, float x, float y)
  BasicGraphicsUtils.getStringWidth(JComponent c, FontMetrics fm, 
String string)


  The current fix adds implementation which uses floating point API for 
these methods.


  The fix also updated the implementation to use floating point API for 
the method:
Utilities.drawTabbedText(Segment s, float x, float y, Graphics2D g, 
TabExpander e, int startOffset)


 Thanks,
 Alexandr.

[9] Review request for 8168899 java.nio.file.InvalidPathException if click button in JFileChooser demo of SwingSet2

[Alexandr Scherbatiy]

Hello,

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

  It has been already discussed in the thread
http://mail.openjdk.java.net/pipermail/swing-dev/2016-October/006803.html

  However, the initial bug id 8167988 has been used for the original 
fix 7067885 reverting.
  The new bug id 8168899 has been created to fix both the original 
issue and the InvalidPathException.


 Thanks,
 Alexandr.

Re: [9] RFR JDK-8048702: Deprecate obsolete classes in javax/swing/plaf/metal/MetalFileChooserUI.java

[Alexandr Scherbatiy]

On 10/26/2016 10:28 AM, Prasanta Sadhukhan wrote:

Hi All,

Please review a fix for the issue where this obsolete class needs to 
be deprecated:


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

diff -r aae3690e53e3 
src/java.desktop/share/classes/javax/swing/plaf/metal/MetalFileChooserUI.java
--- 
a/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalFileChooserUI.java 
Thu Oct 20 14:21:46 2016 +0300
+++ 
b/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalFileChooserUI.java 
Wed Oct 26 12:56:48 2016 +0530

@@ -570,8 +570,9 @@
 }

 /**
- * Obsolete class, not used in this version.
+ * Obsolete class, not used in this version. deprecated as of JDK 
version 9.
  - The deprecated word should start with the capital letter after the 
full stop.

  */
+@Deprecated

- since="1.9" can be added.
- Could you provide the webrev for the fix?

  Thanks,
  Alexandr.


 protected class SingleClickListener extends MouseAdapter {
 /**
  * Constructs an instance of {@code SingleClickListener}.
@@ -583,8 +584,9 @@
 }

 /**
- * Obsolete class, not used in this version.
+ * Obsolete class, not used in this version. deprecated as of JDK 
version 9.

  */
+@Deprecated
 @SuppressWarnings("serial") // Superclass is not serializable 
across versions

 protected class FileRenderer extends DefaultListCellRenderer {
 }

Regards
Prasanta

Re: [9] Review request for 8153522: Update JLightweightFrame to allow non-integer (and X/Y) scales

[Alexandr Scherbatiy]

On 9/29/2016 10:55 AM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

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

LightweightFrame Swing embedding API is updated for capability with 
floating point scale to make JavaFX SwingNode correctly shown on 
Windows platform.


Backward API compatibility is preserved but methods with old 
signatures are marked as deprecated.


JLightweightFrame

 155 new Rectangle(0, 0,
 156   (int)Math.round(bbImage.getWidth() / 
scaleFactorX),
 157   (int)Math.round(bbImage.getHeight() / 
scaleFactorY)));


 303 int startX = (int)(x * scaleX);
 304 int startY = (int)(y * scaleY);
 305 int width  = (int)((x + w) * scaleX + 0.5) - startX;
 306 int height = (int)((y + h) * scaleY + 0.5) - startY;

The usual rule is to use Math.floor() for the top left corner of a 
region and Math.ceil() for for the right bottom corner.


171 imageBufferReset(data, x, y, width, height, linestride, 
(int)scaleX);


May be it is better to use Math.round() here for the scale rounding.

Thanks,
Alexandr.



--Semyon

Re: [9] Review request for 8075904: The regression-swing case failed as Ctrl-F4 can't work with the special options"-client -Dswing.defaultlaf=javax.swing.plaf.nimbus.NimbusLookAndFeel"

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 10/25/2016 2:29 AM, Sergey Bylokhov wrote:

Looks fine.

On 21.10.16 19:22, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

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

Missed key-bindings are added for DesktopPane in Nimbus L

--Semyon

Re: [9] Review request for 8074883: Tab key should move to focused button in a button group

[Alexandr Scherbatiy]

On 10/19/2016 8:14 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

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

To avoid unexpected selection change the selected button of a button 
group should always grab focus when focus is transferred form 
component outside the group to any unselected button inside the group 
in case of traversal or initial container activation actions.
  - It is better to pass the cause and boolean focusInWindow arguments 
to the getGroupSelection() method to avoid some code duplication like 
switching over the same cause values.
  - The fix will require a CCC request because it updates a javadoc for 
the publci method.


  Thanks,
  Alexandr.


--Semyon

Re: RfR JDK-8167213 Move include/bridge/AccessBridgeCalls.c to the source directory

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 10/24/2016 1:18 PM, Erik Joelsson wrote:

The last change looks good and simple to me.

/Erik


On 2016-10-21 06:55, Pete Brunet wrote:

Please see the latest update
http://cr.openjdk.java.net/~ptbrunet/JDK-8167213/webrev.02/

The fix now is to simply remove the copy of the AccessBridgeCalls.c file
into the JDK.

AccessBridgeCalls.c is the implementation of the documented Java Access
Bridge API and is a set of wrapper functions that hides the
complications related to interfacing to JAB's WindowsAccessBridge*.dll.
In the past users of the API would compile and link to
AccessBridgeCalls.c/obj.

Since the interface implementation of AccessBridgeCalls.c will no longer
be provided the JAB API documentation will be updated to instruct a user
how to create an equivalent of AccessBridgeCalls.c.  The documentation
will also contain a reference to the JAB 2.0.2 download
http://www.oracle.com/technetwork/java/javase/downloads/jab-2-0-2-download-354311.html 


which does contain AccessBridgeCalls.c and which is compatible with the
current API and related calls into WindowsAccessBridge*.dll.

Pete

On 10/18/16 12:28 PM, Pete Brunet wrote:

I've updated the webrev.  Please see
http://cr.openjdk.java.net/~ptbrunet/JDK-8167213/webrev.01/

Rather than removing the files needed by Assistive Technology 
developers
we have to provide them in JDK.  However since there is a .c file in 
the

group of files the files were moved from the include directory to a new
javaaccessbridge directory.

On 10/17/16 2:43 AM, Magnus Ihse Bursie wrote:

On 2016-10-14 17:51, Pete Brunet wrote:

Please review the following.

The .h files and .c file provided to allow Assistive Technology to
interface to the Java Access Bridge API are being removed from the 
built
JRE/JDK images.  They are not used much and they can be obtained 
online

via the OpenJDK web site.  The pubs will be updated to mention the
location of the files.

Since there is a .c file in this group of files the directory 
structure

has been changed slightly to remove the include directory.

There was one file missing from the group of files needed by 
developers

and that was moved from the common to the bridge directory.

The make was updated in response to the above.

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

Webrev: http://cr.openjdk.java.net/~ptbrunet/JDK-8167213/webrev.00/

Build changes looks good to me.

/Magnus

Re: [9] Review request for 8027639: JComboBox's popup leaves tracks after closing

[Alexandr Scherbatiy]

On 10/10/2016 12:55 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

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

When the Swing repaint manager paints Swing to a separate off-screen 
buffer the root window is not painted. So in case of non-opaque root 
window having transparent background the repainted buffer area 
transparency is not restored and previously visible Swing components 
can remain visible.


The solution is to paint the root window's background to the repainted 
area in case the root window is non-opaque.

  Should the previous composite be restored after the rect filling?

  Thanks,
  Alexandr.


--Semyon

Re: RFR: PIT Bug 8167988 : java.nio.file.InvalidPathException if click button in JFileChooser demo of SwingSet2

[Alexandr Scherbatiy]

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

  - Path creation is moved under instanceof ShellFolder check
  - Paths.get(file.getPath()) is changed to file.toPath()

  Thanks,
  Alexandr.

On 10/25/2016 9:36 AM, Semyon Sadetsky wrote:


It seems the fix should be:

===
--- src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
+++ src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
@@ -244,10 +244,10 @@
   * @exception FileNotFoundException if file does not exist
   */
  public static ShellFolder getShellFolder(File file) throws 
FileNotFoundException {
-Path path = Paths.get(file.getPath());
  if (file instanceof ShellFolder) {
  return (ShellFolder)file;
  }
+Path path = Paths.get(file.getPath());
  if (!Files.exists(path, LinkOption.NOFOLLOW_LINKS)) {
  throw new FileNotFoundException();
  }
===

--Semyon
On 10/25/2016 1:26 AM, Phil Race wrote:

SQE reports some 35 tests fail as a result of this bug.
I am backing out the change that caused it from our PIT and need a 
reviewer.

SQE will verify this before I push.

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

Fix :

hg diff src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
diff --git 
a/src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java 
b/src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java

--- a/src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
+++ b/src/java.desktop/share/classes/sun/awt/shell/ShellFolder.java
@@ -30,10 +30,6 @@
 import java.awt.Toolkit;
 import java.io.*;
 import java.io.FileNotFoundException;
-import java.nio.file.Files;
-import java.nio.file.LinkOption;
-import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.*;
 import java.util.concurrent.Callable;

@@ -244,11 +240,10 @@
  * @exception FileNotFoundException if file does not exist
  */
 public static ShellFolder getShellFolder(File file) throws 
FileNotFoundException {

-Path path = Paths.get(file.getPath());
 if (file instanceof ShellFolder) {
 return (ShellFolder)file;
 }
-if (!Files.exists(path, LinkOption.NOFOLLOW_LINKS)) {
+if (!file.exists()) {
 throw new FileNotFoundException();
 }
 return shellFolderManager.createShellFolder(file);

Re: [9] Review request for 8162350 RepaintManager shifts repainted region when the floating point UI scale is used

[Alexandr Scherbatiy]

  Hello,

  Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8162350/webrev.01
  - JScrollPane copy area functionality is disabled for floating point 
scale

  - VolatileImage buffer size is recalculates using transform translate

  Using floating point scale leads that drawing the same thing from 
different coordinates gives different results. For example filling a 
rectangle with size (1, 1) from location (0, 0) and UI scale 1.5 gives 
scaled region (0, 0, 1.5, 1.5) which is rounded to (0, 0, 2, 2). The 
same rectangle filled from the location (1, 1) gives the scaled region 
(1.5, 1.5, 3, 3) which is rounded to (2, 2, 3, 3). The first rectangle 
has size 2 in the device space and the second one has 1.
  As a result drawing a component from some coordinates and using 
Graphics.copyArea() to translate an image to a new location could have a 
differ results than just drawing the same component from the new location.
  The fix suggests to disable the JScrollPane area copying for floating 
point scales.


Thanks,
Alexandr.

On 10/7/2016 4:30 PM, Alexandr Scherbatiy wrote:

On 10/6/2016 11:42 PM, Sergey Bylokhov wrote:

Hi, Alexandr.
Can you please provide some standalone small example, which emulates 
this artifacts via java2d API. (The pattern which we use in 
RepainManager). It will help to understand the problem.


The code sample [1]  draws two the same shapes (with different colors) 
one after another into areas (x, y, w, h) and (x+w, y, w, h) 
accordingly in different ways.


The shape is constructed from the following parts:
1. Fill clip area
 - set clip (x, y, w, h)
 - fill the whole image
As a result only clipped area is filled.

2. Fill rect
- fill rect(x, y, w, h) // big rect
- fill rect(x+1, y+1, w-2, h-2) // small rect

3. Draw center lines
- draw line (x, cy, x + w, cy)
- draw line (cx, y, cx, y + h)

The program has the following options:
  RECT - draw two shapes one after another from point (0, 0)
  SHIFTED_RECT - draw two shapes one after another from point (x, y)
  BACKBUFFER - draw the shape into a backbuffer with size (w, h) and 
draw the backbuffer from the point (x, y)
  SCALED_BACKBUFFER - draw the shape into a scaled backbuffer with 
size (ceil(w*scale), ceil(h*scale)) with scaled graphics from point 
(0, 0) and draw it into the rectangle (x, y, w, h)
  ENLARGED_SCALED_BACKBUFFER - draw the shape into a scaled backbuffer 
with size (ceil((x+w)*scale), ceil((y+h)*scale)) with scaled graphics 
from point (x, y) and draw it into the rectangle (0, 0, x+w, y+h)


The resulted images are placed in the directory [2].
Directory name "rect-[7,5,10,8]" means that the rectangles (7,5,10,8) 
was used for the shape drawing.
Each screenshot name follows the template " 
screenshot-N-[x,y,w,h]-TYPE.png" where the type is a program option 
used for the image generation.
Screenshots with suffix "-compare" compares the golden image (shape 
drawn in to the rectangle (x, y, w, h)) with the generated image. The 
golden image is on the top left side. The generated image is shown on 
the right and bottom side.


The RepaintManager has an assumption that drawing something in some 
area (x, y, w, h) or just drawing the same thing into an image with 
translated graphics g.translate(-x1, -y1) and drawing the image into 
the area(x, y, w, h)  has the the same result.


As it is shown on screenshots this statement is not true for floating 
point scales.
For example the same shape drawn from the point (0,0) and (x,y) look 
differently (see [3] and [4]).


The solution could be just to use an enlarged backbuffer with size 
(x+w, y+h) with scaled graphics, draw the component into the rectangle 
(x, y, w, h) and draw the backbuffer into the area (0,0, x+w, y+h).
Even the scaled enlarged backbuffer is used the results can be differ 
(see [5] where the rectangle [7, 5, 11, 9] is used. The backbuffer is 
drawn into bigger size).



[1] code samples: 
http://cr.openjdk.java.net/~alexsch/8162350/code/00/Java2DFPSamples.java
[2] dir with screenshots results: 
http://cr.openjdk.java.net/~alexsch/8162350/results
[3] RECT: 
http://cr.openjdk.java.net/~alexsch/8162350/results/rect-%5b7%2c5%2c10%2c8%5d/screenshot-01-%5b7%2c5%2c10%2c8%5d-rects.png
[4] SHIFTED_RECT: 
http://cr.openjdk.java.net/~alexsch/8162350/results/rect-%5b7%2c5%2c10%2c8%5d/screenshot-02-%5b7%2c5%2c10%2c8%5d-shifted-rects.png
[5] ENLARGED_SCALED_BACKBUFFER compare: 
http://cr.openjdk.java.net/~alexsch/8162350/results/rect-%5b7%2c5%2c11%2c9%5d/screenshot-05-%5b7%2c5%2c11%2c9%5d-enlarged-scaled-backbuffers-compare.png


Thanks,
Alexandr.



On 06.10.16 20:07, Alexandr Scherbatiy wrote:

Hello,

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

  The fix uses the solution suggest by Jim in the email:
http://mail.openjdk.java.net/pipermail/2d-dev/2016-October/007737.html

  To draw to a VolatileImage backbuffer 

Re: [9] Review request for 8167176 Exported elements referring to inaccessible types in java.desktop

[Alexandr Scherbatiy]

Hello,

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

  - JRootPane.defaultPressAction/defaultReleaseAction fields are removed
  - MetalBorders.bumps and MetalScrollBarUI.bumps are made private
  - MetalFileChooserUI.createDirectoryComboBoxRenderer() method is made 
private


Thanks,
Alexandr.

On 10/18/2016 8:23 PM, Phil Race wrote:



On 10/18/2016 03:54 AM, Alexandr Scherbatiy wrote:

On 10/18/2016 12:42 AM, Philip Race wrote:

Hi,

First note that any of the alternatives here require an approved CCC 
before pushing.


As I wrote in the bug the fields in JRootPane are not used. Making 
it a public supertype
is no more useful than just deleting it. This like hiding the peers 
which was a much

bigger change so please just delete these fields.

I don't understand the reasoning behind changing "bumps" to be of 
the super-class type Icon

and introducing a new variable that is what is then used.

Since "MetalBumps" is package private we should be able to safely 
assume no one has
been using these fields without the aid of reflection and I doubt 
anyone found a need.

And that won't work in JDK 9 with upcoming jigsaw changes
If you wanted people to usefully access these then you'd need to 
make MetalBumps public.
I *am not* suggesting that .. just pointing out that the 
alternatives I see are either

make it entirely public, or entirely hide is as being a mistake.
So make the fields all private and do not provide the non-useful 
public replacement.
And I mean remove today. In JDK 9. This is not as radical as it 
seems since left as

it is they are useless to anyone as you cannot use them.

Also createDirectoryComboBoxRenderer(..) can be made private too.
  All these fields and methods can be used with the public 
super-class type in a user code.

  For example the code below successfully compiled with JDK 8 and 9:
  -
public class Test {

static class CustomToolBarBorder extends 
MetalBorders.ToolBarBorder {


public void doSomething(Graphics g) {
Icon metalBumps = bumps;
metalBumps.paintIcon(new JLabel(), g, 0, 0);
}
}

static class CustomMetalFileChooserUI extends MetalFileChooserUI {

public CustomMetalFileChooserUI(JFileChooser filechooser) {
super(filechooser);
}

public void doSomething() {
DefaultListCellRenderer listCellRenderer = 
createDirectoryComboBoxRenderer(getFileChooser());

// do something with the listCellRenderer
}
}


See if you can find any 3rd party code at all that uses any of these ...



static class CustomJRootPane extends JRootPane {

public void doSomething() {
AbstractAction pressAction = defaultPressAction != null
? defaultPressAction
: new CustomAction();
AbstractAction releaseAction = defaultReleaseAction != null
? defaultReleaseAction
: new CustomAction();
}
}
}


This example is weird since defaultPressAction and 
defaultReleaseAction are ALWAYS null

unless the application itself set them to something.
It is completely acceptable to me to delete these fields.



  -

  I just need to be sure that the incompatible change introduced by 
removing these fields or making them private is acceptable.






Finally, when these are resolved you should be able to re-enable lint !
http://hg.openjdk.java.net/jdk9/dev/rev/81435a812f59
  I prepared the hint enabling fix but I believe it should be 
reviewed on the build-dev and jigsaw-dev aliases after the current 
fix is pushed.


I don't think this is at all necessary. The warning has nothing to do 
with jigsaw and isn't a build change per se.
It should be included with this webrev+subsequent push, under this bug 
ID.



-phil.



  Thanks,
  Alexandr.


-phil.

On 10/17/16, 4:22 AM, Alexandr Scherbatiy wrote:


  Hello,

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

  MetalBorders.bumps and MetalScrollBarUI.bumps fields are nor 
marked for removal any more.


  Thanks,
  Alexandr.

On 10/14/2016 3:23 PM, Sergey Bylokhov wrote:
Is it necessary to remove tese fields? For example 
MetalScrollBarUI.bumps looks similar to other fields 
MetalScrollButton.increaseButton/decreaseButton.


On 13.10.16 16:15, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8167176
  webrev: http://cr.openjdk.java.net/~alexsch/8167176/webrev.00
  - Inaccessible classes returned by public API in swing are 
changed to

their public super classes.
  - Fields which expose inaccessible classes are deprecated and 
marked

for removal.

  Changing a public field type to its superclass is incompatible 
change
with previous releases. However it is not possible to use a class 
which

is not exported in JDK 9 because o

Re: [9] Review request for 8167176 Exported elements referring to inaccessible types in java.desktop

[Alexandr Scherbatiy]

On 10/18/2016 12:42 AM, Philip Race wrote:

Hi,

First note that any of the alternatives here require an approved CCC 
before pushing.


As I wrote in the bug the fields in JRootPane are not used. Making it 
a public supertype
is no more useful than just deleting it. This like hiding the peers 
which was a much

bigger change so please just delete these fields.

I don't understand the reasoning behind changing "bumps" to be of the 
super-class type Icon

and introducing a new variable that is what is then used.

Since "MetalBumps" is package private we should be able to safely 
assume no one has
been using these fields without the aid of reflection and I doubt 
anyone found a need.

And that won't work in JDK 9 with upcoming jigsaw changes
If you wanted people to usefully access these then you'd need to make 
MetalBumps public.
I *am not* suggesting that .. just pointing out that the alternatives 
I see are either

make it entirely public, or entirely hide is as being a mistake.
So make the fields all private and do not provide the non-useful 
public replacement.
And I mean remove today. In JDK 9. This is not as radical as it seems 
since left as

it is they are useless to anyone as you cannot use them.

Also createDirectoryComboBoxRenderer(..) can be made private too.
  All these fields and methods can be used with the public super-class 
type in a user code.

  For example the code below successfully compiled with JDK 8 and 9:
  -
public class Test {

static class CustomToolBarBorder extends MetalBorders.ToolBarBorder {

public void doSomething(Graphics g) {
Icon metalBumps = bumps;
metalBumps.paintIcon(new JLabel(), g, 0, 0);
}
}

static class CustomMetalFileChooserUI extends MetalFileChooserUI {

public CustomMetalFileChooserUI(JFileChooser filechooser) {
super(filechooser);
}

public void doSomething() {
DefaultListCellRenderer listCellRenderer = 
createDirectoryComboBoxRenderer(getFileChooser());

// do something with the listCellRenderer
}
}

static class CustomJRootPane extends JRootPane {

public void doSomething() {
AbstractAction pressAction = defaultPressAction != null
? defaultPressAction
: new CustomAction();
AbstractAction releaseAction = defaultReleaseAction != null
? defaultReleaseAction
: new CustomAction();
}
}
}
  -

  I just need to be sure that the incompatible change introduced by 
removing these fields or making them private is acceptable.




Finally, when these are resolved you should be able to re-enable lint !
http://hg.openjdk.java.net/jdk9/dev/rev/81435a812f59
  I prepared the hint enabling fix but I believe it should be reviewed 
on the build-dev and jigsaw-dev aliases after the current fix is pushed.


  Thanks,
  Alexandr.


-phil.

On 10/17/16, 4:22 AM, Alexandr Scherbatiy wrote:


  Hello,

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

  MetalBorders.bumps and MetalScrollBarUI.bumps fields are nor marked 
for removal any more.


  Thanks,
  Alexandr.

On 10/14/2016 3:23 PM, Sergey Bylokhov wrote:
Is it necessary to remove tese fields? For example 
MetalScrollBarUI.bumps looks similar to other fields 
MetalScrollButton.increaseButton/decreaseButton.


On 13.10.16 16:15, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8167176
  webrev: http://cr.openjdk.java.net/~alexsch/8167176/webrev.00
  - Inaccessible classes returned by public API in swing are 
changed to

their public super classes.
  - Fields which expose inaccessible classes are deprecated and marked
for removal.

  Changing a public field type to its superclass is incompatible 
change
with previous releases. However it is not possible to use a class 
which

is not exported in JDK 9 because of the modularization feature.

  Thanks,
  Alexandr.

Re: [9] Review request for 8167176 Exported elements referring to inaccessible types in java.desktop

[Alexandr Scherbatiy]

  Hello,

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

  MetalBorders.bumps and MetalScrollBarUI.bumps fields are nor marked 
for removal any more.


  Thanks,
  Alexandr.

On 10/14/2016 3:23 PM, Sergey Bylokhov wrote:
Is it necessary to remove tese fields? For example 
MetalScrollBarUI.bumps looks similar to other fields 
MetalScrollButton.increaseButton/decreaseButton.


On 13.10.16 16:15, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8167176
  webrev: http://cr.openjdk.java.net/~alexsch/8167176/webrev.00
  - Inaccessible classes returned by public API in swing are changed to
their public super classes.
  - Fields which expose inaccessible classes are deprecated and marked
for removal.

  Changing a public field type to its superclass is incompatible change
with previous releases. However it is not possible to use a class which
is not exported in JDK 9 because of the modularization feature.

  Thanks,
  Alexandr.

Re: [9] Review request for 8163167: [PIT] javax/swing/JTextArea/ScrollbarFlicker/ScrollFlickerTest.java always fail

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 10/14/2016 8:52 PM, Sergey Bylokhov wrote:

On 03.10.16 12:35, Semyon Sadetsky wrote:

But if layout was done, why we get any notifications? Should the check
be (cnt != 0)?

Okay. I have changed it to zero:
http://cr.openjdk.java.net/~ssadetsky/8163167/webrev.01/


Looks fine.






I assume that each notification should be called when the size of
JViewport or View were changed?







On 23.08.16 11:59, Alexander Scherbatiy wrote:

The fix looks good to me.

Thanks,
Alexandr.

On 23/08/16 11:40, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

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

The cause of this test bug is revalidating JScrollPane
layout may
take
various number of iterations on different L

To fix the test the counter listener is added after the
layout
revalidation has been started.

--Semyon

Re: [9] Review request for 8164321: Crash of SwingNode with GTK LaF

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 10/11/2016 8:56 PM, Alexander Zvegintsev wrote:

+1


On 10/6/16 11:18 AM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

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

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

The issue is caused by concurrent XLib access form FX and AWT toolkit 
threads in case of GTK3. To fix the issue wrapping by 
gdk_threads_enter/gdk_threads_leave is added to JDK GTK+ LnF GTK3 
implementation. Since FX GTK3 implementation uses 
gdk_threads_enter/gdk_threads_leave as well this will make GTK3 XLib 
calls serialized.


--Semyon

Re: [9] Review request for 8162350 RepaintManager shifts repainted region when the floating point UI scale is used

[Alexandr Scherbatiy]

On 10/6/2016 11:42 PM, Sergey Bylokhov wrote:

Hi, Alexandr.
Can you please provide some standalone small example, which emulates 
this artifacts via java2d API. (The pattern which we use in 
RepainManager). It will help to understand the problem.


The code sample [1]  draws two the same shapes (with different colors) 
one after another into areas (x, y, w, h) and (x+w, y, w, h) accordingly 
in different ways.


The shape is constructed from the following parts:
1. Fill clip area
 - set clip (x, y, w, h)
 - fill the whole image
As a result only clipped area is filled.

2. Fill rect
- fill rect(x, y, w, h) // big rect
- fill rect(x+1, y+1, w-2, h-2) // small rect

3. Draw center lines
- draw line (x, cy, x + w, cy)
- draw line (cx, y, cx, y + h)

The program has the following options:
  RECT - draw two shapes one after another from point (0, 0)
  SHIFTED_RECT - draw two shapes one after another from point (x, y)
  BACKBUFFER - draw the shape into a backbuffer with size (w, h) and 
draw the backbuffer from the point (x, y)
  SCALED_BACKBUFFER - draw the shape into a scaled backbuffer with size 
(ceil(w*scale), ceil(h*scale)) with scaled graphics from point (0, 0) 
and draw it into the rectangle (x, y, w, h)
  ENLARGED_SCALED_BACKBUFFER - draw the shape into a scaled backbuffer 
with size (ceil((x+w)*scale), ceil((y+h)*scale)) with scaled graphics 
from point (x, y) and draw it into the rectangle (0, 0, x+w, y+h)


The resulted images are placed in the directory [2].
Directory name "rect-[7,5,10,8]" means that the rectangles (7,5,10,8) 
was used for the shape drawing.
Each screenshot name follows the template " 
screenshot-N-[x,y,w,h]-TYPE.png" where the type is a program option used 
for the image generation.
Screenshots with suffix "-compare" compares the golden image (shape 
drawn in to the rectangle (x, y, w, h)) with the generated image. The 
golden image is on the top left side. The generated image is shown on 
the right and bottom side.


The RepaintManager has an assumption that drawing something in some area 
(x, y, w, h) or just drawing the same thing into an image with 
translated graphics g.translate(-x1, -y1) and drawing the image into the 
area(x, y, w, h)  has the the same result.


As it is shown on screenshots this statement is not true for floating 
point scales.
For example the same shape drawn from the point (0,0) and (x,y) look 
differently (see [3] and [4]).


The solution could be just to use an enlarged backbuffer with size (x+w, 
y+h) with scaled graphics, draw the component into the rectangle (x, y, 
w, h) and draw the backbuffer into the area (0,0, x+w, y+h).
Even the scaled enlarged backbuffer is used the results can be differ 
(see [5] where the rectangle [7, 5, 11, 9] is used. The backbuffer is 
drawn into bigger size).



[1] code samples: 
http://cr.openjdk.java.net/~alexsch/8162350/code/00/Java2DFPSamples.java
[2] dir with screenshots results: 
http://cr.openjdk.java.net/~alexsch/8162350/results
[3] RECT: 
http://cr.openjdk.java.net/~alexsch/8162350/results/rect-%5b7%2c5%2c10%2c8%5d/screenshot-01-%5b7%2c5%2c10%2c8%5d-rects.png
[4] SHIFTED_RECT: 
http://cr.openjdk.java.net/~alexsch/8162350/results/rect-%5b7%2c5%2c10%2c8%5d/screenshot-02-%5b7%2c5%2c10%2c8%5d-shifted-rects.png
[5] ENLARGED_SCALED_BACKBUFFER compare: 
http://cr.openjdk.java.net/~alexsch/8162350/results/rect-%5b7%2c5%2c11%2c9%5d/screenshot-05-%5b7%2c5%2c11%2c9%5d-enlarged-scaled-backbuffers-compare.png


Thanks,
Alexandr.



On 06.10.16 20:07, Alexandr Scherbatiy wrote:

Hello,

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

  The fix uses the solution suggest by Jim in the email:
http://mail.openjdk.java.net/pipermail/2d-dev/2016-October/007737.html

  To draw to a VolatileImage backbuffer its graphics transform is set to
identity and device coordinates are used to set the buffer clip.
  Copying the backbuffer image to the graphics has some problems.
  -
// Since there is no drawImage(img, float x, float y)...
destination.translate(pixelx1 / scaleX, pixely1 / scaleY)
destination.drawImage(img, 0, 0)
  -
  This code solves the problem for the top left corner of the region.
All Graphics.drawImage(...) methods scales the image size and it looks
like ceil(img.getWidth() * scaleX) can be differ from the ceil(pixelx1 +
img.getWidth() * scaleX) - pixelx1 so the right bottom corner of the
image does not fit the required point.
  There is also a question could a line drawn from one point and then
from another has a different width in pixels because the graphics scale
is not integer.

  The proposed fix prepares a backbuffer with size [x + w, y + h] in a
user space and a component is drawn in to the region [pixelx1, pixely1,
pixely2, pixely2] in the device space.
  After that the necessary clip is set to the graphics and whole image
is just drawn into it.

[9] Review request for 8162350 RepaintManager shifts repainted region when the floating point UI scale is used

[Alexandr Scherbatiy]

Hello,

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

  The fix uses the solution suggest by Jim in the email:
http://mail.openjdk.java.net/pipermail/2d-dev/2016-October/007737.html

  To draw to a VolatileImage backbuffer its graphics transform is set 
to identity and device coordinates are used to set the buffer clip.

  Copying the backbuffer image to the graphics has some problems.
  -
// Since there is no drawImage(img, float x, float y)...
destination.translate(pixelx1 / scaleX, pixely1 / scaleY)
destination.drawImage(img, 0, 0)
  -
  This code solves the problem for the top left corner of the region. 
All Graphics.drawImage(...) methods scales the image size and it looks 
like ceil(img.getWidth() * scaleX) can be differ from the ceil(pixelx1 + 
img.getWidth() * scaleX) - pixelx1 so the right bottom corner of the 
image does not fit the required point.
  There is also a question could a line drawn from one point and then 
from another has a different width in pixels because the graphics scale 
is not integer.


  The proposed fix prepares a backbuffer with size [x + w, y + h] in a 
user space and a component is drawn in to the region [pixelx1, pixely1, 
pixely2, pixely2] in the device space.
  After that the necessary clip is set to the graphics and whole image 
is just drawn into it.


  The new logic is used only the component graphics configuration is 
scaled the graphics configuration has the same scales. So it possible 
just to copy the backbuffer surface data to the graphics surface data.
  For other cases like for rotated graphics transform it seems it is 
necessary to have more complicated algorithm.


  This solves problems with repainted region but there are still 
artifacts with JInternalFrame moving or a component scrolling, This can 
be related to the RepaintManager.copyArea() method which needs to be 
updated in the similar way. I have created an issue on it:
   JDK-8167305 RepaintManager.copyArea() method should be updated for 
floating point UI scale

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

 Thanks,
 Alexandr.

Re: [9] Review request for 8157065: There is no the focus border on the selected tab.

[Alexandr Scherbatiy]

On 9/20/2016 10:40 AM, Semyon Sadetsky wrote:

On 9/20/2016 12:00 AM, Alexandr Scherbatiy wrote:

On 9/14/2016 11:51 AM, Semyon Sadetsky wrote:

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.
  The GTK L just tries to paint Swing components in the same way as 
native components.
  It is possible to get a default orientation of a native component 
to which the JTabbedPane corresponds to and use it.
All orientation are equivalent and should correspond to the component 
position. Drawing a border for a tab wi

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

[Alexandr Scherbatiy]

  Thank you for the suggested code.

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

  - the provided method is used to draw triangles in BasicArrowButton class
  - shift to one pixel right is added for the triangles drawing
  - AA rendering hint is set to draw a radio button

  Screenshots [1] and [2] shows difference between icons drawing before 
and after the fix for scales 2x and 4x.


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


  Thanks,
  Alexandr.

On 9/20/2016 5:34 PM, Semyon Sadetsky wrote:

I would rewrite the method that draws triangle to simplify it:

private void paintScaledTriangle(Graphics g, double x, double y, 
double size,

 int direction, boolean isEnabled) {
size = Math.max(size , 2);
Path2D.Double path = new Path2D.Double();
path.moveTo(-size, size/2);
path.lineTo(size, size/2);
path.lineTo(0, -size/2);
path.closePath();
AffineTransform affineTransform = new AffineTransform();
affineTransform.rotate(Math.PI * (direction - 1) / 4 );
path.transform(affineTransform);

Graphics2D g2d = (Graphics2D) g;
g2d.translate(x + size / 2. - 1, y + size / 2);
Color oldColor = g.getColor();
if (!isEnabled) {
g2d.translate(1, 0);
g2d.setColor(highlight);
g2d.fill(path);
g2d.translate(-1, 0);
}
g2d.setColor(isEnabled ? darkShadow : shadow);
g2d.fill(path);
g2d.translate( - x - size / 2. + 1, - y - size / 2);
g2d.setColor(oldColor);
}

I'm suggesting to make the paintTriangle() method accept floating 
point coordinates to get more precise location and size for the scaled 
version.


Optionally, 2D AA hints can be added to get better appearance for 
scales 1.25, 1.5, 1.75.


--Semyon


On 9/20/2016 2:58 PM, Alexandr Scherbatiy wrote:


Hello,

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

 The code formatting issues are updated.

On 9/14/2016 8:02 PM, Semyon Sadetsky wrote:

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.
  I have filled an issue on it: JDK-8166368 JComboBox drawing has 
artifacts with Windows Classic L on HiDPI display

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

  Thanks,
  Alexandr.



--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: OpenJDK JDK-7067885 code changes for community review

[Alexandr Scherbatiy]

  Hello Alok,

  Is it possible to update the line where File.exists() is used just 
converting the file to a Path and using java.nio.exists() check?


  Thanks,
  Alexandr.

On 9/16/2016 1:20 PM, Alexey Ivanov wrote:

Hi Alok,

This change should be discussed on swing-dev mailing list because you 
modify behavior of javax.swing.JFileChooser, and on core-libs-dev 
because you also modify java.io.File.


I agree with Alan, using the new API appears to be a better 
alternative than changing java.io.File.



Regards,
Alexey

On 12.09.2016 19:08, Alan Bateman wrote:
Best to follow-up on swing-dev as I assume the right thing is to 
update JFileChooser rather than changing java.io.File to be mutable. 
You did acknowledge near the end of your mail that the new file 
system API supports sym links so it may be that JFileChooser could 
use that.


-Alan


On 12/09/2016 08:41, Sharma, Alok Kumar (OSTL) wrote:
Following is the fix for JDK-7067885. I am not sure which mailing ID 
to post this.


Bug: FileChooser does not display soft link name if link is to 
nonexistent file/directory

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

This bug is unassigned. Can someone please look into these changes 
and get back to me? Explanation of fix is at the end of the source 
code diff.


Mercurial diff for source change:
---
diff -r 021369229cfd src/java.base/share/classes/java/io/File.java
--- a/src/java.base/share/classes/java/io/File.java Tue Sep 06 
13:09:29 2016 -0400
+++ b/src/java.base/share/classes/java/io/File.java Mon Sep 12 
11:27:07 2016 +0530

@@ -164,6 +164,27 @@
  private final String path;

  /**
+ * The flag indicates whether to follow the symlink.
+ */
+private boolean follow_link = true;
+
+   /**
+* Sets the follow_link of file.
+* description: Sets follow_link on or off.
+* @param follow_link the value that determines whether to 
follow the symlink or not.
+*   TRUE: file.io.exists() checks the file existence using 
stat() which follows the symlink.

+*
+*   FALSE: file.io.exists() checks the file existence using 
lstat() if stat() fails to retrieve
+*  the file info. lstat() if file is a symbolic link, then 
it returns information

+*  about the link itself.
+* @return Returns ZERO at success
+*/
+public int set_follow_link(boolean follow_link) {
+this.follow_link=follow_link;
+return 0;
+}
+
+/**
   * Enum type that indicates the status of a file path.
   */
  private static enum PathStatus { INVALID, CHECKED };
diff -r 021369229cfd 
src/java.base/unix/native/libjava/UnixFileSystem_md.c
--- a/src/java.base/unix/native/libjava/UnixFileSystem_md.c Tue Sep 
06 13:09:29 2016 -0400
+++ b/src/java.base/unix/native/libjava/UnixFileSystem_md.c Mon Sep 
12 11:27:07 2016 +0530

@@ -51,6 +51,7 @@
#define dirent64 dirent
#define readdir64_r readdir_r
#define stat64 stat
+#define lstat64 lstat
#ifndef MACOSX
#define statvfs64 statvfs
#endif
@@ -62,6 +63,7 @@
  jfieldID path;
} ids;

+jfieldID ufs_follow_link;

JNIEXPORT void JNICALL
Java_java_io_UnixFileSystem_initIDs(JNIEnv *env, jclass cls)
@@ -70,6 +72,8 @@
  if (!fileClass) return;
  ids.path = (*env)->GetFieldID(env, fileClass,
"path", "Ljava/lang/String;");
+ufs_follow_link = (*env)->GetFieldID(env, fileClass,
+  "follow_link", "Z");
}

/* -- Path operations -- */
@@ -113,20 +117,42 @@
  return JNI_FALSE;
}

+static jboolean
+lstatMode(const char *path, int *mode)
+{
+struct stat64 sb;
+if (lstat64(path, ) == 0) {
+*mode = sb.st_mode;
+return JNI_TRUE;
+}
+return JNI_FALSE;
+}

JNIEXPORT jint JNICALL
Java_java_io_UnixFileSystem_getBooleanAttributes0(JNIEnv *env, 
jobject this,

jobject file)
{
  jint rv = 0;
+jint follow_link = 0;

  WITH_FIELD_PLATFORM_STRING(env, file, ids.path, path) {
  int mode;
-if (statMode(path, )) {
-int fmt = mode & S_IFMT;
-rv = (jint) (java_io_FileSystem_BA_EXISTS
-  | ((fmt == S_IFREG) ? 
java_io_FileSystem_BA_REGULAR : 0)
-  | ((fmt == S_IFDIR) ? 
java_io_FileSystem_BA_DIRECTORY : 0));
+follow_link = (*env)->GetBooleanField(env, file, 
ufs_follow_link);

+if ( follow_link ) {
+  if (statMode(path, )) {
+  int fmt = mode & S_IFMT;
+  rv = (jint) (java_io_FileSystem_BA_EXISTS
+| ((fmt == S_IFREG) ? 
java_io_FileSystem_BA_REGULAR : 0)
+| ((fmt == S_IFDIR) ? 
java_io_FileSystem_BA_DIRECTORY : 0));

+  }
+}
+else {
+  if (lstatMode(path, )) {
+  int fmt = mode & S_IFMT;
+  rv = (jint) (java_io_FileSystem_BA_EXISTS
+| ((fmt == S_IFREG) ? 

Re: [9] Review request for 8163124 Add floating point API support to javax.swing.text.Caret

[Alexandr Scherbatiy]

On 9/20/2016 2:02 PM, Semyon Sadetsky wrote:

Thanks, that helped.

Did you think about changing the DefaultCaret class to use the new 2D 
API in case it is allowed for the component ?

  DefaultCaret class extends Rectangle so it always uses int coordinates.
  I filled an enhancement to support new floating point version of the 
caret.

JDK-8163174 Add DefaultCaret2D which supports floating point API
  https://bugs.openjdk.java.net/browse/JDK-8163174

  Thanks,
  Alexandr.



--Semyon


On 19.09.2016 22:43, Alexandr Scherbatiy wrote:


This is the known issue: JDK-8163175 PlainView.modelToView() method 
should return Rectangle2D
It is included into the fix JDK-8156217 Selected text is shifted on 
HiDPI display.


You can apply the fix [1] JDK-8156217 to the JDK and try the proposed 
sample.

I get the results [2] for scale 2x and [3] for scale 8x.

[1] http://cr.openjdk.java.net/~alexsch/8156217/webrev.07/all
[2] 
http://cr.openjdk.java.net/~alexsch/8156217/screenshots/model-to-view-2x.png
[3] 
http://cr.openjdk.java.net/~alexsch/8156217/screenshots/model-to-view-8x.png


Thanks,
Alexandr.

On 9/15/2016 1:49 PM, Semyon Sadetsky wrote:

Hi Alexander,

Could you run the next test (with x8 scale, for example):

public class New2dApiTest {
public static void main(String[] args) {
JTextArea jTextArea2d = new 
JTextArea("jksxbqhbxniiiaiiaawww") {

@Override
protected void paintComponent(Graphics g) {
super.paintComponent(g);
Graphics2D g2d = (Graphics2D) g;
g2d.setColor(new Color(255, 0, 0, 50));
for (int i = 0; i < getText().length(); i++) {
try {
g2d.fill(modelToView2D(i));
} catch (BadLocationException e) {
e.printStackTrace();
}
}
}
};
JTextArea jTextArea = new JTextArea("jksxbqhbxniiiaiiaawww") {
@Override
protected void paintComponent(Graphics g) {
super.paintComponent(g);
Graphics2D g2d = (Graphics2D) g;
g2d.setColor(new Color(0, 0, 255, 50));
for (int i = 0; i < getText().length(); i++) {
try {
g2d.fill(modelToView(i));
} catch (BadLocationException e) {
e.printStackTrace();
}
}
}
};

JFrame jFrame = new JFrame();
jFrame.getContentPane().setLayout(new GridLayout(2, 1));
jFrame.getContentPane().add(jTextArea2d);
jFrame.getContentPane().add(jTextArea);
jFrame.pack();
jFrame.setVisible(true);

}
}


For some reason I cannot see the difference with the new and old API 
result.


--Semyon


On 04.08.2016 15:44, Alexandr Scherbatiy wrote:


Hello,

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

  The text position can have floating point value on HiDPI display. 
The Caret interface should be updated to allow use floating point 
positions.


  The fix adds the following public API with floating point positions:
  javax.swing.text.Caret.getMagicCaretPosition2D()
  javax.swing.text.Caret.setMagicCaretPosition2D(Point2D p)
  javax.swing.text.JTextComponent.modelToView2D(int pos)
  javax.swing.text.JTextComponent.viewToModel2D(Point2D pt)
  javax.swing.text.ParagraphView.getClosestPositionTo(int pos, 
Position.Bias b, Shape a, int direction, Position.Bias[], int 
rowIndex, float x)



  The fix replaces 
Caret.getMagicCaretPosition()/setMagicCaretPosition(Point p) to 
Caret.getMagicCaretPosition2D()/setMagicCaretPosition2D(Point2D p) 
in all places
  except DefaultCaret because DefaultCaret extends Rectangle so its 
coordinates always have int values.
  I have filled a separated enhancement for this JDK-8163174 Add 
DefaultCaret2D which supports floating point API


  To make a custom caret use floating point API it is also 
necessary that PlainView.modelToView() returns a rectangle with 
floating point values. It can be done after the fix

JDK-8156217 Selected text is shifted on HiDPI display
  which implements Utilities.getTabbedTextWidth(Segment s, 
FontMetrics metrics, float x, TabExpander e, int startOffset) method.

  I have filled a separated issue on it:
JDK-8163175 PlainView.modelToView() method should return 
Rectangle2D


 Thanks,
 Alexandr.

Re: [9] Review request for 8156217 Selected text is shifted on HiDPI display

[Alexandr Scherbatiy]

On 9/20/2016 1:56 PM, Semyon Sadetsky wrote:


Hi Alexander,

why in the TextUI class the new viewToModel2D and modelToView2D 
methods implementations fall-back to the deprecated ones?


  TextUI is a public abstract class. There is no way to add a new 
abstract method to it and keep a backward compatibility with previous 
version. So new viewToModel2D/modelToView2D methods should have an 
implementation. New code which starts to use new 
TextUI.viewToModel2D()/modelToView2D() methods can get a reference to 
old TextUI class implementation which does not override new 
viewToModel2D()/modelToView2D() methods but it expects that it they 
return a meaningful result. The only way to keep compatibility with old 
classes which extend TextUI class is that new 
viewToModel2D()/modelToView2D() methods fall back to old ones.


  Thanks,
  Alexandr.


--Semyon

On 12.09.2016 15:19, Alexandr Scherbatiy wrote:


Hello,

Could you review the updated fix:
  all changes: http://cr.openjdk.java.net/~alexsch/8156217/webrev.07/all
  public API changes: 
http://cr.openjdk.java.net/~alexsch/8156217/webrev.07/public-api


  - @since 9 tag is added to new methods.

  Thanks,
  Alexandr.

On 9/10/2016 2:36 AM, Philip Race wrote:

- * Returns the tab size set for the document, defaulting to 8.
- *
- * @implSpec This implementation calls {@link #getTabSize() 
getTabSize()}.

- *
- * @return the tab size
- */
-protected float getFractionalTabSize() {
-return getTabSize();
-}
-


It seems this was added only in 9.
Since I think I remember that asking a question about it.
I note it has no @since. Moot if you are really removing it but
what has it to do with the rest of this change ?

-phil.

On 9/9/16, 11:51 AM, Alexandr Scherbatiy wrote:


Hello,

Could you review the updated fix:
 all changes: http://cr.openjdk.java.net/~alexsch/8156217/webrev.06/all
 public API changes: 
http://cr.openjdk.java.net/~alexsch/8156217/webrev.06/public-api


 - reflection is used to detect do methods with floating point API 
need to be called.


Thanks,
Alexandr.

On 9/1/2016 9:17 PM, Semyon Sadetsky wrote:




On 9/1/2016 8:26 PM, Alexandr Scherbatiy wrote:

On 9/1/2016 7:27 PM, Alexandr Scherbatiy wrote:

On 9/1/2016 6:49 PM, Semyon Sadetsky wrote:


Alexander, did you consider possibility to check if method is 
really over-riden then to use the old API?



  Could you give a sample how it can be done?
  I think it is possible to use a reflection to found the latest 
overridden method which uses int coordinates and check does it 
has a corresponding overridden method with floating point 
arguments. But I doubt that it is a good solution.

yes. You could use:
useFloatingPointAPI = 
PlainView.class.equals(getClass().getMethod("drawUnselectedText", 
...).getDeclaringClass());


Otherwise, with high probability your new API will never be used.

--Semyon


  Thanks,
  Alexandr.


  Thanks,
  Alexandr.


--Semyon

On 9/1/2016 3:07 PM, Alexandr Scherbatiy wrote:

On 9/1/2016 11:31 AM, Semyon Sadetsky wrote:


Hi Alexander,

It is a good style to add a note recommending what to use 
instead of the method which is being deprecated.


   Could you review the updated public API there "replaced by" 
notes are added to the deprecated methods:

http://cr.openjdk.java.net/~alexsch/8156217/webrev.05/public-api.02


I did not get for what the useFloatingPointAPI property was 
introduced and moreover is set to false by default. If the 
old API is used then it doesn't matter which value it has 
because the float values will receive ints. And for the new 
API I would expect everything having the float precision, and 
it is unclear what may be the reason to switch it off back to 
integer. Especially if


" This allows to draw text properly using graphics with 
scaled transform."


so an improper mode is the default?

  This is has been discussed below. For example new 
drawSelectedText(Graphics2D g, float x, float y, int p0, int 
p1) with floating point coordinates is added to the PlainView 
which has the same method with int coordinates. Suppose 
someone has a custom password component which uses and old 
methods with int coordinates.


 public class CustomPasswordField extends FieldView {

 @Override
 protected int drawSelectedText(Graphics g, int x, int y, 
int p0, int p1) throws BadLocationException {

 // draw echo chars
 }
 }


If we start to call drawSelectedText() with floating point 
values the customization of old components will not be used 
and the CustomPasswordField from the example starts to show 
real text instead of echo chars. This is incompatible change 
with previous JDK releases.


The solution is to switch to new floating point API only when 
it is known that a component properly overrides new methods 
with floating point arguments. After that the 
PlainView.useFloatingPointAPI flag can be set to true.


For exam

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

[Alexandr Scherbatiy]

On 9/20/2016 2:56 PM, Alexandr Scherbatiy wrote:


   [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 


Radio buttons are still not ideal. Have you considered to enable AA
when painting HiDPI icons? On the other hand I'm not sure, whether we
should invest a lot of time into making Windows Classic L to look
perfect on HiDPI screens.
  I think this all questions can be addressed to Java 2D which draws 
arcs and ovals. It also should be mention that 4 pixels for HiDPI 
display with scale 2x and 16 pixels on a display with scale 4x relate 
just to 1 pixel on non HiDPI display.


  Thanks,
  Alexandr.



Best regards,
Andrej Golovnin 

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

[Alexandr Scherbatiy]

Hello,

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

 The code formatting issues are updated.

On 9/14/2016 8:02 PM, Semyon Sadetsky wrote:

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.
  I have filled an issue on it: JDK-8166368 JComboBox drawing has 
artifacts with Windows Classic L on HiDPI display

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

  Thanks,
  Alexandr.



--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: [9] Review request for 8165594 Bad rendering of Swing UI controls with Windows Classic L on HiDPI display

[Alexandr Scherbatiy]

On 9/16/2016 11:22 AM, Andrej Golovnin wrote:

Hi Alexandr,

thanks! It looks much better now.


   http://cr.openjdk.java.net/~alexsch/8165594/webrev.01

src/java.desktop/share/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java

490 if(isScaledGraphics){

Spaces are missed between 'if' and '(' and between ')' and '{'.

The empty lines 491, 520 and 522 are not needed and should be removed.


src/java.desktop/share/classes/javax/swing/plaf/basic/BasicArrowButton.java

240 if(SwingUtilities2.isScaledGraphics(g)){

Spaces are missed between 'if' and '(' and between ')' and '{'.

344 xPoints = new int[]{mid, mid + size, mid - size};

The Java Style Guidelines [1] don't mention this. But I think there
should be a space between 'new int[]' and '{'. The lines 345, 356,
357, 366 and 367 are also affected.


src/java.desktop/share/classes/sun/swing/SwingUtilities2.java

  I will update the fix to properly format the code.


2041 public static boolean isScaledGraphics(Graphics g) {

Have you considered to make this method a public API (move to the
SwingUtilities class), so that other Swing developers may use it too?

  Please, file an enhancement on this if you think it is worth to do:
http://bugs.java.com

  Thanks,
  Alexandr.




   [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

Radio buttons are still not ideal. Have you considered to enable AA
when painting HiDPI icons? On the other hand I'm not sure, whether we
should invest a lot of time into making Windows Classic L to look
perfect on HiDPI screens.

Best regards,
Andrej Golovnin

[1] http://cr.openjdk.java.net/~alundblad/styleguide/

Re: [9] Review request for 8157065: There is no the focus border on the selected tab.

[Alexandr Scherbatiy]

On 9/14/2016 11:51 AM, Semyon Sadetsky wrote:

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.
  The GTK L just tries to paint Swing components in the same way as 
native components.
  It is possible to get a default orientation of a native component to 
which the JTabbedPane corresponds to and use it.


  Thanks,
  Alexandr.


--Semyon


  Thanks,
  Alexandr.


--Semyon


  Thanks,
  Alexandr.


--Semyon


  Thanks,
  Alexandr.


--Semyon


  Thanks,
  Alexandr.


--Semyon


 Thanks,
 Alexandr.




--Semyon

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

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 9/16/2016 5:34 PM, Semyon Sadetsky wrote:



On 15.09.2016 16:11, Alexander Scherbatiy wrote:

On 15/09/16 15:03, Semyon Sadetsky wrote:



On 15.09.2016 12:49, Alexandr Scherbatiy wrote:

On 9/15/2016 11:47 AM, Semyon Sadetsky wrote:

On 14.09.2016 21:00, Alexandr Scherbatiy wrote:

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.
It is possible. But then the group will get focus second (or more) 
time during the cycle. Seems, this is not correct because option 
in the group has already been selected.
  It is not clear is it correct that pressing an arrow key selects 
a radio button in a group. For example each radio button selection 
can trigger some action. So trying to select the latest radio 
button it is necessary to move the focus from the first button to 
the last by arrow keys so radio buttons in the middle will also be 
unnecessary selected. Usually the space key is responsible to a 
button selection.
I did not get the concern. Arrow buttons are aimed to cycle all 
buttons group consequently. Tab key should move focus outside the 
group. Space key doesn't change focus.
Also it looks inconsistent when the second option is skipped in 
the cycle while the third is not.
  This is because the tab key is pressed which can move the focus 
from the group and back. The arrow keys are used to move the focus 
in the group.
  It preserves the tab key behavior before the fix 8033699 except 
radio buttons in the group.
Group of options can be treated as one control which should be 
visited only once during single cycle. Because it is more correct 
from usability point of view: if user has made a selection it is 
unnecessary to return to this control again.
Also what do you propose seems to contradict the 8074883 which 
requires only one selected option to be focused always in the group.
  I see the point that the group of buttons should be treated as one 
control from the focus point of view.
  I read the description of the JDK-8033699 but it is not still clear 
to me is selecting a radio button by pressing an arrow key follows to 
the industry standard?

In native apps and in browsers it works that way.


  One more question which I have relates to a group of radio buttons 
there no one of them is selected. Should tab pressing moves the focus 
to the first button or to the latest one which has been selected or 
just using standard focus order for components?
It is not good style to leave all radio buttons unselected in one 
group. I think the first one should be selected in this case.


Also, yes, the test/javax/swing/JRadioButton/8033699/bug8033699.java 
should be modified.
The updated webrev: 
http://cr.openjdk.java.net/~ssadetsky/8154043/webrev.01/


--Semyon


  Thanks,
  Alexandr.



--Semyon


  Thanks,
  Alexandr.



--Semyon


  Thanks,
  Alexandr.


  Thanks,
  Alexandr.


--Semyon

Re: [9] Review request for 8163124 Add floating point API support to javax.swing.text.Caret

[Alexandr Scherbatiy]

This is the known issue: JDK-8163175 PlainView.modelToView() method 
should return Rectangle2D
It is included into the fix JDK-8156217 Selected text is shifted on 
HiDPI display.


You can apply the fix [1] JDK-8156217 to the JDK and try the proposed 
sample.

I get the results [2] for scale 2x and [3] for scale 8x.

[1] http://cr.openjdk.java.net/~alexsch/8156217/webrev.07/all
[2] 
http://cr.openjdk.java.net/~alexsch/8156217/screenshots/model-to-view-2x.png
[3] 
http://cr.openjdk.java.net/~alexsch/8156217/screenshots/model-to-view-8x.png


Thanks,
Alexandr.

On 9/15/2016 1:49 PM, Semyon Sadetsky wrote:

Hi Alexander,

Could you run the next test (with x8 scale, for example):

public class New2dApiTest {
public static void main(String[] args) {
JTextArea jTextArea2d = new JTextArea("jksxbqhbxniiiaiiaawww") {
@Override
protected void paintComponent(Graphics g) {
super.paintComponent(g);
Graphics2D g2d = (Graphics2D) g;
g2d.setColor(new Color(255, 0, 0, 50));
for (int i = 0; i < getText().length(); i++) {
try {
g2d.fill(modelToView2D(i));
} catch (BadLocationException e) {
e.printStackTrace();
}
}
}
};
JTextArea jTextArea = new JTextArea("jksxbqhbxniiiaiiaawww") {
@Override
protected void paintComponent(Graphics g) {
super.paintComponent(g);
Graphics2D g2d = (Graphics2D) g;
g2d.setColor(new Color(0, 0, 255, 50));
for (int i = 0; i < getText().length(); i++) {
try {
g2d.fill(modelToView(i));
} catch (BadLocationException e) {
e.printStackTrace();
}
}
}
};

JFrame jFrame = new JFrame();
jFrame.getContentPane().setLayout(new GridLayout(2, 1));
jFrame.getContentPane().add(jTextArea2d);
jFrame.getContentPane().add(jTextArea);
jFrame.pack();
jFrame.setVisible(true);

}
}


For some reason I cannot see the difference with the new and old API 
result.


--Semyon


On 04.08.2016 15:44, Alexandr Scherbatiy wrote:


Hello,

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

  The text position can have floating point value on HiDPI display. 
The Caret interface should be updated to allow use floating point 
positions.


  The fix adds the following public API with floating point positions:
  javax.swing.text.Caret.getMagicCaretPosition2D()
  javax.swing.text.Caret.setMagicCaretPosition2D(Point2D p)
  javax.swing.text.JTextComponent.modelToView2D(int pos)
  javax.swing.text.JTextComponent.viewToModel2D(Point2D pt)
  javax.swing.text.ParagraphView.getClosestPositionTo(int pos, 
Position.Bias b, Shape a, int direction, Position.Bias[], int 
rowIndex, float x)



  The fix replaces 
Caret.getMagicCaretPosition()/setMagicCaretPosition(Point p) to 
Caret.getMagicCaretPosition2D()/setMagicCaretPosition2D(Point2D p) in 
all places
  except DefaultCaret because DefaultCaret extends Rectangle so its 
coordinates always have int values.
  I have filled a separated enhancement for this JDK-8163174 Add 
DefaultCaret2D which supports floating point API


  To make a custom caret use floating point API it is also necessary 
that PlainView.modelToView() returns a rectangle with floating point 
values. It can be done after the fix

JDK-8156217 Selected text is shifted on HiDPI display
  which implements Utilities.getTabbedTextWidth(Segment s, 
FontMetrics metrics, float x, TabExpander e, int startOffset) method.

  I have filled a separated issue on it:
JDK-8163175 PlainView.modelToView() method should return Rectangle2D

 Thanks,
 Alexandr.

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

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 9/15/2016 1:01 PM, Sergey Bylokhov wrote:

+1

On 15.09.16 9:24, Rajeev Chamyal wrote:

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/
<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:  [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:  [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:  [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 im

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

[Alexandr Scherbatiy]

  Hello,

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

  The closeOnMouseClick property is renamed to doNotCloseOnMouseClick 
with the default false value.
  The client property is queried in the first place. If it is not set 
the L property value is used instead.


  Thanks,
  Alexandr.

On 9/14/2016 10:03 PM, Sergey Bylokhov wrote:

Hi, Alexandr.
I think we should double check how we operate on this property in case 
of custom L Our code relies that this property is set to "true" by 
default. And if the custom look, 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}

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 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 or component level.

 Thanks,
 Alexandr.

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

[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

[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
<eclipse-javadoc:%E2%98%82=tools/src%3Cjavax>.swing
<eclipse-javadoc:%E2%98%82=tools/src%3Cjavax.swing>.text
<eclipse-javadoc:%E2%98%82=tools/src%3Cjavax.swing.text>.html
<eclipse-javadoc:%E2%98%82=tools/src%3Cjavax.swing.text.html>.parser
<eclipse-javadoc:%E2%98%82=tools/src%3Cjavax.swing.text.html.parser>.NPrintWriter 





void javax <eclipse-javadoc:%E2%98%82=tools/src%3Cjavax>.swing
<eclipse-javadoc:%E2%98%82=tools/src%3Cjavax.swing>.text
<eclipse-javadoc:%E2%98%82=tools/src%3Cjavax.swing.text>.html
<eclipse-javadoc:%E2%98%82=tools/src%3Cjavax.swing.text.html>.parser
<eclipse-javadoc:%E2%98%82=tools/src%3Cjavax.swing.text.html.parser>.NPrintWriter 

<eclipse-javadoc:%E2%98%82=tools/src%3Cjavax.swing.text.html.parser%7BTagStack.java%E2%98%83NPrintWriter>.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: javax.swing.text.html.parser.NPrintWriter

[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.

[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

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

[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/ 
<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:  [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:  [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:  [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);
BufferedImag

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

[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.

Re: [9] Review request for 8157065: There is no the focus border on the selected tab.

[Alexandr Scherbatiy]

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.

  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 PM, Sergey Bylokhov wrote:

On 03.06.16 22:21, Semyon Sadetsky wrote:
What reason? Why it is not public? since I provided the 
code example

where these methods are accessed by the user?

GTK toollkit painting sequence is very different.


What does it mean "different"? Even in this fix you 
implement one of the method according to the spec and 
skip the same method for

Re: [9] Review request for 8157065: There is no the focus border on the selected tab.

[Alexandr Scherbatiy]

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().

  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 PM, Sergey Bylokhov wrote:

On 03.06.16 22:21, Semyon Sadetsky wrote:
What reason? Why it is not public? since I provided the 
code example

where these methods are accessed by the user?

GTK toollkit painting sequence is very different.


What does it mean "different"? Even in this fix you 
implement one of the method according to the spec and skip 
the same method for some unknown reason.




I still did not get why an overload method should have 
the same behavior
as its associates. This is a brand new design principle 
I've never heard

before.


...

That's nice...
Do you have any other concerns?


I still do not understand why the first method with default 
orientation is not implemented.
I guess you meant "is 

Re: [9] Review request for 8157065: There is no the focus border on the selected tab.

[Alexandr Scherbatiy]

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 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.


  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 PM, Sergey Bylokhov wrote:

On 03.06.16 22:21, Semyon Sadetsky wrote:
What reason? Why it is not public? since I provided the 
code example

where these methods are accessed by the user?

GTK toollkit painting sequence is very different.


What does it mean "different"? Even in this fix you implement 
one of the method according to the spec and skip the same 
method for some unknown reason.




I still did not get why an overload method should have the 
same behavior
as its associates. This is a brand new design principle 
I've never heard

before.


...

That's nice...
Do you have any other concerns?


I still do not understand why the first method with default 
orientation is not implemented.
I guess you meant "is not over-ridden". :) Once again: because 
it is never used.

Re: [9] Review request for 8157065: There is no the focus border on the selected tab.

[Alexandr Scherbatiy]

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.

  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 PM, Sergey Bylokhov wrote:

On 03.06.16 22:21, Semyon Sadetsky wrote:
What reason? Why it is not public? since I provided the code 
example

where these methods are accessed by the user?

GTK toollkit painting sequence is very different.


What does it mean "different"? Even in this fix you implement 
one of the method according to the spec and skip the same 
method for some unknown reason.




I still did not get why an overload method should have the 
same behavior
as its associates. This is a brand new design principle I've 
never heard

before.


...

That's nice...
Do you have any other concerns?


I still do not understand why the first method with default 
orientation is not implemented.
I guess you meant "is not over-ridden". :) Once again: because 
it is never used.

Re: RFR: JDK-8165799 Fix license and copyright headers under test/java/swing

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 9/13/2016 12:50 AM, Ken Chen wrote:


**

*Hello,*

*

Please review the patch below which fixes copyright in test files for awt


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

*Webrev: 
http://cr.openjdk.java.net/~shurailine/8165799/webrev.00/*



Thanks,

-Ken* 

Re: [9] Review request for 8157065: There is no the focus border on the selected tab.

[Alexandr Scherbatiy]

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.

 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 PM, Sergey Bylokhov wrote:

On 03.06.16 22:21, Semyon Sadetsky wrote:
What reason? Why it is not public? since I provided the code 
example

where these methods are accessed by the user?

GTK toollkit painting sequence is very different.


What does it mean "different"? Even in this fix you implement one 
of the method according to the spec and skip the same method for 
some unknown reason.




I still did not get why an overload method should have the 
same behavior
as its associates. This is a brand new design principle I've 
never heard

before.


...

That's nice...
Do you have any other concerns?


I still do not understand why the first method with default 
orientation is not implemented.
I guess you meant "is not over-ridden". :) Once again: because it 
is never used.

Re: [9] Review request for 8157065: There is no the focus border on the selected tab.

[Alexandr Scherbatiy]

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.


  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 PM, Sergey Bylokhov wrote:

On 03.06.16 22:21, Semyon Sadetsky wrote:

What reason? Why it is not public? since I provided the code example
where these methods are accessed by the user?

GTK toollkit painting sequence is very different.


What does it mean "different"? Even in this fix you implement one 
of the method according to the spec and skip the same method for 
some unknown reason.




I still did not get why an overload method should have the same 
behavior
as its associates. This is a brand new design principle I've 
never heard

before.


...

That's nice...
Do you have any other concerns?


I still do not understand why the first method with default 
orientation is not implemented.
I guess you meant "is not over-ridden". :) Once again: because it is 
never used.

Re: [9] Review request for 8157065: There is no the focus border on the selected tab.

[Alexandr Scherbatiy]

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 PM, Sergey Bylokhov wrote:

On 03.06.16 22:21, Semyon Sadetsky wrote:

What reason? Why it is not public? since I provided the code example
where these methods are accessed by the user?

GTK toollkit painting sequence is very different.


What does it mean "different"? Even in this fix you implement one of 
the method according to the spec and skip the same method for some 
unknown reason.




I still did not get why an overload method should have the same 
behavior
as its associates. This is a brand new design principle I've never 
heard

before.


...

That's nice...
Do you have any other concerns?


I still do not understand why the first method with default 
orientation is not implemented.
I guess you meant "is not over-ridden". :) Once again: because it is 
never used.