JDK-8146920 : [hidpi] Multi-Monitor issue with HiDpi scaling and undecorated stages
  • Type: Bug
  • Component: javafx
  • Sub-Component: graphics
  • Affected Version: 8u65,9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: windows_7
  • CPU: x86_64
  • Submitted: 2016-01-11
  • Updated: 2020-11-10
  • Resolved: 2016-12-21
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9Fixed
Related Reports
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Description
FULL PRODUCT VERSION :
java version "1.8.0_65"
Java(TM) SE Runtime Environment (build 1.8.0_65-b17)
Java HotSpot(TM) Client VM (build 25.65-b01, mixed mode, sharing)

ADDITIONAL OS VERSION INFORMATION :
Microsoft Windows [Version 6.1.7601]

A DESCRIPTION OF THE PROBLEM :
There's an issue with how the coordinate system works in JavaFx using 150% displays and multiple monitors. When we use an undecorated stage, we manually add a mouse dragged listener to get drag support. When using 2 1080p display screens at 100% display the javaFX X-Coordinate system goes from 0 to 3840(1920*2). When the display is set to 150% display, the X-Coordinate system goes from  0 to 2560(1280*2).

I believe there's an issue with how JavaFX calculates where the first monitor ends and the second monitor begins. scaling primaryStage.setX(1200) and primaryStage.setX(1735) are both placed on the first monitor. primaryStage.SetX(1735) most certainly should be placed on the second monitor. This works correctly at 100% display. 

Here's an image of what I think is happening: http://imgur.com/FRj7MFP

REGRESSION.  Last worked in version 8u45

ADDITIONAL REGRESSION INFORMATION: 
There's no dropdown for this, but the regression version is 8 update 40, not update 45

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
See sample source code. Attempt to drag the box to your second monitor and the stage will "jump" back to the first monitor and continue. Notice how the X-Coordinate of the stage keeps increasing after the jump

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
The box should smoothly transition to the second monitor without a "jump".
ACTUAL -
The box reaches the end of screen 1 then jumps backwards and continues.

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
public class Main extends Application {

    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage primaryStage) throws Exception {
        Label label = new Label();
        StackPane root = new StackPane();
        root.getChildren().add(label);
        Scene scene = new Scene(root, 300, 250);

        AtomicLong xOff = new AtomicLong(0);
        AtomicLong yOff = new AtomicLong(0);
        root.setOnMousePressed(event -> {
            xOff.set((long)event.getX());
            yOff.set((long)event.getY());
        });
        
        root.setOnMouseDragged(event -> {
            primaryStage.setX(event.getScreenX() - xOff.get());
            primaryStage.setY(event.getScreenY() - yOff.get());
            label.setText(String.format("X Cord %s\nY Cord %s", primaryStage.getX(), primaryStage.getY()));
            System.out.println(String.format("X Cord %s\nY Cord %s", primaryStage.getX(), primaryStage.getY()));
        });
        
        primaryStage.initStyle(StageStyle.TRANSPARENT);
        primaryStage.setScene(scene);
        primaryStage.show();
    }
}
---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
Set jvm argument to -Dglass.win.minHiDPI=6 to prevent HiDpi scaling


Comments
http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/adef7339b6fa
21-12-2016

Final webrev (doing a fresh build on Mac to double check): http://cr.openjdk.java.net/~flar/JDK-8146920/webrev.04/ The only difference from webrev.03 is deleting some commented-out print statements and getting rid of the pesky fflush(stdout) that Kevin found. Here is the relevant output from a diff of the patch files (deleting all lines that only showed file timestamp differences): < --- old/modules/javafx.graphics/src/main/native-glass/win/GlassWindow.cpp 2016-12-20 01:53:09.397984100 -0800 < +++ new/modules/javafx.graphics/src/main/native-glass/win/GlassWindow.cpp 2016-12-20 01:53:09.257349800 -0800 --- > --- old/modules/javafx.graphics/src/main/native-glass/win/GlassWindow.cpp 2016-12-20 16:34:33.997479600 -0800 > +++ new/modules/javafx.graphics/src/main/native-glass/win/GlassWindow.cpp 2016-12-20 16:34:33.794348300 -0800 640c635 < +// fprintf(stdout, "Exception from upcall"); --- > +// fprintf(stderr, "Exception from upcall"); 642a638 > +// fflush(stdout); 675d670 < + fflush(stdout); < --- old/modules/javafx.graphics/src/main/java/com/sun/glass/ui/win/WinWindow.java 2016-12-20 01:53:02.901553700 -0800 < +++ new/modules/javafx.graphics/src/main/java/com/sun/glass/ui/win/WinWindow.java 2016-12-20 01:53:02.760922100 -0800 --- > --- old/modules/javafx.graphics/src/main/java/com/sun/glass/ui/win/WinWindow.java 2016-12-20 16:34:27.512750500 -0800 > +++ new/modules/javafx.graphics/src/main/java/com/sun/glass/ui/win/WinWindow.java 2016-12-20 16:34:27.372119100 -0800 178c178 < @@ -48,6 +58,191 @@ --- > @@ -48,6 +58,186 @@ 187d186 < +// System.out.printf("setBounds(%f, %f, %b, %b, %f, %f, %f, %f\n", x, y, xSet, ySet, w, h, cw, ch); 279,282d277 < +// System.out.printf("notifyMoving(%d, %d, %d, %d) (%f, %f, %d, %d, mode=%d) (%d, %d, %d, %d\n", < +// x, y, w, h, < +// fx_x, fx_y, anchorX, anchorY, resizeMode, < +// iLft, iTop, iRgt, iBot);
21-12-2016

