[PATCH 0/1] MR3667: comctl32: Include HORZ_GAP in update region and invalidate if movement is verticle.

[Jacob Czekalla (＠Maaka00524)]

Fixes a bug where the statusbar doesn't correctly paint when the parent window is resized. This can be seen in wine notepad.

[PATCH 1/1] comctl32: Include HORZ_GAP in update region and invalidate if movement is verticle.

[Jacob Czekalla]

From: Jacob Czekalla

---
 dlls/comctl32/status.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/dlls/comctl32/status.c b/dlls/comctl32/status.c
index 5251f3677b0..392c7416eca 100644
--- a/dlls/comctl32/status.c
+++ b/dlls/comctl32/status.c
@@ -62,6 +62,8 @@ typedef struct
     HWND              Notify;
     WORD              numParts;
     UINT              height;
+    UINT              width;
+    UINT              y;
     UINT              minHeight;        /* at least MIN_PANE_HEIGHT, can be increased by SB_SETMINHEIGHT */
     BOOL              simple;
     HWND              hwndToolTip;
@@ -865,6 +867,8 @@ STATUSBAR_WMCreate (HWND hwnd, const CREATESTRUCTA *lpCreate)
     infoPtr->verticalBorder = VERT_BORDER;
     infoPtr->horizontalGap = HORZ_GAP;
     infoPtr->minHeight = GetSystemMetrics(SM_CYSIZE);
+    infoPtr->width = 0;
+    infoPtr->y = 0;
     if (infoPtr->minHeight & 1) infoPtr->minHeight--;

STATUSBAR_NotifyFormat(infoPtr, infoPtr->Notify, NF_REQUERY);
@@ -1050,6 +1054,7 @@ STATUSBAR_WMSize (STATUS_INFO *infoPtr, WORD flags)
 {
     INT  width, x, y;
     RECT parent_rect;
+    RECT update_rect;

/* Need to resize width to match parent */
     TRACE("flags %04x\n", flags);
@@ -1068,8 +1073,20 @@ STATUSBAR_WMSize (STATUS_INFO *infoPtr, WORD flags)
     width = parent_rect.right - parent_rect.left;
     x = parent_rect.left;
     y = parent_rect.bottom - infoPtr->height;
-    MoveWindow (infoPtr->Self, x, y, width, infoPtr->height, TRUE);
+    MoveWindow (infoPtr->Self, x, y, width, infoPtr->height, FALSE);
+    if(y != infoPtr->y)
+        InvalidateRect(infoPtr->Self, NULL, FALSE);
+    else
+    {
+        update_rect.left = min(infoPtr->width-HORZ_GAP, width);
+        update_rect.right = max(width, infoPtr->width-HORZ_GAP);
+        update_rect.top = infoPtr->height;
+        update_rect.bottom = 0;
+        InvalidateRect(infoPtr->Self, &update_rect, FALSE);
+    }
     STATUSBAR_SetPartBounds (infoPtr);
+    infoPtr->width = width;
+    infoPtr->y = y;
     return TRUE;
 }

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/status.c:
>  HWND              Notify;
>  WORD              numParts;

