Merge lp:~albaguirre/qtubuntu/more-new-mir-apis into lp:qtubuntu

Proposed by Alberto Aguirre on 2017-02-07
Status: Merged
Approved by: Gerry Boland on 2017-03-27
Approved revision: 381
Merged at revision: 389
Proposed branch: lp:~albaguirre/qtubuntu/more-new-mir-apis
Merge into: lp:qtubuntu
Prerequisite: lp:~artmello/qtubuntu/qtubuntu-new_mir_api
Diff against target: 164 lines (+41/-40)
4 files modified
src/ubuntumirclient/qmirclientcursor.cpp (+27/-12)
src/ubuntumirclient/qmirclientdebugextension.cpp (+10/-21)
src/ubuntumirclient/qmirclientdebugextension.h (+3/-6)
src/ubuntumirclient/qmirclientintegration.cpp (+1/-1)
To merge this branch: bzr merge lp:~albaguirre/qtubuntu/more-new-mir-apis
Reviewer Review Type Date Requested Status
Gerry Boland (community) Approve on 2017-03-27
Daniel d'Andrada (community) 2017-02-07 Approve on 2017-02-21
Unity8 CI Bot continuous-integration Approve on 2017-02-21
Review via email: mp+316646@code.launchpad.net

Commit message

Remove more uses of deprecated mir apis.

- Use proper extension for translating window coordinates
- Use a window spec to apply a cursor name to a window.

To post a comment you must log in.
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:380
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/175/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4040
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4068
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3909
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3909/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3909
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3909/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3909
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3909/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3909
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3909/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3909
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3909/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3909
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3909/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Daniel d'Andrada (dandrader) wrote :

Code makes sense, but it breaks the cursor. Ie, it no longer changes shape. Maybe it needs changes also on the qtmir side to match?

To test:
$ bzr branch lp:~dandrader/+junk/animatedDemos
$ cd animatedDemos/CustomCursor
$ qmake && make
$ cd ..
$ qmlscene CursorShapes.qml -I . -- --desktop_file_hint=/usr/share/applications/gedit.desktop

Currently there's a problem somewhere (didn't investigate yet) that several of the named cursors are not working anymore. But still, the following ones still work:
UpArrow, Cross, WhatsThis, DragCopy, Busy, DragMove, DragLink.
The big boat an football ball custom cursors should also be set and unset as expected

With this patch, no named cursor work anymore. Also once you hover over the big football ball, it never goes back to the arrow cursor.

review: Needs Fixing
Daniel d'Andrada (dandrader) wrote :

For reference, it's bug 1662827

Alberto Aguirre (albaguirre) wrote :

@Daniel, yeah probably a new codepath needed in qtmir.

Cursor name changes work (with this branch) when I connect directly to a mir_demo_server instance.

Daniel d'Andrada (dandrader) wrote :

> @Daniel, yeah probably a new codepath needed in qtmir.

Will check the qtmir part.

Daniel d'Andrada (dandrader) wrote :

> > @Daniel, yeah probably a new codepath needed in qtmir.
>
> Will check the qtmir part.

alan_g said this actually might be a bug in mir: https://bugs.launchpad.net/mir/+bug/1663197

Daniel d'Andrada (dandrader) wrote :
Download full text (3.3 KiB)

We should be able to merge it now that the fix for bug 1663197 has been released. Unfortunately this branch now conflicts with latest trunk.

"""
$ bzr merge lp:~albaguirre/qtubuntu/more-new-mir-apis
Not attempting to fix packaging branch ancestry, missing pristine tar data for version 0.63+17.04.20170119.2.
 M src/ubuntumirclient/qmirclientcursor.cpp
 M src/ubuntumirclient/qmirclientdebugextension.cpp
 M src/ubuntumirclient/qmirclientdebugextension.h
 M src/ubuntumirclient/qmirclientintegration.cpp
 M src/ubuntumirclient/qmirclientwindow.cpp
Text conflict in src/ubuntumirclient/qmirclientcursor.cpp
Text conflict in src/ubuntumirclient/qmirclientdebugextension.cpp
Text conflict in src/ubuntumirclient/qmirclientdebugextension.h
Text conflict in src/ubuntumirclient/qmirclientwindow.cpp ...

Read more...

review: Needs Fixing
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:381
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/188/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4160
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4188
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4026
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4026/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4026
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4026/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4026
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4026/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4026
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4026/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4026
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4026/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4026
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4026/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Daniel d'Andrada (dandrader) wrote :

Thanks!

review: Approve
Gerry Boland (gerboland) wrote :

https://bugs.launchpad.net/miral/+bug/1670876
The new cursor api has a bug which this exposes. I think we need to hold off accepting this until that bug is fixed.

Gerry Boland (gerboland) wrote :

Mir 0.26.2 released with fix for this aforementioned bug, this should be good to go now

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/ubuntumirclient/qmirclientcursor.cpp'
2--- src/ubuntumirclient/qmirclientcursor.cpp 2017-02-07 16:16:54 +0000
3+++ src/ubuntumirclient/qmirclientcursor.cpp 2017-02-21 18:25:18 +0000
4@@ -133,6 +133,28 @@
5 return "???";
6 }
7 }
8+
9+class CursorWindowSpec
10+{
11+public:
12+ CursorWindowSpec(MirConnection *connection, const char *name)
13+ : spec(mir_create_window_spec(connection))
14+ {
15+ mir_window_spec_set_cursor_name(spec, name);
16+ }
17+
18+ ~CursorWindowSpec()
19+ {
20+ mir_window_spec_release(spec);
21+ }
22+
23+ void apply(MirWindow *window)
24+ {
25+ mir_window_apply_spec(window, spec);
26+ }
27+private:
28+ MirWindowSpec * const spec;
29+};
30 } // anonymous namespace
31
32 void QMirClientCursor::changeCursor(QCursor *windowCursor, QWindow *window)
33@@ -157,12 +179,8 @@
34 applyDefaultCursorConfiguration(mirWindow);
35 } else {
36 const auto &cursorName = mShapeToCursorName.value(windowCursor->shape(), QByteArray("left_ptr"));
37-#pragma GCC diagnostic push
38-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
39- auto cursorConfiguration = mir_cursor_configuration_from_name(cursorName.data());
40-#pragma GCC diagnostic pop
41- mir_window_configure_cursor(mirWindow, cursorConfiguration);
42- mir_cursor_configuration_destroy(cursorConfiguration);
43+ CursorWindowSpec spec(mConnection, cursorName.data());
44+ spec.apply(mirWindow);
45 }
46 } else {
47 applyDefaultCursorConfiguration(mirWindow);
48@@ -206,10 +224,7 @@
49
50 void QMirClientCursor::applyDefaultCursorConfiguration(MirWindow *window)
51 {
52-#pragma GCC diagnostic push
53-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
54- auto cursorConfiguration = mir_cursor_configuration_from_name("left_ptr");
55-#pragma GCC diagnostic pop
56- mir_window_configure_cursor(window, cursorConfiguration);
57- mir_cursor_configuration_destroy(cursorConfiguration);
58+
59+ CursorWindowSpec spec(mConnection, "left_ptr");
60+ spec.apply(window);
61 }
62
63=== modified file 'src/ubuntumirclient/qmirclientdebugextension.cpp'
64--- src/ubuntumirclient/qmirclientdebugextension.cpp 2017-02-07 16:18:59 +0000
65+++ src/ubuntumirclient/qmirclientdebugextension.cpp 2017-02-21 18:25:18 +0000
66@@ -43,42 +43,31 @@
67 #include "qmirclientlogging.h"
68
69 // mir client debug
70-#include <mir_toolkit/debug/surface.h>
71+#include <mir_toolkit/extensions/window_coordinate_translation.h>
72
73 Q_LOGGING_CATEGORY(mirclientDebug, "qt.qpa.mirclient.debug")
74
75-QMirClientDebugExtension::QMirClientDebugExtension()
76- : m_mirclientDebug(QStringLiteral("mirclient-debug-extension"), 1)
77- , m_mapper(nullptr)
78+QMirClientDebugExtension::QMirClientDebugExtension(MirConnection *connection)
79+ : mExtension(mir_extension_window_coordinate_translation_v1(connection))
80 {
81- qCDebug(mirclientDebug) << "NOTICE: Loading mirclient-debug-extension";
82- m_mapper = (MapperPrototype) m_mirclientDebug.resolve("mir_extension_window_coordinate_translation");
83-
84- if (!m_mirclientDebug.isLoaded()) {
85- qCWarning(mirclientDebug) << "ERROR: mirclient-debug-extension failed to load:"
86- << m_mirclientDebug.errorString();
87- } else if (!m_mapper) {
88- qCWarning(mirclientDebug) << "ERROR: unable to find required symbols in mirclient-debug-extension:"
89- << m_mirclientDebug.errorString();
90+ if (mExtension == nullptr) {
91+ qCWarning(mirclientDebug) << "ERROR: no window coordinate translation extension available";
92 }
93 }
94
95 bool QMirClientDebugExtension::isEnabled() const
96 {
97- return m_mirclientDebug.isLoaded() && m_mapper;
98+ return mExtension != nullptr;
99 }
100
101 QPoint QMirClientDebugExtension::mapWindowPointToScreen(MirWindow *window, const QPoint &point)
102 {
103- if (!m_mapper) {
104+ if (!mExtension) {
105 return point;
106 }
107
108 QPoint mappedPoint;
109- bool status = m_mapper(window, point.x(), point.y(), &mappedPoint.rx(), &mappedPoint.ry());
110- if (status) {
111- return mappedPoint;
112- } else {
113- return point;
114- }
115+ mExtension->window_translate_coordinates(window, point.x(), point.y(), &mappedPoint.rx(), &mappedPoint.ry());
116+
117+ return mappedPoint;
118 }
119
120=== modified file 'src/ubuntumirclient/qmirclientdebugextension.h'
121--- src/ubuntumirclient/qmirclientdebugextension.h 2017-02-07 16:16:54 +0000
122+++ src/ubuntumirclient/qmirclientdebugextension.h 2017-02-21 18:25:18 +0000
123@@ -42,25 +42,22 @@
124 #define QMIRCLIENTDEBUGEXTENSION_H
125
126 #include <QPoint>
127-#include <QLibrary>
128
129 #include <mir_toolkit/mir_window.h>
130
131-typedef bool (*MapperPrototype)(MirWindow* window, int x, int y, int* screenX, int* screenY);
132-
133+struct MirExtensionWindowCoordinateTranslationV1;
134
135 class QMirClientDebugExtension
136 {
137 public:
138- QMirClientDebugExtension();
139+ QMirClientDebugExtension(MirConnection *connection);
140
141 bool isEnabled() const;
142
143 QPoint mapWindowPointToScreen(MirWindow *, const QPoint &point);
144
145 private:
146- QLibrary m_mirclientDebug;
147- MapperPrototype m_mapper;
148+ MirExtensionWindowCoordinateTranslationV1 const *mExtension;
149 };
150
151 #endif // QMIRCLIENTDEBUGEXTENSION_H
152
153=== modified file 'src/ubuntumirclient/qmirclientintegration.cpp'
154--- src/ubuntumirclient/qmirclientintegration.cpp 2017-02-07 15:37:20 +0000
155+++ src/ubuntumirclient/qmirclientintegration.cpp 2017-02-21 18:25:18 +0000
156@@ -163,7 +163,7 @@
157 }
158 }
159 if (testability) {
160- mDebugExtension.reset(new QMirClientDebugExtension);
161+ mDebugExtension.reset(new QMirClientDebugExtension(mMirConnection));
162 if (!mDebugExtension->isEnabled()) {
163 mDebugExtension.reset();
164 }

Subscribers

People subscribed via source and target branches