1. Yes, when the value is not AROUND_ANCHOR they are ignored (so they are ignored for FX_ORIGIN) so I could test for that and manually set them to something like 0,0 but rather than duplicate code, I just let them get set to "whatever happens when you decompose the NO_CAPTURE value". 2. OK, missed that. I'll comment that out... I can probably delete it since it is already flushed at line 745 in the common case (which is commented out already) and line 715 uses stderr because it is an error message, so a common flush at the end of the function is ill-conceived anyway...
21-12-2016

+1. Sanity tested on Mac and Linux and didn't notice any regression. I did some targeted review on this big code change and those paths look good to me.
21-12-2016

All my testing looks good. Code changes look good to me. 1. This is just a minor observation. In WinWindow.java setBounds, I see: int resizeMode = (anchor == ANCHOR_NO_CAPTURE) ? RESIZE_TO_FX_ORIGIN : RESIZE_AROUND_ANCHOR; int anchorX = (int) (anchor >> 32); int anchorY = (int) (anchor); I followed the anchorX and anchorY variables and can see that they are unused when resizeMode is RESIZE_TO_FX_ORIGIN? Their values are bogus in that case (0x80000000, 0), which is why I did that. I don't know whether this is worth a comment or not? It's fine either way. 2. In GlassWindow.cpp there is an uncommented "fflush(stdout);" in the GlassWindow::HandleWindowPosChangingEvent function. Maybe comment it out? +1 (no need for a new webrev if you want to comment out the fflush and/or add any other comments)
21-12-2016

Although I can't test it fully, I reviewed the code in .3 and have not issues with it. +1
20-12-2016

Testing on 8u-dev shows that some problems still occur there, but they are much less pronounced than the problems that occur(ed before this fix) on 9. The window never rescales indefinitely to huge or tiny sizes and it only has an occasional glitch to an odd location that it easily recovers from if you keep dragging it. Given the big difference in the code base of 8u-dev I don't believe a direct back port of this fix to 8u-dev would be feasible for so little gain (we'd have to teach the entire 8u-dev code base how to deal with separate X & Y scales, which was a huge change early in JDK9 development, in order for it to accept this code), but perhaps a slight modification of the way that 8u-dev determines which screen to choose for a given FX request coordinate might reduce the impact there - that would be a different fix entirely, though.
20-12-2016

