[PATCH resend 1/6] comctl32/listbox: Handle Mouse Wheel scrolling for multi-column listboxes properly
Multi-column listboxes scroll horizontally, so each wheel tick must go an entire page at a time instead of an item at a time. But we have to limit the amount of scrolling in this case to avoid "jumping over" columns, if the window is too small. This matches Windows behavior.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=22253 Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
These notes apply to all the patches in the series. Some casts have been removed by making variables signed (with an unsigned tmp since SystemParametersInfoW expects UINT), as suggested.
The calculation has been simplified to just integer arithmetic in all cases, since the division (the only operation with a fractional result) was immediately truncated to integer anyway and didn't help with overflow either, as initially assumed.
As you can see, it gets assigned to cLineScroll (which is integer) and then gets multiplied as an *integer* back by WHEEL_DELTA, so if the float were to help with overflow before the division, it would overflow on the next line anyway since you apply the multiply back (but as an integer)...
Anyway, overflow is extremely unlikely (and probably impossible in practice) since WHEEL_DELTA is just 120, and it was already vulnerable to overflow currently.
dlls/comctl32/listbox.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index bd9cffd..2f4d4fe 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -2068,9 +2068,11 @@ static LRESULT LISTBOX_HandleHScroll( LB_DESCR *descr, WORD scrollReq, WORD pos
static LRESULT LISTBOX_HandleMouseWheel(LB_DESCR *descr, SHORT delta ) { - UINT pulScrollLines = 3; + INT pulScrollLines; + UINT tmp = 3;
- SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &pulScrollLines, 0); + SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &tmp, 0); + pulScrollLines = tmp;
/* if scrolling changes direction, ignore left overs */ if ((delta < 0 && descr->wheel_remain < 0) || @@ -2081,10 +2083,21 @@ static LRESULT LISTBOX_HandleMouseWheel(LB_DESCR *descr, SHORT delta )
if (descr->wheel_remain && pulScrollLines) { - int cLineScroll; - pulScrollLines = min((UINT) descr->page_size, pulScrollLines); - cLineScroll = pulScrollLines * (float)descr->wheel_remain / WHEEL_DELTA; - descr->wheel_remain -= WHEEL_DELTA * cLineScroll / (int)pulScrollLines; + INT cLineScroll; + if (descr->style & LBS_MULTICOLUMN) + { + pulScrollLines = min((UINT)descr->width / descr->column_width, pulScrollLines); + pulScrollLines = max(1U, pulScrollLines); + cLineScroll = (pulScrollLines * descr->wheel_remain) / WHEEL_DELTA; + descr->wheel_remain -= (cLineScroll * WHEEL_DELTA) / pulScrollLines; + cLineScroll *= descr->page_size; + } + else + { + pulScrollLines = min((UINT)descr->page_size, pulScrollLines); + cLineScroll = (pulScrollLines * descr->wheel_remain) / WHEEL_DELTA; + descr->wheel_remain -= (cLineScroll * WHEEL_DELTA) / pulScrollLines; + } LISTBOX_SetTopItem( descr, descr->top_item - cLineScroll, TRUE ); } return 0;
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=22253 Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/user32/listbox.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c index 95621b8..663f104 100644 --- a/dlls/user32/listbox.c +++ b/dlls/user32/listbox.c @@ -2082,9 +2082,11 @@ static LRESULT LISTBOX_HandleHScroll( LB_DESCR *descr, WORD scrollReq, WORD pos
static LRESULT LISTBOX_HandleMouseWheel(LB_DESCR *descr, SHORT delta ) { - UINT pulScrollLines = 3; + INT pulScrollLines; + UINT tmp = 3;
- SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &pulScrollLines, 0); + SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &tmp, 0); + pulScrollLines = tmp;
/* if scrolling changes direction, ignore left overs */ if ((delta < 0 && descr->wheel_remain < 0) || @@ -2095,10 +2097,21 @@ static LRESULT LISTBOX_HandleMouseWheel(LB_DESCR *descr, SHORT delta )
if (descr->wheel_remain && pulScrollLines) { - int cLineScroll; - pulScrollLines = min((UINT) descr->page_size, pulScrollLines); - cLineScroll = pulScrollLines * (float)descr->wheel_remain / WHEEL_DELTA; - descr->wheel_remain -= WHEEL_DELTA * cLineScroll / (int)pulScrollLines; + INT cLineScroll; + if (descr->style & LBS_MULTICOLUMN) + { + pulScrollLines = min((UINT)descr->width / descr->column_width, pulScrollLines); + pulScrollLines = max(1U, pulScrollLines); + cLineScroll = (pulScrollLines * descr->wheel_remain) / WHEEL_DELTA; + descr->wheel_remain -= (cLineScroll * WHEEL_DELTA) / pulScrollLines; + cLineScroll *= descr->page_size; + } + else + { + pulScrollLines = min((UINT)descr->page_size, pulScrollLines); + cLineScroll = (pulScrollLines * descr->wheel_remain) / WHEEL_DELTA; + descr->wheel_remain -= (cLineScroll * WHEEL_DELTA) / pulScrollLines; + } LISTBOX_SetTopItem( descr, descr->top_item - cLineScroll, TRUE ); } return 0;
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=48862
Your paranoid android.
=== debian9 (32 bit report) ===
user32: msg.c:8713: Test failed: WaitForSingleObject failed 102 msg.c:8719: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead msg.c:8719: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead msg.c:8719: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/edit.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/dlls/comctl32/edit.c b/dlls/comctl32/edit.c index 1f17276..c724bc6 100644 --- a/dlls/comctl32/edit.c +++ b/dlls/comctl32/edit.c @@ -4975,9 +4975,10 @@ static LRESULT CALLBACK EDIT_WindowProc(HWND hwnd, UINT msg, WPARAM wParam, LPAR
case WM_MOUSEWHEEL: { - int wheelDelta; - UINT pulScrollLines = 3; - SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &pulScrollLines, 0); + INT wheelDelta, pulScrollLines; + UINT tmp = 3; + SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &tmp, 0); + pulScrollLines = tmp;
if (wParam & (MK_SHIFT | MK_CONTROL)) { @@ -4995,10 +4996,10 @@ static LRESULT CALLBACK EDIT_WindowProc(HWND hwnd, UINT msg, WPARAM wParam, LPAR
if (es->wheelDeltaRemainder && pulScrollLines) { - int cLineScroll; - pulScrollLines = (int) min((UINT) es->line_count, pulScrollLines); - cLineScroll = pulScrollLines * (float)es->wheelDeltaRemainder / WHEEL_DELTA; - es->wheelDeltaRemainder -= WHEEL_DELTA * cLineScroll / (int)pulScrollLines; + INT cLineScroll; + pulScrollLines = min((UINT)es->line_count, pulScrollLines); + cLineScroll = (pulScrollLines * es->wheelDeltaRemainder) / WHEEL_DELTA; + es->wheelDeltaRemainder -= (cLineScroll * WHEEL_DELTA) / pulScrollLines; result = EDIT_EM_LineScroll(es, 0, -cLineScroll); } break;
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=48863
Your paranoid android.
=== debian9 (32 bit Chinese:China report) ===
user32: msg.c:14491: Test failed: bad time be5804d
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/user32/edit.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/dlls/user32/edit.c b/dlls/user32/edit.c index 6f8f412..93ec202 100644 --- a/dlls/user32/edit.c +++ b/dlls/user32/edit.c @@ -5102,9 +5102,10 @@ LRESULT EditWndProc_common( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam, B
case WM_MOUSEWHEEL: { - int wheelDelta; - UINT pulScrollLines = 3; - SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &pulScrollLines, 0); + INT wheelDelta, pulScrollLines; + UINT tmp = 3; + SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &tmp, 0); + pulScrollLines = tmp;
if (wParam & (MK_SHIFT | MK_CONTROL)) { result = DefWindowProcW(hwnd, msg, wParam, lParam); @@ -5119,10 +5120,10 @@ LRESULT EditWndProc_common( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam, B es->wheelDeltaRemainder = wheelDelta; if (es->wheelDeltaRemainder && pulScrollLines) { - int cLineScroll; - pulScrollLines = (int) min((UINT) es->line_count, pulScrollLines); - cLineScroll = pulScrollLines * (float)es->wheelDeltaRemainder / WHEEL_DELTA; - es->wheelDeltaRemainder -= WHEEL_DELTA * cLineScroll / (int)pulScrollLines; + INT cLineScroll; + pulScrollLines = min((UINT)es->line_count, pulScrollLines); + cLineScroll = (pulScrollLines * es->wheelDeltaRemainder) / WHEEL_DELTA; + es->wheelDeltaRemainder -= (cLineScroll * WHEEL_DELTA) / pulScrollLines; result = EDIT_EM_LineScroll(es, 0, -cLineScroll); } }
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/listview.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 7fa6996..f6a573f 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -9913,7 +9913,8 @@ static LRESULT LISTVIEW_HScroll(LISTVIEW_INFO *infoPtr, INT nScrollCode,
static LRESULT LISTVIEW_MouseWheel(LISTVIEW_INFO *infoPtr, INT wheelDelta) { - UINT pulScrollLines = 3; + INT pulScrollLines; + UINT tmp;
TRACE("(wheelDelta=%d)\n", wheelDelta);
@@ -9930,7 +9931,9 @@ static LRESULT LISTVIEW_MouseWheel(LISTVIEW_INFO *infoPtr, INT wheelDelta) break;
case LV_VIEW_DETAILS: - SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &pulScrollLines, 0); + tmp = 3; + SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &tmp, 0); + pulScrollLines = tmp;
/* if scrolling changes direction, ignore left overs */ if ((wheelDelta < 0 && infoPtr->cWheelRemainder < 0) || @@ -9940,10 +9943,10 @@ static LRESULT LISTVIEW_MouseWheel(LISTVIEW_INFO *infoPtr, INT wheelDelta) infoPtr->cWheelRemainder = wheelDelta; if (infoPtr->cWheelRemainder && pulScrollLines) { - int cLineScroll; + INT cLineScroll; pulScrollLines = min((UINT)LISTVIEW_GetCountPerColumn(infoPtr), pulScrollLines); - cLineScroll = pulScrollLines * (float)infoPtr->cWheelRemainder / WHEEL_DELTA; - infoPtr->cWheelRemainder -= WHEEL_DELTA * cLineScroll / (int)pulScrollLines; + cLineScroll = (pulScrollLines * infoPtr->cWheelRemainder) / WHEEL_DELTA; + infoPtr->cWheelRemainder -= (cLineScroll * WHEEL_DELTA) / pulScrollLines; LISTVIEW_VScroll(infoPtr, SB_INTERNAL, -cLineScroll); } break;
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=48865
Your paranoid android.
=== debian9 (32 bit report) ===
user32: msg.c:8713: Test failed: WaitForSingleObject failed 102 msg.c:8719: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead msg.c:8719: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead msg.c:8719: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/treeview.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/dlls/comctl32/treeview.c b/dlls/comctl32/treeview.c index bbef651..0f10292 100644 --- a/dlls/comctl32/treeview.c +++ b/dlls/comctl32/treeview.c @@ -5030,8 +5030,9 @@ scroll: static LRESULT TREEVIEW_MouseWheel(TREEVIEW_INFO *infoPtr, WPARAM wParam, LPARAM lParam) { - short wheelDelta; - UINT pulScrollLines = 3; + SHORT wheelDelta; + INT pulScrollLines; + UINT tmp = 3;
if (wParam & (MK_SHIFT | MK_CONTROL)) return DefWindowProcW(infoPtr->hwnd, WM_MOUSEWHEEL, wParam, lParam); @@ -5039,7 +5040,8 @@ TREEVIEW_MouseWheel(TREEVIEW_INFO *infoPtr, WPARAM wParam, LPARAM lParam) if (infoPtr->firstVisible == NULL) return TRUE;
- SystemParametersInfoW(SPI_GETWHEELSCROLLLINES, 0, &pulScrollLines, 0); + SystemParametersInfoW(SPI_GETWHEELSCROLLLINES, 0, &tmp, 0); + pulScrollLines = tmp;
wheelDelta = GET_WHEEL_DELTA_WPARAM(wParam); /* if scrolling changes direction, ignore left overs */ @@ -5051,12 +5053,12 @@ TREEVIEW_MouseWheel(TREEVIEW_INFO *infoPtr, WPARAM wParam, LPARAM lParam)
if (infoPtr->wheelRemainder && pulScrollLines) { - int newDy; - int maxDy; - int lineScroll; + INT newDy; + INT maxDy; + INT lineScroll;
- lineScroll = pulScrollLines * (float)infoPtr->wheelRemainder / WHEEL_DELTA; - infoPtr->wheelRemainder -= WHEEL_DELTA * lineScroll / (int)pulScrollLines; + lineScroll = (pulScrollLines * infoPtr->wheelRemainder) / WHEEL_DELTA; + infoPtr->wheelRemainder -= (lineScroll * WHEEL_DELTA) / pulScrollLines;
newDy = infoPtr->firstVisible->visibleOrder - lineScroll; maxDy = infoPtr->maxVisibleOrder;
-
Gabriel Ivăncescu -
Marvin