Merge lp:~lukas-kde/unity8/reparent-cursor into lp:unity8

[Lukáš Tinkl]

Reparent Cursor/MousePointer to the toplevel rootObject so that it's above everything

Reparent Cursor/MousePointer to the toplevel rootObject so that it's above everything

... including popups that UITK creates

Fixes lp:1667928

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:2840
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3290/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4329
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2585
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2585
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4357
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4191
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4191/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4191
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4191/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4191
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4191/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4191
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4191/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4191
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4191/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4191
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4191/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3290/rebuild

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

QML code decides the parent when it declares the MousePointer (epoxed as Cursor by the Cursor plugin). A qml item shouldn't be magically reparenting itself, completely ignoring what you would expect by reading the qml code.

UITK is the one doing the hacky magical reparenting. We shouldn't go down that route as well just to counter it.

If I'm not mistaken, UITK recently added support for users to choose the "visual parent" (I think that's how it's called) of some components exactly to solve those problems caused by magical reparenting

review: Disapprove

[Lukáš Tinkl (lukas-kde)]

This is not a workaround for that specific "hack" that UITK is doing, imagine all app authors can do the same; it's not a bad thing per se as it works just fine for apps. We're just having this problem only in our shell where our topmost QQuickWindow/QQuickView is in ShellView.cpp.

Do you have a better idea to solve this bug?

[Michael Zanetti (mzanetti)]

I tend to agree with Daniel that this feels very wrong... The thing is, that UITK already does this and with that kinda forces us to do the same... The only other other way I can see would be to change the uitk components to not behave like that. That would actually be better because it also has other issues, like wrong orientations etc (given those uitk elements are parented to above OrientedShell and with that won't get rotation inherited).

Question is really whether at this point it makes sense to start changing those uitk elements... Many apps and other stuff rely on this behavior by now and it feels like a lot of work to make that right again.

Another idea that might get us around this could be to not reparent things inside the MousePointer itself, but instead create the Cursor object from our main.cpp and parent it to the root item from there? This way we would not have the magic hidden inside the cursor, but actually set the parent from the place where we instantiate the cursor. wdyt?

[Daniel d'Andrada (dandrader)]

On 07/03/2017 08:22, Lukáš Tinkl wrote:
> This is not a workaround for that specific "hack" that UITK is doing, imagine all app authors can do the same; it's not a bad thing per se as it works just fine for apps. We're just having this problem only in our shell where our topmost QQuickWindow/QQuickView is in ShellView.cpp.
>
> Do you have a better idea to solve this bug?

I just did in my original comment. That "visual parent" thing I recall
having seem.

In any case, I think this will also cause a bug when Shell gets rotated
by OrientedShell. The cursor position and movement will be all wrong.

Unmerged revisions

2840. By Lukáš Tinkl on 2017-03-06

reparent Cursor to the toplevel rootObject so that it's above everything

including popups that UITK creates

=== modified file 'plugins/Cursor/MousePointer.cpp'
--- plugins/Cursor/MousePointer.cpp	2016-09-07 08:36:59 +0000
+++ plugins/Cursor/MousePointer.cpp	2017-03-06 22:09:45 +0000
@@ -23,6 +23,7 @@
 #include <QQuickWindow>
 #include <QGuiApplication>
 #include <QtMath>
+#include <QQuickView>
 
 #include <qpa/qwindowsysteminterface.h>
 
@@ -151,6 +152,7 @@
     if (change == ItemSceneChange) {
         registerWindow(value.window);
     }
+    QQuickItem::itemChange(change, value);
 }
 
 void MousePointer::registerWindow(QWindow *window)
@@ -166,6 +168,9 @@
     m_registeredWindow = window;
 
     if (m_registeredWindow) {
+        if (m_registeredWindow->inherits("QQuickView")) {
+            setParentItem(static_cast<QQuickView *>(window)->rootObject());
+        }
         connect(window, &QWindow::screenChanged, this, &MousePointer::registerScreen);
         registerScreen(window->screen());
     } else {