[PATCH v4 0/3] MR4484: comctl32: Use selected font to determine default min tab width
This fixes the layout of the tabcontrol when a font other than the system font is used. -- v4: comctl32: Modify test_width to try different fonts comctl32: Fix tabcontrol tests to work with different fonts comctl32: Use selected font to determine default min tab width https://gitlab.winehq.org/wine/wine/-/merge_requests/4484
From: Brendan McGrath <bmcgrath(a)codeweavers.com> This fixes the layout of the tabcontrol when a font other than the system font is used. --- dlls/comctl32/tab.c | 70 +++++++++++++++++++++++++++++---------------- 1 file changed, 45 insertions(+), 25 deletions(-) diff --git a/dlls/comctl32/tab.c b/dlls/comctl32/tab.c index 13abbd8078d..0df005980d3 100644 --- a/dlls/comctl32/tab.c +++ b/dlls/comctl32/tab.c @@ -138,14 +138,12 @@ typedef struct #define BUTTON_SPACINGX 3 #define BUTTON_SPACINGY 3 #define FLAT_BTN_SPACINGX 8 -#define DEFAULT_MIN_TAB_WIDTH 54 #define DEFAULT_PADDING_X 6 #define EXTRA_ICON_PADDING 3 +#define MIN_CHAR_LENGTH 6 #define TAB_GetInfoPtr(hwnd) ((TAB_INFO *)GetWindowLongPtrW(hwnd,0)) -#define GET_DEFAULT_MIN_TAB_WIDTH(infoPtr) (DEFAULT_MIN_TAB_WIDTH - (DEFAULT_PADDING_X - (infoPtr)->uHItemPadding) * 2) - /****************************************************************************** * Hot-tracking timer constants */ @@ -340,16 +338,6 @@ TAB_SetToolTips (TAB_INFO *infoPtr, HWND hwndToolTip) return 0; } -static inline LRESULT -TAB_SetPadding (TAB_INFO *infoPtr, LPARAM lParam) -{ - TRACE("(%p %d %d)\n", infoPtr, LOWORD(lParam), HIWORD(lParam)); - infoPtr->uHItemPadding_s = LOWORD(lParam); - infoPtr->uVItemPadding_s = HIWORD(lParam); - - return 0; -} - /****************************************************************************** * TAB_InternalGetItemRect * @@ -1101,8 +1089,10 @@ static void TAB_SetupScrolling( * It also uses the current font to determine the height of the tab row and * it checks if all the tabs fit in the client area of the window. If they * don't, a scrolling control is added. + * + * Returns the default minimum tab width */ -static void TAB_SetItemBounds (TAB_INFO *infoPtr) +static INT TAB_SetItemBounds (TAB_INFO *infoPtr) { TEXTMETRICW fontMetrics; UINT curItem; @@ -1115,6 +1105,8 @@ static void TAB_SetItemBounds (TAB_INFO *infoPtr) RECT* rcItem; INT iIndex; INT icon_width = 0; + INT default_min_tab_width; + TEXTMETRICW text_metrics; /* * We need to get text information so we need a DC and we need to select @@ -1144,6 +1136,9 @@ static void TAB_SetItemBounds (TAB_INFO *infoPtr) /* Now use hPadding and vPadding */ infoPtr->uHItemPadding = infoPtr->uHItemPadding_s; infoPtr->uVItemPadding = infoPtr->uVItemPadding_s; + + GetTextMetricsW(hdc, &text_metrics); + default_min_tab_width = text_metrics.tmAveCharWidth * MIN_CHAR_LENGTH + infoPtr->uHItemPadding * 2; /* The leftmost item will be "0" aligned */ curItemLeftPos = 0; @@ -1210,13 +1205,13 @@ static void TAB_SetItemBounds (TAB_INFO *infoPtr) { /* If no text use minimum tab width including padding. */ if (infoPtr->tabMinWidth < 0) - curr->rect.right = curr->rect.left + GET_DEFAULT_MIN_TAB_WIDTH(infoPtr); + curr->rect.right = curr->rect.left + default_min_tab_width; else { curr->rect.right = curr->rect.left + infoPtr->tabMinWidth; /* Add extra padding if icon is present */ - if (infoPtr->himl && infoPtr->tabMinWidth > 0 && infoPtr->tabMinWidth < DEFAULT_MIN_TAB_WIDTH + if (infoPtr->himl && infoPtr->tabMinWidth > 0 && infoPtr->tabMinWidth < MIN_CHAR_LENGTH * text_metrics.tmAveCharWidth && infoPtr->uHItemPadding > 1) curr->rect.right += EXTRA_ICON_PADDING * (infoPtr->uHItemPadding-1); } @@ -1232,7 +1227,7 @@ static void TAB_SetItemBounds (TAB_INFO *infoPtr) tabwidth = size.cx + icon_width + 2 * infoPtr->uHItemPadding; if (infoPtr->tabMinWidth < 0) - tabwidth = max(tabwidth, GET_DEFAULT_MIN_TAB_WIDTH(infoPtr)); + tabwidth = max(tabwidth, default_min_tab_width); else tabwidth = max(tabwidth, infoPtr->tabMinWidth); @@ -1461,6 +1456,34 @@ static void TAB_SetItemBounds (TAB_INFO *infoPtr) /* Cleanup */ SelectObject (hdc, hOldFont); ReleaseDC (infoPtr->hwnd, hdc); + + return default_min_tab_width; +} + +static inline LRESULT +TAB_SetPadding (TAB_INFO *infoPtr, LPARAM lParam) +{ + BOOL bNeedPaint = FALSE; + + TRACE("(%p %d %d)\n", infoPtr, LOWORD(lParam), HIWORD(lParam)); + if (infoPtr->uHItemPadding_s != LOWORD(lParam)) + { + infoPtr->uHItemPadding_s = LOWORD(lParam); + bNeedPaint = TRUE; + } + if (infoPtr->uVItemPadding_s != HIWORD(lParam)) + { + infoPtr->uVItemPadding_s = HIWORD(lParam); + bNeedPaint = TRUE; + } + + if (bNeedPaint) + { + TAB_SetItemBounds(infoPtr); + RedrawWindow(infoPtr->hwnd, NULL, NULL, RDW_ERASE | RDW_INVALIDATE | RDW_UPDATENOW); + } + + return 0; } @@ -2690,17 +2713,14 @@ TAB_SetItemSize (TAB_INFO *infoPtr, INT cx, INT cy) static inline LRESULT TAB_SetMinTabWidth (TAB_INFO *infoPtr, INT cx) { - INT oldcx = 0; + INT default_min_tab_width; + INT prevMinWidth = infoPtr->tabMinWidth; + infoPtr->tabMinWidth = cx; TRACE("(%p,%d)\n", infoPtr, cx); - if (infoPtr->tabMinWidth < 0) - oldcx = DEFAULT_MIN_TAB_WIDTH; - else - oldcx = infoPtr->tabMinWidth; - infoPtr->tabMinWidth = cx; - TAB_SetItemBounds(infoPtr); - return oldcx; + default_min_tab_width = TAB_SetItemBounds(infoPtr); + return (prevMinWidth < 0) ? default_min_tab_width : prevMinWidth; } static inline LRESULT -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4484
From: Brendan McGrath <bmcgrath(a)codeweavers.com> The WM_SETFONT was previously passing the hFont as the LPARAM (when it should be the WPARAM). --- dlls/comctl32/tests/tab.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/dlls/comctl32/tests/tab.c b/dlls/comctl32/tests/tab.c index a5c51690689..99116e9e9c6 100644 --- a/dlls/comctl32/tests/tab.c +++ b/dlls/comctl32/tests/tab.c @@ -25,10 +25,10 @@ #include "wine/test.h" #include "msg.h" -#define DEFAULT_MIN_TAB_WIDTH 54 #define TAB_PADDING_X 6 #define EXTRA_ICON_PADDING 3 #define MAX_TABLEN 32 +#define MIN_CHAR_LENGTH 6 #define NUM_MSG_SEQUENCES 2 #define PARENT_SEQ_INDEX 0 @@ -43,7 +43,7 @@ static inline void expect_(unsigned line, DWORD expected, DWORD got) #define expect_str(expected, got)\ ok ( strcmp(expected, got) == 0, "Expected '%s', got '%s'\n", expected, got) -#define TabWidthPadded(padd_x, num) (DEFAULT_MIN_TAB_WIDTH - (TAB_PADDING_X - (padd_x)) * num) +#define TabWidthPadded(default_min_tab_width, padd_x, num) ((default_min_tab_width) - (TAB_PADDING_X - (padd_x)) * num) static HIMAGELIST (WINAPI *pImageList_Create)(INT,INT,UINT,INT,INT); static BOOL (WINAPI *pImageList_Destroy)(HIMAGELIST); @@ -313,7 +313,7 @@ create_tabcontrol (DWORD style, DWORD mask) ok(handle != NULL, "failed to create tab wnd\n"); SetWindowLongA(handle, GWL_STYLE, WS_CLIPSIBLINGS | WS_CLIPCHILDREN | TCS_FOCUSNEVER | style); - SendMessageA(handle, WM_SETFONT, 0, (LPARAM)hFont); + SendMessageA(handle, WM_SETFONT, (WPARAM)hFont, (LPARAM)0); tcNewTab.mask = mask; tcNewTab.pszText = text1; @@ -524,18 +524,19 @@ static void test_tab(INT nMinTabWidth) HDC hdc; HFONT hOldFont; INT i, dpi, exp; + INT default_min_tab_width; + TEXTMETRICW text_metrics; hwTab = create_tabcontrol(TCS_FIXEDWIDTH, TCIF_TEXT|TCIF_IMAGE); SendMessageA(hwTab, TCM_SETMINTABWIDTH, 0, nMinTabWidth); - /* Get System default MinTabWidth */ - if (nMinTabWidth < 0) - nMinTabWidth = SendMessageA(hwTab, TCM_SETMINTABWIDTH, 0, nMinTabWidth); hdc = GetDC(hwTab); dpi = GetDeviceCaps(hdc, LOGPIXELSX); hOldFont = SelectObject(hdc, (HFONT)SendMessageA(hwTab, WM_GETFONT, 0, 0)); GetTextExtentPoint32A(hdc, "Tab 1", strlen("Tab 1"), &size); trace("Tab1 text size: size.cx=%ld size.cy=%ld\n", size.cx, size.cy); + GetTextMetricsW(hdc, &text_metrics); + default_min_tab_width = text_metrics.tmAveCharWidth * MIN_CHAR_LENGTH + TAB_PADDING_X * 2; SelectObject(hdc, hOldFont); ReleaseDC(hwTab, hdc); @@ -603,14 +604,14 @@ static void test_tab(INT nMinTabWidth) SendMessageA(hwTab, TCM_SETMINTABWIDTH, 0, nMinTabWidth); trace (" non fixed width, with text...\n"); - exp = max(size.cx +TAB_PADDING_X*2, (nMinTabWidth < 0) ? DEFAULT_MIN_TAB_WIDTH : nMinTabWidth); + exp = max(size.cx +TAB_PADDING_X*2, (nMinTabWidth < 0) ? default_min_tab_width : nMinTabWidth); SendMessageA( hwTab, TCM_GETITEMRECT, 0, (LPARAM)&rTab ); - ok( rTab.right - rTab.left == exp || broken(rTab.right - rTab.left == DEFAULT_MIN_TAB_WIDTH), + ok( rTab.right - rTab.left == exp || broken(rTab.right - rTab.left == default_min_tab_width), "no icon, default width: Expected width [%d] got [%ld]\n", exp, rTab.right - rTab.left ); for (i=0; i<8; i++) { - INT nTabWidth = (nMinTabWidth < 0) ? TabWidthPadded(i, 2) : nMinTabWidth; + INT nTabWidth = (nMinTabWidth < 0) ? TabWidthPadded(default_min_tab_width, i, 2) : nMinTabWidth; SendMessageA(hwTab, TCM_SETIMAGELIST, 0, 0); SendMessageA(hwTab, TCM_SETPADDING, 0, MAKELPARAM(i, i)); @@ -619,7 +620,7 @@ static void test_tab(INT nMinTabWidth) TABCHECKSETSIZE(hwTab, 0, 1, max(size.cx + i*2, nTabWidth), 1, "no icon, min size"); SendMessageA(hwTab, TCM_SETIMAGELIST, 0, (LPARAM)himl); - nTabWidth = (nMinTabWidth < 0) ? TabWidthPadded(i, 3) : nMinTabWidth; + nTabWidth = (nMinTabWidth < 0) ? TabWidthPadded(default_min_tab_width, i, 3) : nMinTabWidth; TABCHECKSETSIZE(hwTab, 50, 30, max(size.cx + 21 + i*3, nTabWidth), 30, "with icon, set size > icon"); TABCHECKSETSIZE(hwTab, 20, 20, max(size.cx + 21 + i*3, nTabWidth), 20, "with icon, set size < icon"); @@ -631,14 +632,14 @@ static void test_tab(INT nMinTabWidth) SendMessageA(hwTab, TCM_SETMINTABWIDTH, 0, nMinTabWidth); trace (" non fixed width, no text...\n"); - exp = (nMinTabWidth < 0) ? DEFAULT_MIN_TAB_WIDTH : nMinTabWidth; + exp = (nMinTabWidth < 0) ? default_min_tab_width : nMinTabWidth; SendMessageA( hwTab, TCM_GETITEMRECT, 0, (LPARAM)&rTab ); - ok( rTab.right - rTab.left == exp || broken(rTab.right - rTab.left == DEFAULT_MIN_TAB_WIDTH), + ok( rTab.right - rTab.left == exp || broken(rTab.right - rTab.left == default_min_tab_width), "no icon, default width: Expected width [%d] got [%ld]\n", exp, rTab.right - rTab.left ); for (i=0; i<8; i++) { - INT nTabWidth = (nMinTabWidth < 0) ? TabWidthPadded(i, 2) : nMinTabWidth; + INT nTabWidth = (nMinTabWidth < 0) ? TabWidthPadded(default_min_tab_width, i, 2) : nMinTabWidth; SendMessageA(hwTab, TCM_SETIMAGELIST, 0, 0); SendMessageA(hwTab, TCM_SETPADDING, 0, MAKELPARAM(i, i)); @@ -647,7 +648,7 @@ static void test_tab(INT nMinTabWidth) TABCHECKSETSIZE(hwTab, 0, 1, nTabWidth, 1, "no icon, min size"); SendMessageA(hwTab, TCM_SETIMAGELIST, 0, (LPARAM)himl); - if (i > 1 && nMinTabWidth > 0 && nMinTabWidth < DEFAULT_MIN_TAB_WIDTH) + if (i > 1 && nMinTabWidth > 0 && nMinTabWidth < default_min_tab_width) nTabWidth += EXTRA_ICON_PADDING *(i-1); TABCHECKSETSIZE(hwTab, 50, 30, nTabWidth, 30, "with icon, set size > icon"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4484
From: Brendan McGrath <bmcgrath(a)codeweavers.com> --- dlls/comctl32/tests/tab.c | 43 ++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/dlls/comctl32/tests/tab.c b/dlls/comctl32/tests/tab.c index 99116e9e9c6..e0afe7dce27 100644 --- a/dlls/comctl32/tests/tab.c +++ b/dlls/comctl32/tests/tab.c @@ -663,16 +663,39 @@ static void test_tab(INT nMinTabWidth) static void test_width(void) { - trace ("Testing with default MinWidth\n"); - test_tab(-1); - trace ("Testing with MinWidth set to -3\n"); - test_tab(-3); - trace ("Testing with MinWidth set to 24\n"); - test_tab(24); - trace ("Testing with MinWidth set to 54\n"); - test_tab(54); - trace ("Testing with MinWidth set to 94\n"); - test_tab(94); + HFONT oldFont = hFont; + const char *fonts[] = { + "System", + "Arial", + "Tahoma", + "Courier New", + "MS Shell Dlg", + }; + + LOGFONTA logfont = { + .lfHeight = -12, + .lfWeight = FW_NORMAL, + .lfCharSet = ANSI_CHARSET + }; + + for(int i = 0; i < sizeof(fonts)/sizeof(fonts[0]); i++) { + trace ("Testing with the '%s' font\n", fonts[i]); + lstrcpyA(logfont.lfFaceName, fonts[i]); + hFont = CreateFontIndirectA(&logfont); + + trace ("Testing with default MinWidth\n"); + test_tab(-1); + trace ("Testing with MinWidth set to -3\n"); + test_tab(-3); + trace ("Testing with MinWidth set to 24\n"); + test_tab(24); + trace ("Testing with MinWidth set to 54\n"); + test_tab(54); + trace ("Testing with MinWidth set to 94\n"); + test_tab(94); + } + + hFont = oldFont; } static void test_curfocus(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4484
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=140608 Your paranoid android. === debian11 (32 bit ja:JP report) === comctl32: tab.c:654: Test failed: with icon, set size > icon: Expected [57,30] got [54,30] tab.c:655: Test failed: with icon, set size < icon: Expected [57,20] got [54,20] tab.c:656: Test failed: with icon, min size: Expected [57,1] got [54,1] tab.c:654: Test failed: with icon, set size > icon: Expected [60,30] got [54,30] tab.c:655: Test failed: with icon, set size < icon: Expected [60,20] got [54,20] tab.c:656: Test failed: with icon, min size: Expected [60,1] got [54,1] tab.c:654: Test failed: with icon, set size > icon: Expected [63,30] got [54,30] tab.c:655: Test failed: with icon, set size < icon: Expected [63,20] got [54,20] tab.c:656: Test failed: with icon, min size: Expected [63,1] got [54,1] tab.c:654: Test failed: with icon, set size > icon: Expected [66,30] got [54,30] tab.c:655: Test failed: with icon, set size < icon: Expected [66,20] got [54,20] tab.c:656: Test failed: with icon, min size: Expected [66,1] got [54,1] tab.c:654: Test failed: with icon, set size > icon: Expected [69,30] got [54,30] tab.c:655: Test failed: with icon, set size < icon: Expected [69,20] got [54,20] tab.c:656: Test failed: with icon, min size: Expected [69,1] got [54,1] tab.c:654: Test failed: with icon, set size > icon: Expected [72,30] got [54,30] tab.c:655: Test failed: with icon, set size < icon: Expected [72,20] got [54,20] tab.c:656: Test failed: with icon, min size: Expected [72,1] got [54,1]
Nikolay Sivov (@nsivov) commented about dlls/comctl32/tab.c:
+ bNeedPaint = TRUE; + } + if (infoPtr->uVItemPadding_s != HIWORD(lParam)) + { + infoPtr->uVItemPadding_s = HIWORD(lParam); + bNeedPaint = TRUE; + } + + if (bNeedPaint) + { + TAB_SetItemBounds(infoPtr); + RedrawWindow(infoPtr->hwnd, NULL, NULL, RDW_ERASE | RDW_INVALIDATE | RDW_UPDATENOW); + } + + return 0; } Does it make sense to separate SetPadding to its own commit? It's a dedicate message, so it's not obvious why it should also change, including force repaint part.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4484#note_54790
participants (4)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Marvin -
Nikolay Sivov (＠nsivov)