#2063 GetSystemMetrics() in PlatWin.cxx need take current dpi into account

[Zufu Liu]

See the document for GetSystemMetrics() and GetSystemMetricsForDpi()
https://docs.microsoft.com/en-us/windows/desktop/api/winuser/nf-winuser-getsystemmetrics
https://docs.microsoft.com/en-us/windows/desktop/api/winuser/nf-winuser-getsystemmetricsfordpi

[Zufu Liu]

Also in ScintillaWin.cxx.

[Zufu Liu]

Only need changes for per-monitor DPI awareness application.
GetSystemMetrics() returns value based DPI settings on default monitor.

[Neil Hodgson]

PlatWin.cxx only uses it for the ListBox scrollbar and minimum size. ScintillaWin.cxx only for whether a mouse drag is large enough to trigger drag and drop.

Since GetSystemMetricsForDpi is only available from Windows 10 version 1607, any patch will need to allow for older versions of Windows by dynamically accessing GetSystemMetricsForDpi and falling back to GetSystemMetrics.

[Zufu Liu]

GetSystemMetricsForDpi(int nIndex, UINT dpi) is equal to following code:

// UINT dpi = DpiForWindow(hwnd);
int GetSystemMetricsFor(int nIndex, UINT dpi) {
    int value = GetSystemMetrics(nIndex);
    HDC hdc = GetDC(NULL);
    const int logPixelY = GetDeviceCaps(hdc, LOGPIXELSY);
    ReleaseDC(NULL, hdc);
    value = (dpi == logPixelY)? value : MulDiv(value, dpi, logPixelY);
    return value;
}

LOGPIXELSY is not changed after login, GetSystemMetrics returns same values initialized after boot.

The code can be used to calculate reverse arrow cursor size for new DPI.

[Neil Hodgson]

The reverse arror cursor is a complex case as there may be Scintilla windows from one application on different monitors hence different DPIs. The lifetime of of the cursor also becomes more complex in this case as it shouldn't be deleted if it is in use by another Scintilla window.

Its likely that reverseArrowCursor should become a member of ScintillaWin and follow its DPI. that requires overriding DisplayCursor and special casing cursorReverseArrow.

[Zufu Liu]

how about something like:

struct ReverseArrowCursor {
    UINT dpi;
    HCURSOR cursor;

    ReverseArrowCursor(UINT dpi_) noexcept : dpi(dpi_), cursor(MakeReverseArrowCursor(dpi_)) {}
    ~ReverseArrowCursor() {
        if (cursor) {
            ::DestroyCursor(cursor);
        }
    }
};

std::vector<std::unique_ptr<ReverseArrowCursor>> reverseArrowCursorList;

HCURSOR LoadReverseArrowCursor(UINT dpi) {
    for (const auto &cursor : reverseArrowCursorList) {
        if (cursor->dpi == dpi) {
            return cursor->cursor;
        }
    }

    reverseArrowCursorList.push_back(std::make_unique<ReverseArrowCursor>(dpi));
    return reverseArrowCursorList.back()->cursor;
}

ScintillaWin can cache a reverse arrow cursor that matches it's current DPI.

[Neil Hodgson]

IDC_ARROW is determined at startup. If DPI is changed then LoadReverseArrowCursor called then the bitmap of IDC_ARROW is always the same size within one run.

[Zufu Liu]

it can be resized with following pseudo code:

width = GetSystemMetricsFor(SM_CXCURSOR, dpi);
height = GetSystemMetricsFor(SM_CYCURSOR, dpi);
HCURSOR cousor = (HCURSOR)CopyImage(LoadCursor(nullptr, IDC_ARROW), IMAGE_CURSOR, width, height, LR_COPYFROMRESOURCE | LR_COPYRETURNORG);

[Neil Hodgson]

That produces a scaled cursor for me so, if starting with 100% and switching to 200%, its blurry. Its better than having a tiny cursor but there should be a way to get the full resolution arrow bitmap which is visible by just moving the mouse up to the tab bar.

[Zufu Liu]

I feel make dpi a field of Window base class will simplify codes.
Window::SetCursor() can use dpi to load reverse arrow cursor;
ListBoxX need a dpi for GetSystemMetrics;
ScintillaWin can use the dpi for GetSystemMetrics and IME.

[Neil Hodgson]

If its a field of Window, it will require work on other platforms and concern for just when its valid and what its lifetime is. Better to call the API whenever needed.

[Zufu Liu]

