New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Event.MouseWheel.Delta sometimes returns 0 #95

Closed
LaurentGomila opened this Issue Sep 11, 2011 · 29 comments

Comments

Projects
None yet
@LaurentGomila
Member

LaurentGomila commented Sep 11, 2011

On Windows, when a mouse wheel event is triggered, sometimes the Delta parameter is 0. It probably happens with high-resolution mice, which trigger mouse wheel events with an internal delta less than 120. Therefore this code:

event.MouseWheel.Delta = static_cast<Int16>(HIWORD(wParam)) / 120;

... leads to Delta being 0.

I must find a solution to handle correctly both regular and high-resolution mice.

@ghost ghost assigned LaurentGomila Sep 11, 2011

@alexanderzeillinger

This comment has been minimized.

Show comment
Hide comment
@alexanderzeillinger

alexanderzeillinger Sep 28, 2011

So either you make sure the result of the division is always at least 1 or -1

Int16 temp = static_cast<Int16>(HIWORD(wParam));
if (temp > 0 && temp <  120) temp =  120;
if (temp < 0 && temp > -120) temp = -120;
event.MouseWheel.Delta = temp / 120;

or use a float to store the Delta, which would probably be the more consistent solution.

alexanderzeillinger commented Sep 28, 2011

So either you make sure the result of the division is always at least 1 or -1

Int16 temp = static_cast<Int16>(HIWORD(wParam));
if (temp > 0 && temp <  120) temp =  120;
if (temp < 0 && temp > -120) temp = -120;
event.MouseWheel.Delta = temp / 120;

or use a float to store the Delta, which would probably be the more consistent solution.

@l0ud

This comment has been minimized.

Show comment
Hide comment
@l0ud

l0ud Dec 29, 2011

I think it's a good time to remind about this issue. It seems to me that it happens on every Microsoft mouse (even the cheapest ones, without smooth scroll). It wouldn't be that bad if I could somehow read the wheel direction (when delta is 0), but it's not possible in current case.

Anyway, accumulating delta values isn't a good solution for me, cause it require 2 wheel movements to trigger event. I think returning floats should be enough (with info in doc, that delta may be less than 1).

l0ud commented Dec 29, 2011

I think it's a good time to remind about this issue. It seems to me that it happens on every Microsoft mouse (even the cheapest ones, without smooth scroll). It wouldn't be that bad if I could somehow read the wheel direction (when delta is 0), but it's not possible in current case.

Anyway, accumulating delta values isn't a good solution for me, cause it require 2 wheel movements to trigger event. I think returning floats should be enough (with info in doc, that delta may be less than 1).

@retep998

This comment has been minimized.

Show comment
Hide comment
@retep998

retep998 Dec 30, 2011

I vote for providing float values and then letting the user manually round or cast it. This allows for games to take advantage of high precision scrolling.

retep998 commented Dec 30, 2011

I vote for providing float values and then letting the user manually round or cast it. This allows for games to take advantage of high precision scrolling.

@CelticMinstrel

This comment has been minimized.

Show comment
Hide comment
@CelticMinstrel

CelticMinstrel Jun 19, 2012

It's always zero for me if I scroll by one "tick" on the mouse; it'll go up to ±3 or so if I flick the scroll wheel, but the inability to detect small scroll wheel movements is a bit of a drawback.

I'm on a Mac, but not using an Apple mouse.

CelticMinstrel commented Jun 19, 2012

It's always zero for me if I scroll by one "tick" on the mouse; it'll go up to ±3 or so if I flick the scroll wheel, but the inability to detect small scroll wheel movements is a bit of a drawback.

I'm on a Mac, but not using an Apple mouse.

@sardonic2

This comment has been minimized.

Show comment
Hide comment
@sardonic2

sardonic2 Jun 22, 2012

With RC1 on windows, the delta for both my mouse and trackpad show only 0.

sardonic2 commented Jun 22, 2012

With RC1 on windows, the delta for both my mouse and trackpad show only 0.

@Qix-

This comment has been minimized.

Show comment
Hide comment
@Qix-

Qix- Jan 13, 2013

Still an issue.

or use a float to store the Delta

+1

Qix- commented Jan 13, 2013

Still an issue.

or use a float to store the Delta

+1

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Jan 13, 2013

Member

Still an issue.

Of course it is, otherwise the task would be closed.

+1

Do you think it will change anything? I'll implement the best solution, not the one that has the most +1s :p
I'm more interested in technical arguments for or against one of the proposed solutions; or new solutions.

Member

LaurentGomila commented Jan 13, 2013

Still an issue.

Of course it is, otherwise the task would be closed.

+1

Do you think it will change anything? I'll implement the best solution, not the one that has the most +1s :p
I'm more interested in technical arguments for or against one of the proposed solutions; or new solutions.

@Qix-

This comment has been minimized.

Show comment
Hide comment
@Qix-

Qix- Jan 13, 2013

Okay Laurent, calm down. I didn't have any better technical ideas so I voiced that floats or normalization was something I thought would be a fine approach.

Qix- commented Jan 13, 2013

Okay Laurent, calm down. I didn't have any better technical ideas so I voiced that floats or normalization was something I thought would be a fine approach.

@CelticMinstrel

This comment has been minimized.

Show comment
Hide comment
@CelticMinstrel

CelticMinstrel Jan 22, 2013

I'm not sure about making it a float, but I think it would at least be a good idea to expose the raw delta rather than dividing by 120.

CelticMinstrel commented Jan 22, 2013

I'm not sure about making it a float, but I think it would at least be a good idea to expose the raw delta rather than dividing by 120.

@retep998

This comment has been minimized.

Show comment
Hide comment
@retep998

retep998 Jan 23, 2013

The way I see it, there's only one choice to be made here, which is how to provide fine-grained data on scrolling so applications using SFML can have really smooth scrolling for those with that sort of scroll wheel. Either returning the raw value directly, or returning a normalized float, would solve this problem. The raw value has the benefit of no precision loss whatsoever, while the normalized float has the benefit of being easier to work with because the developer doesn't have to figure out that 1 scroll == 120.
As for the matter of providing a discrete scroll value, which is useful for providing a simple scroll wheel events to developers who don't want to concern themselves with fractional scrolling, I recommend maintaining some sort of accumulation buffer so no matter how fast or slow the user scrolls, no fractional part is ever lost, but we still have the benefit of retaining a discrete wheel event.
Of course, this causes complications by providing two sorts of mouse wheel events. So, I propose in order to access the high precision scrolling value, there be a function to call to get the current position of the mouse wheel (just return the scrolling accumulation buffer and never modulus that buffer). This way developers using the event system don't notice any changes, the user gets better mouse wheel handling, and for the developers who want to take advantage of high precision scrolling, there is a function to let them obtain that value.

retep998 commented Jan 23, 2013

The way I see it, there's only one choice to be made here, which is how to provide fine-grained data on scrolling so applications using SFML can have really smooth scrolling for those with that sort of scroll wheel. Either returning the raw value directly, or returning a normalized float, would solve this problem. The raw value has the benefit of no precision loss whatsoever, while the normalized float has the benefit of being easier to work with because the developer doesn't have to figure out that 1 scroll == 120.
As for the matter of providing a discrete scroll value, which is useful for providing a simple scroll wheel events to developers who don't want to concern themselves with fractional scrolling, I recommend maintaining some sort of accumulation buffer so no matter how fast or slow the user scrolls, no fractional part is ever lost, but we still have the benefit of retaining a discrete wheel event.
Of course, this causes complications by providing two sorts of mouse wheel events. So, I propose in order to access the high precision scrolling value, there be a function to call to get the current position of the mouse wheel (just return the scrolling accumulation buffer and never modulus that buffer). This way developers using the event system don't notice any changes, the user gets better mouse wheel handling, and for the developers who want to take advantage of high precision scrolling, there is a function to let them obtain that value.

@Qix-

This comment has been minimized.

Show comment
Hide comment
@Qix-

Qix- Jan 24, 2013

@retep998 Perhaps you should make a PR?

Qix- commented Jan 24, 2013

@retep998 Perhaps you should make a PR?

@fabianschuiki

This comment has been minimized.

Show comment
Hide comment
@fabianschuiki

fabianschuiki Mar 4, 2013

On Mac OS X, NSEvent provides the mouse wheel delta as a double (on 64bit machines, i.e. all recent Macs). I'd like to argue for float/double deltas as this is less hardware dependent and SFML might combine the deltas provided by the operating system with scroll speed settings in the OS (Cocoa does this automatically).

It might also be a good idea to add two more deltas, namely deltaX, deltaY and deltaZ, since at least Mac OS provides them. This would allow SFML apps to leverage the precise 2D trackpad of MacBooks. There are also 3D Mice around that generate three dimensional scrolls… you never know what people use SFML for.

I'd argue you should leverage such OS capabilities where they are provided by the OS, or provide a useful default value (like 0 for scroll deltas) where they are not. Better try to accommodate as much functionality as possible, rather than going for the largest set of common functionality (which is probably quite small considering all the OSes).

Cheers

fabianschuiki commented Mar 4, 2013

On Mac OS X, NSEvent provides the mouse wheel delta as a double (on 64bit machines, i.e. all recent Macs). I'd like to argue for float/double deltas as this is less hardware dependent and SFML might combine the deltas provided by the operating system with scroll speed settings in the OS (Cocoa does this automatically).

It might also be a good idea to add two more deltas, namely deltaX, deltaY and deltaZ, since at least Mac OS provides them. This would allow SFML apps to leverage the precise 2D trackpad of MacBooks. There are also 3D Mice around that generate three dimensional scrolls… you never know what people use SFML for.

I'd argue you should leverage such OS capabilities where they are provided by the OS, or provide a useful default value (like 0 for scroll deltas) where they are not. Better try to accommodate as much functionality as possible, rather than going for the largest set of common functionality (which is probably quite small considering all the OSes).

Cheers

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Jul 5, 2014

Member

So... the final verdict on this issue is... what?

There are 2 routes we can go down:

  • Add up the internal deltas and only emit mouse wheel events when they exceed the threshold
  • Convert the mouse wheel delta to a float to support fine scrolling with high-resolution mice

The first option is simple to implement with a static counter in the Win32 window implementation. The second one requires more changes, but exposes more modern features to the user that would also mean being more future proof. On platforms (like X11) that wouldn't produce non-integer deltas, the float would always be set to integer values.

For those pedants who are worried that floats might not be able to represent the same values as ints, a single-precision float has 23 significand bits, meaning that it can represent ints in the range of -(2^23) to (2^23-1) without any loss of precision. I think that is enough for our purposes 😉. The size of a mouse wheel event wouldn't change either since floats and ints are both 32-bit values.

Member

binary1248 commented Jul 5, 2014

So... the final verdict on this issue is... what?

There are 2 routes we can go down:

  • Add up the internal deltas and only emit mouse wheel events when they exceed the threshold
  • Convert the mouse wheel delta to a float to support fine scrolling with high-resolution mice

The first option is simple to implement with a static counter in the Win32 window implementation. The second one requires more changes, but exposes more modern features to the user that would also mean being more future proof. On platforms (like X11) that wouldn't produce non-integer deltas, the float would always be set to integer values.

For those pedants who are worried that floats might not be able to represent the same values as ints, a single-precision float has 23 significand bits, meaning that it can represent ints in the range of -(2^23) to (2^23-1) without any loss of precision. I think that is enough for our purposes 😉. The size of a mouse wheel event wouldn't change either since floats and ints are both 32-bit values.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Aug 20, 2014

Member

@LaurentGomila do you still intend on working on this? 😁

This is one of the few issues that you left assigned to yourself, so I assumed you wanted to fix this too. If not, if you specify what is required of a solution to this (taking into account my previous comment), I can probably come up with a fix for it.

Member

binary1248 commented Aug 20, 2014

@LaurentGomila do you still intend on working on this? 😁

This is one of the few issues that you left assigned to yourself, so I assumed you wanted to fix this too. If not, if you specify what is required of a solution to this (taking into account my previous comment), I can probably come up with a fix for it.

@LaurentGomila LaurentGomila removed their assignment Aug 20, 2014

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Aug 20, 2014

Member

I don't know, I'd like to hear more opinions about this change if possible.

Member

LaurentGomila commented Aug 20, 2014

I don't know, I'd like to hear more opinions about this change if possible.

@TankOs

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Oct 2, 2014

Member

or use a float to store the Delta

+1

SCNR... 😺 But yes, I'm really voting for the float solution. ;-)

Member

TankOs commented Oct 2, 2014

or use a float to store the Delta

+1

SCNR... 😺 But yes, I'm really voting for the float solution. ;-)

@eXpl0it3r eXpl0it3r removed this from the 2.x milestone Nov 13, 2014

@LaurentGomila LaurentGomila removed this from the 2.x milestone Nov 13, 2014

@eXpl0it3r eXpl0it3r removed the s:unassigned label Jan 8, 2015

@Addisonbean

This comment has been minimized.

Show comment
Hide comment
@Addisonbean

Addisonbean Feb 17, 2015

Is there a workaround for this in the meantime?

Addisonbean commented Feb 17, 2015

Is there a workaround for this in the meantime?

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Feb 25, 2015

Member

Is there a workaround for this in the meantime?

Not really...

So... floats it is then? 😁

Member

binary1248 commented Feb 25, 2015

Is there a workaround for this in the meantime?

Not really...

So... floats it is then? 😁

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Feb 25, 2015

Member

Sounds good to me.

Member

eXpl0it3r commented Feb 25, 2015

Sounds good to me.

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Feb 25, 2015

Member

This would be an API breaking change, right?

Member

LaurentGomila commented Feb 25, 2015

This would be an API breaking change, right?

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Feb 25, 2015

Member

Since floats can be cast back to integers (with warnings if you have them enabled) I don't see how this would "break" the API. Strictly speaking, yes it would break the API but the same code could be compiled even if floats were used, so in that sense it doesn't really "break" anything for the developer. The ABI would change yes... but that would happen no matter how we choose to solve this problem I think.

Bonus: If the developer already uses C++11 automatic type deduction and has type independent code, they won't even notice anything.

Member

binary1248 commented Feb 25, 2015

Since floats can be cast back to integers (with warnings if you have them enabled) I don't see how this would "break" the API. Strictly speaking, yes it would break the API but the same code could be compiled even if floats were used, so in that sense it doesn't really "break" anything for the developer. The ABI would change yes... but that would happen no matter how we choose to solve this problem I think.

Bonus: If the developer already uses C++11 automatic type deduction and has type independent code, they won't even notice anything.

@zsbzsb

This comment has been minimized.

Show comment
Hide comment
@zsbzsb

zsbzsb Feb 26, 2015

Member

This would be an API breaking change, right?

I don't see how this would be any different than changing font metrics from ints to floats.

Member

zsbzsb commented Feb 26, 2015

This would be an API breaking change, right?

I don't see how this would be any different than changing font metrics from ints to floats.

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Feb 26, 2015

Member
if (event.mouseWheel.delta == 1)
    up();
else if (event.mouseWheel.delta == -1)
    down();

Not the most clever code, but you get the idea, code that deals with this event may do something that works with integers but not floats (in this case, event.mouseWheel.delta might be 1.00001).

I don't see how this would be any different than changing font metrics from ints to floats.

These sf::Font functions are much less likely to be used in a way that works with integers and not floats, and less likely to be used in user code anyway.

I know we are not 100% strict about API breaks, but I'm afraid that this one would really break user code.

Member

LaurentGomila commented Feb 26, 2015

if (event.mouseWheel.delta == 1)
    up();
else if (event.mouseWheel.delta == -1)
    down();

