Skip to content
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

Horizontal scrolling is not reversed #49

Closed
rituraj22 opened this issue Mar 12, 2018 · 14 comments
Closed

Horizontal scrolling is not reversed #49

rituraj22 opened this issue Mar 12, 2018 · 14 comments

Comments

@rituraj22
Copy link

Vertical scrolling is reversed, but horizontal is not. So scrolling is confusing.

@rom1v
Copy link
Collaborator

rom1v commented Mar 12, 2018

Thank you for the report. I currently have no way to test horizontal scrolling.

However, the documentation says:

Movements to the left generate negative x values and to the right generate positive x values. Movements down (scroll backward) generate negative y values and up (scroll forward) generate positive y values.

This seems inconsistent to me.

Since the vertical scrolling works as expected (same behavior as on desktop environments on Linux), we should reverse the horizontal scrolling.

Could you apply this change and confirms it is ok to you, please?

--- a/app/src/convert.c
+++ b/app/src/convert.c
@@ -172,7 +172,7 @@ SDL_bool mouse_wheel_from_sdl_to_android(const SDL_MouseWheelEvent *from,
     to->scroll_event.position = position;
 
     int mul = from->direction == SDL_MOUSEWHEEL_NORMAL ? 1 : -1;
-    to->scroll_event.hscroll = mul * from->x;
+    to->scroll_event.hscroll = -mul * from->x;
     to->scroll_event.vscroll = mul * from->y;
 
     return SDL_TRUE;

@rituraj22
Copy link
Author

@rom1v Multiplying by -1 did work correctly (horizontal scrolling became reversed).
FYI, I am testing it in webview on
Micromax Canvas A1 (AQ4501)
sprout4
Android 6.0.1

@rom1v
Copy link
Collaborator

rom1v commented Mar 12, 2018

Thank you 👍

rom1v added a commit that referenced this issue Mar 12, 2018
The SDL mouse wheel event seems inconsistent between horizontal and
vertical scrolling.

> Movements to the left generate negative x values and to the right
> generate positive x values. Movements down (scroll backward) generate
> negative y values and up (scroll forward) generate positive y values.

<https://wiki.libsdl.org/SDL_MouseWheelEvent#Remarks>

Reverse the horizontal.

Fixes <#49>.
@rom1v
Copy link
Collaborator

rom1v commented Mar 12, 2018

Fixed in master.

@rom1v rom1v closed this as completed Mar 12, 2018
@rituraj22
Copy link
Author

Well I tested it on another phone with android 7.1 (Lineage OS 14.1), and in it the older version horizontal scrolling works fine and newer one has issues, so I guess it is device specific.

@rom1v rom1v reopened this Mar 12, 2018
@rom1v
Copy link
Collaborator

rom1v commented Mar 12, 2018

@rituraj22 😞

The new version seems more consistent though, since the client and Android part are expressed in the same directions.

Could you add this log and provide the scrolling values for both devices for both horizontal and vertical, please?

--- a/app/src/convert.c
+++ b/app/src/convert.c
@@ -1,4 +1,5 @@
 #include "convert.h"
+#include "log.h"
 
 #define MAP(FROM, TO) case FROM: *to = TO; return SDL_TRUE
 #define FAIL default: return SDL_FALSE
@@ -172,6 +173,7 @@ SDL_bool mouse_wheel_from_sdl_to_android(const SDL_MouseWheelEvent *from,
     to->scroll_event.position = position;
 
     int mul = from->direction == SDL_MOUSEWHEEL_NORMAL ? 1 : -1;
+    LOGI("WHEEL mul=%d; (%d, %d)\n", mul, from->x, from->y);
     // SDL behavior seems inconsistent between horizontal and vertical scrolling
     // so reverse the horizontal
     // <https://wiki.libsdl.org/SDL_MouseWheelEvent#Remarks>

@rituraj22
Copy link
Author

Log shows (-1,0) on both devices when i scroll to right. But the motion on both devices are opposite.
On the phone with 6.0, (-1,0) scrollls to right, while on 7.1, (-1,0) scrolls to left.

@rom1v
Copy link
Collaborator

rom1v commented Mar 12, 2018

@rituraj22 Could you provide the logs as printed please? (I also need the mul value) 😉

@rituraj22
Copy link
Author

INFO: WHEEL mul=1; (-1, 0)
is on all cases on scroll to right

@rom1v
Copy link
Collaborator

rom1v commented Mar 12, 2018

On the phone with 6.0, (-1,0) scrollls to right

This is very unexpected.

Just for confirmation, it does not print:

INFO: WHEEL mul=-1; (-1, 0)

but really prints:

INFO: WHEEL mul=1; (-1, 0)

?

EDIT: my question is stupid, of course it's the same mul value, since you use the same computer.

I don't understand why it behaves that way…

@rituraj22
Copy link
Author

rituraj22 commented Mar 12, 2018

Yeah on all devices it prints

For android 6.0,

INFO: Initial texture: 480x854
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (0, 1)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)

On android 7.1,

INFO: Initial texture: 1080x1920
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (0, 1)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)
INFO: WHEEL mul=1; (-1, 0)

@rom1v
Copy link
Collaborator

rom1v commented Mar 12, 2018

I said:

On the phone with 6.0, (-1,0) scrollls to right

This is very unexpected.

In fact, since I reversed the horizontal direction, the unexpected behavior is:

while on 7.1, (-1,0) scrolls to left.

@rom1v
Copy link
Collaborator

rom1v commented Dec 2, 2019

@rituraj22 Please confirm that #966 (comment) does not break the behavior you want.

@rom1v
Copy link
Collaborator

rom1v commented Apr 9, 2020

Had been fixed (see #966).

@rom1v rom1v closed this as completed Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants