<AWT Dev> [11] Review Request: 8196030 and 8190326

[Phil Race]

+1

-phil.

On 05/23/2018 12:41 PM, Sergey Bylokhov wrote:
> The fix is updated:
> http://cr.openjdk.java.net/~serb/8196030/webrev.06
>
> These methods were moved to sun.java2d.SunGraphicsEnvironment
>
> On 21/05/2018 12:31, Phil Race wrote:
>> I'd like to have no more "AWT code depends on Swing" in the code when 
>> unavoidably
>> "Swing depends on AWT."
>>
>> Put them in 2D if needed but not in SU2.
>>
>> -phil.
>>
>> On 05/21/2018 12:15 PM, Sergey Bylokhov wrote:
>>> But the code itself is unrelated to awt, coordinate conversion is a 
>>> java2d stuff, what about sun.java2d.SunGraphicsEnvironment?
>>>
>>> On 21/05/2018 11:52, Phil Race wrote:
>>>> All the actual cases seem to be in AWT so we should put these 
>>>> somewhere in AWT, not SU2.
>>>>
>>>> -phil.
>>>>
>>>> On 05/21/2018 11:28 AM, Sergey Bylokhov wrote:
>>>>> It is also used by, Window peer on windows:
>>>>> http://hg.openjdk.java.net/jdk/jdk/file/f4735ff8d17d/src/java.desktop/windows/classes/sun/awt/windows/WWindowPeer.java#l662 
>>>>>
>>>>>
>>>>> I guess that unix peers should use it as well in the same way as 
>>>>> on windows, actually all code which try to move something using 
>>>>> x,y coordinates  in user space should use this method.
>>>>>
>>>>> On 21/05/2018 10:44, Phil Race wrote:
>>>>>> Well I don't understand why that was put into SU2 either.
>>>>>> Since it is also used just by the AWT Robot.
>>>>>> It is from the fix for 
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8176097
>>>>>>
>>>>>> So I think these should both be moved out of SU2 and into Robot 
>>>>>> code.
>>>>>>
>>>>>> -phil.
>>>>>>
>>>>>> On 05/21/2018 10:16 AM, Sergey Bylokhov wrote:
>>>>>>> This method call getGraphicsConfigurationAtPoint() which is 
>>>>>>> located in the SwingUtilities2 and the robot class already 
>>>>>>> depends on it:
>>>>>>> http://hg.openjdk.java.net/jdk/jdk/file/f4735ff8d17d/src/java.desktop/share/classes/java/awt/Robot.java#l508 
>>>>>>>
>>>>>>>
>>>>>>> On 21/05/2018 10:03, Phil Race wrote:
>>>>>>>> I don't understand why the new method is added in 
>>>>>>>> SwingUtilities2 instead
>>>>>>>> of directly in WRobotPeer.java since
>>>>>>>> a) This makes AWT internals depend on Swing internals.
>>>>>>>> b) There's no other client of this method.
>>>>>>>>
>>>>>>>> -phil.
>>>>>>>>
>>>>>>>>
>>>>>>>> On 05/18/2018 12:38 PM, Sergey Bylokhov wrote:
>>>>>>>>> Hello.
>>>>>>>>> Please review the fix for jdk11.
>>>>>>>>>
>>>>>>>>> Bugs:
>>>>>>>>>     8196030: AWT Robot mouseMove fails on Windows 10 1709 with 
>>>>>>>>> HiDPI
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8196030
>>>>>>>>>     8190326: Robot.mouseMove uses scaling factor of main 
>>>>>>>>> display on unscaled second display
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8190326
>>>>>>>>>
>>>>>>>>> Webrev: http://cr.openjdk.java.net/~serb/8196030/webrev.04
>>>>>>>>>
>>>>>>>>> Description:
>>>>>>>>>  This change will fix two issues:
>>>>>>>>>  - 8196030: In the Windows 10 relative mouse moving is broken. 
>>>>>>>>> Solution is to change the code to use the absolute mouse 
>>>>>>>>> location. Actually the fix reverts the changes which were done 
>>>>>>>>> in JDK-4288230 [1](It is interesting that previously the 
>>>>>>>>> absolute mouse position was broken). Take a look to the 
>>>>>>>>> function which is used to calculate the mouse position, this 
>>>>>>>>> is the only way I found to align coordinates which passed to 
>>>>>>>>> windows(::SendInput() and coordinate which returned by 
>>>>>>>>> windows(::GetCursorPos().
>>>>>>>>>
>>>>>>>>>  - 8190326: the logic how we convert coordinates from the user 
>>>>>>>>> space to device space is changed. Previously we use the 
>>>>>>>>> transformation of the main screen, now we will find the 
>>>>>>>>> appropriate screen(where coordinates are located) and then use 
>>>>>>>>> transformation of this screen.
>>>>>>>>>
>>>>>>>>> I have tested the fix on win7/10 using 
>>>>>>>>> scales:100%,125%,%150,%175,%200 in multi-monitor 
>>>>>>>>> configuration. But the new test is quite strict and may fail 
>>>>>>>>> if there are some rounding error in some variations of screen 
>>>>>>>>> resolution+scale+screen locations. So I expect some reports 
>>>>>>>>> for this test.
>>>>>>>>>
>>>>>>>>> Notes:
>>>>>>>>>  - The logic in the new method in SwingUtilities2 is similar 
>>>>>>>>> to Robot.createCompatibleImage(), but the new method uses 
>>>>>>>>> Region.clipRound() instead of floor/ceil. The reason is that 
>>>>>>>>> we use same logic in native. I think that 
>>>>>>>>> Robot.createCompatibleImage() should use clipRound() as well. 
>>>>>>>>> Will check this later in separate bug.
>>>>>>>>>
>>>>>>>>>  - Similar but not the same bug exists in 
>>>>>>>>> Robot.getPixelColor() which uses the scale of the main screen, 
>>>>>>>>> and uses simple cast to (int), but it affects all platforms. 
>>>>>>>>> Will check this later in separate bug.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-4288230
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>