On 08/22/2018 06:49 PM, Ken Thomases wrote:
To be honest, I don't clearly remember why I put the float cast in there. I think perhaps to avoid potential (if unlikely) integer overflow. We want something like MulDiv() but always rounding toward zero. Perhaps just casting to INT64 would be better.
I like MulDiv better. Gabriel, do you think it's possible to get rid of some casts by changing variable types?
-Ken
On Aug 22, 2018, at 7:02 AM, Gabriel Ivăncescu gabrielopcode@gmail.com wrote:
Well the float "appears" to fix a bug, because pulScrollLines is unsigned, so an INT cast is needed, otherwise the promotion rules will do an unsigned multiplication. Float takes precedence so it converted it to float and then it worked.
But IMO it's not the correct way to fix this at all, just cast pulScrollLines to INT, as it's done on the next line as well, and now the multiplication will be signed and it works fine (tested it rigorously!)
On Wed, Aug 22, 2018 at 11:32 AM, Nikolay Sivov nsivov@codeweavers.com wrote:
On 08/20/2018 09:53 PM, Gabriel Ivăncescu wrote:
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.
The calculation has also 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, so the float cast was useless. This works fine because the multiplication is done before the division (parentheses have been added to emphasize this point).
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 991ab87..7256095 100644 --- a/dlls/user32/listbox.c +++ b/dlls/user32/listbox.c @@ -2007,10 +2007,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(1, pulScrollLines);cLineScroll = ((INT)pulScrollLines * descr->wheel_remain) /WHEEL_DELTA;
descr->wheel_remain -= (cLineScroll * WHEEL_DELTA) /(INT)pulScrollLines;
cLineScroll *= descr->page_size;}else{pulScrollLines = min((UINT) descr->page_size,pulScrollLines);
cLineScroll = ((INT)pulScrollLines * descr->wheel_remain) /WHEEL_DELTA;
descr->wheel_remain -= (cLineScroll * WHEEL_DELTA) /(INT)pulScrollLines;
} LISTBOX_SetTopItem( descr, descr->top_item - cLineScroll, TRUE); } return 0;
Float cast was a deliberate addition it seems, and relatively recent one, see f42cfc04eb05fa266cfae0c64452686a68c24151, CC'ing Ken.