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

[Jim Graham]

Hi Alexandr,

Looks fine.  +1

                        ...jim

On 11/21/16 5:59 AM, 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 -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