Do you mean `vertical` in the commit subject?

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/status.c:
>  width = parent_rect.right - parent_rect.left;
>  x = parent_rect.left;
>  y = parent_rect.bottom - infoPtr->height;
> - MoveWindow (infoPtr->Self, x, y, width, infoPtr->height, TRUE);
> - MoveWindow (infoPtr->Self, x, y, width, infoPtr->height, FALSE);
> - if(y != infoPtr->y)
> -
>    InvalidateRect(infoPtr->Self, NULL, FALSE);
> - else
> - {
> -
>    update_rect.left = min(infoPtr->width-HORZ_GAP, width);

Add space before and after the minus sign.

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/status.c:
>  width = parent_rect.right - parent_rect.left;
>  x = parent_rect.left;
>  y = parent_rect.bottom - infoPtr->height;
> - MoveWindow (infoPtr->Self, x, y, width, infoPtr->height, TRUE);
> - MoveWindow (infoPtr->Self, x, y, width, infoPtr->height, FALSE);
> - if(y != infoPtr->y)
> -
>    InvalidateRect(infoPtr->Self, NULL, FALSE);
> - else
> - {
> -
>    update_rect.left = min(infoPtr->width-HORZ_GAP, width);
> -
>    update_rect.right = max(width, infoPtr->width-HORZ_GAP);

I think you should use infoPtr->horizontalGap instead. HORZ_GAP is only the default for infoPtr->horizontalGap.

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/status.c:
>  width = parent_rect.right - parent_rect.left;
>  x = parent_rect.left;
>  y = parent_rect.bottom - infoPtr->height;
> - MoveWindow (infoPtr->Self, x, y, width, infoPtr->height, TRUE);
> - MoveWindow (infoPtr->Self, x, y, width, infoPtr->height, FALSE);
> - if(y != infoPtr->y)
> -
>    InvalidateRect(infoPtr->Self, NULL, FALSE);
> - else
> - {
> -
>    update_rect.left = min(infoPtr->width-HORZ_GAP, width);
> -
>    update_rect.right = max(width, infoPtr->width-HORZ_GAP);
> -
>    update_rect.top = infoPtr->height;
> -
>    update_rect.bottom = 0;
> -
>    InvalidateRect(infoPtr->Self, &update_rect, FALSE);

I think you need to look into why MoveWindow(infoPtr->Self, x, y, width, infoPtr->height, TRUE); doesn't actually repaint the window. I think it's the missing SWP_NOCOPYBITS for NtUserSetWindowPos() in NtUserMoveWindow(). But you need to test and see if that's actually the case. First, you test whether NtUserMoveWindow() calls NtUserSetWindowPos() with SWP_NOCOPYBITS by adding some message tests, e.g., is there a SWP_NOCOPYBITS in WM_WINDOWPOSCHANGING or not. You can search SWP_NOCOPYBITS in user32/tests/msg.c for examples. Depending on the result, if MoveWindow() needs to add SWP_NOCOPYBITS, then you fix MoveWindow(). If not, then adding an InvalidateRect(infoPtr->Self, NULL, FALSE); should be enough. A similar bug is https://bugs.winehq.org/show_bug.cgi?id=52515.

[PATCH 0/1] MR3667: comctl32: Include HORZ_GAP in update region and invalidate if movement is vertical.

[Jacob Czekalla (＠Maaka00524)]

On Tue Aug 29 03:55:11 2023 +0000, Zhiyi Zhang wrote:
> Do you mean `vertical` in the commit subject?

Yes.

[PATCH 0/1] MR3667: comctl32: Include HORZ_GAP in update region and invalidate if movement is vertical.

[Zhiyi Zhang (＠zhiyi)]

On Tue Aug 29 03:55:17 2023 +0000, Zhiyi Zhang wrote:
> I think you need to look into why MoveWindow(infoPtr->Self, x, y, width,
> infoPtr->height, TRUE); doesn't actually repaint the window. I think
> it's the missing SWP_NOCOPYBITS for NtUserSetWindowPos() in
> NtUserMoveWindow(). But you need to test and see if that's actually the
> case. First, you test whether NtUserMoveWindow() calls
> NtUserSetWindowPos() with SWP_NOCOPYBITS by adding some message tests,
> e.g., is there a SWP_NOCOPYBITS in WM_WINDOWPOSCHANGING or not. You can
> search SWP_NOCOPYBITS in user32/tests/msg.c for examples. Depending on
> the result, if MoveWindow() needs to add SWP_NOCOPYBITS, then you fix
> MoveWindow(). If not, then adding an InvalidateRect(infoPtr->Self, NULL,
> FALSE); should be enough. A similar bug is https://bugs.winehq.org/show_bug.cgi?id=52515.

I also wonder if SWP_NOCOPYBITS is automatically added in certain cases. You can test moving the window to a new position, shrinking or expanding the size, and see what happens.

[PATCH 0/1] MR3667: comctl32: Include HORZ_GAP in update region and invalidate if movement is vertical.

[Jacob Czekalla (＠Maaka00524)]

On Tue Aug 29 04:11:26 2023 +0000, Zhiyi Zhang wrote:
> I also wonder if SWP_NOCOPYBITS is automatically added in certain cases.
> You can test moving the window to a new position, shrinking or expanding
> the size, and see what happens.

I believe the issue is in comctl32 and not in MoveWindow() due to the fact that wine notepad has the same issue on windows if wine's comctl32 is brought over with it and used instead of windows comctl32 (Built notepad using an external comctl32 dll). I will look into MoveWindow(). Thank you.

[PATCH 0/1] MR3667: comctl32: Include HORZ_GAP in update region and invalidate if movement is vertical.

[Nikolay Sivov (＠nsivov)]

On Tue Aug 29 05:51:47 2023 +0000, Jacob Czekalla wrote:
> I believed the issue was in comctl32 and not in MoveWindow() due to the
> fact that wine notepad has the same issue on windows if wine's comctl32
> is brought over with it and used instead of windows comctl32 (Built
> notepad using an external comctl32 dll). I will look into MoveWindow().
> Thank you.

Another thing to try is to block WM_SIZE, and see how that works on Windows. We had problems with such caching before.