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

[Jim Graham]

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...


...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 - 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 

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

[Jim Graham]

Hi Alexandr,

On 11/14/2016 7:51 AM, Alexandr Scherbatiy wrote:

  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.


Why not just ensure that you are translating by integer pixel amounts? 
That would work for any scale without add modulus calculations which 
still add slop to the amount of room you need in the buffer...


...jim

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

[Semyon Sadetsky]

+1

--Semyon


On 14.11.2016 20:19, Alexandr Scherbatiy wrote:

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: 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 different colors)
one after 

Re: 8167160: [TEST_BUG][PIT] failure of javax/swing/JRadioButton/8033699/bug8033699.java

[Avik Niyogi]

If I use UIManager.setLookAndFeel() to metalLookAndFeel then it works but for 
default settings on mac it is not working.

> On 14-Nov-2016, at 8:03 pm, Sergey Bylokhov  
> wrote:
> 
> On 14.11.16 17:18, Avik Niyogi wrote:
>> I checked with MetalLAF as well. does not seem to work. also, tried with a 
>> native Mac app. Does not have any option for radio button focus traversal.
> 
> Can you please clarify how you set the L for this test? I also checked it 
> and the test passed on the current jdk9-client in case of 
> "-Dswing.defaultlaf=javax.swing.plaf.metal.MetalLookAndFeel"
> 
>> 
>>> On 14-Nov-2016, at 6:22 pm, Sergey Bylokhov  
>>> wrote:
>>> 
>>> On 14.11.16 6:43, Avik Niyogi wrote:
 This is OS X Specific.
>>> 
>>> Are you sure? I guess the test will pass if will be run using 
>>> MetalLookAndFeel.
>>> 
> On 14-Nov-2016, at 2:18 am, Sergey Bylokhov  
> wrote:
> 
> On 10.11.16 9:53, Avik Niyogi wrote:
>> Arrow buttons for traversing a radio button group is not the expected OS 
>> X behaviour.
> 
> This behavior is OSX specific or it is related to the Aqua L which is 
> default on OSX?
> 
>> In OS X in default OS X apps, for radio button focus traversing to work, 
>> custom actions must be set using System Preferences.
>>> On 09-Nov-2016, at 6:51 pm, Sergey Bylokhov 
>>>  wrote:
>>> 
>>> On 09.11.16 11:06, Avik Niyogi wrote:
 *Bug: https://bugs.openjdk.java.net/browse/JDK-8167160*
 
 *Webrev: http://cr.openjdk.java.net/~aniyogi/8167160/webrev.00/*
 
 *Issue: *The test case :
 javax/swing/JRadioButton/8033699/bug8033699.java fails on OS X
 
 *Cause: * The test case does not apply for OS X and should work for
 windows and Linux
>>> 
>>> What is the reason why the test does not work on OSX?
>>> 
>>> 
>>> --
>>> Best regards, Sergey.
>> 
> 
> 
> --
> Best regards, Sergey.
 
>>> 
>>> 
>>> --
>>> Best regards, Sergey.
>> 
> 
> 
> -- 
> Best regards, Sergey.

Re: 8167160: [TEST_BUG][PIT] failure of javax/swing/JRadioButton/8033699/bug8033699.java

[Sergey Bylokhov]

On 14.11.16 17:18, Avik Niyogi wrote:

I checked with MetalLAF as well. does not seem to work. also, tried with a 
native Mac app. Does not have any option for radio button focus traversal.


Can you please clarify how you set the L for this test? I also checked 
it and the test passed on the current jdk9-client in case of 
"-Dswing.defaultlaf=javax.swing.plaf.metal.MetalLookAndFeel"





On 14-Nov-2016, at 6:22 pm, Sergey Bylokhov  wrote:

On 14.11.16 6:43, Avik Niyogi wrote:

This is OS X Specific.


Are you sure? I guess the test will pass if will be run using MetalLookAndFeel.


On 14-Nov-2016, at 2:18 am, Sergey Bylokhov  wrote:

On 10.11.16 9:53, Avik Niyogi wrote:

Arrow buttons for traversing a radio button group is not the expected OS X 
behaviour.


This behavior is OSX specific or it is related to the Aqua L which is default 
on OSX?


In OS X in default OS X apps, for radio button focus traversing to work, custom 
actions must be set using System Preferences.

On 09-Nov-2016, at 6:51 pm, Sergey Bylokhov  wrote:

On 09.11.16 11:06, Avik Niyogi wrote:

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

*Webrev: http://cr.openjdk.java.net/~aniyogi/8167160/webrev.00/*

*Issue: *The test case :
javax/swing/JRadioButton/8033699/bug8033699.java fails on OS X

*Cause: * The test case does not apply for OS X and should work for
windows and Linux


What is the reason why the test does not work on OSX?


--
Best regards, Sergey.





--
Best regards, Sergey.





--
Best regards, Sergey.





--
Best regards, Sergey.

Re: 8167160: [TEST_BUG][PIT] failure of javax/swing/JRadioButton/8033699/bug8033699.java

[Avik Niyogi]

I checked with MetalLAF as well. does not seem to work. also, tried with a 
native Mac app. Does not have any option for radio button focus traversal.

> On 14-Nov-2016, at 6:22 pm, Sergey Bylokhov  
> wrote:
> 
> On 14.11.16 6:43, Avik Niyogi wrote:
>> This is OS X Specific.
> 
> Are you sure? I guess the test will pass if will be run using 
> MetalLookAndFeel.
> 
>>> On 14-Nov-2016, at 2:18 am, Sergey Bylokhov  
>>> wrote:
>>> 
>>> On 10.11.16 9:53, Avik Niyogi wrote:
 Arrow buttons for traversing a radio button group is not the expected OS X 
 behaviour.
>>> 
>>> This behavior is OSX specific or it is related to the Aqua L which is 
>>> default on OSX?
>>> 
 In OS X in default OS X apps, for radio button focus traversing to work, 
 custom actions must be set using System Preferences.
> On 09-Nov-2016, at 6:51 pm, Sergey Bylokhov  
> wrote:
> 
> On 09.11.16 11:06, Avik Niyogi wrote:
>> *Bug: https://bugs.openjdk.java.net/browse/JDK-8167160*
>> 
>> *Webrev: http://cr.openjdk.java.net/~aniyogi/8167160/webrev.00/*
>> 
>> *Issue: *The test case :
>> javax/swing/JRadioButton/8033699/bug8033699.java fails on OS X
>> 
>> *Cause: * The test case does not apply for OS X and should work for
>> windows and Linux
> 
> What is the reason why the test does not work on OSX?
> 
> 
> --
> Best regards, Sergey.
 
>>> 
>>> 
>>> --
>>> Best regards, Sergey.
>> 
> 
> 
> -- 
> Best regards, Sergey.

Re: 8167160: [TEST_BUG][PIT] failure of javax/swing/JRadioButton/8033699/bug8033699.java

[Sergey Bylokhov]

On 14.11.16 6:43, Avik Niyogi wrote:

This is OS X Specific.


Are you sure? I guess the test will pass if will be run using 
MetalLookAndFeel.



On 14-Nov-2016, at 2:18 am, Sergey Bylokhov  wrote:

On 10.11.16 9:53, Avik Niyogi wrote:

Arrow buttons for traversing a radio button group is not the expected OS X 
behaviour.


This behavior is OSX specific or it is related to the Aqua L which is default 
on OSX?


In OS X in default OS X apps, for radio button focus traversing to work, custom 
actions must be set using System Preferences.

On 09-Nov-2016, at 6:51 pm, Sergey Bylokhov  wrote:

On 09.11.16 11:06, Avik Niyogi wrote:

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

*Webrev: http://cr.openjdk.java.net/~aniyogi/8167160/webrev.00/*

*Issue: *The test case :
javax/swing/JRadioButton/8033699/bug8033699.java fails on OS X

*Cause: * The test case does not apply for OS X and should work for
windows and Linux


What is the reason why the test does not work on OSX?


--
Best regards, Sergey.





--
Best regards, Sergey.





--
Best regards, Sergey.

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

[Semyon Sadetsky]

On 11/8/2016 4:00 PM, Sergey Bylokhov wrote:

On 02.11.16 10:51, Semyon Sadetsky wrote:

On 11/1/2016 10:37 PM, Sergey Bylokhov wrote:


On 28.10.16 11:20, Semyon Sadetsky wrote:
probably it is possible to change the while loop to something? 
just to

hide the usage of Enumeration? like
Enumiration.asIterator().forEachRemaining()?

I did not get why. What is wrong with Enumeration?


It is an old style iterator, and we can hide its usage.

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


