Re: [9] RFR JDK-7190578: Nimbus: css test for 4936917 fails

Fix looks good. Regards, Ajit -Original Message- From: Sergey Bylokhov Sent: Thursday, November 24, 2016 3:55 PM To: Prasanta Sadhukhan; Ajit Ghaisas; Alexander Scherbatiy; Avik Niyogi; swing-dev@openjdk.java.net Subject: Re: [9] RFR JDK-7190578: Nimbus: css test for 4936917 fails

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

+1 -phil. On 11/28/2016 08:41 AM, 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

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

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...? clipRound() has appropriate logic

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

Looks okay to me. Mandy > On Nov 28, 2016, at 8:41 AM, 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

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

I took another look at the use of clipScale/clipRound in the SG2D.drawHiDPI method. The sub-image parameters are given relative to the base image and the intention (ignoring scaling and DPI variants) is to extract those complete pixels from the (base) source images and do the rendering

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

I've filed JDK-8170514 to track this issue: https://bugs.openjdk.java.net/browse/JDK-8170514 ...jim On 11/29/16 10:11 PM, Jim Graham wrote: I took another look at the use of clipScale/clipRound in the SG2D.drawHiDPI method. The sub-image parameters are given relative

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

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.

Re: [9] Review Request: [JDK-8169879] [TEST_BUG] javax/swing/text/GlyphPainter2/6427244/bug6427244.java - compilation failed

Thank you for your time in review Prasanta. Please find my responses inline. > I guess JComponent.isVisible() does not throw IllegalStateException so > 141 } catch (IllegalStateException ex) { > is not valid here. As you rightly pointed, IllegalStateException is not invoked by

Re: [9] Review Request: [JDK-8169879] [TEST_BUG] javax/swing/text/GlyphPainter2/6427244/bug6427244.java - compilation failed

I guess JComponent.isVisible() does not throw IllegalStateException so 141 } catch (IllegalStateException ex) { is not valid here. Probably you can call JComponent.getLocationOnScreen() which throws IllegalStateException in blockTillDisplayed() and update "Point". I also think there is no

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

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.

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

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

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

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.

Re: [9] Review request for 8170387: JLightweightFrame#syncCopyBuffer() may throw IOOBE

Hi, Semyon. It is a little bit strange that the coordinates which were used to create/recreate the image can produce IOOBE when we copy this image later. It seems that we have loss of precision when we divide/multiply coordinate by the scalefactor. Or probably the reason is that we use the

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

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

Re: [9] Review request for 8075918: The regression-swing case failed as the long Tab titles are not clipped with dots at the end with the special options"-client -Dswing.defaultlaf=javax.s

On 11/8/2016 4:39 PM, Sergey Bylokhov wrote: On 02.11.16 10:48, Semyon Sadetsky wrote: On 11/1/2016 10:44 PM, Sergey Bylokhov wrote: On 28.10.16 12:35, Semyon Sadetsky wrote: Probably this clipping should be done in the 633 paintText(ss, g, tabPlacement, font, metrics, 634

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

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

Re: [9] Review request for 8170387: JLightweightFrame#syncCopyBuffer() may throw IOOBE

On 11/29/2016 5:51 PM, Sergey Bylokhov wrote: Hi, Semyon. It is a little bit strange that the coordinates which were used to create/recreate the image can produce IOOBE when we copy this image later. It seems that we have loss of precision when we divide/multiply coordinate by the

Re: [9] Review request for 8170387: JLightweightFrame#syncCopyBuffer() may throw IOOBE

On 11/29/2016 7:59 PM, Sergey Bylokhov wrote: On 29.11.16 18:50, Semyon Sadetsky wrote: I did not get this. The area doesn't determine any sizes or location on screen. It's simply an optimization to update area of the buffer image which was really changed. It cannot be bigger than the buffer

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

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

Re: [9] Review request for 8170387: JLightweightFrame#syncCopyBuffer() may throw IOOBE

On 29.11.16 18:50, Semyon Sadetsky wrote: I did not get this. The area doesn't determine any sizes or location on screen. It's simply an optimization to update area of the buffer image which was really changed. It cannot be bigger than the buffer image size. The logic of the rounding is to

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

Looks fine to me. The only question I have: is "enable" state missing in the javadoc of the parent method, or is was intentional? take a look to the parent method where the next states are covered "displayable, focusable, visible"(enable state is missing??). -- Best regards, Sergey.

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

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()