[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: [OpenJDK 2D-Dev] RFR: 8211999 Window positioning bugs due to overlapping GraphicsDevice bounds (Wind
From: Sergey Bylokhov <Sergey.Bylokhov () oracle ! com>
Date: 2020-07-23 4:49:13
Message-ID: 2cf06130-58d8-502b-e41f-41d4c5252e31 () oracle ! com
[Download RAW message or body]
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.
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic