<Swing Dev> [9] Review request for 8162350 RepaintManager shifts repainted region when the floating point UI scale is used

Thu Dec 1 00:07:58 UTC 2016

On 30.11.16 2:11, Jim Graham wrote:
> - 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.

We have a bug(i cannot find a bugid) that the shaped windows are aliased 
on the hidpi screens. Initially it was implemented on OSX this way, 
because the value which passed to the peers is Region not Shape/Area. 
According to Anthony the shared code uses Region because of performance 
issues, so replacement of Region->Area was deferred by Anthony, because 
the changes in shared was out of scope of support the shaped window/HiDPI.

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

-- 
Best regards, Sergey.