[PATCH 0/2] MR8320: comctl32/listview: Remove header rect caching when drawing LVS_REPORT listviews.
Tests how the rects for the listviews header should be refreshed during painting. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8320
From: Jacob Czekalla <jczekalla(a)codeweavers.com> Tests how the rects for the listviews header should be refreshed during painting. --- dlls/comctl32/tests/listview.c | 66 ++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c index b767106e97a..8487394b1e8 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -608,6 +608,24 @@ static const struct message listview_end_label_edit_kill_focus[] = { { 0 } }; +static const struct message listview_invalidate_rect[] = { + { WM_PAINT, sent }, + { WM_PAINT, sent }, + { WM_NCPAINT, sent|defwinproc }, + { WM_ERASEBKGND, sent|defwinproc }, + { WM_NOTIFY, sent|defwinproc }, + { HDM_ORDERTOINDEX, sent|wparam, 0 }, + { HDM_GETITEMW, sent|wparam|optional, 0 }, /* todo: not sent by wine yet */ + { HDM_GETITEMRECT, sent|wparam, 0 }, + { HDM_ORDERTOINDEX, sent|wparam, 1 }, + { HDM_GETITEMW, sent|wparam|optional, 1 }, /* todo: not sent by wine yet */ + { HDM_GETITEMRECT, sent|wparam, 1 }, + { HDM_ORDERTOINDEX, sent|wparam, 2 }, + { HDM_GETITEMW, sent|wparam|optional, 2 }, /* todo: not sent by wine yet */ + { HDM_GETITEMRECT, sent|wparam, 2 }, + { 0 } +}; + static void hold_key(int vk) { BYTE kstate[256]; @@ -7489,6 +7507,53 @@ static void test_LVM_GETHOTCURSOR(void) DestroyWindow(hwnd); } +static void test_column_rect_caching(void) +{ + HWND listview; + LVCOLUMNA column = { 0 }; + LVITEMA item = { 0 }; + char a_text[] = "A", b_text[] = "B", c_text[] = "C"; + + listview = create_listview_control(LVS_REPORT); + subclass_header(listview); + + column.mask = LVCF_TEXT | LVCF_WIDTH | LVCF_SUBITEM; + column.cx = 50; + column.pszText = a_text; + column.iSubItem = 0; + SendMessageA(listview, LVM_INSERTCOLUMNA, 0, (LPARAM)&column); + column.cx = 50; + column.pszText = b_text; + column.iSubItem = 1; + SendMessageA(listview, LVM_INSERTCOLUMNA, 1, (LPARAM)&column); + column.cx = 50; + column.pszText = c_text; + column.iSubItem = 2; + SendMessageA(listview, LVM_INSERTCOLUMNA, 2, (LPARAM)&column); + + item.mask = LVIF_TEXT; + item.iItem = 0; + item.pszText = a_text; + SendMessageA(listview, LVM_INSERTITEMA, 0, (LPARAM)&item); + item.iSubItem = 1; + item.iItem = 0; + item.pszText = b_text; + SendMessageA(listview, LVM_SETITEMTEXTA, 0, (LPARAM)&item); + item.iSubItem = 2; + item.iItem = 0; + item.pszText = c_text; + SendMessageA(listview, LVM_SETITEMTEXTA, 0, (LPARAM)&item); + + flush_events(); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + InvalidateRect(listview, NULL, FALSE); + flush_events(); + ok_sequence(sequences, LISTVIEW_SEQ_INDEX, listview_invalidate_rect, "column order caching in refresh", TRUE); + + DestroyWindow(listview); +} + START_TEST(listview) { ULONG_PTR ctx_cookie; @@ -7559,6 +7624,7 @@ START_TEST(listview) test_custom_sort(); test_LVM_GETNEXTITEM(); test_LVM_GETHOTCURSOR(); + test_column_rect_caching(); if (!load_v6_module(&ctx_cookie, &hCtx)) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8320
From: Jacob Czekalla <jczekalla(a)codeweavers.com> During painting for an LVS_REPORT listview the cached header rects should be refreshed. Listview columns can be reorderd through the header which was not accounted for. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57594 --- dlls/comctl32/listview.c | 6 ++++-- dlls/comctl32/tests/listview.c | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 31f8a933e56..86c60a2b2ae 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -5015,10 +5015,12 @@ static void LISTVIEW_RefreshReport(LISTVIEW_INFO *infoPtr, ITERATOR *i, HDC hdc, /* narrow down the columns we need to paint */ for(col = 0; col < DPA_GetPtrCount(infoPtr->hdpaColumns); col++) { + COLUMN_INFO* col_info; INT index = SendMessageW(infoPtr->hwndHeader, HDM_ORDERTOINDEX, col, 0); - LISTVIEW_GetHeaderRect(infoPtr, index, &rcItem); - if ((rcItem.right + Origin.x >= rcClip.left) && (rcItem.left + Origin.x < rcClip.right)) + col_info = DPA_GetPtr(infoPtr->hdpaColumns, index); + SendMessageW(infoPtr->hwndHeader, HDM_GETITEMRECT, index, (LPARAM)&col_info->rcHeader); + if ((col_info->rcHeader.right + Origin.x >= rcClip.left) && (col_info->rcHeader.left + Origin.x < rcClip.right)) ranges_additem(colRanges, index); } iterator_rangesitems(&j, colRanges); diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c index 8487394b1e8..4c08838ffd8 100644 --- a/dlls/comctl32/tests/listview.c +++ b/dlls/comctl32/tests/listview.c @@ -7549,7 +7549,7 @@ static void test_column_rect_caching(void) InvalidateRect(listview, NULL, FALSE); flush_events(); - ok_sequence(sequences, LISTVIEW_SEQ_INDEX, listview_invalidate_rect, "column order caching in refresh", TRUE); + ok_sequence(sequences, LISTVIEW_SEQ_INDEX, listview_invalidate_rect, "column order caching in refresh", FALSE); DestroyWindow(listview); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8320
Nikolay Sivov (@nsivov) commented about dlls/comctl32/listview.c:
/* narrow down the columns we need to paint */ for(col = 0; col < DPA_GetPtrCount(infoPtr->hdpaColumns); col++) { + COLUMN_INFO* col_info; INT index = SendMessageW(infoPtr->hwndHeader, HDM_ORDERTOINDEX, col, 0);
- LISTVIEW_GetHeaderRect(infoPtr, index, &rcItem); - if ((rcItem.right + Origin.x >= rcClip.left) && (rcItem.left + Origin.x < rcClip.right)) + col_info = DPA_GetPtr(infoPtr->hdpaColumns, index); + SendMessageW(infoPtr->hwndHeader, HDM_GETITEMRECT, index, (LPARAM)&col_info->rcHeader); + if ((col_info->rcHeader.right + Origin.x >= rcClip.left) && (col_info->rcHeader.left + Origin.x < rcClip.right)) ranges_additem(colRanges, index); }
GetHeaderRect() is used in many places, changing one of them will likely cause inconsistencies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8320#note_106608
Nikolay Sivov (@nsivov) commented about dlls/comctl32/tests/listview.c:
+ item.iSubItem = 1; + item.iItem = 0; + item.pszText = b_text; + SendMessageA(listview, LVM_SETITEMTEXTA, 0, (LPARAM)&item); + item.iSubItem = 2; + item.iItem = 0; + item.pszText = c_text; + SendMessageA(listview, LVM_SETITEMTEXTA, 0, (LPARAM)&item); + + flush_events(); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + InvalidateRect(listview, NULL, FALSE); + flush_events(); + ok_sequence(sequences, LISTVIEW_SEQ_INDEX, listview_invalidate_rect, "column order caching in refresh", FALSE); + Does this message sequence work with comctl32 v6? If it doesn't I would remove it. What we need to test is that HDM_SETORDERARRAY without LVM_SETCOLUMNORDERARRAY works as expected.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8320#note_106609
On Sun Jun 15 01:02:45 2025 +0000, Nikolay Sivov wrote:
GetHeaderRect() is used in many places, changing one of them will likely cause inconsistencies. What I mean here is could we change GetHeaderRect() to always use HDM_GETITEMRECT, and get rid of cached rcHeader complete, in one go? I haven't tried myself, so I don't know if any issues might come up.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8320#note_107020
On Wed Jun 18 11:48:57 2025 +0000, Nikolay Sivov wrote:
What I mean here is could we change GetHeaderRect() to always use HDM_GETITEMRECT, and get rid of cached rcHeader complete, in one go? I haven't tried myself, so I don't know if any issues might come up. It seems the caching is still needed. Many tests will fail if the caching is gone.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8320#note_107231
participants (3)
-
Jacob Czekalla -
Jacob Czekalla (＠JacobCzekalla)
-
Nikolay Sivov (＠nsivov)