PDCursesMod icon indicating copy to clipboard operation
PDCursesMod copied to clipboard

WinGUI: Add HiDPI support

Open xobs opened this issue 7 years ago • 11 comments

This patchset builds on pull #45, and adds to it by registering HiDPI support before the first window is created.

xobs avatar Apr 17 '18 07:04 xobs

Here is what it looks like on my system, with the current HEAD: 01-original

Here's what it looks like after the first patch, which informs Windows that the application supports HiDPI: 02-register-hidpi

And finally, here's what it looks like with the final patch to scale the fonts by the current DPI setting: 03-scaling-font

The two patches can be merged together, but I made them separate both for attribution reasons and because they affect different modules.

xobs avatar Apr 17 '18 08:04 xobs

Is this relevant to wincon (win32), too?

GitMensch avatar Apr 17 '18 10:04 GitMensch

@xobs I've did the following:

make DLL=Y clean
make DLL=Y
./tuidemo.exe   # all fine, set font size to 16, quit
./tuidemo.exe   # all fine, font size still at 16

then applied your patches

make DLL=Y clean
make DLL=Y
./tuidemo.exe   # chrash (gdb shows SIGSEGV)
make DLL=Y clean
make DLL=Y DEBUG=Y
./tuidemo.exe   # no chrash, font size still at 16

Do you have any idea where the SIGSEGV occurs? (PDC_DEBUG_LOG showed that PDC_scr_open finished).

Edit: checked further - only happens if gcc -O is used, stepping through the program results in

PDC_scr_open (argc=argc@entry=0, argv=argv@entry=0x0) at ../win32a/pdcscrn.c:2496
2496            set_process_dpi_awareness_func = (set_process_dpi_awareness_t)GetProcAddress(shcoredll, "SetProcessDpiAwareness");
(gdb)
2497            if (set_process_dpi_awareness_func)
2498              set_process_dpi_awareness_func(1 /*PROCESS_SYSTEM_DPI_AWARE*/);
(gdb) p set_process_dpi_awareness_func
$2 = <optimized out>
(gdb) s

Program received signal SIGSEGV, Segmentation fault.
0x00000000 in ?? ()
(gdb)

changing it from static to volatile:

2498              set_process_dpi_awareness_func(1 /*PROCESS_SYSTEM_DPI_AWARE*/);
(gdb) p set_process_dpi_awareness_func
$1 = (volatile set_process_dpi_awareness_t) 0x760abd80 <SetProcessDpiAwareness>
(gdb) s

Program received signal SIGSEGV, Segmentation fault.
0x00000000 in ?? ()
(gdb)

Strangely the debug log shows that PDC_scr_open is finished already (the SIGSEGV happens in the middle of it), I assume this is a threading issue...

Can you reproduce the crash?

GitMensch avatar Apr 17 '18 10:04 GitMensch

I'm not able to produce the crash.

I'm building on Windows 10 using VS 2017 Community.

Strangely, the DLL doesn't build on a freshly-checked-out system:

D:\Code\PDCurses\win32a>nmake -f .\vcwin32.mak DLL=Y

Microsoft (R) Program Maintenance Utility Version 14.13.26128.0
Copyright (C) Microsoft Corporation.  All rights reserved.

NMAKE : fatal error U1073: don't know how to make '..\exp-base.def'
Stop.

D:\Code\PDCurses\win32a>

However, that's easy enough to fix:

D:\Code\PDCurses\win32a>copy NUL: ..\exp-base.def
        1 file(s) copied.

D:\Code\PDCurses\win32a>nmake -f .\vcwin32.mak DLL=Y

Microsoft (R) Program Maintenance Utility Version 14.13.26128.0
Copyright (C) Microsoft Corporation.  All rights reserved.

        echo LIBRARY pdcurses > pdcurses.def
        echo EXPORTS >> pdcurses.def
        type ..\exp-base.def >> pdcurses.def
        rc /r /fopdcurses.res ..\win32a\pdcurses.rc
Microsoft (R) Windows (R) Resource Compiler Version 10.0.10011.16384
Copyright (C) Microsoft Corporation.  All rights reserved.

        cvtres /MACHINE:X64 /NOLOGO /OUT:pdcurses.obj pdcurses.res
...
        link.exe -nologo  rain.obj pdcurses.lib user32.lib gdi32.lib advapi32.lib shell32.lib comdlg32.lib
        cl.exe -nologo -I.. -c -Ox -MT -W3 -D_CRT_SECURE_NO_WARNINGS  -DPDC_DLL_BUILD    ../demos\worm.c
worm.c
        link.exe -nologo  worm.obj pdcurses.lib user32.lib gdi32.lib advapi32.lib shell32.lib comdlg32.lib

D:\Code\PDCurses\win32a>.\tuidemo.exe ; No crash

D:\Code\PDCurses\win32a>

I do see some older references to wine issues with SetProcessDpiAwareness.

What if we use SetProcessDPIAware() instead? That is exported from user32.dll, and looks like it might be good enough. E.g. https://github.com/OoliteProject/oolite/blob/ab0302d42127f3b890ff137a6d53be0123d1f2dd/src/SDL/MyOpenGLView.m#L822

xobs avatar Apr 17 '18 14:04 xobs

@xobs

Strangely, the DLL doesn't build on a freshly-checked-out system

all references to the def files are removed from master since some versions, I suggest to re-check what you actually builded...

GitMensch avatar Apr 17 '18 19:04 GitMensch

