[PATCH 0/3] MR4484: comctl32: Use selected font to determine default min tab width

List overview All Threads

newer

older

[PATCH 0/2] MR4485: Implement...

[PATCH v5 0/6] MR4449:...

Brendan McGrath (＠redmcg)

24 Nov 2023 24 Nov '23

12:10 a.m.

This fixes the layout of the tabcontrol when a font other than the system font is used.

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4484

Show replies by date

Brendan McGrath

24 Nov 24 Nov

12:10 a.m.

New subject: [PATCH 1/3] comctl32: Use selected font to determine default min tab width

From: Brendan McGrath bmcgrath@codeweavers.com

This fixes the layout of the tabcontrol when a font other than the system font is used. --- dlls/comctl32/tab.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/dlls/comctl32/tab.c b/dlls/comctl32/tab.c index 13abbd8078d..a3766bdbc01 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 */ @@ -1115,6 +1113,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 +1144,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 +1213,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 +1235,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);

@@ -2694,10 +2697,17 @@ static inline LRESULT TAB_SetMinTabWidth (TAB_INFO *infoPtr, INT cx)

TRACE("(%p,%d)\n", infoPtr, cx);

- if (infoPtr->tabMinWidth < 0) - oldcx = DEFAULT_MIN_TAB_WIDTH; - else + if (infoPtr->tabMinWidth < 0) { + TEXTMETRICW text_metrics; + HDC hdc = GetDC(infoPtr->hwnd); + HFONT oldFont = SelectObject(hdc, infoPtr->hFont); + GetTextMetricsW(hdc, &text_metrics); + oldcx = text_metrics.tmAveCharWidth * MIN_CHAR_LENGTH + infoPtr->uHItemPadding * 2; + SelectObject(hdc, oldFont); + ReleaseDC(infoPtr->hwnd, hdc); + } else { oldcx = infoPtr->tabMinWidth; + } infoPtr->tabMinWidth = cx; TAB_SetItemBounds(infoPtr); return oldcx;

-- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4484

Brendan McGrath

12:10 a.m.

New subject: [PATCH 2/3] comctl32: Fix tabcontrol tests to work with different fonts

From: Brendan McGrath bmcgrath@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

Brendan McGrath

12:10 a.m.

New subject: [PATCH 3/3] comctl32: Modify test_width to try different fonts

From: Brendan McGrath bmcgrath@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

Marvin

12:49 a.m.

New subject: [PATCH 3/3] comctl32: Modify test_width to try different fonts

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=140274

Your paranoid android.

=== debian11 (32 bit report) ===

=== debian11 (32 bit ar:MA report) ===

comctl32: tab.c:619: Test failed: no icon, set size: Expected [31,20] got [43,20] tab.c:647: Test failed: no icon, set size: Expected [30,20] got [42,20] tab.c:619: Test failed: no icon, set size: Expected [31,20] got [43,20] tab.c:647: Test failed: no icon, set size: Expected [30,20] got [42,20] tab.c:619: Test failed: no icon, set size: Expected [31,20] got [43,20] tab.c:619: Test failed: no icon, set size: Expected [42,20] got [54,20] tab.c:647: Test failed: no icon, set size: Expected [42,20] got [54,20] tab.c:619: Test failed: no icon, set size: Expected [42,20] got [54,20] tab.c:647: Test failed: no icon, set size: Expected [42,20] got [54,20] tab.c:619: Test failed: no icon, set size: Expected [35,20] got [47,20] tab.c:619: Test failed: no icon, set size: Expected [31,20] got [43,20] tab.c:647: Test failed: no icon, set size: Expected [30,20] got [42,20] tab.c:619: Test failed: no icon, set size: Expected [31,20] got [43,20] tab.c:647: Test failed: no icon, set size: Expected [30,20] got [42,20] tab.c:619: Test failed: no icon, set size: Expected [31,20] got [43,20]

=== debian11 (32 bit de report) ===

=== debian11 (32 bit fr report) ===

=== debian11 (32 bit he:IL report) ===

=== debian11 (32 bit hi:IN report) ===

=== 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] tab.c:619: Test failed: no icon, set size: Expected [31,20] got [43,20] tab.c:647: Test failed: no icon, set size: Expected [30,20] got [42,20] tab.c:619: Test failed: no icon, set size: Expected [31,20] got [43,20] tab.c:647: Test failed: no icon, set size: Expected [30,20] got [42,20] tab.c:619: Test failed: no icon, set size: Expected [31,20] got [43,20] tab.c:619: Test failed: no icon, set size: Expected [42,20] got [54,20] tab.c:647: Test failed: no icon, set size: Expected [42,20] got [54,20] tab.c:619: Test failed: no icon, set size: Expected [42,20] got [54,20] tab.c:647: Test failed: no icon, set size: Expected [42,20] got [54,20] tab.c:619: Test failed: no icon, set size: Expected [35,20] got [47,20] tab.c:619: Test failed: no icon, set size: Expected [31,20] got [43,20] tab.c:647: Test failed: no icon, set size: Expected [30,20] got [42,20] tab.c:619: Test failed: no icon, set size: Expected [31,20] got [43,20] tab.c:647: Test failed: no icon, set size: Expected [30,20] got [42,20] tab.c:619: Test failed: no icon, set size: Expected [31,20] got [43,20]

=== debian11 (32 bit zh:CN report) ===

=== debian11b (32 bit WoW report) ===

=== debian11b (64 bit WoW report) ===

441

Age (days ago)

441

Last active (days ago)

wine-gitlab@winehq.org

4 comments

3 participants

tags (0)

participants (3)

Brendan McGrath
Brendan McGrath (＠redmcg)
Marvin