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 scroll of the timeline using SHIFT+scroll wheel #3857

Merged
merged 6 commits into from
Nov 28, 2020

Conversation

MatthieuMeert
Copy link

This PR adds a new functionality that allows the user to horizontally scroll the timeline using SHIFT+scroll. This is widely used in other programs, and particularly useful when using a mouse (indeed, horizontal scroll is already handled for trackpads). It allows not to have to manually click on the scroll bar to move horizontally. The same convention for the correspondence of scrolling down/up resulting in moving right/left was used to be consistent with other programs.

The scroll speed is proportional to the timeline window, which means that it doesn't scroll faster if the timeline is zoomed in or not (which honestly would be really annoying). The scroll speed was set arbitrarily based on my feeling of the most comfortable.

@ferdnyc
Copy link
Contributor

ferdnyc commented Nov 23, 2020

@MatthieuMeert

Hmm. Interesting... I'm a little confused by this PR, because — OpenShot already horizontally scrolls the Timeline for me, when I scroll while holding down Shift! This may be a bug to be corrected on certain systems, more than a missing feature. (But I'm certainly glad you brought it to our attention that it isn't working everywhere.)

What OS are you running OpenShot on, and are you using the 2.5.1 release or the more recent 2.5.1-dev2 development builds?

@ferdnyc
Copy link
Contributor

ferdnyc commented Nov 23, 2020

Looking at the events in GammaRay, it appears that the mousewheel events (which are Vertical scroll events with ShiftModifier and NoButton, like you'd expect) are delivered to MainWindow, and then directly to the QtWebEngineCore::RenderWidgetHostViewQtDelegateWidget. The WebView itself is never involved.

If you're on Windows, it's possible that the shifted scroll events are a QtWebEngine feature that's not supported by WebKit (which we're still using on Windows, out of necessity). If that's the case, it may make sense to install the wheel event handler in the TimelineWebKitView class in src/windows/views/webview_backend/webkit.py, so that it'll only be used when WebKit is the selected backend.

(If you're on a non-Windows system, it's possible to have both backends installed and available on the system, in which case you can choose your poison by launching OpenShot with the optional --web-backend argument:

$ ./launch.py --help
usage: launch.py [-h] [-l LANG] [--list-languages] [--path PY_PATH]
                 [--test-models] [-b {auto,webkit,webengine}] [-d]
                 [--debug-file] [--debug-console] [-V]

OpenShot version 2.5.1-dev2

optional arguments:
------8<------8<------
  -b {auto,webkit,webengine}, --web-backend {auto,webkit,webengine}
                        Web backend to use for Timeline

...Which is something I'm going to do now, to see if switching over to the WebKit backend affects this at all.

@ferdnyc
Copy link
Contributor

ferdnyc commented Nov 23, 2020

Aha! Yes, with the WebKit backend the situation is the same, except the events are delivered to the TimelineWebView class instead which is not reactive to shifted scrolls. So, this may be something we need to add to the WebKit backend class, to support it there (and most especially, to support it on Windows)..

@ferdnyc
Copy link
Contributor

ferdnyc commented Nov 23, 2020

Hrm. By the same token, I'm only just now noticing that Ctrl+Wheel zooming is functional with WebKit, but again not with WebEngine.

Clearly our attempts at a wheel event handler are proving fruitless there, the events never even get delivered to the class. They're all intercepted by that preposterously-named QtWebEngineCore::RenderWidgetHostViewQtDelegateWidget.

@MatthieuMeert
Copy link
Author

MatthieuMeert commented Nov 23, 2020

Hi, I'm very glad to have feedback on this!

What OS are you running OpenShot on, and are you using the 2.5.1 release or the more recent 2.5.1-dev2 development builds?

I'm on Linux, Ubuntu 18.04, with version 2.5.1.dev2. Here are my system and version details (that appears when launching Openshot locally):

python3 launch.py 
Loaded modules from: /home/matthieu/OpenShot/openshot-qt/src
INFO app: ------------------------------------------------
INFO app:             Tue Nov 24 00:11:35 2020            
INFO app:               Starting new session              
INFO app: ------------------------------------------------
INFO app:          OpenShot (version 2.5.1-dev2)          
INFO app: ------------------------------------------------
INFO app: openshot-qt version: 2.5.1-dev2
INFO app: libopenshot version: 0.2.5-dev2
INFO app: platform: Linux-5.4.0-54-generic-x86_64-with-Ubuntu-18.04-bionic
INFO app: processor: x86_64
INFO app: machine: x86_64
INFO app: python version: 3.6.9
INFO app: qt5 version: 5.9.5
INFO app: pyqt5 version: 5.10.1

If you're on Windows, it's possible that the shifted scroll events are a QtWebEngine feature that's not supported by WebKit (which we're still using on Windows, out of necessity). If that's the case, it may make sense to install the wheel event handler in the TimelineWebKitView class in src/windows/views/webview_backend/webkit.py, so that it'll only be used when WebKit is the selected backend.

I think webkit is my default selected backend, as this also appears in the terminal when simply launching OpenShot without choosing a particular backend:

INFO webkit: Registering objects with WebKit
INFO main_window: foundCurrentVersion: Found the latest version: 2.5.1

(If you're on a non-Windows system, it's possible to have both backends installed and available on the system, in which case you can choose your poison by launching OpenShot with the optional --web-backend argument:

Trying to launch using --web-backend webkit launches normally, but using --web-backend webengine throws an error:

File "/home/matthieu/OpenShot/openshot-qt/src/windows/views/webview_backend/webengine.py", line 38, in <module>
    from PyQt5.QtWebEngineWidgets import QWebEngineView, QWebEnginePage
ModuleNotFoundError: No module named 'PyQt5.QtWebEngineWidgets'

I guess this confirms that I have webkit as default backend, and that I currently can't try it using webengine.

@MatthieuMeert
Copy link
Author

MatthieuMeert commented Nov 27, 2020

Hello,

I have tried to add the wheel event handler in the TimelineWebKitView (@ferdnyc as you suggested) in order to only modify the wrong behaviour when using the webkit backend.

Unfortunately, I have had some trouble trying to make it work (in fact it still doesn't work), due to the limitations of using webkit. I was able to catch the shifted scroll event inside the newly created wheelEvent function in TimelineWebKitView (the logs on the following code do appear when using shift+scroll).

def wheelEvent(self, event):
    """ Wheel callback for timeline """
    if event.modifiers() & Qt.ShiftModifier:
        log.info("shift+scroll event noticed!")
        # should return a wheelEvent with vertical scroll modified to horizontal
        log.info("y scroll value: %d", event.angleDelta().y())
        log.info("x scroll value: %d", event.angleDelta().x())
        # those two values should be inverted in the event to make it work
    return QWebView.wheelEvent(self, event)

The problem is that it doesn't seem possible, using webkit, to modify an existing event, or to create a new one from scratch, so that the wheel handler returns the desired event (horizontal scroll). Maybe I'm missing something here (which I hope I am so we can easily fix that bug) but is seems impossible to me to resolve the problem directly in TimelineWebKitView.

What do you suggest?

NOTE: I wanted to try OpenShot with qtwebengine backend, so I have installed OpenShot version 2.5.1 on a VM running Ubuntu 20.04.1 (assuming it would indeed be launched with qtwebengine), and tested the shift+scroll, which didn't work either. I guess it means that it also used webkit, which I tried to verify by cloning the repo and launching OpenShot locally on my VM, but I had problems during the installation so I didn't take the time to go deeper and verify this. But still, shift+scroll is not handled on my Ubuntu 20.04.1 VM.

NOTE 2: I also tried OpenShot on my Winows10, which (as we expected) results in the same behaviour (shift+scroll not handled).

@ferdnyc
Copy link
Contributor

ferdnyc commented Nov 27, 2020

@MatthieuMeert Apologies, didn't mean to ignore your previous comment, somehow it snuck by me. Let's see here...

Re: WebEngine, with 2.5.1-dev2 it's the preferred backend, and should be used if available. If it's not available, it's probably just a matter of installing it. Even on Bionic, running

sudo apt install python3-pyqt5.qtwebengine python3-pyqt5.qtopengl

(WebEngine uses an OpenGL rendering context) should make the WebEngine backend available.

I'll read over the rest of your message after I send this, just wanted to get that info to you for starters.

@ferdnyc
Copy link
Contributor

ferdnyc commented Nov 27, 2020

Hrm. This is a tricky one. It appears we can't scroll the widget, because that scrolls the entire widget, and then you get this:

image

So, that's no good at all. (It was very performant, though! It screwed up the widget rendering quick as you please.)

It seems like we'd want to scroll the QWebFrame for the view (which has a scrollPosition property, but in my testing trying to update that doesn't work. Unless I was doing something wrong with this:

if event.modifiers() & Qt.ShiftModifier:
    event.accept()
    frame = self.page().mainFrame()
    # Compute scroll offset from wheel motion
    tick_scale = 120
    steps = int(event.angleDelta().y() / tick_scale)
    delta = -(steps * 100)
    log.debug("Scrolling horizontally by %d pixels", delta)
    # Get current scroll position and modify
    scroll_pos = frame.scrollPosition()
    scroll_pos.setX(scroll_pos.x() + delta)
    frame.setScrollPosition(scroll_pos)

In my testing, trying to scroll using JavaScript (like the timeline's scrollLeft() method does) works, but it's DOG slow. Unusably slow, in fact. (That was when trying to do JQuery scrolling in the event handler, like so):

if event.modifiers() & Qt.ShiftModifier:
    event.accept()
    frame = self.page().mainFrame()
    # Compute scroll offset from wheel motion
    tick_scale = 120
    steps = int(event.angleDelta().y() / tick_scale)
    delta = -(steps * 100)
    log.debug("Scrolling horizontally by %d pixels", delta)
    # Get current scroll position and modify
    js = f'$("#scrolling_tracks").scrollLeft($("#scrolling_tracks").scrollLeft() + {delta});'
    frame.evaluateJavaScript(js)

The view did scroll, and it scrolled correctly, but it took on the order of a second or two to respond to each wheel motion. I'm guessing that has something to do with the AngularJS digest not being run. I suppose we could also try using the $scope.scrollLeft() method, instead of direct JQuery.

Your idea about possibly transforming the event from a vertical wheel motion to a horizontal one is a clever one, my only concern is whether or not QWebView even supports horizontal wheel scrolling. (My current mouse is lacking a tilt wheel, annoyingly.) But that also seems like another avenue worth exploring. I'm not sure if events can be modified (I expect no), so we'd probably have to construct a new one. event.angleDelta() and event.pixelDelta() are QPoints, so they can be modified easily enough, and then the new event just gets constructed with the old values (except the deltas). Something like:

if event.modifiers() & Qt.ShiftModifier:
    # Transform vertical scroll to horizontal
    angle_delta = event.angleDelta()
    angle_delta.setX(angle_delta.y())
    angle_delta.setY(0)
    pixel_delta = event.pixelDelta()
    pixel_delta.setX(pixel_delta.y())
    pixel_delta.setY(0)
    # Create new wheelEvent
    new_event = QWheelEvent(
        event.pos(), event.globalPos(),
        pixel_delta, angle_delta,
        event.buttons(), event.modifiers(),
        event.phase(), event.inverted(), event.source()
    ) 
    # Trigger parent event handler with modified event
    super().wheelEvent(new_event)

@ferdnyc
Copy link
Contributor

ferdnyc commented Nov 27, 2020

Oh! Never mind, the AngularJS scrolling actually works great. Here's the event handler I added to webkit.py:

    def wheelEvent(self, event):
        """ Mousewheel scrolling """
        if event.modifiers() & Qt.ShiftModifier:
            event.accept()
            frame = self.page().mainFrame()
            # Compute scroll offset from wheel motion
            tick_scale = 120
            steps = int(event.angleDelta().y() / tick_scale)
            delta = -(steps * 100)
            log.debug("Scrolling horizontally by %d pixels", delta)
            # Update the scroll position using AngularJS
            js = f"$('body').scope().scrollLeft({delta});"
            frame.evaluateJavaScript(js)
        else:
            super().wheelEvent(event)

And I modified the webview.py event handler to:

    def wheelEvent(self, event):
        if event.modifiers() & Qt.ControlModifier:
            event.accept()
            zoom = self.window.sliderZoom
            # For each 120 (standard scroll unit) adjust the zoom slider
            tick_scale = 120
            steps = int(event.angleDelta().y() / tick_scale)
            delta = zoom.pageStep() * steps
            log.debug("Zooming by %d steps", -steps)
            zoom.setValue(zoom.value() - delta)
        else:
            super().wheelEvent(event)

...Still doesn't make Ctrl+Wheel zooming work with the QtWebEngine backend. But that didn't work before, so not our problem (in the context of this change).

Edit: Corrected comment.

@ferdnyc
Copy link
Contributor

ferdnyc commented Nov 27, 2020

@MatthieuMeert (BTW, to see log.debug() messages, pass the --debug flag when launching OpenShot.)

@MatthieuMeert
Copy link
Author

Firstly,

Apologies, didn't mean to ignore your previous comment, somehow it snuck by me. Let's see here...

No problem with that!

Secondly,

Oh! Never mind, the AngularJS scrolling actually works great.

It's great, it indeed woks on my side too!
Thank you very much for your explanations, it was really instructive for me.

Practical question here: Should I update my pull request with your modifications or should I let you do it, since you actually wrote it?

@MatthieuMeert
Copy link
Author

As for:

Still doesn't make Ctrl+Wheel zooming work with the QtWebEngine backend.

On my computer, the Ctrl+Wheel seems to work fine using webengine as well as using webkit.

@ferdnyc
Copy link
Contributor

ferdnyc commented Nov 28, 2020

@MatthieuMeert

On my computer, the Ctrl+Wheel seems to work fine using webengine as well as using webkit.

Huh! That's interesting. I wonder what the difference is there?

Practical question here: Should I update my pull request with your modifications or should I let you do it, since you actually wrote it?

As far as I'm concerned, feel free, I'm not real hung up on attribution. I'll probably be the one who commits it anyway, so my name will end up on it regardless.

I'll take a look, maybe I can submit some of my changes as a Suggestion. Github will list me as co-author then, which feels like the most accurate representation of our roles. (And that would've been the more helpful way for me to post my changes to begin with, instead of pasting them into a comment.) But, ultimately I'm really not hung up about it.

Anyway, I'm on the subway right now, but I'll take a look when I get home in about 45 minutes.

@MatthieuMeert
Copy link
Author

I'm not especially attached to having my name on the modifications. So any way is fine for me!

I just commited the changes then, so that you don't have to do it, and so you can see the exact code with which the ctrl+scroll with webengine works on my computer.

delta = -(steps * 100)
log.debug("Scrolling horizontally by %d pixels", delta)
# Update the scroll position using AngularJS
js = f"$('body').scope().scrollLeft({delta});"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the build failure is my fault. I used an f-string, forgetting that we're still testing on Xenial which is running Python 3.5 (despite it being EOL since last spring). *sigh* Until we can get rid of Xenial when it reaches EOL in April, I guess this will have to be...

Suggested change
js = f"$('body').scope().scrollLeft({delta});"
js = "$('body').scope().scrollLeft({});".format(delta)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just going to apply that change directly, to kick off a new build and hopefully wrap this up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that was it. Now I'm really frustrated that the error message there wasn't clearer. I guess we should log the exception from the failed WebKit backend load, before raising the RuntimeError from the WebEngine exception. Otherwise the reason for the WebKit import failure gets lost.

Well, I'll fix that in another PR. For this change, merging! Many thanks @MatthieuMeert!

@ferdnyc ferdnyc changed the title WIP: Horizontal scroll of the timeline using SHIFT+scroll wheel Horizontal scroll of the timeline using SHIFT+scroll wheel Nov 28, 2020
@ferdnyc ferdnyc merged commit e27625e into OpenShot:develop Nov 28, 2020
SuslikV referenced this pull request in SuslikV/openshot-qt Nov 30, 2020
Shift + Mouse Wheel will scroll Timeline horrizontally.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants