[PATCH 0/1] MR3667: comctl32: Include HORZ_GAP in update region and invalidate if movement is verticle.
Fixes a bug where the statusbar doesn't correctly paint when the parent window is resized. This can be seen in wine notepad. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3667
From: Jacob Czekalla <jacobczekalla(a)gmail.com> --- 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; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3667
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/status.c:
HWND Notify; WORD numParts;
Do you mean `vertical` in the commit subject? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3667#note_43638
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3667#note_43639
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3667#note_43640
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3667#note_43641
On Tue Aug 29 03:55:11 2023 +0000, Zhiyi Zhang wrote:
Do you mean `vertical` in the commit subject? Yes.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3667#note_43642
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3667#note_43644
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3667#note_43645
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3667#note_43678
participants (4)
-
Jacob Czekalla -
Jacob Czekalla (＠Maaka00524)
-
Nikolay Sivov (＠nsivov)
-
Zhiyi Zhang (＠zhiyi)