Some details on the primary changes involved in the fix: - native code also no longer fields the WM_DPICHANGED event which was not reliable in when it asked for a DPI change. It was stable for dragging a window by the border, but if you programmatically moved a window near a screen edge it could get stuck in an infinite loop of "make it smaller, uh oh, now it's on the old screen, make it bigger, uh oh, now it's on the new screen, ad infinitum" - native code no longer evaluates screen changes in HandleWindowPosChanged(), that is now done in Java code leveraged from multiple places. - native code uses HandleWindowPosChanging() to both evaluate screen changes and override the bounds (as it would have done in the WM_DPICHANGED event handling) and to update the scales by passing the new bounds up to a Java method in WinWindow. It passes up the current mouse position if it is captured by the window to be used as a "resize anchor" and it passes up the window insets so the code can manage rescaling the client area without rescaling the window borders. - native code tracks WM_SIZING and WM_MOVING only to provide a hint to WindowPosChanging() about whether this is a resize operation or a move operation which affects how the rescaling is done. I chose for resize events to not change the window bounds and only scale the client portion because that is how many native apps do it and it felt odd to have the window flopping about as you were trying to establish its new size with the mouse. - minor change to GlassScreen.cpp to keep screens lined up by their top or bottom FX coordinates if they were lined up by their top or bottom pixel coordinates. Before this we were getting strange negative numbers for secondary screen origins. This didn't help fix the bug, but made the concept of how the screens line up in FX coordinates a little more sane to those who looked at the numbers. On the Java side: - WindowStage.setBounds() used to deal with interpreting the mapping of the FX bounds to platform bounds, but doing that on Windows required special care so I pushed all of that down into the platform window instance (i.e. Window.setBounds()). - Window.setBounds() (the default implementation) is now much simpler than the code removed from WindowStage because it just has to rescale the coordinates, not try to do mis-matched-scales selection of screens. - WinWindow.setBounds() has the complexity from WindowStage with a more rock-solid implementation. Some key points: - We remember the original FX requested size for rescale operations - We remember the "snapped client size" so we can tell if it has changed - We then rely on a common "notifyMoving" method to evaluate if screen has changed - setBounds() calls notifyMoving() to pick a new screen if needed - native WindowPosChanging() also uses notifyMoving() - WinWindow.notifyResize() looks at the last requested client pixel size and the new reported client size to determine if this is just an echo back of the new bounds we supplied, or a new size entirely generated from the system. If it's the latter, then it will determine new unscaled FX client size values for future use, otherwise it maintains the original client size request that came from above for more accurate rescaling. - WinWindow.notifyMove() is the method that has all of the magic now. - If the window is still more than 50% on the screen it came from, as measured in pixels, it leaves it there - If not, then we scan all screens and see if a new screen has a stronger claim on the window - A window must be at least 60% on a new screen to be switched, for hysteresis - A new screen must also contain more of the window than the original, percentage-wise - There are 3 different ways we can resize the window as we interpret "new screens": - RESIZE_DISABLED always uses the incoming pixel bounds - RESIZE_AROUND_ANCHOR resizes window around the specified pixel coordinates - RESIZE_TO_FX_ORIGIN maintains the FX coordinates for the window origin - setBounds() chooses ANCHOR if the mouse is captured, or FX_ORIGIN otherwise - PosChanging event chooses DISABLED after a WM_SIZING event, or ANCHOR otherwise (If the mouse is not captured, the UL corner is the anchor)
20-12-2016

Here is a potentially final webrev with everything cleaned up and all of the loose ends hopefully tied: http://cr.openjdk.java.net/~flar/JDK-8146920/webrev.03/ - lots of dead code deleted - webrev.02 was missing an initialization of the screen in createChildWindow...fixed now - webrev.02 was not updating the view coordinates when the scale changed...fixed now - testing now complete on 4 platforms (Win7, Win10, Mac, Ubuntu 14.04) (Note that 4-platform testing was done on webrev.02, but only sanity checks on Win10 and Mac were done for webrev.03 since it was mainly cleanup) The only remaining issue that might need some attention is perhaps adding thread checks for the appropriate thread for any of the new methods to be called on, if it matters for those cases.
20-12-2016

Windows backwards compatibility testing completed using: - Windows 7 - Windows 10 set to "one scale for all screens" mode in display settings - Windows 10 with the -Dglass.win.uiScale=1.5 override on the FX command line All 3 of those scenarios worked pretty much identically and showed no problems as would be expected (since all screens being the same scale is just a degenerate simplified case for the code that can handle individual scales for every screen). I still need to do some more dead code removal since I created a number of methods and functions during the testing and development of the fix that never contributed to the final fix - I have to find those unneeded new functions (and a couple of unneeded old functions) and delete them. I also need to test and fix the case of creating an FX stage as a child of an existing window which I'm pretty sure I may have sabotaged so as to not have a "screen" object associated with it - it will be a simple fix to restore that functionality on top of the new code.
20-12-2016

No issues found testing on Ubuntu 14.04.
20-12-2016

No issues found on Mac testing.
20-12-2016

I now have a fix for the Windows 10 code where every screen can have a different scale. Further testing is needed for: - Mac/Linux - Windows running with a single scale for all screens (ala Win7, Win8-before-8.1) http://cr.openjdk.java.net/~flar/JDK-8146920/webrev.02/ I don't think this will back-port well to 8u-dev, but I haven't tried yet. In any case, since 8u-dev did not support per-monitor-DPI it may not require this complete fix and a simpler fix might suffice, but further investigation of the problem on 8u-dev is needed before we can determine what we have to do there...
20-12-2016

Switching Windows 10 to "one scale factor for all" which simulates the 7/8.1 systems illustrates the problems mentioned above very obviously in the latest JDK8u-dev repo, but the 9-dev repo works flawlessly. Possibly some of the fixes for JDK-8091932 can be applied to the JDK8 code base to fix this particular issue for an 8 update, but additional fixes will still be necessary on JDK9 for the non-single-scale case.
19-08-2016

There are issues on Windows 10 as well when crossing the screen, even on the JDK9 code base. In some cases they can cause the scaling of the window to constantly switch back and forth and then miss a few transitions so the end result is doubly-scaled up or down.
19-08-2016

Aug 11, 2016: pending Assignee's evaluation
11-08-2016