[PATCH 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.
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. Temporary variable has been used to get rid of many casts, 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.
Note that the float doesn't help with overflow either. As you can see, it gets assigned to cLineScroll (which is integer) and then gets multiplied back by WHEEL_DELTA, so if the float were to help with overflow in the first place, it would overflow on the next line regardless.
Anyway, overflow is probably impossible in practice since WHEEL_DELTA is just 120, but even if so, it was already overflowing with float, so there's no reason to have that cast...
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 cb645b4..9f56a2f 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -2012,9 +2012,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) || @@ -2025,10 +2027,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 594c956..26d5410 100644 --- a/dlls/user32/listbox.c +++ b/dlls/user32/listbox.c @@ -2025,9 +2025,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) || @@ -2038,10 +2040,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;
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;
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 | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 1230c55..8c04106 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 = 3;
TRACE("(wheelDelta=%d)\n", wheelDelta);
@@ -9930,7 +9931,8 @@ static LRESULT LISTVIEW_MouseWheel(LISTVIEW_INFO *infoPtr, INT wheelDelta) break;
case LV_VIEW_DETAILS: - SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &pulScrollLines, 0); + SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &tmp, 0); + pulScrollLines = tmp;
/* if scrolling changes direction, ignore left overs */ if ((wheelDelta < 0 && infoPtr->cWheelRemainder < 0) || @@ -9940,10 +9942,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;
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;
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=47115
Your paranoid android.
=== debian9 (64 bit WoW 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
-
Gabriel Ivăncescu -
Marvin