I did a rebase, and I'm able to build it on Windows now without creating that .def file. However, I'm still unable to reproduce the crash.

I'm installing a mingw-w64 toolchain to try building it under Linux and gcc so that I can reproduce your environment.

xobs avatar Apr 18 '18 05:04 xobs

I'm unable to reproduce the crash. I'm running wine-3.6. Admittedly, this is via a remote X server:

xobs@nas /d/x/C/P/win32a> i686-w64-mingw32-objdump -x ma^C
xobs@nas /d/x/C/P/win32a> make -f mingwin32.mak DLL=Y
i686-w64-mingw32-gcc -c -O4 -Wall -pedantic -I.. -DPDC_DLL_BUILD ../pdcurses/addch.c
i686-w64-mingw32-gcc -c -O4 -Wall -pedantic -I.. -DPDC_DLL_BUILD ../pdcurses/addchstr.c
i686-w64-mingw32-gcc -c -O4 -Wall -pedantic -I.. -DPDC_DLL_BUILD ../pdcurses/addstr.c
i686-w64-mingw32-gcc -c -O4 -Wall -pedantic -I.. -DPDC_DLL_BUILD ../pdcurses/attr.c
i686-w64-mingw32-gcc -c -O4 -Wall -pedantic -I.. -DPDC_DLL_BUILD ../pdcurses/beep.c
...
i686-w64-mingw32-gcc -O4 -Wall -pedantic -I.. -DPDC_DLL_BUILD -mwindows -orain.exe ../demos/rain.c pdcurses.dll
i686-w64-mingw32-gcc -O4 -Wall -pedantic -I.. -DPDC_DLL_BUILD -mwindows -oworm.exe ../demos/worm.c pdcurses.dll
i686-w64-mingw32-strip *.exe
xobs@nas /d/x/C/P/win32a> wine ./tuidemo.exe
000b:fixme:winediag:start_process Wine Staging 3.6 is a testing version containing experimental patches.
000b:fixme:winediag:start_process Please mention your exact version when filing bug reports on winehq.org.
0011:err:xrandr:xrandr12_init_modes Failed to get primary CRTC info.
002b:err:xrandr:xrandr12_init_modes Failed to get primary CRTC info.
000b:err:xrandr:xrandr12_init_modes Failed to get primary CRTC info.
000d:err:xrandr:xrandr12_init_modes Failed to get primary CRTC info.
0009:err:xrandr:xrandr12_init_modes Failed to get primary CRTC info.
Xlib:  extension "MIT-SHM" missing on display "localhost:10.0".
xobs@nas /d/x/C/P/win32a> # No segfault

xobs avatar Apr 18 '18 07:04 xobs

I can only 'sort of' test this. Under Wine 3.0, when we attempt the LoadLibrary call for User32.dll, that (of course) works; but there's no GetDpiForSystem function. So scale_font_for_current_dpi just goes with the default twelve-point font.

Bill-Gray avatar Nov 30 '18 00:11 Bill-Gray

Is this relevant to wincon (win32), too?

No, that depends on the console to do the rendering, and the console is already high-DPI aware. (In fact, wincon has no concept of fonts or font sizes at all.)

SDL is affected (see https://github.com/wmcbrine/PDCurses/issues/18 ), but of course is somewhat different...

wmcbrine avatar Dec 02 '18 18:12 wmcbrine

I'm interested in this patch for use in Cursive, which another patch on this repo referenced. Is there a good way to test it out myself?

wycats avatar May 21 '19 04:05 wycats

Any update on this? Does someone mind (especially @xobs, @clangen) to create a new PR against current master for this change, providing a screenshot before/after on a HiDPI display with testcurs demo?

Side question: Would a similar patch fix wmcbrine/PDCurses#18 for Win32 or would this be fixed by sdl-only code?

GitMensch avatar Sep 01 '20 06:09 GitMensch

Any update on this? Does someone mind to create a new PR against current master for this change, providing a screenshot before/after on a HiDPI display with testcurs demo?

Side question: Would a similar patch fix wmcbrine/PDCurses#18 for Win32 or would this be fixed by sdl-only code?

Friendly ping @xobs @clangen @Bill-Gray.

GitMensch avatar Dec 29 '22 10:12 GitMensch

I no longer have the setup in order to debug this code. However, someone else is free to take my patches and forward-port them with or without attribution.

xobs avatar Dec 29 '22 11:12 xobs

I was going to say this won't help with SDLn. On further investigation, though... it appears this change is simpler than I thought. It's looking to see if we can determine the screen DPI; if we can, we scale up the font size, proportional to a default USER_DEFAULT_SCREEN_DPI (seems to default to 96 DPI). So if the 'real' screen DPI is 300 pixels/inch, we increase the font size by a factor of 300/96.

I'm open to correction, but it looks to me as if this logic could be shared between WinGUI and SDLn.

If there is an SDL-specific function to determine screen DPI, we should probably use that instead. (If not, we could at least make it work on Windows.) If there is a similar function for X11, we could use that to make the X11 port "fixed" (and could use it on SDLn for *nix if there's no SDL-specific function available).

Bill-Gray avatar Dec 29 '22 20:12 Bill-Gray

Here's a fresh PR against the master branch without any conflicts: https://github.com/Bill-Gray/PDCursesMod/pull/263

clangen avatar Dec 29 '22 20:12 clangen

Thanks to the updated PR - as the original one won't be adjusted - I'm closing this one. Big Thanks to both @xobs and @clangen for working on this!

GitMensch avatar Dec 29 '22 21:12 GitMensch