Unified button mask behavior across platforms #20063
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
leonkrause
suggested changes
Jul 9, 2018
| @@ -248,11 +248,17 @@ static EM_BOOL _wheel_callback(int event_type, const EmscriptenWheelEvent *wheel | |||
|
|
|||
| // Different browsers give wildly different delta values, and we can't | |||
| // interpret deltaMode, so use default value for wheel events' factor | |||
| int mask = _input->get_mouse_button_mask(); | |||
| int button_flag = 1 << (ev->get_button_index() - 1); | |||
There was a problem hiding this comment.
This block confusingly looks like it's related to the preceding comment, put a newline inbetween.
|
|
||
| ev->set_pressed(true); | ||
| mask |= button_flag; | ||
| ev->set_button_mask(mask); |
There was a problem hiding this comment.
set_button_mask is already called once in line 229, remove that original invocation since we're calling it here.
| _input->parse_input_event(ev); | ||
|
|
||
| ev->set_pressed(false); | ||
| mask &= ~button_flag; | ||
| ev->set_button_mask(mask); |
There was a problem hiding this comment.
I don't feel the mask variable helps readability, just this would be clearer:
ev->set_button_mask(ev->get_button_mask() & ~button_flag);
moiman100
force-pushed
the
fix-button-mask
branch
from
47f83c1
to
d7cf201
Compare
|
Made requested changes |
leonkrause
approved these changes
Jul 9, 2018
moiman100
force-pushed
the
fix-button-mask
branch
from
d7cf201
to
87d34a0
Compare
|
resolved conflicts |
moiman100
force-pushed
the
fix-button-mask
branch
from
87d34a0
to
bd9c592
Compare
|
Sorry for the delay. More testing from users and contributors would be nice to ensure that no regressions happen, but I guess this will be best done once in the master branch :) |
|
Thanks! |
This was referenced Dec 14, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before on linux InputEvent would report button mask like this (pseudo)
InputEventMouseButton button_index=x is_pressed=true button_mask=0
InputEventMouseButton button_index=x is_pressed=false button_mask=button_mask_x
On all other platforms button mask is reported like this
InputEventMouseButton button_index=x is_pressed=true button_mask=button_mask_x
InputEventMouseButton button_index=x is_pressed=false button_mask=0
After this commit linux reports it like other platforms.
Before on linux InputEvent reported button mask for mouse wheel buttons, while other platforms didn't.
After this commit InputEvent reports mouse wheel button mask on all platforms.
Also someone should probably check horizontal mouse wheel buttons, because I don't have a mouse with tilting wheel. For example Logitech m705 has all supported functionality.
NOTE: OSX is NOT tested because I don't have a mac.
NOTE 2: os_windows.cpp diff looks really messy in GitHub because I removed an unnecessary indentation.
It looks fine, when comparing the files using something decent like KDiff3.