Previously the window handle was retrieved through private platform
interface. It was recently changed to use `QWindow::winId()` instead,
as it is public and more convenient to use.
However, `QWindow::winId()` creates the platform window if it does
not exist. This is normally not a problem, but since `WinId()` is
may be called within the native event handler, it is a problem.
I did not realize that `WinId()` was called at inappropriate times
(either during destruction of the platform window when the window
is destroyed, or after that as we don't want to re-create the platform
window when closing the interface), that's why I did not pay attention
on having a check here. Now that the problem is obvious, we should
fix this mistake made in 7e063c67.
This is trivial to fix, we can simply check if the pointer returned
by either `QWindow::handle()` or `QSurface::surfaceHandle()` is a
null pointer, and return early if it is before calling `QWindow::winId()`.
As a side note, I believe that having a const method named as `winId()`
which mutates the window by creating platform window if it does not
exist is a really bad idea. I really wonder why Qt behaves like that,
especially considering there is already `QWindow::create()` that
creates the platform window.
- `GetSystemMetrics()` function already scales most of the metrics for the
primary monitor [1], evidently because Qt makes the application DPI aware.
- Currently by multiplying the `GetSystemMetrics()` result with
`windowDpi / screenDpi`, we apply the window's DPI which is what is wanted
here.
- If the window is in the primary monitor, there is no problem. If there
are multiple monitors but they share the same DPI, there should also be no
problem. But if there are multiple monitors that have different DPIs, in
that case dragging would be problematic.
- The solution that Windows recommends [2] is using `GetSystemMetricsForDpi`
and `GetDpiForWindow`. Unfortunately these are only available on Windows 10,
so I dynamically load the necessary library and resolve the symbols.
- Handling `WM_DPICHANGED` and calling `SetWindowPos()` should not be
necessary, because Qt already calls it to resize the window according to
the new dpi.
[1] https://learn.microsoft.com/en-us/windows/win32/gdi/multiple-monitor-system-metrics
[2] https://learn.microsoft.com/en-us/windows/win32/hidpi/high-dpi-desktop-application-development-on-windows#updating-existing-applications
We can use the transparent area to restrict the client
size and use that remaining extra size for resizing.
This size needs to be adjusted with regard to the dpi.
Note that on Windows 10 and Windows 7, this brings
borders.
- WM_NCHITTEST handler should not unconditionally unset
hover status of all buttons. It does not make sense to
alter hover status during hit testing.
- WM_NCMOUSEMOVE should set the hover status of the
hovered button, and unset/reset the hover status of
rest of the buttons.
Although it depends on the platform whether press or
release triggers the click action, on Windows press
has never been considered as click.
It is not clear why `WM_NCLBUTTONDOWN` was used to
click here. `WM_NCLBUTTONUP` should have been used
to trigger the click action.
It appears that OLE 1.0 is not utilized, so it should
be beneficial to set this flag.
From "Initializing the COM Library":
> ... it is a good idea to set the COINIT_DISABLE_OLE1DDE
> flag in the dwCoInit parameter. Setting this flag avoids
> some overhead associated with Object Linking and Embedding
> (OLE) 1.0, an obsolete technology.
This wil allow removing some sed hacks in contribs, and removes the
dependency on contribs for making sure that the private header directory
is added to the -I flags on win32.
Fatih Uzunoglu
Pierre Lamot
Prince Gupta
Steve Lhomme
Johannes Kauffmann
Romain Vimont