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
=== modified file 'src/ubuntumirclient/qmirclientcursor.cpp'
--- src/ubuntumirclient/qmirclientcursor.cpp 2017-02-07 16:16:54 +0000
+++ src/ubuntumirclient/qmirclientcursor.cpp 2017-02-21 18:25:18 +0000
@@ -133,6 +133,28 @@
133 return "???";133 return "???";
134 }134 }
135}135}
136
137class CursorWindowSpec
138{
139public:
140 CursorWindowSpec(MirConnection *connection, const char *name)
141 : spec(mir_create_window_spec(connection))
142 {
143 mir_window_spec_set_cursor_name(spec, name);
144 }
145
146 ~CursorWindowSpec()
147 {
148 mir_window_spec_release(spec);
149 }
150
151 void apply(MirWindow *window)
152 {
153 mir_window_apply_spec(window, spec);
154 }
155private:
156 MirWindowSpec * const spec;
157};
136} // anonymous namespace158} // anonymous namespace
137159
138void QMirClientCursor::changeCursor(QCursor *windowCursor, QWindow *window)160void QMirClientCursor::changeCursor(QCursor *windowCursor, QWindow *window)
@@ -157,12 +179,8 @@
157 applyDefaultCursorConfiguration(mirWindow);179 applyDefaultCursorConfiguration(mirWindow);
158 } else {180 } else {
159 const auto &cursorName = mShapeToCursorName.value(windowCursor->shape(), QByteArray("left_ptr"));181 const auto &cursorName = mShapeToCursorName.value(windowCursor->shape(), QByteArray("left_ptr"));
160#pragma GCC diagnostic push182 CursorWindowSpec spec(mConnection, cursorName.data());
161#pragma GCC diagnostic ignored "-Wdeprecated-declarations"183 spec.apply(mirWindow);
162 auto cursorConfiguration = mir_cursor_configuration_from_name(cursorName.data());
163#pragma GCC diagnostic pop
164 mir_window_configure_cursor(mirWindow, cursorConfiguration);
165 mir_cursor_configuration_destroy(cursorConfiguration);
166 }184 }
167 } else {185 } else {
168 applyDefaultCursorConfiguration(mirWindow);186 applyDefaultCursorConfiguration(mirWindow);
@@ -206,10 +224,7 @@
206224
207void QMirClientCursor::applyDefaultCursorConfiguration(MirWindow *window)225void QMirClientCursor::applyDefaultCursorConfiguration(MirWindow *window)
208{226{
209#pragma GCC diagnostic push227
210#pragma GCC diagnostic ignored "-Wdeprecated-declarations"228 CursorWindowSpec spec(mConnection, "left_ptr");
211 auto cursorConfiguration = mir_cursor_configuration_from_name("left_ptr");229 spec.apply(window);
212#pragma GCC diagnostic pop
213 mir_window_configure_cursor(window, cursorConfiguration);
214 mir_cursor_configuration_destroy(cursorConfiguration);
215}230}
216231
=== modified file 'src/ubuntumirclient/qmirclientdebugextension.cpp'
--- src/ubuntumirclient/qmirclientdebugextension.cpp 2017-02-07 16:18:59 +0000
+++ src/ubuntumirclient/qmirclientdebugextension.cpp 2017-02-21 18:25:18 +0000
@@ -43,42 +43,31 @@
43#include "qmirclientlogging.h"43#include "qmirclientlogging.h"
4444
45// mir client debug45// mir client debug
46#include <mir_toolkit/debug/surface.h>46#include <mir_toolkit/extensions/window_coordinate_translation.h>
4747
48Q_LOGGING_CATEGORY(mirclientDebug, "qt.qpa.mirclient.debug")48Q_LOGGING_CATEGORY(mirclientDebug, "qt.qpa.mirclient.debug")
4949
50QMirClientDebugExtension::QMirClientDebugExtension()50QMirClientDebugExtension::QMirClientDebugExtension(MirConnection *connection)
51 : m_mirclientDebug(QStringLiteral("mirclient-debug-extension"), 1)51 : mExtension(mir_extension_window_coordinate_translation_v1(connection))
52 , m_mapper(nullptr)
53{52{
54 qCDebug(mirclientDebug) << "NOTICE: Loading mirclient-debug-extension";53 if (mExtension == nullptr) {
55 m_mapper = (MapperPrototype) m_mirclientDebug.resolve("mir_extension_window_coordinate_translation");54 qCWarning(mirclientDebug) << "ERROR: no window coordinate translation extension available";
56
57 if (!m_mirclientDebug.isLoaded()) {
58 qCWarning(mirclientDebug) << "ERROR: mirclient-debug-extension failed to load:"
59 << m_mirclientDebug.errorString();
60 } else if (!m_mapper) {
61 qCWarning(mirclientDebug) << "ERROR: unable to find required symbols in mirclient-debug-extension:"
62 << m_mirclientDebug.errorString();
63 }55 }
64}56}
6557
66bool QMirClientDebugExtension::isEnabled() const58bool QMirClientDebugExtension::isEnabled() const
67{59{
68 return m_mirclientDebug.isLoaded() && m_mapper;60 return mExtension != nullptr;
69}61}
7062
71QPoint QMirClientDebugExtension::mapWindowPointToScreen(MirWindow *window, const QPoint &point)63QPoint QMirClientDebugExtension::mapWindowPointToScreen(MirWindow *window, const QPoint &point)
72{64{
73 if (!m_mapper) {65 if (!mExtension) {
74 return point;66 return point;
75 }67 }
7668
77 QPoint mappedPoint;69 QPoint mappedPoint;
78 bool status = m_mapper(window, point.x(), point.y(), &mappedPoint.rx(), &mappedPoint.ry());70 mExtension->window_translate_coordinates(window, point.x(), point.y(), &mappedPoint.rx(), &mappedPoint.ry());
79 if (status) {71
80 return mappedPoint;72 return mappedPoint;
81 } else {
82 return point;
83 }
84}73}
8574
=== modified file 'src/ubuntumirclient/qmirclientdebugextension.h'
--- src/ubuntumirclient/qmirclientdebugextension.h 2017-02-07 16:16:54 +0000
+++ src/ubuntumirclient/qmirclientdebugextension.h 2017-02-21 18:25:18 +0000
@@ -42,25 +42,22 @@
42#define QMIRCLIENTDEBUGEXTENSION_H42#define QMIRCLIENTDEBUGEXTENSION_H
4343
44#include <QPoint>44#include <QPoint>
45#include <QLibrary>
4645
47#include <mir_toolkit/mir_window.h>46#include <mir_toolkit/mir_window.h>
4847
49typedef bool (*MapperPrototype)(MirWindow* window, int x, int y, int* screenX, int* screenY);48struct MirExtensionWindowCoordinateTranslationV1;
50
5149
52class QMirClientDebugExtension50class QMirClientDebugExtension
53{51{
54public:52public:
55 QMirClientDebugExtension();53 QMirClientDebugExtension(MirConnection *connection);
5654
57 bool isEnabled() const;55 bool isEnabled() const;
5856
59 QPoint mapWindowPointToScreen(MirWindow *, const QPoint &point);57 QPoint mapWindowPointToScreen(MirWindow *, const QPoint &point);
6058
61private:59private:
62 QLibrary m_mirclientDebug;60 MirExtensionWindowCoordinateTranslationV1 const *mExtension;
63 MapperPrototype m_mapper;
64};61};
6562
66#endif // QMIRCLIENTDEBUGEXTENSION_H63#endif // QMIRCLIENTDEBUGEXTENSION_H
6764
=== modified file 'src/ubuntumirclient/qmirclientintegration.cpp'
--- src/ubuntumirclient/qmirclientintegration.cpp 2017-02-07 15:37:20 +0000
+++ src/ubuntumirclient/qmirclientintegration.cpp 2017-02-21 18:25:18 +0000
@@ -163,7 +163,7 @@
163 }163 }
164 }164 }
165 if (testability) {165 if (testability) {
166 mDebugExtension.reset(new QMirClientDebugExtension);166 mDebugExtension.reset(new QMirClientDebugExtension(mMirConnection));
167 if (!mDebugExtension->isEnabled()) {167 if (!mDebugExtension->isEnabled()) {
168 mDebugExtension.reset();168 mDebugExtension.reset();
169 }169 }

Subscribers

People subscribed via source and target branches