But what about the question to specification, is it the spac below is 
true when the selected toggle button is disabled?:


48 * If this toggle button is a member of the {@link ButtonGroup} 
which has an another focusable toggle button selected, and the focus 
cause argument denotes window activation or focus traversal action of 
any direction the result of the method execution is the same as 
calling {@link Component#requestFocus(FocusEvent.Cause)} on the toggle 
button selected in the group.


Because the current implementation filter out disabled components(the 
same question is about invisible, non-displayable), but according to 
the spec results should be the same like we call the method directly 
on the selected button.

Probably you missed my previous answer to this question:

If a component is disabled it cannot receive input focus, see 
java.awt.Component#isEnabled specs.  The proposed spec clearly states :


 247  * If this toggle button is a member of the {@link 
ButtonGroup} which has
 248  * an another ***focusable*** toggle button selected, and the 
focus cause argument


in Swing a component may not receive focus if it is disabled, invisible, 
non-displayable...
According to the spec the result will be the same if you call 
Component#requestFocus on ***this*** toggle button.







If a component is disabled it cannot receive input focus, see
java.awt.Component#isEnabled specs.  The proposed spec clearly 
states :


 247  * If this toggle button is a member of the {@link 
ButtonGroup}

which has
 248  * an another ***focusable*** toggle button selected, and the
focus cause argument


It seems that the code in getGroupSelection() will focus the first
element in the group, but what elements will be focused if we call
Component#requestFocus(FocusEvent.Cause) directly for this disabled
compoenent? Will the the same(first) element be selected?
I did find any mentions of "first element" in the proposed spec. 
Please

clarify this question.
According to the proposed spec the case when
Component#requestFocus(FocusEvent.Cause) is called on disabled 
component

will be handled as:

 253  * In all other cases the result of the method is the same as
calling
 254  * {@link Component#requestFocus(FocusEvent.Cause)} on this
toggle button.


The specification states that the call to
this.requestFocus(FocusEvent.Cause cause);
and the call to
selected.requestFocus(FocusEvent.Cause cause);
produce the same result "If this toggle button is a member of the
{@link ButtonGroup} which has an another focusable toggle button
selected, and the focus cause argument denotes window activation or
focus traversal action of any direction"

The question was "is that always true if the selected element is
disabled(but focusable)"? I guess that the implementation in the fix
will select the first "this"(the button on which requestFocus() was
called), but in the second case something different will be selected.


On 10/25/2016 3:14 PM, Alexandr Scherbatiy wrote:

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: [9] Review request for 8027639: JComboBox's popup leaves tracks after closing

[Semyon Sadetsky]

Please review the updated webrev: 
http://cr.openjdk.java.net/~ssadetsky/8027639/webrev.01/

- buffer fill color is changed to 0,0,0,0
- graphics context background color and composition is restored
- scenario when the buffered JComponent has transparent background and 
opaque=true is taken into account


--Semyon


On 11/7/2016 7:01 PM, Sergey Bylokhov wrote:

On 07.11.16 18:31, Semyon Sadetsky wrote:

If it is src-over mean that in the window you will get a composite of
colors, which was drawn to the backbuffer and the colors which was
drawn in the window(which was drawn by the window itself). And in this
case you will get a different results when you paint via backbuffer or
when you skip it.

I did not get this. You've state if JRootPane has own different
transparent color than it may be painted twice.


I am talking about the color of the window, you said that it is always 
painted.

http://mail.openjdk.java.net/pipermail/swing-dev/2016-October/006854.html
So if it always painted in case of non-opaque windows and you paint it 
to the backbuffer means that you paint it twice, no?



At first, I'm not sure that JRootPane may have such color. Because we
only support window translucency if window is non-opaque and having
non-opaque window with opaque JRootPane seems incorrect usage.


The components can be opaque/non-opaque even if the window is 
opaque/non-opaque. They have a different meaning. For window this 
means that it has a transparent background, for components it means 
that before the component is painted all its containers should be 
painted first.



But anyway I don't see the way how the JRootPane transparent color may
be pained twice. For non-opaque JRootPane it's background color is not
painted, regardless of transparency. With opaque JRootPane the parent
window paint() method will not be called and  window background will not
be painted.


  Should the previous composite be restored after the rect 
filling?

SRC should be the default composite type.


default composite type should be srcOver, and it should be restored
before call paintToOffscreen().