Merge lp:~jhodapp/qtubuntu/qtubuntu_add-live-orientation into lp:qtubuntu

Proposed by Jim Hodapp
Status: Merged
Approved by: Loïc Molinari
Approved revision: 120
Merged at revision: 119
Proposed branch: lp:~jhodapp/qtubuntu/qtubuntu_add-live-orientation
Merge into: lp:qtubuntu
Diff against target: 188 lines (+99/-5)
4 files modified
debian/control (+6/-2)
src/platforms/ubuntu/screen.cc (+77/-1)
src/platforms/ubuntu/screen.h (+15/-1)
src/platforms/ubuntu/ubuntu.pro (+1/-1)
To merge this branch: bzr merge lp:~jhodapp/qtubuntu/qtubuntu_add-live-orientation
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Loïc Molinari (community) Approve
Review via email: mp+150809@code.launchpad.net

Commit message

* Implements live orientation support for client applications using the QtSensors OrientationSensor backend.
* Add qtsensors5-dev as a package build dependency.
* Update the binary package dependencies to include the sensor related libraries.

Description of the change

* Implements live orientation support for client applications using the QtSensors OrientationSensor backend.
* Add qtsensors5-dev as a package build dependency.
* Update the binary package dependencies to include the sensor related libraries.

To post a comment you must log in.
118. By Jim Hodapp

Don't need to include QObject header file in base/screen.h

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
119. By Jim Hodapp

Fix compile error when setting debug config via: qmake CONFIG+=debug

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Loïc Molinari (loic.molinari) wrote :

Hi Jim,

That is good, code is working well.

I only have a few coding style remarks, it's not written anywhere in the project but QtUbuntu respects the coding guidelines from Google (http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml):

- Not sure about the "Depends" change in debian/control, might need to be validated by ricmm.

