Bug 1688

Created attachment 1147 [details] [review]
Patch to the mouse acceleration code

Created attachment 1308 [details] [review]
Patch without floats (suggested version)

The other acceleration method isn't exponential but a power function. Just to
correct the mistake. :)

Comment on attachment 1308 [details] [review]
Patch without floats (suggested version)

Jan_B:
Only release wranglers can grant approval for attachments (the '+' state of the
flag), normal users can only request approval ('?' state of the flag dropdown
widget) or clear the approval request (the first choice of the dropdown).

Setting approval flag to '?' (=requesting approval) ...

The patch looks fine to me, it makes good sense.  But actually, I think
attachment 1147 [details] [review] is better, since the pow() based acceleration code already uses
floats, so there's no reason to jump through hoops and introduce extra struct
fields to avoid using floats.

I've obsoleted the integer-only patch and nominated the float patch instead.

> [..]there's no reason to jump through hoops and introduce extra struct
> fields to avoid using floats.
That's what I thought first but after some input from Jim Gettys and Keith
Packard, I found out that casting from float to integer is very slow in C, at
least with x86 cpus. Additionally, there are target platforms that don't have a FPU.

For desktop computers, the difference is not noticeable of course, even with
future 10'000 Hz mice but without an FPU, this could make a system crawl. :)

I don't know what others think but I now like the integer version better. I hate
to waste CPU time even if it's not much. Plus the integer version is
theoretically more accurate.

I guess the mouse acceleration code is going to be revised to a more
sophisticated algorithm in the future anyway so I don't care a lot about which
patch gets accepted now as I see it as a needed fix.

(In reply to comment #6)
> I guess the mouse acceleration code is going to be revised to a more
> sophisticated algorithm in the future anyway so I don't care a lot about which
> patch gets accepted now as I see it as a needed fix.

The current code is sort-of broken, and in a future rewrite (not for 6.8.2) it
would be nice to get rid of the float code altogether.  If you want to look into
that, that would be cool, but for 6.8.2 I think a smaller, less intrusive patch
is better.  Since the polynomial acceleration code already use floats and has
the remainder fields in the struct, I think that's the way to go for the stable
release.

I didn't mean to obsolete your integer patch as a long term solution, I was just
evaluating it in the context of 6.8.2.

Jan,

Thanks for all your efforts.

I think Kristian is right; we'll go with your original patch for the maintenence
release, and contemplate a full rewrite for a future release, and in the new
input stuff that Kristian and I are beginning work on.

> I think Kristian is right; we'll go with your original patch for the maintenence
> release, and contemplate a full rewrite for a future release, and in the new
> input stuff that Kristian and I are beginning work on.
OK.

Comment on attachment 1147 [details] [review]
Patch to the mouse acceleration code

Approval for X11R6.8.x stable branch granted in the 2004-12-08
release-wranglers phone call.
Please don't commit, I'll do that myself...

Created attachment 1530 [details] [review]
[FIXED_X11R68x] Patch including changelog comment (same as attachment #1147 [details] [review])

Comment on attachment 1530 [details] [review]
[FIXED_X11R68x] Patch including changelog comment (same as attachment #1147 [details] [review])

Carry over the approval flag (the patches are identical except the added entry
for xc/Changelog)

Comment on attachment 1530 [details] [review]
[FIXED_X11R68x] Patch including changelog comment (same as attachment #1147 [details] [review])

Patch checked-in into Xorg trunk:

/cvs/xorg/xc/ChangeLog,v  <--  ChangeLog
new revision: 1.602; previous revision: 1.601
cvs commit: Using deprecated info format strings.  Convert your scripts to use
the new argument format and remove '1's from your info file format strings.
/cvs/xorg/xc/programs/Xserver/hw/xfree86/common/xf86Xinput.c,v	<-- 
xf86Xinput.c
new revision: 1.2; previous revision: 1.1
cvs commit: Using deprecated info format strings.  Convert your scripts to use
the new argument format and remove '1's from your info file format strings.
Mailing the commit message to xorg-commit@lists.freedesktop.org...

Comment on attachment 1530 [details] [review]
[FIXED_X11R68x] Patch including changelog comment (same as attachment #1147 [details] [review])

Patch checked-in into X11R6.8.x stable branch:

/cvs/xorg/xc/ChangeLog,v  <--  ChangeLog
new revision: 1.365.2.24; previous revision: 1.365.2.23
cvs commit: Using deprecated info format strings.  Convert your scripts to use
the new argument format and remove '1's from your info file format strings.
/cvs/xorg/xc/programs/Xserver/hw/xfree86/common/xf86Xinput.c,v	<-- 
xf86Xinput.c
new revision: 1.1.1.2.6.1; previous revision: 1.1.1.2
cvs commit: Using deprecated info format strings.  Convert your scripts to use
the new argument format and remove '1's from your info file format strings.
Mailing the commit message to xorg-commit@lists.freedesktop.org...

Patch has been commited to Xorg trunk and X11R6.8.x stable branch...

... marking bug as FIXED (please reopen if there are any issues left).