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

Properly emulate old scrolling functionality #345

Closed
wants to merge 1 commit into from
Closed

Properly emulate old scrolling functionality #345

wants to merge 1 commit into from

Conversation

@iirelu
Copy link
Contributor

@iirelu iirelu commented Jun 17, 2015

The logic for the old step-wise scrolling functionality was improporly recreated in the new smooth-scrolling-based logic as of 189e195. This makes it exactly the same as it was before.

I'm not familiar with this code so there might be some subtleties I'm missing that have to be accounted for, in which case just tell me and I'll fix it.

Right now it seems that this is only used if you untick the "Handle smooth scrolling events when devices send them" in your preferences, so I think there's still problems with the code not properly figuring out what's actually a smooth scrolling event.

The logic for the old step-wise scrolling functionality was improporly
recreated in the new smooth-scrolling-based logic as of 189e195. This
makes it *exactly* the same as it was before.
@achadwick
Copy link
Member

@achadwick achadwick commented Jun 17, 2015

Bear in mind that the "Handle smooth scrolling events when devices send them" setting is one that requires MyPaint to be restarted.

@achadwick
Copy link
Member

@achadwick achadwick commented Jun 17, 2015

@iirelu You're seeing a lot of weirdness that I'm not. I'm wondering if your device is sending a mixed stream of smooth and non-smooth events for the same wheel rotation. Maybe that's just lingering worry from dealing with #344 though.

Could you stick a few debug statements in there (on master) to rule that out please?

diff --git a/gui/mode.py b/gui/mode.py
index 3f4149f..a26bc3e 100644
--- a/gui/mode.py
+++ b/gui/mode.py
@@ -470,6 +470,10 @@ class ScrollableModeMixin (InteractionMode):
         # We don't rotate any more though. Srsly, that was awful.
         if scroll_action == gui.device.ScrollAction.ZOOM:
             if direction == gdk.SCROLL_SMOOTH:
+                logger.debug(
+                    "PR#345: zoom-SMOOTH dx=%r,dy=%r",
+                    event.delta_x, event.delta_y,
+                )
                 if constrain_smooth:
                     # Needs to work in an identical fashion to old-style
                     # zooming.
@@ -501,9 +505,11 @@ class ScrollableModeMixin (InteractionMode):
                     self.__reset_delta_totals()
             # Old-style zooming
             elif direction == gdk.SCROLL_UP:
+                logger.debug("PR#345: zoom-nonsmooth-INWARDS")
                 doc.zoom(doc.ZOOM_INWARDS)
                 self.__reset_delta_totals()
             elif direction == gdk.SCROLL_DOWN:
+                logger.debug("PR#345: zoom-nonsmooth-OUTWARDS")
                 doc.zoom(doc.ZOOM_OUTWARDS)
                 self.__reset_delta_totals()

And run with

$ MYPAINT_DEBUG=1 ./mypaint -c /tmp/cfgtmp1 2>&1 | grep 'PR#345'

There should be no interleaving of the "zoom-nonsmooth" and "zoom-SMOOTH" messages at any time, even when "Handle smooth scrolling events when devices send them" is set to "When devices send them" (and after MyPaint is restarted if you change it). If they interleave, we need to redo this bit of code pronto.

I'd also like to see what the dx and dy values your device reports for the smooth flavour of events are. For this scrollwheel mouse here, it's ±1.0 exactly for each wheel notch.

@iirelu
Copy link
Contributor Author

@iirelu iirelu commented Jun 17, 2015

Bear in mind that the "Handle smooth scrolling events when devices send them" setting is one that requires MyPaint to be restarted.

...Ah.

Whoops.

@iirelu iirelu closed this Jun 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants