Merge into trunk : reparent-cursor : Code : Unity 8

Status:

Rejected

Rejected by:

Lukáš Tinkl on 2017-03-07

Proposed branch:

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

Merge into:

lp:unity8

Diff against target:

29 lines (+5/-0)

1 file modified

plugins/Cursor/MousePointer.cpp (+5/-0)

To merge this branch:

bzr merge lp:~lukas-kde/unity8/reparent-cursor

Undecided

New

Link a bug report

Reviewer	Review Type	Date Requested	Status
Daniel d'Andrada (community)		2017-03-06	Disapprove on 2017-03-07
Unity8 CI Bot	continuous-integration		Approve on 2017-03-07
Review via email: mp+319135@code.launchpad.net

Commit Message

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

Description of the Change

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) wrote on 2017-03-07:

#

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) wrote on 2017-03-07:

#

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) wrote on 2017-03-07:

#

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) wrote on 2017-03-07:

#

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) wrote on 2017-03-07:

#

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

1	=== modified file 'plugins/Cursor/MousePointer.cpp'
2	--- plugins/Cursor/MousePointer.cpp 2016-09-07 08:36:59 +0000
3	+++ plugins/Cursor/MousePointer.cpp 2017-03-06 22:09:45 +0000
4	@@ -23,6 +23,7 @@
5	#include <QQuickWindow>
6	#include <QGuiApplication>
7	#include <QtMath>
8	+#include <QQuickView>
9
10	#include <qpa/qwindowsysteminterface.h>
11
12	@@ -151,6 +152,7 @@
13	if (change == ItemSceneChange) {
14	registerWindow(value.window);
15	}
16	+ QQuickItem::itemChange(change, value);
17	}
18
19	void MousePointer::registerWindow(QWindow *window)
20	@@ -166,6 +168,9 @@
21	m_registeredWindow = window;
22
23	if (m_registeredWindow) {
24	+ if (m_registeredWindow->inherits("QQuickView")) {
25	+ setParentItem(static_cast<QQuickView *>(window)->rootObject());
26	+ }
27	connect(window, &QWindow::screenChanged, this, &MousePointer::registerScreen);
28	registerScreen(window->screen());
29	} else {

Unity 8

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

Commit Message

Description of the Change

Unmerged revisions

Preview Diff

Subscribers