Not the most clever code, but you get the idea, code that deals with this event may do something that works with integers but not floats (in this case, event.mouseWheel.delta might be 1.00001).

I don't see how this would be any different than changing font metrics from ints to floats.

These sf::Font functions are much less likely to be used in a way that works with integers and not floats, and less likely to be used in user code anyway.

I know we are not 100% strict about API breaks, but I'm afraid that this one would really break user code.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Feb 26, 2015

Member

Well... we could always consider just adding a .deltaHighPrecision field to the struct instead.

Member

binary1248 commented Feb 26, 2015

Well... we could always consider just adding a .deltaHighPrecision field to the struct instead.

@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch

MarioLiebisch Feb 28, 2015

Member

Not the most clever code, but you get the idea, code that deals with this event may do something that works with integers but not floats (in this case, event.mouseWheel.delta might be 1.00001).

I'd probably blame the compiler since it's not warning about the potential issue. :)

But considering this would be very isolated and pretty easy to locate/fix, I'd ignore that. As mentioned in the other thread, keeping the integer comparison will be buggy as well. So do we want a consistent and clean fix or do we want to keep a broken system just to lessen the impact on existing code, which would essentially be broken anyway?

Member

MarioLiebisch commented Feb 28, 2015

Not the most clever code, but you get the idea, code that deals with this event may do something that works with integers but not floats (in this case, event.mouseWheel.delta might be 1.00001).

I'd probably blame the compiler since it's not warning about the potential issue. :)

But considering this would be very isolated and pretty easy to locate/fix, I'd ignore that. As mentioned in the other thread, keeping the integer comparison will be buggy as well. So do we want a consistent and clean fix or do we want to keep a broken system just to lessen the impact on existing code, which would essentially be broken anyway?

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Feb 28, 2015

Member

We provide a clean way of doing things for those who care. There are always going to be people who can't care less about their broken code/warnings, and for them the current implementation is just "good enough". They will start caring however, if we break (i.e. no longer function "good enough") their code. I don't think annoying people who don't want to be annoyed is the right way to go. For those who do care and commented on this issue, there will be the new system with which they can write their code and benefit from.

The API isn't meant to teach people how to write their code. If they can't be bothered using it properly and are fine with the results thereof, then so be it.

Member

binary1248 commented Feb 28, 2015

We provide a clean way of doing things for those who care. There are always going to be people who can't care less about their broken code/warnings, and for them the current implementation is just "good enough". They will start caring however, if we break (i.e. no longer function "good enough") their code. I don't think annoying people who don't want to be annoyed is the right way to go. For those who do care and commented on this issue, there will be the new system with which they can write their code and benefit from.

The API isn't meant to teach people how to write their code. If they can't be bothered using it properly and are fine with the results thereof, then so be it.

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Feb 28, 2015

Member

Not breaking the API between minor versions, especially with such nasty bugs that may end up being hard to track for users, is very important if we want SFML to be considered a little more seriously. Users who update from 2.2 to 2.3 expect their old code to continue to work as expected, not to have to track stupid bugs that the dev introduced.

Member

LaurentGomila commented Feb 28, 2015

Not breaking the API between minor versions, especially with such nasty bugs that may end up being hard to track for users, is very important if we want SFML to be considered a little more seriously. Users who update from 2.2 to 2.3 expect their old code to continue to work as expected, not to have to track stupid bugs that the dev introduced.

@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch

MarioLiebisch Feb 28, 2015

Member

Yeah, well, agree - just no fan of having "duplicate" members.

Member

MarioLiebisch commented Feb 28, 2015

Yeah, well, agree - just no fan of having "duplicate" members.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Mar 16, 2015

Member

Fixed in #810

Member

eXpl0it3r commented Mar 16, 2015

Fixed in #810

@eXpl0it3r eXpl0it3r closed this Mar 16, 2015

@eXpl0it3r eXpl0it3r added this to the 2.3 milestone Mar 16, 2015

@eXpl0it3r eXpl0it3r assigned eXpl0it3r and binary1248 and unassigned eXpl0it3r Mar 16, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment