[PATCH 0/4] MR10261: Fix toolbar wrapping issues

[Ivan Ivlev (＠iviv)]

This MR resolves two bugs with toolbar wrapping I have discovered.

1. Toolbar missing four buttons when app window is maximized (wrap should not appear at all in this case).
2. Last toolbar button disappears when window size (toolbar size) is slightly narrower then button right border.

Reasons of this behavior:

1. Toolbar buttons have BTNS_AUTOSIZE style, toolbar width is set using CCS_NORESIZE and equals to widths of all autosized buttons combined.

   cx (button width) value in TOOLBAR_WrapToolbar is set from nButtonWidth, which contains widest button width. Since buttons in toolbar have different widths, TOOLBAR_WrapToolbar miscalculates total width, because actual buttons widths are less then nButtonWidth. This results in TOOLBAR_WrapToolbar setting TBSTATE_WRAP when it is not not needed, and since app does not paint second button row, wrapped buttons just disappear.
2. After I fixed first issue, I noticed, that last button disappears, when toolbar become narrower then its right border. Then, when toolbar right border approximately in the middle of disappeared button, wrap happens. In windows system, wrapping occurs immediately when toolbar becomes narrower.

Found solutions:

1. I noticed, that TOOLBAR_LayoutToolbar calculates buttons rects differently, thats because it checks if button have BTNS_AUTOSIZE style and then calculate size. So i moved this BTNS_AUTOSIZE size calculation in separate function for it to then be used in TOOLBAR_LayoutToolbar and TOOLBAR_WrapToolbar to guarantee that cx values for specific button will be equal in both functions.
2. TOOLBAR_WrapToolbar function decides to wrap in case `if ((x + cx - (infoPtr->nButtonWidth - infoPtr->nBitmapWidth) / 2 > width).` This `(infoPtr->nButtonWidth - infoPtr->nBitmapWidth) / 2` turns out, makes this gap, where button right border (x + cx) exceed toolbar right border (width), but wrap wont appear. I did not found any reasons to make this half-button gap, so I just removed it and it solved this issue. The comment above says /\* The layout makes sure the bitmap is visible, but not the button. \*/, but it was introduced in 1999, when TOOLBAR_LayoutToolbar did not existed, and I think its irrelevant now, since layout calculates cx taking both bitmap and string into account.

I wrote tests for both fixes:

1. Creates CCS_NORESIZE toolbar, then add BTNS_AUTOSIZE buttons of different widths one by one and checks if wrap happened. In normal case (in windows) wrap should only happen on fourth button, but without fix wrap happens on third.
2. Creates CCS_NORESIZE toolbar with 4 BTNS_AUTOSIZE buttons. Toolbar width at the creation is calculated beforehand and equals exactly buttons widths combined. Then test reduces toolbar width by 1 and checks if wrap happened, in normal case wrap happens, without the fix its not.

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

[PATCH 1/4] comctl32/tests/toolbar.c: Test wrapping for fixed size toolbar and autosized buttons

[Ivan Ivlev]

From: Ivan Ivlev <[e-mail]>

Signed-off-by: Ivan Ivlev <[e-mail]>
---
 dlls/comctl32/tests/toolbar.c | 43 +++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/dlls/comctl32/tests/toolbar.c b/dlls/comctl32/tests/toolbar.c
index eed70df2a64..0a8a35c7042 100644
--- a/dlls/comctl32/tests/toolbar.c
+++ b/dlls/comctl32/tests/toolbar.c
@@ -3102,6 +3102,48 @@ static void test_WM_PAINT(BOOL v6)
     UnregisterClassW(wc.lpszClassName, 0);
 }
 
