Hello.
Please review the fix for jdk/client.

Bug: https://bugs.openjdk.java.net/browse/JDK-8211999
Fix: http://cr.openjdk.java.net/~serb/8211999/webrev.04

(Note: the fix use API available since Windows 8.1: WM_DPICHANGED, but it 
should be fine for
Windows 7, because it does not support different DPI for different mnitors)

========================================================
Short description:
    In the multi-screen configurations when each screen have different DPI
    we calculate the bounds of each monitor by these formulas:

        userSpaceBounds = deviceX / scaleX, deviceY / scaleY, deviceW / scaleX, 
deviceH / scaleY
        devSpaceBounds  = userX * scaleX, userY * scaleY, userW * scaleX, userH 
* scaleY

    This formula makes the next configuration completely broken:
     - The main screen on the left and 100% DPI
     - The second screen on the right and 200% DPI
    When we translate the bounds of the config from the device space to the 
user's space,
    the bounds of both screen overlap in the user's space, because we use 
bounds of
    the main screen as-is, and the X/Y of the second screen are divided by the 
scaleX/Y.

    Since the screens are overlapped we cannot be sure how to translate the 
user's space
    coordinates to device space in the overlapped zone.
    As a result => we use the wrong screen
                  => got wrong coordinates in the device space
                    => show top-level windows/popups/tooltips/menus/etc on the 
wrong screen

    The proposed solution for this bug is to change the formulas to these:
        userSpaceBounds = deviceX, deviceY, deviceW / scaleX, deviceH / scaleY
        devSpaceBounds  = userX, userY, userW * scaleX, userH * scaleY

    In other words, we should not transform the X and Y coordinates of the 
screen. This will
    complicate the way of how we transform coordinates on the screen: user's 
<--> device spaces:

    Before the fix:
        userX = deviceX * scaleX;
        deviceX = userX / scaleX;

    After the fix(only the part appeared on the screen is scaled)
        userX = screenX + (deviceX - screenX) * scaleX
        deviceX = screenX + (userX - screenX) / scaleX

    Note that this new formulas are applicable only for the coordinates on the 
screen such as X and Y.
    If we will need to calculate the "size" such as W and H then the old 
formula should be used.

    The main changes for the problem above are:
    - Skip transformation of X and Y of the screen bounds:
      
http://cr.openjdk.java.net/~serb/8211999/webrev.04/src/java.desktop/windows/native/libawt/windows/awt_Win32GraphicsConfig.cpp.sdiff.html
    - A number of utility methods in java and native:
      
http://cr.openjdk.java.net/~serb/8211999/webrev.04/src/java.desktop/windows/native/libawt/windows/awt_Win32GraphicsDevice.cpp.sdiff.html
      
http://cr.openjdk.java.net/~serb/8211999/webrev.04/src/java.desktop/share/classes/sun/java2d/SunGraphicsEnvironment.java.sdiff.html
========================================================
Long description:
    Unfortunately the changes above are not enough to fix the problem when 
different monitors
    have different DPI, even if the bounds are *NOT* overlapped.

  - Currently when we try to set the bounds of the window, we manually convert it in 
"java" to the
    expected device position based on the current GraphicsConfiguration of the 
peer, and then
    additionally tweak it in native using the device scale stored in native 
code. Unfortunately
    this two scale might not be in sync:(after we use the GraphicsConfiguration 
scale in peer,
    the config might be changed and the tweak in native will use different 
screen).

    As a fix I have moved all transformation from the peer to the native code, 
it will be executed
    on the toolkit thread:
    See the change in WWindowPeer.setBounds() and awt_Window.cpp 
AwtWindow::Reshape
        
http://cr.openjdk.java.net/~serb/8211999/webrev.04/src/java.desktop/windows/classes/sun/awt/windows/WWindowPeer.java.sdiff.html
        
http://cr.openjdk.java.net/~serb/8211999/webrev.04/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp.sdiff.html
    I think at some point we should delete all transformation in java, and 
apply it in the native code only.
  - We had a special code that tracked the dragging of the window by the user 
from one screen to another,
    and change the size of the window only when the user stop the drag 