- No need to test for orientationSensor_ validity in ~QUbuntuScreen, just delete it. "delete" takes care of NULL pointers and the sensor is stopped in its destructor.

 - Add DLOGs in OrientationReadingEvent constructor and destructor as it's done in src/modules/application/application_image.cc for instance.

 - Add DLOGs in onOrientationReadingChanged() and customEvent() at the beginning as it's done in every other methods.

 - I think using a single switch in customEvent() would make the code easier to read and maintain (also Google coding guidelines recommend to use brackets for cases, 2 spaces indentation, 4 spaces line wrap), something like that:

  switch (oReadingEvent->orientation_) {
    case QOrientationReading::LeftUp: {
      currentOrientation_ = (nativeOrientation_ == Qt::LandscapeOrientation) ?
          Qt::InvertedPortraitOrientation : Qt::LandscapeOrientation;;
      break;
    }
    case ...
  }

 - QtUbuntu uses full include paths for Qt5 classes (QtCore/QObject, QtSensors/QOrientationSensor, QtGui/QScreen, QtCore/QThread, etc). If you don't know where a class is located, "find /usr/include/qt5 -name QFoo" can help you.

 - Open curly brace should be on the same line than the last parameter of a function or the last initialization parameter of a constructor (see lines 47, 87, 98 and 143 in screen.cc). Google coding guidelines recommend Egyptian brackets (http://www.codinghorror.com/blog/2012/07/new-programming-jargon.html).

 - "Qt::ScreenOrientation QUbuntuScreen::orientation() const" should be defined in the header.

 - Bad indentation in QUbuntuScreen: QOBJECT macro and methods should be indented 2 spaces, private, public and protected keywords should be indented 1 space.

 - For pointer declarations, space is following the asterisk.

 - I'd remove the comment on line 73 in screen.cc, code is explicit enough.

review: Needs Fixing
120. By Jim Hodapp

Code [style] cleanup per a merge request review by loicm.

Revision history for this message
Loïc Molinari (loic.molinari) wrote :

Excellent, thanks.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/control'
--- debian/control 2013-02-12 16:56:42 +0000
+++ debian/control 2013-03-01 14:53:21 +0000
@@ -15,7 +15,8 @@
15 libfontconfig1-dev,15 libfontconfig1-dev,
16 libfreetype6-dev,16 libfreetype6-dev,
17 libdbus-1-dev,17 libdbus-1-dev,
18 libatspi2.0-dev18 libatspi2.0-dev,
19 qtsensors5-dev
19Standards-Version: 3.9.320Standards-Version: 3.9.3
20Section: libs21Section: libs
2122
@@ -24,7 +25,10 @@
24Breaks: qthybris (<< 0.37)25Breaks: qthybris (<< 0.37)
25Section: libs26Section: libs
26Architecture: armel armhf27Architecture: armel armhf
27Depends: ${shlibs:Depends}, ${misc:Depends}28Depends: ${shlibs:Depends}, ${misc:Depends},
29 libqt5sensors5,
30 qtubuntu-sensors[armel],
31 qtubuntu-sensors[armhf],
28Description: Qt plugins for Ubuntu Platform API32Description: Qt plugins for Ubuntu Platform API
29 QtUbuntu is a set of Qt5 components for the Ubuntu Platform API. It33 QtUbuntu is a set of Qt5 components for the Ubuntu Platform API. It
30 contains a QPA (Qt Platform Abstraction) plugin based on the Ubuntu34 contains a QPA (Qt Platform Abstraction) plugin based on the Ubuntu
3135
=== modified file 'src/platforms/ubuntu/screen.cc'
--- src/platforms/ubuntu/screen.cc 2013-02-26 01:03:00 +0000
+++ src/platforms/ubuntu/screen.cc 2013-03-01 14:53:21 +0000
@@ -17,9 +17,32 @@
1717
18#include "screen.h"18#include "screen.h"
19#include "base/logging.h"19#include "base/logging.h"
20#include <QtCore/QCoreApplication>
20#include <QtCore/qmath.h>21#include <QtCore/qmath.h>
22#include <QtSensors/QOrientationSensor>
23#include <QtSensors/QOrientationReading>
24#include <QtGui/QScreen>
25#include <QtCore/QThread>
26#include <qpa/qwindowsysteminterface.h>
21#include <ubuntu/application/ui/ubuntu_application_ui.h>27#include <ubuntu/application/ui/ubuntu_application_ui.h>
2228
29class OrientationReadingEvent : public QEvent {
30 public:
31 OrientationReadingEvent(QEvent::Type type, QOrientationReading::Orientation orientation)
32 : QEvent(type)
33 , orientation_(orientation) {
34 DLOG("OrientationReadingEvent::OrientationReadingEvent()");
35 }
36 ~OrientationReadingEvent() {
37 DLOG("OrientationReadingEvent::~OrientationReadingEvent()");
38 }
39 static const QEvent::Type type_;
40 QOrientationReading::Orientation orientation_;
41};
42
43const QEvent::Type OrientationReadingEvent::type_ =
44 static_cast<QEvent::Type>(QEvent::registerEventType());
45
23// Grid unit used if GRID_UNIT_PX is not in the environment.46// Grid unit used if GRID_UNIT_PX is not in the environment.
24const int kDefaultGridUnit = 8;47const int kDefaultGridUnit = 8;
2548
@@ -79,11 +102,20 @@
79 DLOG("QUbuntuScreen::QUbuntuScreen (this=%p)", this);102 DLOG("QUbuntuScreen::QUbuntuScreen (this=%p)", this);
80103
81 // Set the default orientation based on the initial screen dimmensions.104 // Set the default orientation based on the initial screen dimmensions.
82 nativeOrientation_ = kScreenWidth >= kScreenHeight ? Qt::LandscapeOrientation : Qt::PortraitOrientation;105 nativeOrientation_ = (kScreenWidth >= kScreenHeight) ? Qt::LandscapeOrientation : Qt::PortraitOrientation;
106
107 // If it's a landscape device (i.e. some tablets), start in landscape, otherwise portrait
108 currentOrientation_ = (nativeOrientation_ == Qt::LandscapeOrientation) ? Qt::LandscapeOrientation : Qt::PortraitOrientation;
109
110 orientationSensor_ = new QOrientationSensor();
111 QObject::connect(orientationSensor_, SIGNAL(readingChanged()), this, SLOT(onOrientationReadingChanged()));
112 orientationSensor_->start();
83}113}
84114
85QUbuntuScreen::~QUbuntuScreen() {115QUbuntuScreen::~QUbuntuScreen() {
86 DLOG("QUbuntuScreen::~QUbuntuScreen");116 DLOG("QUbuntuScreen::~QUbuntuScreen");
117
118 delete orientationSensor_;
87}119}
88120
89int QUbuntuScreen::gridUnitToPixel(int value) const {121int QUbuntuScreen::gridUnitToPixel(int value) const {
@@ -100,3 +132,47 @@
100 return static_cast<int>(qRound(value * densityPixelRatio_));132 return static_cast<int>(qRound(value * densityPixelRatio_));
101 }133 }
102}134}
135
136void QUbuntuScreen::customEvent(QEvent* event) {
137 DLOG("QUbuntuScreen::customEvent (event: %p)", event);
138 DASSERT(QThread::currentThread() == thread());
139
140 OrientationReadingEvent* oReadingEvent = static_cast<OrientationReadingEvent*>(event);
141 switch (oReadingEvent->orientation_) {
142 case QOrientationReading::LeftUp: {
143 currentOrientation_ = (nativeOrientation_ == Qt::LandscapeOrientation) ?
144 Qt::InvertedPortraitOrientation : Qt::LandscapeOrientation;
145 break;
146 }
147 case QOrientationReading::TopUp: {
148 currentOrientation_ = (nativeOrientation_ == Qt::LandscapeOrientation) ?
149 Qt::LandscapeOrientation : Qt::PortraitOrientation;
150 break;
151 }
152 case QOrientationReading::RightUp: {
153 currentOrientation_ = (nativeOrientation_ == Qt::LandscapeOrientation) ?
154 Qt::PortraitOrientation : Qt::InvertedLandscapeOrientation;
155 break;
156 }
157 case QOrientationReading::TopDown: {
158 currentOrientation_ = (nativeOrientation_ == Qt::LandscapeOrientation) ?
159 Qt::InvertedLandscapeOrientation : Qt::InvertedPortraitOrientation;
160 break;
161 }
162 default: {
163 DLOG("Unknown orientation.");
164 }
165 }
166
167 // Raise the event signal so that client apps know the orientation changed
168 QWindowSystemInterface::handleScreenOrientationChange(screen(), currentOrientation_);
169}
170
171void QUbuntuScreen::onOrientationReadingChanged() {
172 DLOG("QUbuntuScreen::onOrientationReadingChanged");
173 DASSERT(orientationSensor_ != NULL);
174
175 // Make sure to switch to the main Qt thread context
176 QCoreApplication::postEvent(this, new OrientationReadingEvent(
177 OrientationReadingEvent::type_, orientationSensor_->reading()->orientation()));
178}
103179
=== modified file 'src/platforms/ubuntu/screen.h'
--- src/platforms/ubuntu/screen.h 2013-02-14 16:31:33 +0000
+++ src/platforms/ubuntu/screen.h 2013-03-01 14:53:21 +0000
@@ -18,7 +18,12 @@
1818
19#include "base/screen.h"19#include "base/screen.h"
2020
21class QUbuntuScreen : public QUbuntuBaseScreen {21#include <QObject>
22
23class QOrientationSensor;
24
25class QUbuntuScreen : public QObject, public QUbuntuBaseScreen {
26 Q_OBJECT
22 public:27 public:
23 QUbuntuScreen();28 QUbuntuScreen();
24 ~QUbuntuScreen();29 ~QUbuntuScreen();
@@ -28,15 +33,24 @@
28 QRect availableGeometry() const { return availableGeometry_; }33 QRect availableGeometry() const { return availableGeometry_; }
2934
30 Qt::ScreenOrientation nativeOrientation() const { return nativeOrientation_; }35 Qt::ScreenOrientation nativeOrientation() const { return nativeOrientation_; }
36 Qt::ScreenOrientation orientation() const { return currentOrientation_; }
31 int gridUnitToPixel(int value) const;37 int gridUnitToPixel(int value) const;
32 int densityPixelToPixel(int value) const;38 int densityPixelToPixel(int value) const;
3339
40 // QObject methods.
41 void customEvent(QEvent* event);
42
43 public Q_SLOTS:
44 void onOrientationReadingChanged();
45
34 private:46 private:
35 QRect geometry_;47 QRect geometry_;
36 QRect availableGeometry_;48 QRect availableGeometry_;
37 int gridUnit_;49 int gridUnit_;
38 float densityPixelRatio_;50 float densityPixelRatio_;
39 Qt::ScreenOrientation nativeOrientation_;51 Qt::ScreenOrientation nativeOrientation_;
52 Qt::ScreenOrientation currentOrientation_;
53 QOrientationSensor* orientationSensor_;
40};54};
4155
42#endif // QUBUNTUSCREEN_H56#endif // QUBUNTUSCREEN_H
4357
=== modified file 'src/platforms/ubuntu/ubuntu.pro'
--- src/platforms/ubuntu/ubuntu.pro 2013-02-07 13:54:11 +0000
+++ src/platforms/ubuntu/ubuntu.pro 2013-03-01 14:53:21 +0000
@@ -1,7 +1,7 @@
1TARGET = qubuntu1TARGET = qubuntu
2TEMPLATE = lib2TEMPLATE = lib
33
4QT += core-private gui-private platformsupport-private4QT += core-private gui-private platformsupport-private sensors-private
55
6DEFINES += MESA_EGL_NO_X11_HEADERS6DEFINES += MESA_EGL_NO_X11_HEADERS
7QMAKE_CXXFLAGS += -fvisibility=hidden -fvisibility-inlines-hidden7QMAKE_CXXFLAGS += -fvisibility=hidden -fvisibility-inlines-hidden

Subscribers

People subscribed via source and target branches