<Swing Dev> [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
>>>
>