[PATCH v2 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 and didn't help with overflow either, because the line that assigns to descr->wheel_remain operates on integers, so if it overflowed in the initial float operation, it would overflow on this line anyway.
In other words, the current code didn't protect from overflow, so we can simply get rid of the float cast (WHEEL_DELTA is too small for overflow to be a concern, though).
dlls/comctl32/listbox.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 8fd556c..991baac 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -2068,7 +2068,7 @@ 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 = 3;
SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &pulScrollLines, 0);
@@ -2082,9 +2082,20 @@ 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; + 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 -= (WHEEL_DELTA * cLineScroll) / pulScrollLines; + cLineScroll *= descr->page_size; + } + else + { + pulScrollLines = min((UINT)descr->page_size, pulScrollLines); + cLineScroll = (pulScrollLines * descr->wheel_remain) / WHEEL_DELTA; + descr->wheel_remain -= (WHEEL_DELTA * cLineScroll) / 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 | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c index 52f01a7..8b8b1a6 100644 --- a/dlls/user32/listbox.c +++ b/dlls/user32/listbox.c @@ -2083,7 +2083,7 @@ 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 = 3;
SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &pulScrollLines, 0);
@@ -2097,9 +2097,20 @@ 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; + 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 -= (WHEEL_DELTA * cLineScroll) / pulScrollLines; + cLineScroll *= descr->page_size; + } + else + { + pulScrollLines = min((UINT)descr->page_size, pulScrollLines); + cLineScroll = (pulScrollLines * descr->wheel_remain) / WHEEL_DELTA; + descr->wheel_remain -= (WHEEL_DELTA * cLineScroll) / pulScrollLines; + } LISTBOX_SetTopItem( descr, descr->top_item - cLineScroll, TRUE ); } return 0;
Hi,
While running your changed tests, 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=53318
Your paranoid android.
=== debian9 (64 bit WoW report) ===
user32: clipboard.c:1179: Test failed: OpenClipboard failed: 5 clipboard.c:1181: Test failed: sequence diff 0 clipboard.c:1185: Test failed: CloseClipboard failed: 1418 clipboard.c:1017: Test failed: wait failed clipboard.c:1188: Test failed: WM_DRAWCLIPBOARD not received clipboard.c:1188: Test failed: WM_CLIPBOARDUPDATE not received 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 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/dlls/comctl32/edit.c b/dlls/comctl32/edit.c index e64e260..0b07575 100644 --- a/dlls/comctl32/edit.c +++ b/dlls/comctl32/edit.c @@ -5017,7 +5017,7 @@ static LRESULT CALLBACK EDIT_WindowProc(HWND hwnd, UINT msg, WPARAM wParam, LPAR case WM_MOUSEWHEEL: { int wheelDelta; - UINT pulScrollLines = 3; + INT pulScrollLines = 3; SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &pulScrollLines, 0);
if (wParam & (MK_SHIFT | MK_CONTROL)) @@ -5037,9 +5037,9 @@ 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; + pulScrollLines = min((UINT) es->line_count, pulScrollLines); + cLineScroll = (pulScrollLines * es->wheelDeltaRemainder) / WHEEL_DELTA; + es->wheelDeltaRemainder -= (WHEEL_DELTA * cLineScroll) / pulScrollLines; result = EDIT_EM_LineScroll(es, 0, -cLineScroll); } break;
Hi,
While running your changed tests, 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=53319
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
Hi,
All the test failures for this patch series seem to have nothing to do with it, but with some race condition (?) making WaitForSingleObject fail. So, pre-existing / random failures.
On 6/5/19 3:08 PM, Marvin wrote:
Hi,
While running your changed tests, 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=53319
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/user32/edit.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/dlls/user32/edit.c b/dlls/user32/edit.c index 3c5331f..7955cb5 100644 --- a/dlls/user32/edit.c +++ b/dlls/user32/edit.c @@ -5158,7 +5158,7 @@ LRESULT EditWndProc_common( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam, B case WM_MOUSEWHEEL: { int wheelDelta; - UINT pulScrollLines = 3; + INT pulScrollLines = 3; SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &pulScrollLines, 0);
if (wParam & (MK_SHIFT | MK_CONTROL)) { @@ -5175,9 +5175,9 @@ LRESULT EditWndProc_common( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam, B 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; + pulScrollLines = min((UINT) es->line_count, pulScrollLines); + cLineScroll = (pulScrollLines * es->wheelDeltaRemainder) / WHEEL_DELTA; + es->wheelDeltaRemainder -= (WHEEL_DELTA * cLineScroll) / pulScrollLines; result = EDIT_EM_LineScroll(es, 0, -cLineScroll); } }
Hi,
While running your changed tests, 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=53320
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
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/listview.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 4ecff20..213d55a 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -9909,7 +9909,7 @@ static LRESULT LISTVIEW_HScroll(LISTVIEW_INFO *infoPtr, INT nScrollCode,
static LRESULT LISTVIEW_MouseWheel(LISTVIEW_INFO *infoPtr, INT wheelDelta) { - UINT pulScrollLines = 3; + INT pulScrollLines = 3;
TRACE("(wheelDelta=%d)\n", wheelDelta);
@@ -9938,8 +9938,8 @@ static LRESULT LISTVIEW_MouseWheel(LISTVIEW_INFO *infoPtr, INT wheelDelta) { 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 -= (WHEEL_DELTA * cLineScroll) / pulScrollLines; LISTVIEW_VScroll(infoPtr, SB_INTERNAL, -cLineScroll); } break;
Hi,
While running your changed tests, 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=53321
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/treeview.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/dlls/comctl32/treeview.c b/dlls/comctl32/treeview.c index 6854b0c..cbb69bb 100644 --- a/dlls/comctl32/treeview.c +++ b/dlls/comctl32/treeview.c @@ -5027,7 +5027,7 @@ static LRESULT TREEVIEW_MouseWheel(TREEVIEW_INFO *infoPtr, WPARAM wParam, LPARAM lParam) { short wheelDelta; - UINT pulScrollLines = 3; + INT pulScrollLines = 3;
if (wParam & (MK_SHIFT | MK_CONTROL)) return DefWindowProcW(infoPtr->hwnd, WM_MOUSEWHEEL, wParam, lParam); @@ -5051,8 +5051,8 @@ TREEVIEW_MouseWheel(TREEVIEW_INFO *infoPtr, WPARAM wParam, LPARAM lParam) 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 -= (WHEEL_DELTA * lineScroll) / pulScrollLines;
newDy = infoPtr->firstVisible->visibleOrder - lineScroll; maxDy = infoPtr->maxVisibleOrder;
-
Gabriel Ivăncescu -
Marvin