Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IsWindows8Point1OrGreater() returns FALSE for win10 if no manifest file targeting win10 exists #4200

Closed
radjkarl opened this issue Jun 5, 2021 · 3 comments

Comments

@radjkarl
Copy link

radjkarl commented Jun 5, 2021

Hi,
This is related to multi DPI support.

IsWindows8Point1OrGreater() will return false on win8.1 or win10 is no manifest file exists:
https://docs.microsoft.com/en-us/windows/win32/api/versionhelpers/nf-versionhelpers-iswindows8point1orgreater

In consequence ImGui_ImplWin32_GetDpiScaleForMonitor() fails to provide monitor dependent scaling as the monitor handler is ignored for versions below 8.1, see

ImGui_ImplWin32_GetDpiScaleForMonitor()
{
    // if not win8.1 or higher:
    const HDC dc = ::GetDC(NULL); // <- this is not monitor specific
    xdpi = ::GetDeviceCaps(dc, LOGPIXELSX);
    ydpi = ::GetDeviceCaps(dc, LOGPIXELSY);
}

This issue has been debated and fixed here:
#glfw/glfw#1294

My fix is mostly based on:
glfw/glfw@83aefa9
https://github.com/glfw/glfw/blob/061a0263a9783c1442ad96a061c717c167ab4a76/src/win32_platform.h

In file imgui_impl_win32.cpp replace function IsWindowsVersionOrGreater() with:

typedef LONG NTSTATUS, * PNTSTATUS;
#define STATUS_SUCCESS ((NTSTATUS)0x00000000)
#define STATUS_REVISION_MISMATCH ((NTSTATUS)0xC0000059)
typedef LONG(WINAPI* PFN_RtlVerifyVersionInfo)(OSVERSIONINFOEXW*, ULONG, ULONGLONG);

// HACK: Define versionhelpers.h functions manually as MinGW lacks the header
BOOL IsWindowsVersionOrGreater(WORD major, WORD minor)
{
    static PFN_RtlVerifyVersionInfo RtlVerifyVersionInfoFn = NULL;
    if (!RtlVerifyVersionInfoFn)
    {
        HMODULE ntdllModule = GetModuleHandleW(L"ntdll.dll");
        if (ntdllModule)
        {
            RtlVerifyVersionInfoFn = (PFN_RtlVerifyVersionInfo)GetProcAddress(ntdllModule, "RtlVerifyVersionInfo");
        }
    }
    RTL_OSVERSIONINFOEXW versionInfo = { 0 };
    NTSTATUS status;
    ULONGLONG conditionMask = 0;
    versionInfo.dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW);
    versionInfo.dwMajorVersion = major;
    versionInfo.dwMinorVersion = minor;

    VER_SET_CONDITION(conditionMask, VER_MAJORVERSION, VER_GREATER_EQUAL);
    VER_SET_CONDITION(conditionMask, VER_MINORVERSION, VER_GREATER_EQUAL);

    status = RtlVerifyVersionInfoFn(&versionInfo,
        VER_MAJORVERSION | VER_MINORVERSION,
        conditionMask);

    if (status == STATUS_SUCCESS)
        return TRUE;

    return FALSE;
}

and remove the last argument from the three IsWindowsVersionOrGreater() calls:

#define IsWindowsVistaOrGreater()   IsWindowsVersionOrGreater(HIBYTE(0x0600), LOBYTE(0x0600)) // _WIN32_WINNT_VISTA
#define IsWindows8OrGreater()       IsWindowsVersionOrGreater(HIBYTE(0x0602), LOBYTE(0x0602)) // _WIN32_WINNT_WIN8
#define IsWindows8Point1OrGreater() IsWindowsVersionOrGreater(HIBYTE(0x0603), LOBYTE(0x0603)) // _WIN32_WINNT_WINBLUE
@ocornut
Copy link
Owner

ocornut commented Jun 5, 2021 via email

@ocornut
Copy link
Owner

ocornut commented Jun 8, 2021

While looking at this, I noticed two things:

  • For ImGui_ImplWin32_EnableDpiAwareness() we had already caved in and ignored the IsWindows10OrGreater() version check for the same reason. Instead we just check the availability of function in the DLL. (note that it was intuitively elmindreda's first intuition in IsWindows8Point1OrGreater fails on Windows 10 glfw/glfw#1294 (comment) before RtlGetVersion got mentioned). I think it may be a viable route to simply do that...
  • That path in ImGui_ImplWin32_GetDpiScaleForMonitor() would lead to call GetProcAddress() every frame on every monitor which is properly not ideal, so that's something we can fix as well.

ocornut added a commit that referenced this issue Jun 8, 2021
@ocornut
Copy link
Owner

ocornut commented Jun 8, 2021

I have now applied your suggestion with b66529f (+ improved ImGui_ImplWin32_GetDpiScaleForMonitor() as well).

- Backends: Win32: Rework to handle certains Windows 8.1/10 features without a manifest. (#4200, #4191)
  - ImGui_ImplWin32_GetDpiScaleForMonitor() will handle per-monitor DPI on Windows 10 without a manifest.
  - ImGui_ImplWin32_EnableDpiAwareness() will call SetProcessDpiAwareness() fallback on Windows 8.1 without a manifest.

Thank you!

@ocornut ocornut closed this as completed Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants