Merge lp:~gerboland/qtubuntu/exposeOrientation into lp:qtubuntu

Proposed by Gerry Boland
Status: Merged
Approved by: Daniel d'Andrada
Approved revision: 249
Merged at revision: 243
Proposed branch: lp:~gerboland/qtubuntu/exposeOrientation
Merge into: lp:qtubuntu
Diff against target: 323 lines (+120/-54)
7 files modified
debian/control (+1/-1)
src/ubuntumirclient/input.cpp (+74/-0)
src/ubuntumirclient/input.h (+1/-0)
src/ubuntumirclient/orientationchangeevent_p.h (+36/-0)
src/ubuntumirclient/screen.cpp (+7/-45)
src/ubuntumirclient/screen.h (+0/-8)
src/ubuntumirclient/ubuntumirclient.pro (+1/-0)
To merge this branch: bzr merge lp:~gerboland/qtubuntu/exposeOrientation
Reviewer Review Type Date Requested Status
Daniel d'Andrada (community) Approve
PS Jenkins bot continuous-integration Needs Fixing
Review via email: mp+232252@code.launchpad.net

Commit message

Expose Mir surface orientation property instead of connecting to orientation sensor directly

To post a comment you must log in.
244. By Gerry Boland

Depend on PAPI 2.2.1

245. By Gerry Boland

Alphabetize

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

In src/ubuntumirclient/input.cpp

"""
    LOG("ORIENTATION direction:%d", event->orientation.direction);
"""

Convert the enum value to a nice human-readable string, please.

""""
    Qt::ScreenOrientation orientation;
    switch (event->orientation.direction) {
    case U_ORIENTATION_NORMAL:
        orientation = Qt::PortraitOrientation;
        break;
    case U_ORIENTATION_LEFT:
        orientation = Qt::LandscapeOrientation;
        break;
    case U_ORIENTATION_INVERTED:
        orientation = Qt::InvertedPortraitOrientation;
        break;
    case U_ORIENTATION_RIGHT:
        orientation = Qt::InvertedLandscapeOrientation;
        break;
    default:
        DLOG("No such orientation %d", event->orientation.direction);
        return;
    }

    // Dispatch orientation event to [Platform]Screen, as that is where Qt reads it. Screen will handle
    // notifying Qt of the actual orientation change - done to prevent multiple Windows each creating
    // an identical orientation change event and passing it directly to Qt.
    // [Platform]Screen can also factor in the native orientation.
    QCoreApplication::postEvent(static_cast<UbuntuScreen*>(window->screen()->handle()),
                                new OrientationChangeEvent(OrientationChangeEvent::mType, orientation));
""""

Then please use the same nomenclature that the mir event uses in that internal OrientationChangeEvent, thus avoiding mentioning landscape or portrait at this point, which is misleading. You could stick with the QOrientationReading::Orientation enum.

review: Needs Fixing
Revision history for this message
Gerry Boland (gerboland) wrote :

> Then please use the same nomenclature that the mir event uses in that internal
> OrientationChangeEvent, thus avoiding mentioning landscape or portrait at this
> point, which is misleading. You could stick with the
> QOrientationReading::Orientation enum.

Can you explain your reasoning why this is misleading? We need to convert from Mir event nomenclature (normal,left,right,inverted) to QScreenOrientation (portrait, landscape, invertedlandscape, invertedportrait) somewhere, it struck me as a good idea to convert to Qt's definition as quickly as possible.

Do I understand you want me to add an intermediary 3rd nomenclature, the QOrientationReading::Orientation. So then we have Mir->QOrientationReading->QScreenOrientation ?

246. By Gerry Boland

Add text description for a debug output

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> > Then please use the same nomenclature that the mir event uses in that
> internal
> > OrientationChangeEvent, thus avoiding mentioning landscape or portrait at
> this
> > point, which is misleading. You could stick with the
> > QOrientationReading::Orientation enum.
>
> Can you explain your reasoning why this is misleading? We need to convert from
> Mir event nomenclature (normal,left,right,inverted) to QScreenOrientation
> (portrait, landscape, invertedlandscape, invertedportrait) somewhere, it
> struck me as a good idea to convert to Qt's definition as quickly as possible.

Because in UbuntuInput::dispatchOrientationEvent you're doing a wrong conversion as you're assuming that the native orientation is portrait, just to later fix it in QScreen. By converting it to QOrientationReading you don't have to make any such assumptions. For what it's worth, QOrientationReading is also a "Qt definition".

>
> Do I understand you want me to add an intermediary 3rd nomenclature, the
> QOrientationReading::Orientation. So then we have
> Mir->QOrientationReading->QScreenOrientation ?

Yes. If I understood QOrientationReading correctly, it's a one-to-one mapping of the Mir enum.

But if you don't like that intermediate enum, then I'm fine if you store the Mir enum value in that internal QEvent as well. I just don't want the wrong conversion I mentioned above.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

"""
LOG("ORIENTATION direction: %d", nativeOrientationDirectionToStr(event->orientation.direction));
"""

Should be '%s', not '%d'. Did it work at all like that?

review: Needs Fixing
247. By Gerry Boland

oops, %s not %d

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
248. By Gerry Boland

Use QOrientationReading::Orientation in the OrientationChangeEvent

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

In src/ubuntumirclient/screen.cpp:

"""
- case QOrientationReading::LeftUp: {
+ case QOrientationReading::RightUp: {
             mCurrentOrientation = (mNativeOrientation == Qt::LandscapeOrientation) ?
- Qt::InvertedPortraitOrientation : Qt::LandscapeOrientation;
+ Qt::InvertedPortraitOrientation : Qt::InvertedLandscapeOrientation;
"""

This is wrong. The original code had the correct conversion.

"""
- case QOrientationReading::RightUp: {
+ case QOrientationReading::LeftUp: {
             mCurrentOrientation = (mNativeOrientation == Qt::LandscapeOrientation) ?
- Qt::PortraitOrientation : Qt::InvertedLandscapeOrientation;
+ Qt::PortraitOrientation : Qt::LandscapeOrientation;
"""

Same here.

See https://www.dropbox.com/s/4ctoecuq4269j5m/QtScreenOrientation.png?dl=0

review: Needs Fixing
249. By Gerry Boland

Undo changes to mapping of OrientationReading->ScreenOrientation, better the old way

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

Looking good.

review: Approve
250. By Gerry Boland

