Merge lp:~lukas-kde/qtubuntu/fixWheel into lp:qtubuntu

[Lukáš Tinkl]

Fix inconsistent mouse wheel scrolling behavior

Fix inconsistent mouse wheel scrolling behavior

Need to pass a nullptr as a target, QWindowSystemInterface will internally choose the window to send the event to based on the globalPos only (QCursor::pos() in this case).

Related branch: lp:~lukas-kde/qtmir/fixWheel

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:284
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/269/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/60
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/60/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtubuntu-ci/269/rebuild

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

Not specifying the window only for wheel events make the code inconsistent. So mouse movement can go to a window A but mouse wheel events to a window B.

It also breaks the contract of the method dispatchPointerEvent. Its first parameter is the QWindow where events are supposed to go to. It should obey it.

review: Needs Fixing

[lp:~lukas-kde/qtubuntu/fixWheel]

285. By Lukáš Tinkl on 2015-11-09

merge trunk

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:285
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/272/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/63
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/63/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtubuntu-ci/272/rebuild

review: Approve (continuous-integration)

[lp:~lukas-kde/qtubuntu/fixWheel]

286. By Lukáš Tinkl on 2015-11-09

pass wheel events to window

[Lukáš Tinkl (lukas-kde)]

> Not specifying the window only for wheel events make the code inconsistent. So
> mouse movement can go to a window A but mouse wheel events to a window B.
>
> It also breaks the contract of the method dispatchPointerEvent. Its first
> parameter is the QWindow where events are supposed to go to. It should obey
> it.

Fixed

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:286
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/273/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/64
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/64/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtubuntu-ci/273/rebuild

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

I don't think s/localPoint/QCursor::pos() makes any difference as I think we are currently not informing the global position of the cursor.

I think we can find out the global cursor position by doing:
window->position() + localPoint

What do you think?

review: Needs Information

[Daniel d'Andrada (dandrader)]

wow, I said "think" too many times on my last comment :)

[Lukáš Tinkl (lukas-kde)]

> I don't think s/localPoint/QCursor::pos() makes any difference as I think we
> are currently not informing the global position of the cursor.
>
> I think we can find out the global cursor position by doing:
> window->position() + localPoint
>
> What do you think?

OK, I can try that but still I guess we should stay as close to qtmir as possible, what do you think? :p

[Lukáš Tinkl (lukas-kde)]

For the record, both work equally fine.

[Daniel d'Andrada (dandrader)]

On 10/11/2015 17:15, Lukáš Tinkl wrote:
>> I don't think s/localPoint/QCursor::pos() makes any difference as I think we
>> are currently not informing the global position of the cursor.
>>
>> I think we can find out the global cursor position by doing:
>> window->position() + localPoint
>>
>> What do you think?
> OK, I can try that but still I guess we should stay as close to qtmir as possible, what do you think? :p

The thing is the way mir events are formatted, there's no way for
qtmir/unity8 to tell the global position of the cursor to applications.
That's why qtubuntu should do this math on its own.

[Daniel d'Andrada (dandrader)]

On 10/11/2015 17:36, Lukáš Tinkl wrote:
> For the record, both work equally fine.

Both will return the same in a single-monitor scenario.

[lp:~lukas-kde/qtubuntu/fixWheel]

287. By Lukáš Tinkl on 2015-11-10

use window->position() + localPoint() instead of globalPos

[Lukáš Tinkl (lukas-kde)]

> On 10/11/2015 17:15, Lukáš Tinkl wrote:
> >> I don't think s/localPoint/QCursor::pos() makes any difference as I think
> we
> >> are currently not informing the global position of the cursor.
> >>
> >> I think we can find out the global cursor position by doing:
> >> window->position() + localPoint
> >>
> >> What do you think?
> > OK, I can try that but still I guess we should stay as close to qtmir as
> possible, what do you think? :p
>
>
> The thing is the way mir events are formatted, there's no way for
> qtmir/unity8 to tell the global position of the cursor to applications.
> That's why qtubuntu should do this math on its own.

Ok, done

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:287
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/275/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/66
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/66/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtubuntu-ci/275/rebuild

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

Code looks ok, didn't test though.

review: Approve

[Daniel d'Andrada (dandrader)]

review: Approve (code)

[Daniel d'Andrada (dandrader)]

And works too.

review: Approve

Unmerged revisions

=== modified file 'src/ubuntumirclient/input.cpp'
--- src/ubuntumirclient/input.cpp	2015-10-26 18:27:26 +0000
+++ src/ubuntumirclient/input.cpp	2015-11-10 22:33:20 +0000
@@ -448,17 +448,16 @@
 
         if (hDelta != 0 || vDelta != 0) {
             const QPoint angleDelta = QPoint(hDelta * 15, vDelta * 15);
-            QWindowSystemInterface::handleWheelEvent(window, timestamp, localPoint, localPoint,
+            QWindowSystemInterface::handleWheelEvent(window, timestamp, localPoint, window->position() + localPoint,
                                                      QPoint(), angleDelta, modifiers, Qt::ScrollUpdate);
-        } else {
-            auto buttons = extract_buttons(pev);
-            QWindowSystemInterface::handleMouseEvent(window, timestamp, localPoint, localPoint /* Should we omit global point instead? */,
-                                                     buttons, modifiers);
         }
+        auto buttons = extract_buttons(pev);
+        QWindowSystemInterface::handleMouseEvent(window, timestamp, localPoint, window->position() + localPoint /* Should we omit global point instead? */,
+                                                 buttons, modifiers);
         break;
     }
     case mir_pointer_action_enter:
-        QWindowSystemInterface::handleEnterEvent(window, localPoint, localPoint);
+        QWindowSystemInterface::handleEnterEvent(window, localPoint, window->position() + localPoint);
         break;
     case mir_pointer_action_leave:
         QWindowSystemInterface::handleLeaveEvent(window);