operation. I've propose to use standard Windows
    machinery for that via WM_DPICHANGED: 
https://docs.microsoft.com/en-us/windows/win32/hidpi/wm-dpichanged
    As a result the Windows will provide a "best" windows bounds on the new 
screen. Note that the code have a
    workaround for https://bugs.openjdk.java.net/browse/JDK-8249164
- I've also fix variation of the bug previously fixed on macOS https://hg.openjdk.java.net/jdk/jdk/rev/b5cdba232fca
    If we try to change the position of the window, and Windows ignores this 
request then we need to
    call all "callbacks" manually, otherwise the shared code will uses the 
bounds ignored by the Windows.
    See the end of void AwtWindow::Reshape():
    
http://cr.openjdk.java.net/~serb/8211999/webrev.04/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp.sdiff.html

 -  Currently the HW componets such as Canvas scales everything based on such 
code:
       770 int AwtComponent::ScaleUpX(int x) {
       4771     int screen = 
AwtWin32GraphicsDevice::DeviceIndexForWindow(GetHWnd());
       4772     Devices::InstanceAccess devices;
       4773     AwtWin32GraphicsDevice* device = devices->GetDevice(screen);
       4774     return device == NULL ? x : device->ScaleUpX(x);
       4775 }
    But it does not work well if the smaller part of the top-level window is 
located on one screen1(DPI=100) but
    the biggest part is located on the screen2(DPI=200). If the Canvas is located on the 
"smaller" part of the
    window, then the canvas will use DPI=100 for scale, but the whole window 
will use DPI=200

    As a fix, all HW components will try to use the scale factor of the 
top-level window, see AwtComponent::GetScreenImOn:
        
http://cr.openjdk.java.net/~serb/8211999/webrev.04/src/java.desktop/windows/native/libawt/windows/awt_Component.cpp.sdiff.html

  - Our current implementation does not take care about the case when the HW 
component bounds are updated when the top-level
    window is moved to other screen. For example if the window does not use 
LayoutManager and the user set some specific bounds
    for the Canvas. It is expected that the size of the Canvas will be updated 
when the window will be moved to other screen, but
    only HW top-level window and Swing components will update its size.

    As a fix I suggest to resync the bounds of the HW components if the 
GraphicsCOnfiguration is changed, see WComponentPeer.syncBounds():
        
http://cr.openjdk.java.net/~serb/8211999/webrev.04/src/java.desktop/windows/classes/sun/awt/windows/WComponentPeer.java.sdiff.html

  - Note that the current fix is for Windows only, but the bug exists on Linux as well, 
so I have to create a kind of "ifdef" in the
    Robot class, on windows we will use the new logic, on other platforms the 
old logic will be used.

========================================================
Some other bugs which are not fixed.

   - We have a lot of places where we mix(unintentionally) the coordinate in 
user's space and device space.
     For example when we create the component we read x/y/width/height by the 
JNI(of course in a user's space)
     but pass this coordinates as-is to the ::CreateWindowEx() which is 
incorrect. It works currently
     because later we reshape the component to the correct bounds. Will fix it 
later.

   - When the frame is iconized and we try to save a new bounds for the future 
use, we store the
     coordinates in the user's space to the field which use device space:
     
https://github.com/openjdk/jdk/blob/master/src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp#L664

Probably there are some other bugs and possibility to cleanups, but I would 
like to do that in a separate issues.
========================================================
The tests
   - I have updated a couple of the tests to check multiple screen if possible, 
it helps to found some bugs
     in my initial implementation
   - Three new tests were added: SetComponentsBounds, SlowMotion, 
WindowSizeDifferentScreens
   - One test is moved from the closed repo(I made a small cleanup of it): 
ListMultipleSelectTest

========================================================
I have run jck, regression tests in a different configurations, when the main 
screen is on the left, up, down,
right, and have different DPI such as 100, 125, 150 and 200. No new issues were 
found so far.

The mach5 is also green, but I still investigate some common intermittent 
failures.


PS: hope I did not forget some important information, will send it later if any.

--
Best regards, Sergey.

Reply via email to