The changes with GetSystemMetricsFor and cursor. The reverse arrow cursor is not that bad compared to Visual Studio (at 175%).

[Neil Hodgson]

reverseArrowCursorList is global mutable state which should be avoided. That is why the reverse arrow is better stored on ScintillaWin.

There is also some complexity with 'created' that appears to be providing a backup so NULL isn't returned from LoadReverseArrowCursor. May be better to just return NULL and have the caller deal with it.

DpiForWindow(HWND) and DpiForWindow(WindowID) [void *] may not differ sufficiently depending on system headers or open source derived headers so there is too big of a chance of problems. See, for example, the role of STRICT in the SDK. Change the name for one or the other.

USER_DEFAULT_SCREEN_DPI can be used without the fallback as it should be available in minimum supported compilers now. This requires updating _WIN32_WINNT and WINVER to at least 0x0600. The main reason for keeping it at 0x0500 was to see warnings when calls are made that should be dynamically loaded to allow running on old systems.

GetDC({}) is used where other code uses CreateCompatibleDC({}). Unless there is a reason, its better to be consistent.

[Zufu Liu]

Do you means override Editor::DisplayCursor() in ScintillaWin?

That is why the reverse arrow is better stored on ScintillaWin.

USER_DEFAULT_SCREEN_DPI is not available when build for XP. because some structures (like NONCLIENTMETRICSA) difference, define _WIN32_WINNT to 0x0600 for XP build may cause other problems.

[Zufu Liu]

Moved reverseArrowCursor into ScintillaWin, reverted other changes to DpiForWindow and WM_DPICHANGED.

[Zufu Liu]

Renamed GetSystemMetricsFor to SystemMetricsForDpi.

[Neil Hodgson]

Commited as 2 patches [2dd165] for SystemMetricsForDpi and [01940b] for reverse arrow cursor.

Changed SystemMetricsForDpi to noexcept as it is and it was called from noexcept code.

The assignment of dpi in ListBoxX::Create may get the wrong DPI from the main screen rather than the screen it is displayed on as the window is created with fixed coordinates and later moved. It may be better to use the DPI of the parent window or adapt when the list box window is moved.

If the application calls SCI_SETCURSOR(SC_CURSORREVERSEARROW) then the patch called wMain.SetCursor(SC_CURSORREVERSEARROW) which doesn't work. Rearranged ScintillaWin::DisplayCursor to handle this case.

[Zufu Liu]

Changed dpi for ListBox to hwndParent, fixed AdjustWindowRect.

I think ScintillaWin::CheckDpiChanged() is not needed, in case it's needed I moved it into WM_DPICHANGED_AFTERPARENT.
Following are the order for these messages:

1. ScintillaWin receives WM_DPICHANGED_BEFOREPARENT
2. after ScintillaWin (and other children windows) processed WM_DPICHANGED_BEFOREPARENT, application window receives WM_DPICHANGED
3. application window processes WM_DPICHANGED and forwards WM_DPICHANGED to ScintillaWin
4. ScintillaWin receives WM_DPICHANGED
5. after application window processed WM_DPICHANGED, ScintillaWin receives WM_DPICHANGED_AFTERPARENT

[Neil Hodgson]

The change to AdjustWindowRectExForDpi was committed as [5899b5] .

WM_DPICHANGED_AFTERPARENT is relatively recent (Windows 10, version 1703) compared to WM_DPICHANGED (Windows 8.1) so this could miss some cases. Committed with [61ff49] anyway as applications can forward WM_DPICHANGED if they want.

With 4.4.0 near, more change should be avoided now, except for regressions and other strong cases.

[Zufu Liu]

Current implementation (all loaded functions from user32.dll) need Win10 1607.

Windows 8.1 need GetDpiForMonitor.

[Neil Hodgson]

Loading and freeing DLLs is too problematic to be added late in a release cycle.

Its likely that there should be changes to check that it is safe to use LOAD_LIBRARY_SEARCH_SYSTEM32 as is done in LoadD2D. Freeing libraries is rarely worth the potential for trouble.

These can be examined after 4.4.0.

[Zufu Liu]

Shcore.dll (Windows 8.1) is not available on earlier systems, LOAD_LIBRARY_SEARCH_SYSTEM32 (Windows 8 without KB) is safe.

[Zufu Liu]

how about call LoadDpiForWindow() inside ScintillaWin::SWndProc():

if (iMessage == WM_CREATE) {
    LoadDpiForWindow();
    // Create C++ object associated with window
    sci = new ScintillaWin(hWnd);
}