[PATCH 0/2] MR2849: comdlg32: Make item dialog and file access dialogs a bit more HiDPI friendly.
Toolbar buttons won't adjust its size to fit the DPI setting. This two patches set a DPI-considered size to those buttons to make them look better in HiDPI. Here are some comparisons on Wine with 192 DPI:  vs   vs  -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2849
From: Jactry Zeng <jzeng(a)codeweavers.com> --- dlls/comdlg32/itemdlg.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dlls/comdlg32/itemdlg.c b/dlls/comdlg32/itemdlg.c index 95778c2f358..e6f4ecb55b9 100644 --- a/dlls/comdlg32/itemdlg.c +++ b/dlls/comdlg32/itemdlg.c @@ -1896,6 +1896,7 @@ static void init_toolbar(FileDialogImpl *This, HWND hwnd) HWND htoolbar; TBADDBITMAP tbab; TBBUTTON button[2]; + int height; htoolbar = CreateWindowExW(0, TOOLBARCLASSNAMEW, NULL, TBSTYLE_FLAT | WS_CHILD | WS_VISIBLE, 0, 0, 0, 0, @@ -1920,7 +1921,8 @@ static void init_toolbar(FileDialogImpl *This, HWND hwnd) button[1].iString = 0; SendMessageW(htoolbar, TB_ADDBUTTONSW, 2, (LPARAM)button); - SendMessageW(htoolbar, TB_SETBUTTONSIZE, 0, MAKELPARAM(24,24)); + height = MulDiv(24, This->dpi_y, USER_DEFAULT_SCREEN_DPI); + SendMessageW(htoolbar, TB_SETBUTTONSIZE, 0, MAKELPARAM(height, height)); SendMessageW(htoolbar, TB_AUTOSIZE, 0, 0); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2849
From: Jactry Zeng <jzeng(a)codeweavers.com> The default size is too small for HiDPI, so calculate a size by the height of the look-in combo box and set it as the size of these buttons. --- dlls/comdlg32/filedlg.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/comdlg32/filedlg.c b/dlls/comdlg32/filedlg.c index 415c0580461..5d4b9649820 100644 --- a/dlls/comdlg32/filedlg.c +++ b/dlls/comdlg32/filedlg.c @@ -1486,6 +1486,7 @@ static LRESULT FILEDLG95_InitControls(HWND hwnd) BOOL win98plus = FALSE; BOOL handledPath = FALSE; OSVERSIONINFOW osVi; + int height; static const TBBUTTON tbb[] = { @@ -1551,6 +1552,7 @@ static LRESULT FILEDLG95_InitControls(HWND hwnd) rectTB.bottom = rectlook.top - 1 + rectTB.bottom - rectTB.top; rectTB.left = rectlook.right; rectTB.top = rectlook.top-1; + height = rectlook.bottom - rectlook.top; if (fodInfos->unicode) fodInfos->DlgInfos.hwndTB = CreateWindowExW(0, TOOLBARCLASSNAMEW, NULL, @@ -1584,6 +1586,7 @@ static LRESULT FILEDLG95_InitControls(HWND hwnd) /* Finish Toolbar Construction */ SendMessageW(fodInfos->DlgInfos.hwndTB, TB_ADDBUTTONSW, 9, (LPARAM) tbb); + SendMessageW(fodInfos->DlgInfos.hwndTB, TB_SETBUTTONSIZE, 0, MAKELPARAM(height, height)); SendMessageW(fodInfos->DlgInfos.hwndTB, TB_AUTOSIZE, 0, 0); if (is_places_bar_enabled(fodInfos)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2849
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=132802 Your paranoid android. === debian11 (32 bit report) === winhttp: notification.c:118: Test failed: 994: expected status 0x20000 got 0x4000 notification.c:118: Test failed: 994: expected status 0x20000 got 0x1 notification.c:123: Test failed: 994: expected callback 0x1 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x2 notification.c:123: Test failed: 994: expected callback 0x2 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x4 notification.c:123: Test failed: 994: expected callback 0x4 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x8 notification.c:123: Test failed: 994: expected callback 0x8 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x10 notification.c:123: Test failed: 994: expected callback 0x10 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x20 notification.c:123: Test failed: 994: expected callback 0x20 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x40 notification.c:123: Test failed: 994: expected callback 0x40 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x80 notification.c:123: Test failed: 994: expected callback 0x80 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x4000 notification.c:123: Test failed: 994: expected callback 0x4000 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x10 notification.c:123: Test failed: 994: expected callback 0x10 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x20 notification.c:123: Test failed: 994: expected callback 0x20 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x40 notification.c:123: Test failed: 994: expected callback 0x40 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x80 notification.c:123: Test failed: 994: expected callback 0x80 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x4000 notification.c:123: Test failed: 994: expected callback 0x4000 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x10 notification.c:123: Test failed: 994: expected callback 0x10 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x20 notification.c:123: Test failed: 994: expected callback 0x20 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x40 notification.c:123: Test failed: 994: expected callback 0x40 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x80 notification.c:123: Test failed: 994: expected callback 0x80 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x4000 notification.c:123: Test failed: 994: expected callback 0x4000 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x10 notification.c:123: Test failed: 994: expected callback 0x10 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x20 notification.c:123: Test failed: 994: expected callback 0x20 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x40 notification.c:123: Test failed: 994: expected callback 0x40 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x80 notification.c:123: Test failed: 994: expected callback 0x80 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x4000 notification.c:123: Test failed: 994: expected callback 0x4000 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x10 notification.c:123: Test failed: 994: expected callback 0x10 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x20 notification.c:123: Test failed: 994: expected callback 0x20 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x40 notification.c:123: Test failed: 994: expected callback 0x40 to be called from the same thread notification.c:118: Test failed: 994: expected status 0x20000 got 0x80 notification.c:123: Test failed: 994: expected callback 0x80 to be called from the same thread notification.c:123: Test failed: 994: expected callback 0x20000 to be called from the same thread notification.c:1174: Test failed: got 400 notification.c:118: Test failed: 1181: expected status 0x80000 got 0x200000 notification: Timeout winhttp.c:3543: Test failed: got 8 winhttp.c:3549: Test failed: got 12019 winhttp.c:3550: Test failed: got 3735928559 Unhandled exception: page fault on write access to 0x00000008 in 32-bit code (0x63c04786).
For filedlg, is there a reason not to use the size of the toolbar itself? I'd expect that it's already being scaled to DPI. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2849#note_33103
On Thu May 18 16:00:26 2023 +0000, Esme Povirk wrote:
For filedlg, is there a reason not to use the size of the toolbar itself? I'd expect that it's already being scaled to DPI. Hi Esme,
Yes, in theory, we can (by "height = rectTB.bottom - rectTB.top"). It will look like this:  The buttons on toolbar are much bigger than the button on the combo box, which looks not beautiful to my taste. :) But sure, this is mainly my personal preference. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2849#note_33253
On Fri May 19 14:56:52 2023 +0000, Jactry Zeng wrote:
Hi Esme, Yes, in theory, we can (by "height = rectTB.bottom - rectTB.top"). It will look like this:  The buttons on toolbar are much bigger than the button on the combo box, which looks not beautiful to my taste. :) But sure, this is mainly my personal preference. I see, even at the default DPI, the toolbar window is significantly larger than the toolbar is visually. But the "Look in" label control is not as tall as the combo box (which sizes itself automatically, ignoring the height in the dialog template), and to me it doesn't seem to line up.
My suggestion would be to use GetComboBoxInfo to find the height of the combo box's drop-down arrow. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2849#note_33257
participants (4)
-
Esme Povirk (＠madewokherd) -
Jactry Zeng
-
Jactry Zeng (＠jactry)
-
Marvin