[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.
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, it would overflow on the next line anyway... 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 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;
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=45596
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
=== debian9 (32 bit Chinese:China 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;
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 48c65ea..af2084b 100644 --- a/dlls/user32/edit.c +++ b/dlls/user32/edit.c @@ -5098,9 +5098,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); @@ -5115,10 +5116,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); } }
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=45598
Your paranoid android.
=== debian9 (32 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
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 1230c55..94d47f6 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=45599
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
=== debian9 (32 bit Chinese:China 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