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

List overview All Threads

newer

older

[PATCH 0/1] MR4570: winegstreamer:...

Re: [PATCH v3 0/1] MR4555: shcore:...

Brendan McGrath (＠redmcg)

1 Dec 2023 1 Dec '23

2:48 a.m.

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

-- v3: 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

Show replies by date

Brendan McGrath

1 Dec 1 Dec

2:48 a.m.

New subject: [PATCH v3 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 | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/dlls/comctl32/tab.c b/dlls/comctl32/tab.c index 13abbd8078d..d2310ea1567 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 */ @@ -1101,8 +1099,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 +1115,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 +1146,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 +1215,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 +1237,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 +1466,8 @@ static void TAB_SetItemBounds (TAB_INFO *infoPtr) /* Cleanup */ SelectObject (hdc, hOldFont); ReleaseDC (infoPtr->hwnd, hdc); + + return default_min_tab_width; }

@@ -2690,17 +2697,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

Brendan McGrath

2:48 a.m.

New subject: [PATCH v3 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

2:48 a.m.

New subject: [PATCH v3 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

3:35 a.m.

New subject: [PATCH v3 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=140606

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

Brendan McGrath (＠redmcg)

2:51 a.m.

On Fri Dec 1 02:48:44 2023 +0000, Brendan McGrath wrote:

...

changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/4484/diffs?diff_id=87330&start_sha=a4cdfc9b695220f581ad8f28e1c67141f281dd6c#a7af78b5641e9ec756ea963b78d868ada0627dd1_2712_2706)

Good idea. That's been done.

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

432

Age (days ago)

432

Last active (days ago)

wine-gitlab@winehq.org

5 comments

3 participants

tags (0)

participants (3)

Brendan McGrath
Brendan McGrath (＠redmcg)
Marvin