<Swing Dev> RFR: 8211999: Window positioning bugs due to overlapping GraphicsDevice bounds (Windows/HiDPI)

Victor Dyakov vdyakov at openjdk.java.net
Fri Oct 2 16:50:42 UTC 2020


On Fri, 2 Oct 2020 16:46:11 GMT, Victor Dyakov <vdyakov at openjdk.org> wrote:

>> Hello.
>> Please review the fix for jdk.
>> 
>> Old review request:
>> https://mail.openjdk.java.net/pipermail/awt-dev/2020-July/015991.html
>> 
>> 
>> (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 monitors)
>> 
>> ========================================================
>> 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(the top/left corner). 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 this screen should be scaled)
>> 
>>          userX = screenX + (deviceX - screenX) * scaleX
>>          deviceX = screenX + (userX - screenX) / scaleX
>> 
>>    Note that these 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 a 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 stops the drag operation. I've proposed to use standard Windows
>>      machinery for that via WM_DPICHANGED: https://docs.microsoft.com/en-us/windows/win32/hidpi/wm-dpichanged
>>      As a result, Windows will provide the "best" windows bounds on the new screen. Note that the code has a
>>      workaround for https://bugs.openjdk.java.net/browse/JDK-8249164
>>   
>>    - I've also fix a 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 use 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 of the case when the HW component bounds are updated when the top-level
>>      the window is moved to another screen. For example, if the window does not use LayoutManager and the user sets 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 another 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.
>> 
>>    - The win32GraphicsDevice incorrectly sets the size of the full-screen window. It uses the display mode
>>      width/height(which are stored in pixels), but the bounds of the graphics config(which are stored in user's space)
>>      should 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 separate issues.
>>      
>> 
>> ========================================================
>> The tests
>>     - I have updated a couple of the tests to check multiple screens if possible, it helps to found some bugs
>>       in my initial implementation
>>     - Four new tests were added: SetComponentsBounds, SlowMotion, WindowSizeDifferentScreens, FullscreenWindowProps
>>     - One test is moved from the closed repo(I made a small cleanup of it): ListMultipleSelectTest
>> 
>> ========================================================
>> I have run jck, regression tests in 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.
>> 
>> PS: hope I did not forget some important information, will send it later if any.
>
> @prsadhuk  can you review this?

@prrace Can you review this?

-------------

PR: https://git.openjdk.java.net/jdk/pull/375


More information about the swing-dev mailing list