Dump dependency on papi to 2.4.0

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/control'
--- debian/control 2014-07-09 17:17:05 +0000
+++ debian/control 2014-09-19 12:18:46 +0000
@@ -8,7 +8,7 @@
8 libfreetype6-dev,8 libfreetype6-dev,
9 libgles2-mesa-dev,9 libgles2-mesa-dev,
10 libglib2.0-dev,10 libglib2.0-dev,
11 libubuntu-application-api-dev (>= 2.2.0),11 libubuntu-application-api-dev (>= 2.4.0),
12 libudev-dev,12 libudev-dev,
13 libxrender-dev,13 libxrender-dev,
14 libxkbcommon-dev,14 libxkbcommon-dev,
1515
=== modified file 'src/ubuntumirclient/input.cpp'
--- src/ubuntumirclient/input.cpp 2014-08-14 09:45:04 +0000
+++ src/ubuntumirclient/input.cpp 2014-09-19 12:18:46 +0000
@@ -18,8 +18,10 @@
18#include "input.h"18#include "input.h"
19#include "integration.h"19#include "integration.h"
20#include "nativeinterface.h"20#include "nativeinterface.h"
21#include "screen.h"
21#include "window.h"22#include "window.h"
22#include "logging.h"23#include "logging.h"
24#include "orientationchangeevent_p.h"
2325
24// Qt26// Qt
25#if !defined(QT_NO_DEBUG)27#if !defined(QT_NO_DEBUG)
@@ -36,6 +38,7 @@
3638
37// Platform API39// Platform API
38#include <ubuntu/application/ui/input/event.h>40#include <ubuntu/application/ui/input/event.h>
41#include <ubuntu/application/ui/window_orientation.h>
3942
40#define LOG_EVENTS 043#define LOG_EVENTS 0
4144
@@ -174,6 +177,10 @@
174 break;177 break;
175 case SURFACE_WEVENT_TYPE:178 case SURFACE_WEVENT_TYPE:
176 return "SURFACE_WEVENT_TYPE";179 return "SURFACE_WEVENT_TYPE";
180 break;
181 case ORIENTATION_WEVENT_TYPE:
182 return "ORIENTATION_WEVENT_TYPE";
183 break;
177 default:184 default:
178 return "INVALID!";185 return "INVALID!";
179 }186 }
@@ -218,6 +225,9 @@
218 ubuntuEvent->window->handleSurfaceFocusChange(nativeEvent->surface.value == 1);225 ubuntuEvent->window->handleSurfaceFocusChange(nativeEvent->surface.value == 1);
219 }226 }
220 break;227 break;
228 case ORIENTATION_WEVENT_TYPE:
229 dispatchOrientationEvent(ubuntuEvent->window->window(), nativeEvent);
230 break;
221 default:231 default:
222 DLOG("unhandled event type %d", nativeEvent->type);232 DLOG("unhandled event type %d", nativeEvent->type);
223 }233 }
@@ -408,3 +418,67 @@
408418
409 QWindowSystemInterface::handleKeyEvent(window, timestamp, keyType, sym, modifiers, text);419 QWindowSystemInterface::handleKeyEvent(window, timestamp, keyType, sym, modifiers, text);
410}420}
421
422#if (LOG_EVENTS != 0)
423static const char* nativeOrientationDirectionToStr(const int orientation)
424{
425 switch (orientation) {
426 case U_ORIENTATION_NORMAL:
427 return "Normal";
428 break;
429 case U_ORIENTATION_LEFT:
430 return "Left";
431 break;
432 case U_ORIENTATION_INVERTED:
433 return "Inverted";
434 break;
435 case U_ORIENTATION_RIGHT:
436 return "Right";
437 break;
438 default:
439 return "INVALID!";
440 }
441}
442#endif
443
444void UbuntuInput::dispatchOrientationEvent(QWindow* window, const void* ev)
445{
446 const WindowEvent* event = reinterpret_cast<const WindowEvent*>(ev);
447
448 #if (LOG_EVENTS != 0)
449 // Orientation event logging.
450 LOG("ORIENTATION direction: %s", nativeOrientationDirectionToStr(event->orientation.direction));
451 #endif
452
453 if (!window->screen()) {
454 DLOG("Window has no associated screen, dropping orientation event");
455 return;
456 }
457
458 QOrientationReading::Orientation orientation;
459 switch (event->orientation.direction) {
460 case U_ORIENTATION_NORMAL:
461 orientation = QOrientationReading::TopUp;
462 break;
463 case U_ORIENTATION_LEFT:
464 orientation = QOrientationReading::LeftUp;
465 break;
466 case U_ORIENTATION_INVERTED:
467 orientation = QOrientationReading::TopDown;
468 break;
469 case U_ORIENTATION_RIGHT:
470 orientation = QOrientationReading::RightUp;
471 break;
472 default:
473 DLOG("No such orientation %d", event->orientation.direction);
474 return;
475 }
476
477 // Dispatch orientation event to [Platform]Screen, as that is where Qt reads it. Screen will handle
478 // notifying Qt of the actual orientation change - done to prevent multiple Windows each creating
479 // an identical orientation change event and passing it directly to Qt.
480 // [Platform]Screen can also factor in the native orientation.
481 QCoreApplication::postEvent(static_cast<UbuntuScreen*>(window->screen()->handle()),
482 new OrientationChangeEvent(OrientationChangeEvent::mType, orientation));
483}
484
411485
=== modified file 'src/ubuntumirclient/input.h'
--- src/ubuntumirclient/input.h 2014-07-15 12:45:14 +0000
+++ src/ubuntumirclient/input.h 2014-09-19 12:18:46 +0000
@@ -40,6 +40,7 @@
40protected:40protected:
41 void dispatchKeyEvent(QWindow* window, const void* event);41 void dispatchKeyEvent(QWindow* window, const void* event);
42 void dispatchMotionEvent(QWindow* window, const void* event);42 void dispatchMotionEvent(QWindow* window, const void* event);
43 void dispatchOrientationEvent(QWindow* window, const void* event);
4344
44private:45private:
45 UbuntuClientIntegration* mIntegration;46 UbuntuClientIntegration* mIntegration;
4647
=== added file 'src/ubuntumirclient/orientationchangeevent_p.h'
--- src/ubuntumirclient/orientationchangeevent_p.h 1970-01-01 00:00:00 +0000
+++ src/ubuntumirclient/orientationchangeevent_p.h 2014-09-19 12:18:46 +0000
@@ -0,0 +1,36 @@
1/*
2 * Copyright (C) 2014 Canonical, Ltd.
3 *
4 * This program is free software: you can redistribute it and/or modify it under
5 * the terms of the GNU Lesser General Public License version 3, as published by
6 * the Free Software Foundation.
7 *
8 * This program is distributed in the hope that it will be useful, but WITHOUT
9 * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
10 * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
11 * Lesser General Public License for more details.
12 *
13 * You should have received a copy of the GNU Lesser General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 */
16
17#ifndef ORIENTATIONCHANGEEVENT_P_H
18#define ORIENTATIONCHANGEEVENT_P_H
19
20#include <QEvent>
21#include <QOrientationReading>
22#include "logging.h"
23
24class OrientationChangeEvent : public QEvent {
25public:
26 OrientationChangeEvent(QEvent::Type type, QOrientationReading::Orientation orientation)
27 : QEvent(type)
28 , mOrientation(orientation)
29 {
30 }
31
32 static const QEvent::Type mType;
33 QOrientationReading::Orientation mOrientation;
34};
35
36#endif // ORIENTATIONCHANGEEVENT_P_H
037
=== modified file 'src/ubuntumirclient/screen.cpp'
--- src/ubuntumirclient/screen.cpp 2014-06-30 14:40:47 +0000
+++ src/ubuntumirclient/screen.cpp 2014-09-19 12:18:46 +0000
@@ -22,13 +22,10 @@
22#include <qpa/qwindowsysteminterface.h>22#include <qpa/qwindowsysteminterface.h>
23#include <QtPlatformSupport/private/qeglconvenience_p.h>23#include <QtPlatformSupport/private/qeglconvenience_p.h>
2424
25// Qt sensors
26#include <QtSensors/QOrientationSensor>
27#include <QtSensors/QOrientationReading>
28
29// local25// local
30#include "screen.h"26#include "screen.h"
31#include "logging.h"27#include "logging.h"
28#include "orientationchangeevent_p.h"
3229
33// platform-api30// platform-api
34#include <ubuntu/application/ui/display.h>31#include <ubuntu/application/ui/display.h>
@@ -85,21 +82,8 @@
85}82}
86#endif83#endif
8784
88class OrientationReadingEvent : public QEvent {
89public:
90 OrientationReadingEvent(QEvent::Type type, QOrientationReading::Orientation orientation)
91 : QEvent(type)
92 , mOrientation(orientation) {
93 DLOG("OrientationReadingEvent::OrientationReadingEvent()");
94 }
95 ~OrientationReadingEvent() {
96 DLOG("OrientationReadingEvent::~OrientationReadingEvent()");
97 }
98 static const QEvent::Type mType;
99 QOrientationReading::Orientation mOrientation;
100};
10185
102const QEvent::Type OrientationReadingEvent::mType =86const QEvent::Type OrientationChangeEvent::mType =
103 static_cast<QEvent::Type>(QEvent::registerEventType());87 static_cast<QEvent::Type>(QEvent::registerEventType());
10488
10589
@@ -171,31 +155,17 @@
171155
172 // If it's a landscape device (i.e. some tablets), start in landscape, otherwise portrait156 // If it's a landscape device (i.e. some tablets), start in landscape, otherwise portrait
173 mCurrentOrientation = (mNativeOrientation == Qt::LandscapeOrientation) ? Qt::LandscapeOrientation : Qt::PortraitOrientation;157 mCurrentOrientation = (mNativeOrientation == Qt::LandscapeOrientation) ? Qt::LandscapeOrientation : Qt::PortraitOrientation;
174
175 mOrientationSensor = new QOrientationSensor();
176 QObject::connect(mOrientationSensor, SIGNAL(readingChanged()), this, SLOT(onOrientationReadingChanged()));
177 mOrientationSensor->start();
178}158}
179159
180UbuntuScreen::~UbuntuScreen()160UbuntuScreen::~UbuntuScreen()
181{161{
182 eglTerminate(mEglDisplay);162 eglTerminate(mEglDisplay);
183 delete mOrientationSensor;
184}
185
186void UbuntuScreen::toggleSensors(const bool enable) const {
187 DLOG("UbuntuScreen::toggleSensors (this=%p, enable=%d)", this, enable);
188 if (enable)
189 mOrientationSensor->start();
190 else
191 mOrientationSensor->stop();
192}163}
193164
194void UbuntuScreen::customEvent(QEvent* event) {165void UbuntuScreen::customEvent(QEvent* event) {
195 DLOG("UbuntuScreen::customEvent (event: %p)", event);
196 DASSERT(QThread::currentThread() == thread());166 DASSERT(QThread::currentThread() == thread());
197167
198 OrientationReadingEvent* oReadingEvent = static_cast<OrientationReadingEvent*>(event);168 OrientationChangeEvent* oReadingEvent = static_cast<OrientationChangeEvent*>(event);
199 switch (oReadingEvent->mOrientation) {169 switch (oReadingEvent->mOrientation) {
200 case QOrientationReading::LeftUp: {170 case QOrientationReading::LeftUp: {
201 mCurrentOrientation = (mNativeOrientation == Qt::LandscapeOrientation) ?171 mCurrentOrientation = (mNativeOrientation == Qt::LandscapeOrientation) ?
@@ -218,20 +188,12 @@
218 break;188 break;
219 }189 }
220 default: {190 default: {
221 DLOG("Unknown orientation.");191 DLOG("UbuntuScreen::customEvent - Unknown orientation.");
192 return;
222 }193 }
223 }194 }
224195
225 // Raise the event signal so that client apps know the orientation changed196 // Raise the event signal so that client apps know the orientation changed
226 QWindowSystemInterface::handleScreenOrientationChange(screen(), mCurrentOrientation);197 QWindowSystemInterface::handleScreenOrientationChange(screen(), mCurrentOrientation);
227}198 DLOG("UbuntuScreen::customEvent - handling orientation change to %d", mCurrentOrientation);
228199}
229void UbuntuScreen::onOrientationReadingChanged() {
230 DLOG("UbuntuScreen::onOrientationReadingChanged");
231 DASSERT(mOrientationSensor != NULL);
232
233 // Make sure to switch to the main Qt thread context
234 QCoreApplication::postEvent(this, new OrientationReadingEvent(
235 OrientationReadingEvent::mType, mOrientationSensor->reading()->orientation()));
236}
237
238200
=== modified file 'src/ubuntumirclient/screen.h'
--- src/ubuntumirclient/screen.h 2014-06-30 14:40:47 +0000
+++ src/ubuntumirclient/screen.h 2014-09-19 12:18:46 +0000
@@ -21,8 +21,6 @@
21#include <QSurfaceFormat>21#include <QSurfaceFormat>
22#include <EGL/egl.h>22#include <EGL/egl.h>
2323
24class QOrientationSensor;
25
26class UbuntuScreen : public QObject, public QPlatformScreen24class UbuntuScreen : public QObject, public QPlatformScreen
27{25{
28 Q_OBJECT26 Q_OBJECT
@@ -44,20 +42,14 @@
44 EGLConfig eglConfig() const { return mEglConfig; }42 EGLConfig eglConfig() const { return mEglConfig; }
45 EGLNativeDisplayType eglNativeDisplay() const { return mEglNativeDisplay; }43 EGLNativeDisplayType eglNativeDisplay() const { return mEglNativeDisplay; }
4644
47 void toggleSensors(bool enable) const;
48
49 // QObject methods.45 // QObject methods.
50 void customEvent(QEvent* event);46 void customEvent(QEvent* event);
5147
52public Q_SLOTS:
53 void onOrientationReadingChanged();
54
55private:48private:
56 QRect mGeometry;49 QRect mGeometry;
57 QRect mAvailableGeometry;50 QRect mAvailableGeometry;
58 Qt::ScreenOrientation mNativeOrientation;51 Qt::ScreenOrientation mNativeOrientation;
59 Qt::ScreenOrientation mCurrentOrientation;52 Qt::ScreenOrientation mCurrentOrientation;
60 QOrientationSensor* mOrientationSensor;
61 QImage::Format mFormat;53 QImage::Format mFormat;
62 int mDepth;54 int mDepth;
63 QSurfaceFormat mSurfaceFormat;55 QSurfaceFormat mSurfaceFormat;
6456
=== modified file 'src/ubuntumirclient/ubuntumirclient.pro'
--- src/ubuntumirclient/ubuntumirclient.pro 2014-08-14 13:18:24 +0000
+++ src/ubuntumirclient/ubuntumirclient.pro 2014-09-19 12:18:46 +0000
@@ -37,6 +37,7 @@
37 integration.h \37 integration.h \
38 logging.h \38 logging.h \
39 nativeinterface.h \39 nativeinterface.h \
40 orientationchangeevent_p.h \
40 platformservices.h \41 platformservices.h \
41 plugin.h \42 plugin.h \
42 screen.h \43 screen.h \

Subscribers

People subscribed via source and target branches