8372415: Stage size should match visual window bounds

[mstr2]

On Windows, the Stage.width and Stage.height correspond to the window size as returned by GetWindowRect.

Up until Windows 10, the size of a window was identical to its visual borders. However, since Windows 10 has introduced thin visual window borders, the window manager adds an invisible border of a few pixels around the window to make it easier to resize the window. Since GetWindowRect returns the window size including these invisible borders, the location and size of a Stage isn't exactly what we'd expect.

For example, if we place a Stage at setX(0) and setY(0), the window appears with a small distance from the screen edge, and the window size extends a few pixels beyond its visual borders (in the following images, the screenshot size corresponds to the window size; note the invisible padding around the edges):

What we actually want is to have the visual borders line up with the edges of the screen, and have the window size correspond to the visual borders:

The implementation is quite simple: instead of GetWindowRect, we use DwmGetWindowAttribute(DWMA_EXTENDED_FRAME_BOUNDS). This gives us the bounds of the visual window borders. If this function fails, we fall back to GetWindowRect (now, I don't know why DwmGetWindowAttribute(DWMA_EXTENDED_FRAME_BOUNDS) would ever fail... maybe an old Windows version in a remote desktop scenario?).

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issues

• JDK-8372415: Stage size should match visual window bounds (Bug - P4)
• JDK-8285983: JavaFX reports maximized window size larger than the physical screen (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1982/head:pull/1982
$ git checkout pull/1982

Update a local copy of the PR:
$ git checkout pull/1982
$ git pull https://git.openjdk.org/jfx.git pull/1982/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1982

View PR using the GUI difftool:
$ git pr show -t 1982

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1982.diff

Using Webrev

Link to Webrev Comment

[mstr2]

/reviewers 2

[bridgekeeper]

👋 Welcome back mstrauss! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@mstr2
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[mlbridge]

Webrevs

• 03: Full - Incremental (8f586174)
• 02: Full - Incremental (f7232ce8)
• 01: Full - Incremental (defe0df7)
• 00: Full (164d9b73)

[beldenfox]

I haven't uncovered any problems so far but it took me a while to get my bearings. You're working with two coordinate systems, the one used by JavaFX which doesn't include the invisible border and the one used by Windows which does. The naming convention is confusing since the "extended" rectangle is actually the smaller of the two. It would also be good to see some comments clarifying that m_insets, m_maxSize, and m_minSize are all specified in the JavaFX system.

[mstr2]

I haven't uncovered any problems so far but it took me a while to get my bearings. You're working with two coordinate systems, the one used by JavaFX which doesn't include the invisible border and the one used by Windows which does. The naming convention is confusing since the "extended" rectangle is actually the smaller of the two. It would also be good to see some comments clarifying that m_insets, m_maxSize, and m_minSize are all specified in the JavaFX system.

I don’t think it’s quite "two coordinate systems" (JavaFX vs Windows). Stage.width/height are intended to include the normal system decorations (title bar + borders), which historically also matches what people mean by "window size": it includes the non-client area, but not purely visual effects like drop shadows. The weirdness on Windows 10+ is that GetWindowRect started including an extra invisible resize border that’s not part of the visible frame, so the window ends up being reported a few pixels larger than it looks. Using DwmGetWindowAttribute(DWMA_EXTENDED_FRAME_BOUNDS) is basically a way to get the visually perceived bounds back.

[beldenfox]

I was thinking that JavaFX and Windows have different ideas of where the window origin (0, 0) lies; Windows thinks it's at the top left of drop shadow area and JavaFX thinks it's at the top left of the title bar (in this PR). But you're right, what's relevant is that there are two rectangles of different sizes. I still think it could be clearer that, say, m_insets applies to the smaller of the two (which is the extended one).

[mstr2]

I've added a comment to clarify that m_minSize, m_maxSize, and m_insets are specified with respect to the extended frame bounds.

[mstr2]

With this fix applied, the problem described in JDK-8285983 disappears and the reported position and size of a full-screen window is as a sane person would expect it to be (position = 0,0 and size = visual bounds of the screen).

I suggest we re-open the bug (which was closed as "not an issue") and add it to this PR.

[beldenfox]

The existing changes look fine. GitHub doesn't allow me to comment on unchanged code so I'll just have to write up my comments here.

In GlassView.cpp the _getX and _getY routines return the location of the scene relative to the upper-left corner of the window. I believe these offsets should be relative to the extended frame bounds instead of GetWindowRect.

In GlassWindow.cpp there's a call to the Java routine notifyMoving (which lives in WinWindow.java). At that call site the x and y coordinates are based on GetWindowRect and the anchor is relative to GetWindowRect but the insets are relative to the extended frame bounds. notifyMoving is also called from the setBounds routine in WinWindow.java. In that case I believe it's being called with the coordinates of the extended frame bounds, an anchor relative to GetWindowRect, and insets relative to the extended frame bounds. I think this all needs further review.

[mstr2]

I've looked at every call to GetWindowRect and, where appropriate, replaced it with GetExtendedFrameBounds.
In addition to that, I also tested the case when JavaFX is running in a DPI-unaware process. I've added code that detects this situation and applies the correct mapping from physical coordinates into virtualized coordinates.

[mstr2]

/issue add JDK-8285983

[openjdk]

@mstr2
Adding additional issue to issue list: 8285983: JavaFX reports maximized window size larger than the physical screen.

[beldenfox]

What versions of Windows does JavaFX support? I just spent a half hour searching and couldn't find any statement on this for either JavaFX or the JDK.

As of October of this year the original version of Windows 10 left Microsoft's Long Term Servicing Channel. As of now the earliest version of Windows supported by Microsoft is Win10 2016 (1607), the version where the thread DPI awareness calls were first added. Is JavaFX supporting and testing earlier versions of Windows? If not you should be able to just make direct calls to the thread DPI awareness API's.

This is probably a topic for the mailing list. I'm still surprised I couldn't find this information anywhere.

[mstr2]

Even if we didn't officially support Windows 10, I think it's a little too early to remove functionality given that this OS still has a ~40% share of Windows installations.

[kevinrushforth]

Definitely. We don't need to worry about Windows 8 or earlier, but it's premature to break Windows 10.

[beldenfox]

I don't want to stop supporting Windows 10. I'm referring specifically to Windows 10 RTM, the very first version of 10, which has fallen entirely out of Microsoft's support system. If we only need to support versions of Windows that Microsoft is supporting that would be Windows 10 2016 (1607) in which case there's no need to mess with all the DLL and function pointer retrieval in this code. You can just issue direct calls to SetThreadDpiAwarenessContext and friends.

I will add that though we might say in this thread that we don't need to worry about Windows 8 or earlier I really and truly could not find any public statement to that effect anywhere.

[mstr2]

If we removed that path, then for Windows 10 < 1607, we wouldn't be able to skip coordinate mapping for the usual (99.9%) case when the Java process is running with per-monitor DPI awareness. This wouldn't cause JavaFX to stop working, we'd just be back at the status quo where we don't account for the visible window frame.

We still need to dynamically load the function though, as it definitely won't be available on Win10 < 1607.

[beldenfox]

We still need to dynamically load the function though, as it definitely won't be available on Win10 < 1607.

Officially you're doing the right thing by writing code that works on all versions of Windows 10. But this assumes there are users out there with the original release of Windows 10 (no updates past 2015) who will want to install a JavaFX 26-based app. Are there? Can we even test this code? I don't see how this is sustainable going forward.

This is not really a question for you. I should take it to the mailing list.

[mstr2]

I can't test this in a systematic way, as I can't be bothered to get an installation of an ancient version of Windows. However, in this particular case, I did test what would happen if the new functions can't be loaded by simply commenting out the code that loads the new functions.