+void AddButton(HWND tb, const char *text, int id)
+{
+    TBBUTTON btn = {0};
+    btn.iBitmap   = I_IMAGENONE;
+    btn.idCommand = id;
+    btn.fsState   = TBSTATE_ENABLED;
+    btn.fsStyle   = BTNS_BUTTON | BTNS_AUTOSIZE;
+    btn.iString   = (INT_PTR)text;
+    SendMessageA(tb, TB_ADDBUTTONSA, 1, (LPARAM)&btn);
+}
+
+static void test_wrap(void)
+{
+    int result;
+    HWND hToolbar = CreateWindowExA(0, TOOLBARCLASSNAMEA, NULL, WS_CHILD | WS_VISIBLE | CCS_NORESIZE | TBSTYLE_WRAPABLE,
+            0, 0, 150, 30, hMainWnd, NULL, GetModuleHandleA(NULL), NULL);
+
+    SendMessageA(hToolbar, TB_BUTTONSTRUCTSIZE, sizeof(TBBUTTON), 0);
+
+    AddButton(hToolbar, "T", 0);
+    SendMessageA(hToolbar, TB_AUTOSIZE, 0, 0);
+    result = SendMessageA(hToolbar, TB_GETROWS, 0, 0);
+    ok(result == 1, "Got unexpected nRows: %d.\n", result);
+
+    AddButton(hToolbar, "TEST", 1);
+    SendMessageA(hToolbar, TB_AUTOSIZE, 0, 0);
+    result = SendMessageA(hToolbar, TB_GETROWS, 0, 0);
+    ok(result == 1, "Got unexpected nRows: %d.\n", result);
+
+    AddButton(hToolbar, "TESTTESTTEST", 2);
+    SendMessageA(hToolbar, TB_AUTOSIZE, 0, 0);
+    result = SendMessageA(hToolbar, TB_GETROWS, 0, 0);
+    ok(result == 1, "Got unexpected nRows: %d.\n", result);
+
+    AddButton(hToolbar, "TESTTESTTESTTESTTESTTE", 3);
+    SendMessageA(hToolbar, TB_AUTOSIZE, 0, 0);
+    result = SendMessageA(hToolbar, TB_GETROWS, 0, 0);
+    ok(result == 2, "Got unexpected nRows: %d.\n", result);
+
+    DestroyWindow(hToolbar);
+}
+
 START_TEST(toolbar)
 {
     ULONG_PTR ctx_cookie;
@@ -3157,6 +3199,7 @@ START_TEST(toolbar)
     test_unicode_format();
     test_WM_ERASEBKGND(FALSE);
     test_WM_PAINT(FALSE);
+    test_wrap();
 
     if (!load_v6_module(&ctx_cookie, &ctx))
         return;
-- 
GitLab


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

[PATCH 2/4] comctl32/toolbar.c: Move BTNS_AUTOSIZE processing from LayoutToolbar in a separate function and use it in WrapToolbar

[Ivan Ivlev]

From: Ivan Ivlev <[e-mail]>

Signed-off-by: Ivan Ivlev <[e-mail]>
---
 dlls/comctl32/toolbar.c | 45 +++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/dlls/comctl32/toolbar.c b/dlls/comctl32/toolbar.c
index 3826b18d4c6..5dffa4687c9 100644
--- a/dlls/comctl32/toolbar.c
+++ b/dlls/comctl32/toolbar.c
@@ -248,6 +248,7 @@ static void TOOLBAR_TooltipAddTool(const TOOLBAR_INFO *infoPtr, const TBUTTON_IN
 static void TOOLBAR_TooltipSetRect(const TOOLBAR_INFO *infoPtr, const TBUTTON_INFO *button);
 static LRESULT TOOLBAR_SetButtonInfo(TOOLBAR_INFO *infoPtr, INT Id,
                                      const TBBUTTONINFOW *lptbbi, BOOL isW);
+static int TOOLBAR_AutoSizeButtonWidth(TOOLBAR_INFO *infoPtr, TBUTTON_INFO *btnPtr, int btnIdx);
 
 
 static inline int default_top_margin(const TOOLBAR_INFO *infoPtr)
@@ -1414,6 +1415,8 @@ TOOLBAR_WrapToolbar(TOOLBAR_INFO *infoPtr)
 	else if ((btnPtr[i].fsStyle & BTNS_SEP) &&
             !(infoPtr->dwStyle & CCS_VERT))
             cx = (btnPtr[i].iBitmap > 0) ? btnPtr[i].iBitmap : SEPARATOR_WIDTH;
+    else if (btnPtr[i].fsStyle & BTNS_AUTOSIZE)
+        cx = TOOLBAR_AutoSizeButtonWidth(infoPtr, &btnPtr[i], i);
 	else
 	    cx = infoPtr->nButtonWidth;
 
@@ -1687,6 +1690,27 @@ static inline SIZE TOOLBAR_MeasureButton(const TOOLBAR_INFO *infoPtr, SIZE sizeS
     return sizeButton;
 }
 
+static int
+TOOLBAR_AutoSizeButtonWidth(TOOLBAR_INFO *infoPtr, TBUTTON_INFO *btnPtr, int btnIdx)
+{
+    SIZE sz, sizeButton;
+    HDC hdc;
+    HFONT hOldFont;
+    BOOL validImageList = TOOLBAR_IsValidImageList(infoPtr, 0);
+
+    hdc = GetDC (infoPtr->hwndSelf);
+    hOldFont = SelectObject (hdc, infoPtr->hFont);
+
+    TOOLBAR_MeasureString(infoPtr, btnPtr, hdc, &sz);
+
+    SelectObject (hdc, hOldFont);
+    ReleaseDC (infoPtr->hwndSelf, hdc);
+
+    sizeButton = TOOLBAR_MeasureButton(infoPtr, sz,
+        TOOLBAR_IsValidBitmapIndex(infoPtr, btnPtr[btnIdx].iBitmap),
+        validImageList);
+    return sizeButton.cx;
+}
 
 /***********************************************************************
 * 		TOOLBAR_CalcToolbar
@@ -1727,11 +1751,9 @@ static void
 TOOLBAR_LayoutToolbar(TOOLBAR_INFO *infoPtr)
 {
     TBUTTON_INFO *btnPtr;
-    SIZE sizeButton;
     INT i, nRows, nSepRows;
     INT x, y, cx, cy;
     BOOL bWrap;
-    BOOL validImageList = TOOLBAR_IsValidImageList(infoPtr, 0);
 
     TOOLBAR_WrapToolbar(infoPtr);
 
@@ -1777,24 +1799,7 @@ TOOLBAR_LayoutToolbar(TOOLBAR_INFO *infoPtr)
             if (btnPtr->cx)
               cx = btnPtr->cx;
             else if (btnPtr->fsStyle & BTNS_AUTOSIZE)
-            {
-              SIZE sz;
-	      HDC hdc;
-	      HFONT hOldFont;
-
-	      hdc = GetDC (infoPtr->hwndSelf);
-	      hOldFont = SelectObject (hdc, infoPtr->hFont);
-
-              TOOLBAR_MeasureString(infoPtr, btnPtr, hdc, &sz);
-
-	      SelectObject (hdc, hOldFont);
-	      ReleaseDC (infoPtr->hwndSelf, hdc);
-
-              sizeButton = TOOLBAR_MeasureButton(infoPtr, sz,
-                  TOOLBAR_IsValidBitmapIndex(infoPtr, infoPtr->buttons[i].iBitmap),
-                  validImageList);
-              cx = sizeButton.cx;
-            }
+                cx = TOOLBAR_AutoSizeButtonWidth(infoPtr, btnPtr, i);
             else
 	      cx = infoPtr->nButtonWidth;
 
-- 
GitLab


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

[PATCH 3/4] comctl32/tests/toolbar.c: Test wrapping occurs immediately when needed

[Ivan Ivlev]

From: Ivan Ivlev <[e-mail]>

Signed-off-by: Ivan Ivlev <[e-mail]>
---
 dlls/comctl32/tests/toolbar.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/dlls/comctl32/tests/toolbar.c b/dlls/comctl32/tests/toolbar.c
index 0a8a35c7042..8bdd22dda6f 100644
--- a/dlls/comctl32/tests/toolbar.c
+++ b/dlls/comctl32/tests/toolbar.c
@@ -3142,6 +3142,28 @@ static void test_wrap(void)
     ok(result == 2, "Got unexpected nRows: %d.\n", result);
 
     DestroyWindow(hToolbar);
+
+    hToolbar = CreateWindowExA(0, TOOLBARCLASSNAMEA, NULL, WS_CHILD | WS_VISIBLE | CCS_NORESIZE | TBSTYLE_WRAPABLE,
+            0, 0, 428, 30, hMainWnd, NULL, GetModuleHandleA(NULL), NULL);
+
+    SendMessageA(hToolbar, TB_BUTTONSTRUCTSIZE, sizeof(TBBUTTON), 0);
+
+    //total button width is 428
+    AddButton(hToolbar, "................................", 0);
+    AddButton(hToolbar, "................................", 1);
+    AddButton(hToolbar, "................................", 2);
+    AddButton(hToolbar, "................................", 3);
+
+    SendMessageA(hToolbar, TB_AUTOSIZE, 0, 0);
+    result = SendMessageA(hToolbar, TB_GETROWS, 0, 0);
+    ok(result == 1, "Got unexpected nRows: %d.\n", result);
+
+    SetWindowPos(hToolbar, NULL, 0, 0, 427, 30, SWP_NOMOVE | SWP_NOZORDER);
+    SendMessageA(hToolbar, TB_AUTOSIZE, 0, 0);
+    result = SendMessageA(hToolbar, TB_GETROWS, 0, 0);
+    ok(result == 2, "Got unexpected nRows: %d.\n", result);
+
+    DestroyWindow(hToolbar);
 }
 
 START_TEST(toolbar)
-- 
GitLab


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

[PATCH 4/4] comctl32/toolbar.c: Wrap toolbar immediately after width exceeded

[Ivan Ivlev]

From: Ivan Ivlev <[e-mail]>

Signed-off-by: Ivan Ivlev <[e-mail]>
---
 dlls/comctl32/toolbar.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/dlls/comctl32/toolbar.c b/dlls/comctl32/toolbar.c
index 5dffa4687c9..e40c9cd2ef9 100644
--- a/dlls/comctl32/toolbar.c
+++ b/dlls/comctl32/toolbar.c
@@ -1439,10 +1439,9 @@ TOOLBAR_WrapToolbar(TOOLBAR_INFO *infoPtr)
 	    continue;
 	}
 
-	/* The layout makes sure the bitmap is visible, but not the button. */
 	/* Test added to also wrap after a button that starts a row but     */
 	/* is bigger than the area.  - GA  8/01                             */
-        if ((x + cx - (infoPtr->nButtonWidth - infoPtr->nBitmapWidth) / 2 > width) ||
+        if ((x + cx > width) ||
             ((x == infoPtr->nIndent) && (cx > width)))
 	{
 	    BOOL bFound = FALSE;
-- 
GitLab

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