Bug 184063 - Logitech Revolution mouse doesn't scroll down editor when in "free-spinning" mode
Summary: Logitech Revolution mouse doesn't scroll down editor when in "free-spinning" ...
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 3.2.1   Edit
Hardware: PC Windows XP
: P3 minor (vote)
Target Milestone: 3.4 M5   Edit
Assignee: Felipe Heidrich CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-04-25 13:28 EDT by glymor CLA
Modified: 2020-07-22 08:27 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description glymor CLA 2007-04-25 13:28:55 EDT
Build ID: M20060921-0945

Steps To Reproduce:
1. Spend too much on a mouse.
2. Turn scroll acceleration to minimum (or this will somewhat mask the effect)
3. Scrolling down on free-spinning mode only occurs when spun at high speed, where as scrolling up works normally.
4. Spinning very fast in either direction causes strangeness and generally odd behavour with free-spinning scrolling all round.

More information:
Logitech's new mouse has a scrolling mode where the wheel spins smoothly. This works immediately in most applications but eclipse demonstrates odd behaviour when using the mode.
Comment 1 Steve Northover CLA 2007-05-01 11:58:56 EDT
> 1. Spend too much on a mouse.

Hilarious.
Comment 2 Steve Northover CLA 2007-05-01 12:00:44 EDT
Ok, I don't have the hardware.  You and I are going to have to debug this remotely.  

1) Does the scrolling go insane in a tree or table control or only in the Java editor?
2) Does scrolling work in other applications?  Explorer, Notepad, Wordpad ...
Comment 3 glymor CLA 2007-05-01 13:11:54 EDT
Step 3 from the original bug report, scrolling down not working, has stopped happening. It may have been a temporary thing. 

The problem in step 4 occurs only when flicking the wheel hard and you also have to have acceleration set high (the default) or the speed set high. It scrolls for a bit then reverses direction and repeats this a few times. A bit like I would imagine it would do if there was a numeric overflow. Opera has the same problem but most other applications don't exhibit the behaviour.

However, not scrolling down was the main problem; since it's no longer occurring I'm fine on my end for the bug to be closed.
Comment 4 glymor CLA 2007-05-01 13:15:19 EDT
Also, sorry, the problem in step 4 doesn't occur in the tree view or list views.
Comment 5 Vadim Dmitriev CLA 2007-05-16 16:33:18 EDT
Have the same nasty "insane scroll" thing. It happens in some other applications (i.e. adobe reader), but not as constantly, as in eclipse. Only in editor window, regardless of actual editor used (java, xml, class, etc.).
P.S.
Since this thing shows itself even with logitech's software unloaded, i think it can be reproduced by issuing wm_mousewheel messages at high rate (50-80/second maybe).
Comment 6 Steve Northover CLA 2007-05-16 19:28:49 EDT
Get SWT from HEAD (http://www.eclipse.org/swt/cvs.php) and investigate/hack Scrollable.WM_MOUSEWHEEL().  This method sends fake WM_VSCROLL and WM_SCROLL messages to custom controls like the StyledText widget that have scroll bars but don't implement mouse wheel so that they will scroll when there is a mouse wheel.  BTW, Non-native controls like StyledText are identified in this method by the CANVAS bit.

Good luck!
Comment 7 Daniel Potter CLA 2007-06-24 14:55:05 EDT
I think I know what causes this. Windows Vista (and, I'm guessing, the Logitech drivers for the reporter's mouse) support sending "fractional" scroll deltas.

Currently SWT expects all delta values to be multiples of 120. This is no longer the case as of Windows Vista and certain mice. (Like the one I use, the Wireless IntelliMouse Explorer 2.0 - yes, they really named it that.) Vista using certain mice will send "fractional" scroll events. Instead of a delta that's a multiple of 120, they'll send deltas that are multiples of 30. (Although I wouldn't count on that remaining the case, either.)

Anyway, I've got a patch to work around this behavior, but I think I'm going to open a new bug and submit it there as I know that's causing problems on my system and have tested the fix. (Thankfully it's a small change in two files, Widget.java and Scrollable.java.)
Comment 8 Steve Northover CLA 2007-06-25 12:33:50 EDT
Vadim and glmor, can you confirm that the patch in 194143 causes the crazy scrolling to go away on XP?  If so, then this bug (or the other) is a dup.
Comment 9 Steve Northover CLA 2007-06-25 13:00:11 EDT
The patch is in bug 194193.
Comment 10 Steve Northover CLA 2007-06-25 13:00:55 EDT
Aghhhhh!  The patch is really in bug 194143.
Comment 11 Felipe Heidrich CLA 2007-06-27 17:31:02 EDT
Daniel: This bug has nothing to do with bug 194143.

I got a Logitech Revolution. The problem with this mouse is that the driver sends crazy number in wParam of WM_MOUSEWHEEL when you spin the wheel to fast.
I'm sure I don't have a number overflow in my code. I'm actually using windows macro to extract the delta from wParam (GET_WHEEL_DELTA_WPARAM).
It is possible the overflow in the driver or in windows.

Accoring to MSDN the delta is "expressed in multiples or divisions of WHEEL_DELTA, which is 120", so I can detect when the delta value is bogus by using:

int delta = OS.GET_WHEEL_DELTA_WPARAM (wParam);
boolean goodDelta = delta % OS.WHEEL_DELTA == 0 || OS.WHEEL_DELTA % delta == 0;
if (!goodDelta) return result;

I'll wait Steve to get back before I close this problem.
Comment 12 Steve Northover CLA 2007-12-10 23:48:52 EST
Felipe, you fix this already?
Comment 13 Felipe Heidrich CLA 2007-12-11 11:57:12 EST
(In reply to comment #12)
> Felipe, you fix this already?

I don't think so. 
This problem is not related to bug 194143 (fractional deltas)
The fix I suggested in comment 11 doesn't work.
I believe this is a bug in the driver.
Comment 14 Felipe Heidrich CLA 2008-01-16 11:33:30 EST
From bug 194143 comment 14 by Francois Valdy:

"I had issues with my free-spin (otherwise called "smooth scroll", which is a
very bad name considering the actual behavior) mouse (logitech MX revolution),
and I suggest you re-consider this fix.

Basically this mousewheel isn't working at all with any of the implementation
(tested 3.3 as well as 3.4M4, which includes this bug fix).

Bug detail:
- scrolling amount is a short (maximum 273 lines = SHORT_MAX/120)
- using free spin, a single scroll event can easily scroll more than 10 lines
- swt Scrollable implementation generates n LINEDOWN messages per scroll event
- eclipse java editor can't consume those events faster than Scrollable is
generating them
- the result being:
 o eclipse uses full CPU when scrolling this mode
 o scrolling ends tens of seconds after you stop the wheel (because of the
loop)
 o because the WM_MOUSEWHEEL isn't consumed fast enough, Win32/Logitech drivers
generates less messages with even bigger values (hence making the situation
even worse)
 o short overflow isn't handled in Win32/Logitech message generation, and when
the SHORT_MAX is reach, the amount suddendly gets negative, making the
scrolling the other way (as I said, highly unusable)

So one overflow bug in Win or Logitech side (didn't check further) who never
thought the short limit could be reached with click-by-click mousewheels.

But more importantly, mousewheel message in SWT should be consumed in one go,
and not changed to X LINE(UP/DOWN). That's fine for the usual 120/240 amounts
you get with click-by-click devices, but don't count on it any longer (no
standard Win32 application does).

I attached the working little patch I did for my 3.3.1.1 eclipse plaform for
reference, but don't expect a lot from it (apart from being simple), as it's a
dirty hack probably breaking a lot of other applications.

Some comments about this patch and logitech devices:
 - MX Revolutions base wheel amounts in free-spin are -80 for down and 180 for
up (different probably because spinning the wheel downwards is easier)
 - those aren't exact values and they can differ with acceleration setup in
logitech driver
 - the 9/4 ratio (180/80) that you see is a personnal touch because I don't
like the difference between up/down amounts, and I wanted to hack a way around
it (not configurable in logitech driver)
 - what my patch does is that I send only one LINEUP/LINEDOWN message, but when
handling it in wmScroll(), I actually scroll by the required (dirtily saved)
amount, and both skips the rounding to line number (could replace your
"remaining" fix that I don't find so clean either) and the multiplication of
messages
 - best solution to me would be to entirely avoid the LINEUP/DOWN messages and
really implement the wmMouseWheel, but my Win32/SWT knowledge didn't allow me
to do that

Result seems fully working (today was my first day of SWT, so I probably missed
a lot of things, let me know), and I can finally scroll through pages and pages
of java code.
(overflow bug from logitech remains but only if I set the acceleration at max
value, and that's not really a useful setup anyway).

Thanks for reading, I'm available to validate any fix you might find to this
problem."

The patch 
https://bugs.eclipse.org/bugs/attachment.cgi?id=86994
Comment 15 Francois CLA 2008-01-16 12:13:02 EST
Thanks Felipe for driving me to this bug, I hadn't found it.

Sounds like conclusion is the same, I agree on the short overflow in windows/logitech (I've got absolutely no idea which one is actually "aggregating" wheel events when they aren't consumed fast enough).

But I also posted on bug 194143 because I don't like the "fix" that was done for the "remaining of floating wheel amounts":

First if the mouse sends -60, 60, 60, SWT would scroll one line (discarding the first remaining because the direction is different).
That's not correct. Expected behavior from a user's pov is to scroll half a line at most, or wait the 60 amount to scroll.
Likewise, if the driver sends -60, 60, 60, -60, 60, 60, your fix will scroll 2 lines instead of the expected single line scrolling.

Secondly, as expressed in my comment, I don't understand why (when the component permits it, which seems to be the case of Scrollable) we don't scroll by the required ratio.
We're mapping ratio -> line count -> X times the bar increment.
Please tell me why not going directly to "ratio by the bar increment" as my patch suggests, it makes the scrolling much more smooth and consumes much less CPU.

It seems AWT is going almost the same way you did (http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6524352)
but I'd rather not see that in SWT.

No ad here, but using eclipse with the patch and a MX revolution is a real pleasure.
Comment 16 Felipe Heidrich CLA 2008-01-16 13:06:34 EST
(In reply to comment #15)
> First if the mouse sends -60, 60, 60, SWT would scroll one line 
>(discarding the first remaining because the direction is different).
> That's not correct. 
Please read http://msdn2.microsoft.com/en-us/library/ms997498.aspx
It says:
"The remainder must be zeroed when the wheel rotation switches directions or when window focus changes."

> Secondly, as expressed in my comment, I don't understand why (when 
> the component permits it, which seems to be the case of Scrollable)
> we don't scroll by the required ratio.

Unfortunately at this point I don't think I can change it without breaking API.
Comment 17 Francois CLA 2008-01-16 13:32:58 EST
Article already read.
But.

"It is preferable for the application to scroll the view a fractional number of lines if possible."

When you follow this rule, the scrolling from -60, 60, 60 is half a line, not a full line, and based on the fact that the messages received do not depend on you following or not this rule, I'd suggest you take some distance from this 2004 article.

From my MX revolution experience, I can tell you it's a horrible feeling doing single step up/down with the mouse and NEVER going down (always up, because the down step is -80, less than 120, and the up step is 180).

So don't assume the driver remembers the remaining when the direction switchs, because it doesn't, and because it can't do (because of applications that support fractionnal scrolls).

MSDN is good, not but god.
Comment 18 Felipe Heidrich CLA 2008-01-17 11:52:36 EST
Fixed in HEAD > 20080117

"It is preferable for the application to scroll the view a fractional number of
lines if possible."
Done that.


I can only reproduce this problem on Windows Vista. On Windows XP Logitech Revolution always worked fine. Is that true for everybody ?
Comment 19 Francois CLA 2008-01-17 12:00:00 EST
I'm on winXP, but using SetPoint logitech drivers.
Without SetPoint, the mouse behaves "standardly" with 120/-120 throttles (hence not causing any fraction problem).

Don't you have any issue even if you move the wheel fast ?

Thanks for the fix, I'll validate it asap.
Comment 20 Felipe Heidrich CLA 2008-01-17 13:20:29 EST
(In reply to comment #19)
> Don't you have any issue even if you move the wheel fast ?

Not on XP, I have logitech and setpoint 4.00.121 installed.

> Thanks for the fix, I'll validate it asap.
In that case you will have to enable the fix for XP (currently it only runs on Vista).
In Scrollable#WM_MOUSEWHEEL() find this line:
if (!OS.IsWinCE && OS.WIN32_VERSION >= OS.VERSION (6, 0)) {
and change to "if (true) {"
Comment 21 Francois CLA 2008-01-17 13:41:52 EST
I've got SetPoint 4.24.99, never tested with any other version (I got my mouse a week ago).

Quick remark (before any test, because I'd have to build a full eclipse version or port your patch to 3.3.1.1, I noticed the version check too).

Is it really necessary to perform twice the " OS.SetScrollInfo " call ?

(once in WM_MOUSEWHEEL and once in the wall to wmScroll during the sendMessage handling)

Sounds to me like unnecessary overhead. Is there a reason behind that ?
Can't we perform the wmScroll directly without re-sending a SB_THUMBPOSITION message ?
And if we can't, can we at least set the rePaint param to false in the first call ?

Sorry if there's any obvious reason, I'm quite new to all this.
Comment 22 Felipe Heidrich CLA 2008-01-17 13:59:59 EST
(In reply to comment #21)
> Quick remark (before any test, because I'd have to build a full eclipse version
> or port your patch to 3.3.1.1, I noticed the version check too).
You can load swt from CVS http://www.eclipse.org/swt/cvs.php and self host eclipse to test the code.


> Is it really necessary to perform twice the " OS.SetScrollInfo " call ?
> (once in WM_MOUSEWHEEL and once in the wall to wmScroll during the sendMessage
> handling)
> Sounds to me like unnecessary overhead. Is there a reason behind that ?

Well, try to removed the first one and test the code.
(the second we need cause we don't know from where the call comes from)

> Can't we perform the wmScroll directly without re-sending a SB_THUMBPOSITION
> message ?

Yes we could, but we are trying to be a good win32 citzen by making all calls go through winproc.

> And if we can't, can we at least set the rePaint param to false in the first
> call ?
It doesn't matter, change and see.

> Sorry if there's any obvious reason, I'm quite new to all this.

Thanks for you contribution, you did a great job!

Comment 23 Francois CLA 2008-01-22 10:05:11 EST
(In reply to comment #22)
> Well, try to removed the first one and test the code.
> (the second we need cause we don't know from where the call comes from)

Obviously if I remove the first one, the scroll position wouldn't change :)

> Yes we could, but we are trying to be a good win32 citzen by making all calls
> go through winproc.

I like being a good citizen, that's enough of an explanation.

> > And if we can't, can we at least set the rePaint param to false in the first
> > call ?
> It doesn't matter, change and see.

Changed and tested, doesn't seem to have any (visual) effect.
From the "Wine" implementation of the same layer, it should achieve a better performance, but I can't tell anything about microsoft one.

I tested & validated your patch on WinXP, with SetPoint 4.24.99 and MX Revolution mouse.
Was tested on Eclipse 3.3.1.1 with the following modifications:
 - changed the if clause to if (!OS.IsWinCE && OS.WIN32_VERSION >= OS.VERSION (5, 1))
 - removed the scrollRemainder usage because not implemented in 3.3.1.1 (not used anyway)
 - tested & working with both true/false on OS.SetScrollInfo call. 

Thanks a million.