Ensure Rotation functionality in Qt Port after removing meegotouch

RESOLVED FIXED

Status

Fennec Graveyard
General
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: jbos, Unassigned)

Tracking

Dependency tree / graph

Details

Attachments

(2 attachments, 17 obsolete attachments)

16.12 KB, patch
wolfiR
: review+
Mitch
: review+
Details | Diff | Splinter Review
21.98 KB, patch
wolfiR
: review+
khuey
: review+
Details | Diff | Splinter Review
(Reporter)

Description

7 years ago
Ensure Rotation functionality in Qt Port  after removing meegotouch by using QtMobility.
(Reporter)

Comment 1

7 years ago
For this we should use something like described here:

http://doc.qt.nokia.com/qtmobility-1.2.0-tp1/sensors-api.html
http://doc.qt.nokia.com/qtmobility-1.2.0-tp1/qaccelerometer.html
http://doc.qt.nokia.com/qtmobility-1.2.0-tp1/qaccelerometerreading.html

With that we get live data about the sensor position (rotation in all 3 axis!)
(Reporter)

Comment 2

7 years ago
Or this:

http://doc.qt.nokia.com/qtmobility-1.2.0-tp1/qorientationreading.html
(Reporter)

Comment 3

7 years ago
Created attachment 509532 [details] [diff] [review]
WIP, adds functionality but include is somehow still wrong

What do you think, should qtmobility more separated from qt, its default since qt 4.7.2 


Currently this ends in: /usr/include/qt4/QtSensors/qsensor.h:45: fatal error: qmobilityglobal.h: No such file or directory
Attachment #509532 - Flags: feedback?(romaxa)
Depends on: 607936
(Reporter)

Comment 4

7 years ago
meant qt 4.7.0 not 4.7.2
Comment on attachment 509532 [details] [diff] [review]
WIP, adds functionality but include is somehow still wrong


> #include <QtGui/QStyleOptionGraphicsItem>
> #include <QPaintEngine>
> 
>+
Please no, empty lines...
>+        gfxMatrix matr;
>+        matr.Translate(gfxPoint(aPainter->transform().dx(), aPainter->transform().dy()));
>+#if (QT_VERSION >= QT_VERSION_CHECK(4, 7, 0))
>+        QOrientationReading* reading = mOrientation.reading();
IIUC this is works only with Qt mobility, so Qt 4.7 does not have it -> so we need Qt mobility define stuff...

Please move it into separate function, we would like to use it in GL backend too

>+#if (QT_VERSION >= QT_VERSION_CHECK(4, 7, 0))

IIUC Qt mobility define needed here

>+#include <QtSensors/QOrientationSensor>
>+#endif // QT version check 4.7
>+
Attachment #509532 - Flags: feedback?(romaxa) → feedback-
(Reporter)

Comment 6

7 years ago
> 
> Please move it into separate function, we would like to use it in GL backend
> too
> 
Where do you like to place it?
(Reporter)

Comment 7

7 years ago
Created attachment 509716 [details] [diff] [review]
WIP, second WIP Version.

Updated Version, it does compile now. Still needs testing. 

Added as supposed a MOZ_ENABLE_QTMOBILITY. The strange thing is that there is no QtMobility.pc, the way to get QtMobility include is by using QtLocation.pc...
A QtSensory.pc is also missing, there is no pc file defining -I/usr/include/qt4/QtSensors/
Attachment #509532 - Attachment is obsolete: true
Attachment #509716 - Flags: feedback?(romaxa)
(Reporter)

Comment 8

7 years ago
Created attachment 509718 [details] [diff] [review]
WIP, second WIP Version. Fixed Layout

Seen that there are some layout flaws... fixed.
Attachment #509716 - Attachment is obsolete: true
Attachment #509718 - Flags: feedback?(romaxa)
Attachment #509716 - Flags: feedback?(romaxa)
(Reporter)

Comment 9

7 years ago
Created attachment 509720 [details] [diff] [review]
WIP, second WIP Version. Fixed Layout

one more.
Attachment #509718 - Attachment is obsolete: true
Attachment #509720 - Flags: feedback?(romaxa)
Attachment #509718 - Flags: feedback?(romaxa)
(Reporter)

Comment 10

7 years ago
Created attachment 509736 [details] [diff] [review]
WIP, third WIP version.

Third version. I filed a bug against qt, for missing pc files 
http://bugreports.qt.nokia.com/browse/QTMOBILITY-1178

Added hardcoded pathes since the library path for qtsensors is nowhere to be found in any pc file.
Attachment #509720 - Attachment is obsolete: true
Attachment #509736 - Flags: feedback?(romaxa)
Attachment #509720 - Flags: feedback?(romaxa)
Comment on attachment 509736 [details] [diff] [review]
WIP, third WIP version.


>-      AC_DEFINE(MOZ_ENABLE_QTNETWORK)
>+       MOZ_ENABLE_QTNETWORK=1
>+       AC_DEFINE(MOZ_ENABLE_QTNETWORK)

dont change indentation int code which is not related to your bugfix 

>+    fi
>+
>+    MOZ_ENABLE_QTNETWORK==
>+    PKG_CHECK_MODULES(_QTMOBILITY, QtDeclarative,
>+                      MOZ_ENABLE_QTMOBILITY=1,
>+                      MOZ_ENABLE_QTMOBILITY=)
>+    if test "$MOZ_ENABLE_QTMOBILITY"; then
>+       MOZ_ENABLE_QTMOBILITY=1
>+       MOZ_QT_CFLAGS="$MOZ_QT_CFLAGS -I/usr/include/qt4/QtMobility"
>+       MOZ_QT_CFLAGS="$MOZ_QT_CFLAGS -I/usr/include/qt4/QtSensors"

would it be possible to get mobility flags with pkg-config?

>+       MOZ_QT_LIBS="$MOZ_QT_LIBS -lQtSensors"
>+       AC_DEFINE(MOZ_ENABLE_QTMOBILITY)
>     fi
> fi
> 
>+
>+#ifdef MOZ_ENABLE_QTMOBILITY
>+    mWindowRotationAngle = 0;

don't see why we need own variable here... we don't use it without GetWindowRotationAngle call, so just kill it.
or add signal listener which track orientation change and remember in mWindowRotationAngle



>+        gfxMatrix matr;
>+        matr.Translate(gfxPoint(aPainter->transform().dx(), aPainter->transform().dy()));
>+#ifdef MOZ_ENABLE_QTMOBILITY
>+        // This is needed for rotate transformation on MeeGo
>+        // This will work very slow if pixman does not handle rotation very well
>+        matr.Rotate((M_PI/180)*GetWindowRotationAngle());
>+        NS_ASSERTION(PIXMAN_VERSION > PIXMAN_VERSION_ENCODE(0, 21, 2) ||
>+            !angle, "Old pixman and rotate transform, it is going to be slow");
>+#endif //MOZ_ENABLE_QTMOBILITY

bug 607936 already fixed, add please related rotation usage for GL backend.
Attachment #509736 - Flags: feedback?(romaxa) → feedback-
(Reporter)

Comment 12

7 years ago
(In reply to comment #11)
> Comment on attachment 509736 [details] [diff] [review]
> WIP, third WIP version.
> 
> 
> >-      AC_DEFINE(MOZ_ENABLE_QTNETWORK)
> >+       MOZ_ENABLE_QTNETWORK=1
> >+       AC_DEFINE(MOZ_ENABLE_QTNETWORK)
> 
> dont change indentation int code which is not related to your bugfix 
> 
ack

> >+    fi
> >+
> >+    MOZ_ENABLE_QTNETWORK==
> >+    PKG_CHECK_MODULES(_QTMOBILITY, QtDeclarative,
> >+                      MOZ_ENABLE_QTMOBILITY=1,
> >+                      MOZ_ENABLE_QTMOBILITY=)
> >+    if test "$MOZ_ENABLE_QTMOBILITY"; then
> >+       MOZ_ENABLE_QTMOBILITY=1
> >+       MOZ_QT_CFLAGS="$MOZ_QT_CFLAGS -I/usr/include/qt4/QtMobility"
> >+       MOZ_QT_CFLAGS="$MOZ_QT_CFLAGS -I/usr/include/qt4/QtSensors"
> 
> would it be possible to get mobility flags with pkg-config?
> 

no, see bug http://bugreports.qt.nokia.com/browse/QTMOBILITY-1178


> >+       MOZ_QT_LIBS="$MOZ_QT_LIBS -lQtSensors"
> >+       AC_DEFINE(MOZ_ENABLE_QTMOBILITY)
> >     fi
> > fi
> > 
> >+
> >+#ifdef MOZ_ENABLE_QTMOBILITY
> >+    mWindowRotationAngle = 0;
> 
> don't see why we need own variable here... we don't use it without
> GetWindowRotationAngle call, so just kill it.
> or add signal listener which track orientation change and remember in
> mWindowRotationAngle
> 

Because you also get Top/Down Rotation, see http://doc.qt.nokia.com/qtmobility-1.2.0-tp1/qorientationreading.html

For this case we do need to return the last rotation. since rotation angle gets polled each time... 

> 
> 
> >+        gfxMatrix matr;
> >+        matr.Translate(gfxPoint(aPainter->transform().dx(), aPainter->transform().dy()));
> >+#ifdef MOZ_ENABLE_QTMOBILITY
> >+        // This is needed for rotate transformation on MeeGo
> >+        // This will work very slow if pixman does not handle rotation very well
> >+        matr.Rotate((M_PI/180)*GetWindowRotationAngle());
> >+        NS_ASSERTION(PIXMAN_VERSION > PIXMAN_VERSION_ENCODE(0, 21, 2) ||
> >+            !angle, "Old pixman and rotate transform, it is going to be slow");
> >+#endif //MOZ_ENABLE_QTMOBILITY
> 
> bug 607936 already fixed, add please related rotation usage for GL backend.
Comment on attachment 509736 [details] [diff] [review]
WIP, third WIP version.


>+        switch (reading->orientation()) {
>+            //The Top edge of the device is pointing up.
>+            case QOrientationReading::TopUp:
          ^^^^ - no space here

would it be better to move mWindowRotationAngle and mOrientation into GetWindowRotationAngle, and make that function static?

and yeah, idea in general is good.
Attachment #509736 - Flags: feedback- → feedback+
Comment on attachment 509736 [details] [diff] [review]
WIP, third WIP version.


>+        // This is needed for rotate transformation on MeeGo
>+        // This will work very slow if pixman does not handle rotation very well
>+        matr.Rotate((M_PI/180)*GetWindowRotationAngle());
                                ^ space needed here

one more question is how expensive mOrientation.reading->orientation call, should not we listen for some signals and update internal mWindowRotationAngle value only when real rotation happens?
ok, no worry, QOrientation caching this value internally.
(Reporter)

Comment 16

7 years ago
:) will push updated patch tomorrow. than for review.
and don't forget required initialization for mOrientation.class.. like backend init, start/stop...
and test it on real device.
(Reporter)

Comment 19

7 years ago
(In reply to comment #17)
> and don't forget required initialization for mOrientation.class.. like backend
> init, start/stop...

ok, now i am puzzled?? What are you talking about?

- there are no signals
- no init
- no start/stop
(Reporter)

Comment 20

7 years ago
see http://doc.qt.nokia.com/qtmobility/qorientationreading.html
I was checking some APIs and qtmobility examples, and found there:
.connectToBackend();
.start();

.stop();

panoramaWithSense/orientationcontroller.cpp
hmm cannot get it working at all on maemo6... "reading" is always null for me...

have you tested it anywhere? btw, I think we should do something else together with orientation change... resize widget... don't we?
(Reporter)

Comment 23

7 years ago
Ah i see, well that seems to be a qml / qtmobility example, Connecting QML to some c++ backend.

see http://qt.gitorious.org/qt-mobility/simulator-mobility/blobs/4f8e947f256a01a6398455b81a52cfc7f3e596df/examples/sensors/panoramaWithSense/orientationcontroller.cpp#line48

So what we do is using a prepared c++ class, no qml stuff involved.

----

Do you have libqtm-4-1.1 installed on our Maemo5?  (PR1.3)
(Reporter)

Comment 24

7 years ago
(note that there a two libqtm versions on maemo,... important is the 1.1
ah, ok. got it.
(Reporter)

Comment 26

7 years ago
Alright, it took me quite a while to figure out how this works. Basically the docs are pretty much useless and don't tell you what you need.

I had not yet the time to finally test on n900, but at least meego started to work/signaling rotation changes. (still something wrong ... for portrait its crashing...). The patch needs still time due to complete undocumented functionality. But it will come.
(Reporter)

Comment 27

7 years ago
Created attachment 512779 [details] [diff] [review]
Still a WIP, forth Version

This actually get the events and send a signal, rotation basically works. But unlucky it is that window goes black for all angles != 0. I was not able to get it fixed. I don't think it has something todo with Size/Screen, but is a bug in the rendering code. 

Oleg, you have more insight - so maybe you can help to fix that.
Attachment #509736 - Attachment is obsolete: true
Attachment #512779 - Flags: review?(romaxa)
Comment on attachment 512779 [details] [diff] [review]
Still a WIP, forth Version


>+#ifdef MOZ_ENABLE_QTMOBILITY
>+class OrientationSensorFilter : public QObject, public QOrientationFilter
move second public to next line


>+        switch (reading->orientation()) {
>+            //The Top edge of the device is pointing up.
>+            case QOrientationReading::TopUp:
          ^^^^ - no need this indentation space

see here:
http://mxr.mozilla.org/mozilla-central/source/widget/src/qt/nsWindow.cpp#1850

>+    int gWindowRotationAngle;
Add empty line here
>+signals:
>+    void orientationChanged(int angle);
>+
No empty line here ;)
>+};
>+#endif


>+        if (!this->scene())
>+        {
>+            return;
>+        }
>+
>+        if (!this->scene()->views().size())
>+        {
>+            return;
>+        }

Combine this into one condition

also use  "if (...) {"

>+            qDebug() <<"Portrait Mode";
use MOZ logging, no qdebug

>+    if (!gOrientation.isActive()) {
>+        qWarning("Orientationsensor didn't start!");

NS_WARNING

>+        // This will work very slow if pixman does not handle rotation very well
>+        matr.Rotate((M_PI/180)*GetWindowRotationAngle());
                                ^ - spaces


>+nsWindow::GetWindowRotationAngle()
>+{
>+    return filter.gWindowRotationAngle;

this is not compiling... IIUC it should be gOrientationFilter.g...
Attachment #512779 - Flags: review?(romaxa) → review-
IIUC when QOrientationReading::RightUp: happen we should set gWindowRotationAngle = 0;
Comment on attachment 512779 [details] [diff] [review]
Still a WIP, forth Version


>+    MOZ_ENABLE_QTNETWORK==
what is this?

>+    PKG_CHECK_MODULES(_QTMOBILITY, QtDeclarative,

Why we enabling QtMobility when QtDeclarative enabled? on desktop I have QtDeclarative, but don't have mobility

>+                      MOZ_ENABLE_QTMOBILITY=1,
(Reporter)

Comment 31

7 years ago
Created attachment 514164 [details] [diff] [review]
Version Five, now with real rotation

Ok, this version finaly does rotate. in Portrait there is some issue still with resizeing the window. (sidebars height seem mostly ok - but width is somehow not perfect...) also parts of fennecs ui seem not to be resized...

Anyway. One step in the right direction.
Attachment #512779 - Attachment is obsolete: true
Attachment #514164 - Flags: review?(romaxa)
(Reporter)

Comment 32

7 years ago
Created attachment 514178 [details] [diff] [review]
Version Five, now with real rotation .1

 what does still not work on rotation:
 * fennec is only party resized in portrait
 * vkb and other dialogs do not appear rotated
 * fennec sidebar height is correct
 ** fennec width in portrait is a bit to much
 ** website size is not correctly resized in portrait
 ** touch-able area seem also not resized in portrait
Attachment #514164 - Attachment is obsolete: true
Attachment #514164 - Flags: review?(romaxa)
Comment on attachment 514178 [details] [diff] [review]
Version Five, now with real rotation .1

Yep, this is does visible rotation change!
You need to resize Scene/View after rotation change (that should resize nsWindow)
(Reporter)

Comment 34

7 years ago
Created attachment 514786 [details] [diff] [review]
Version Six, now with rotated touch

Touch rotated now to, but still missing:

* fennec ui not fully resized
* vkb appears still on wrong edge
Attachment #514178 - Attachment is obsolete: true
(Reporter)

Comment 35

7 years ago
Created attachment 514799 [details] [diff] [review]
Version Six, now with rotated touch v2
Attachment #514786 - Attachment is obsolete: true
> * fennec ui not fully resized

Ok, I noticed that you have missing nsScreenQt.cpp, that is need to be fixed in order to get content sized properly.

> * vkb appears still on wrong edge

Any ideas what need to be done to make that work? 

Tested this patch with additional 
QTransform transform.rotate(gOrientationFilter.GetWindowRotationAngle());
QRect r = transform.mapRect(rn);

and content sized properly.
(Reporter)

Comment 37

7 years ago
Thanks! This was driving me crazy. I had some mail / irc contact to VKB Devs, (Dominik - which was contributing also to Firefox Mobile) is working on 'vkb selfrotation'.)

This means currently you can not rotate the meegotouch VKB without depending to MeegoTouch. They plan to introduce some special DBUS Signal for Applications to cause VKB rotation 

So, the answer is, there is simply no way to fix VKB currently. 

(I suggested to them to use qt mobility sensors - like we do here -. So they would not depend on apps, but than they seem to miss interfaces to cover cases when an application does not rotate at all. So how ever you look on it, there seem always be a hack involved...) 

Do you finalize the patch, or should I do it?
> So, the answer is, there is simply no way to fix VKB currently. 

But how then VKB work with meegotuch application? do they call activeWindow API directly?

> Do you finalize the patch, or should I do it?

you  do it ;)
(Reporter)

Comment 39

7 years ago
Currently they use some magic within the MSceneManager, which seems to be 'kind of global' and related with MWindows...
Can we report bug for them and ask them to fix it?
(Reporter)

Comment 41

7 years ago
I think they work on it here:

http://www.meego.gitorious.org/~rottsches/meegotouch/dros-meegotouch-inputmethodframework/commits/selfrotation
This branch is not related to keyboard orientation signaling. It's meant for VKB rotation animation and relies on such previously existing signals from the Meegotouch app. As I mentioned in my private email to you, Jeremias, as of now orientation changes are signaled to VKB using MInputMethod and MInputMethodState. 

> "They plan to introduce some special DBUS Signal for Applications to cause VKB rotation."

I don't know where this information comes from. That wasn't in my mail at least.
(Reporter)

Comment 43

7 years ago
> orientation changes are signaled to VKB using MInputMethod and
> MInputMethodState. 

Which we can not use, since we must not link against MeegoTouch.
(Reporter)

Comment 44

7 years ago
Created attachment 515879 [details] [diff] [review]
Version Seven, now with rotated touch and resized

Ok finally got it cleaned up,
Attachment #514799 - Attachment is obsolete: true
Attachment #515879 - Flags: review?(romaxa)
(Reporter)

Comment 45

7 years ago
Created attachment 515881 [details] [diff] [review]
Version Seven, now with rotated touch and resized v2
Attachment #515879 - Attachment is obsolete: true
Attachment #515879 - Flags: review?(romaxa)
(Reporter)

Updated

7 years ago
Attachment #515881 - Flags: review?(romaxa)
Comment on attachment 515881 [details] [diff] [review]
Version Seven, now with rotated touch and resized v2


>+       MOZ_ENABLE_QTNETWORK=1
>+       AC_DEFINE(MOZ_ENABLE_QTNETWORK)
>+    fi
>+
>+    MOZ_ENABLE_QTMOBILITY==
>+    PKG_CHECK_MODULES(_QTMOBILITY, QtDeclarative,
this is wrong, see comments above


>+bool
>+MozQOrientationSensorFilter::filter(QOrientationReading *reading)
>+{
>+    switch (reading->orientation()) {
>+        //The Top edge of the device is pointing up.
      ^^^^ - no need for this space, compact it

>+        case QOrientationReading::Undefined:
>+        default: break;
>+    }
Add line here
>+    return true; // don't store the reading in the sensor
>+}

>+ *
>+ * ***** END LICENSE BLOCK ***** */
Add empty Line here
>+#ifndef MOZQORIENTATIONSENSORFILTER_H
>+#define MOZQORIENTATIONSENSORFILTER_H
>+

>+
>+#endif
>\ No newline at end of file

get rid of this warning, add line

>  *
>  * ***** END LICENSE BLOCK ***** */
>-
don't remove this 
> #ifndef MOZQWIDGET_H
> #define MOZQWIDGET_H
> 
>@@ -42,12 +41,26 @@
> #include <QtGui/QGraphicsView>
> #include <QtGui/QGraphicsWidget>
> 
>+
>+
don't add this

> #include "nsIWidget.h"
> #include "prenv.h"
> 
> #include "nsIObserverService.h"
> #include "mozilla/Services.h"
> 
>+#ifdef MOZ_ENABLE_QTMOBILITY
>+#include "mozqorientationsensorfilter.h"
>+#ifdef MOZ_X11
>+#include <QX11Info>
>+#include <X11/Xlib.h>
>+#include <X11/Xatom.h>
>+# undef KeyPress
>+# undef KeyRelease
>+# undef CursorShape
do you really need this? can you just move it up/down, or even move function impl into cpp file

> 
>+#ifdef MOZ_ENABLE_QTMOBILITY
>+    void orientationChanged(int orientation)
>+    {
..................
>+
>+        if (orientation == 90 || orientation ==  270) {
>+            screenSize = QSizeF(screenSize.height() < screenSize.width() ? screenSize.height() : screenSize.width(),
>+                                screenSize.height() < screenSize.width() ? screenSize.width() : screenSize.height());
>+
I don't like these hacks, can you create Transform matrix and apply it to Rect/Size/Points?


>+#ifdef MOZ_X11
>+        Display *display = QX11Info::display();
>+        if (!display)
>+            return;
{}

>+
>+        Atom orientationAngleAtom = XInternAtom(display, "_MEEGOTOUCH_ORIENTATION_ANGLE", False);
>+        XChangeProperty(display, scene()->views()[0]->effectiveWinId(), orientationAngleAtom, XA_CARDINAL, 32,
>+                    PropModeReplace, (unsigned char*)&orientation, 1);
                  fix indent here


> 
>     *outTop = r.x();
>     *outLeft = r.y();
>-    *outWidth = r.width();
>-    *outHeight = r.height();
>+#ifdef MOZ_ENABLE_QTMOBILITY
>+    if (MozQOrientationSensorFilter::GetWindowRotationAngle() == 90 ||
>+        MozQOrientationSensorFilter::GetWindowRotationAngle() == 270) {
>+        *outWidth = r.height();
>+        *outHeight = r.width();
>+    } else
>+#endif
>+    {
>+        *outWidth = r.width();
>+        *outHeight = r.height();
>+    }

Same here, use transform matrix and apply it... probably instead of GetWindowRotationAngle make sense to create GetTransform()


>@@ -80,11 +94,21 @@
> nsScreenQt::GetAvailRect(PRInt32 *outLeft,PRInt32 *outTop,
>                          PRInt32 *outWidth,PRInt32 *outHeight)
> {
>-    QRect r = QApplication::desktop()->availableGeometry(mScreen);
>+    QRect r = QApplication::desktop()->screenGeometry(mScreen);
>+
>     *outTop = r.x();
>     *outLeft = r.y();
>-    *outWidth = r.width();
>-    *outHeight = r.height();
>+#ifdef MOZ_ENABLE_QTMOBILITY
>+     if (MozQOrientationSensorFilter::GetWindowRotationAngle() == 90 ||
>+         MozQOrientationSensorFilter::GetWindowRotationAngle() == 270) {
>+         *outWidth = r.height();
>+         *outHeight = r.width();
>+     } else
>+#endif
>+     {
>+        *outWidth = r.width();
>+        *outHeight = r.height();
>+     }
same here, use transform matrix


layers::LayerManagerOGL*>(GetLayerManager(nsnull))->
>             SetClippingRegion(event.region);
>+#ifdef MOZ_ENABLE_QTMOBILITY
>+        gfxMatrix matr;
>+        matr.Translate(gfxPoint(aPainter->transform().dx(), aPainter->transform().dy()));
Translate should be out of this define... see SW-Rendering code below


>+#ifdef MOZ_ENABLE_QTMOBILITY
>+         // This is needed for rotate transformation on MeeGo
>+         // This will work very slow if pixman does not handle rotation very well
>+         matr.Rotate((M_PI/180)*GetWindowRotationAngle());
>+         NS_ASSERTION(PIXMAN_VERSION > PIXMAN_VERSION_ENCODE(0, 21, 2) ||
>+             !angle, "Old pixman and rotate transform, it is going to be slow");
                ^ - this will break debug build

>@@ -1202,6 +1239,7 @@
>     return status;
> }
> 
>+
no empty lines

> nsEventStatus
> nsWindow::OnCloseEvent(QCloseEvent *aEvent)
> {
>@@ -2592,6 +2630,9 @@
>             newView->viewport()->setAttribute(Qt::WA_PaintOnScreen, true);
>             newView->viewport()->setAttribute(Qt::WA_NoSystemBackground, true);
>         }
>+#ifdef MOZ_ENABLE_QTMOBILITY
>+        QObject::connect((QObject*) &gOrientationFilter, SIGNAL(orientationChanged(int)), widget, SLOT(orientationChanged(int)));
move arguments to next line, try keep code ~80 symbols
Attachment #515881 - Flags: review?(romaxa) → review-
(Reporter)

Comment 47

7 years ago
Created attachment 516923 [details] [diff] [review]
again wip cause of changes wanted, does not really work yet.

See commented code, i was not able to make it work / rotate correctly with transformation. 

Romaxa, take a look, what do i miss...
Attachment #516923 - Flags: review?(romaxa)
Comment on attachment 516923 [details] [diff] [review]
again wip cause of changes wanted, does not really work yet.


>+
>+dnl we need to check for QtMultimedia since package config of qtmobility 1.1 is broken
>+dnl this is why we also need to set unclude pathes and lib by hand
>+    MOZ_ENABLE_QTMOBILITY==
>+    PKG_CHECK_MODULES(_QTMOBILITY, QtMultimedia,
>+                      MOZ_ENABLE_QTMOBILITY=1,
>+                      MOZ_ENABLE_QTMOBILITY=)

Still did not get why QtMultimedia defining mobility sensors...
Practically we need only libqtm-sensors-dev, and that package contain 
/usr/lib/pkgconfig/QtSensors.pc
/usr/lib/libQtSensors.so

why we are not using this? Probably it better to change define from QTMOBILITY -> QTSENSORS or QTMOBILITY_SENSORS

>+    if test "$MOZ_ENABLE_QTMOBILITY"; then
>+       MOZ_ENABLE_QTMOBILITY=1
>+       MOZ_QT_CFLAGS="$MOZ_QT_CFLAGS -I/usr/include/qt4/QtMobility"

And we don't really need QtMobility, we need only sensors here...

>+       MOZ_QT_CFLAGS="$MOZ_QT_CFLAGS -I/us
>+
>+//         if (orientation == 90 ||
>+//             orientation ==  270) {
>+//             screenSize = QSizeF(screenSize.height() < screenSize.width() ? screenSize.height() : screenSize.width(),
>+//                                 screenSize.height() < screenSize.width() ? screenSize.width() : screenSize.height());
>+// 
>+//             QPointF position((screenSize.width() - screenSize.height()) / 2, (screenSize.width() - screenSize.height()) / 2);
>+// 
>+//             setTransformOriginPoint(screenSize.height() / 2, screenSize.width() / 2);
>+//             setPos(position);
>+//         } else {
>+//             setPos(0,0);
>+//             screenSize = QSizeF(screenSize.height() < screenSize.width() ? screenSize.width() : screenSize.height(),
>+//                                 screenSize.height() < screenSize.width() ? screenSize.height() : screenSize.width());
>+//             setTransformOriginPoint(screenSize.width() / 2, screenSize.height() / 2);
>+//         }
>+//      resize(screenSize);
>+//      scene()->setSceneRect(QRectF(pos(), screenSize);

Can we just kill this stuff now? don't keep commented code if it is obsolete


The rest of patch looks good now
(Reporter)

Comment 49

7 years ago
(In reply to comment #48)
> Comment on attachment 516923 [details] [diff] [review]
> again wip cause of changes wanted, does not really work yet.
> 
> 
> >+
> >+dnl we need to check for QtMultimedia since package config of qtmobility 1.1 is broken
> >+dnl this is why we also need to set unclude pathes and lib by hand
> >+    MOZ_ENABLE_QTMOBILITY==
> >+    PKG_CHECK_MODULES(_QTMOBILITY, QtMultimedia,
> >+                      MOZ_ENABLE_QTMOBILITY=1,
> >+                      MOZ_ENABLE_QTMOBILITY=)
> 
> Still did not get why QtMultimedia defining mobility sensors...

See: http://bugreports.qt.nokia.com/browse/QTMOBILITY-1178

There is no sensors in 1.1

> Practically we need only libqtm-sensors-dev, and that package contain 
> /usr/lib/pkgconfig/QtSensors.pc
> /usr/lib/libQtSensors.so
>
Its not there in 1.1
 
> why we are not using this? Probably it better to change define from QTMOBILITY
> -> QTSENSORS or QTMOBILITY_SENSORS
> 
> >+    if test "$MOZ_ENABLE_QTMOBILITY"; then
> >+       MOZ_ENABLE_QTMOBILITY=1
> >+       MOZ_QT_CFLAGS="$MOZ_QT_CFLAGS -I/usr/include/qt4/QtMobility"
> 
> And we don't really need QtMobility, we need only sensors here...
>

We need, sensors headers include file which need QtMobility in include path.
 

> >+       MOZ_QT_CFLAGS="$MOZ_QT_CFLAGS -I/us
> >+
> >+//         if (orientation == 90 ||
> >+//             orientation ==  270) {
> >+//             screenSize = QSizeF(screenSize.height() < screenSize.width() ? screenSize.height() : screenSize.width(),
> >+//                                 screenSize.height() < screenSize.width() ? screenSize.width() : screenSize.height());
> >+// 
> >+//             QPointF position((screenSize.width() - screenSize.height()) / 2, (screenSize.width() - screenSize.height()) / 2);
> >+// 
> >+//             setTransformOriginPoint(screenSize.height() / 2, screenSize.width() / 2);
> >+//             setPos(position);
> >+//         } else {
> >+//             setPos(0,0);
> >+//             screenSize = QSizeF(screenSize.height() < screenSize.width() ? screenSize.width() : screenSize.height(),
> >+//                                 screenSize.height() < screenSize.width() ? screenSize.height() : screenSize.width());
> >+//             setTransformOriginPoint(screenSize.width() / 2, screenSize.height() / 2);
> >+//         }
> >+//      resize(screenSize);
> >+//      scene()->setSceneRect(QRectF(pos(), screenSize);
> 
> Can we just kill this stuff now? don't keep commented code if it is obsolete
>

Its not yet working (transform) as i wrote, i did not find a working tranform approach. Please take a look for fix.
 
> 
> The rest of patch looks good now
> /usr/lib/pkgconfig/QtSensors.pc
> /usr/lib/libQtSensors.so
>
Its not there in 1.1

why do we care about broken packages? can we ask related SDK to upgrade that package, and set dependency with version requirement?
please don't mix package .pc files to the libraries which are not existing in those packages.... it will break desktop compilation.
it is better to use in this case AC_CHECK_LIB AC_CHECK_HEADER macros in such cases
Created attachment 517366 [details] [diff] [review]
Update patch, seems to work fine

I've updated configure stuff and cleaned up a bit transform stuff... still a bit hacky but now it looks more readable.
Attachment #516923 - Attachment is obsolete: true
Attachment #517366 - Flags: feedback?
Attachment #516923 - Flags: review?(romaxa)
Attachment #517366 - Flags: feedback? → feedback?(jeremias.bosch)
I found one problem: if device in portrait mode on moment of Fennec start then fennec started in Landscape mode.... can we fix it somehow?
(Reporter)

Comment 54

7 years ago
(In reply to comment #53)
> I found one problem: if device in portrait mode on moment of Fennec start then
> fennec started in Landscape mode.... can we fix it somehow?

http://bugreports.qt.nokia.com/browse/QTMOBILITY-1385
(Reporter)

Comment 55

7 years ago
Comment on attachment 517366 [details] [diff] [review]
Update patch, seems to work fine

> +
> +#include "mozqorientationsensorfilter.h"
> +
> +#include <stdio.h>
> +
> +int MozQOrientationSensorFilter::mWindowRotationAngle = 0;
> +QTransform MozQOrientationSensorFilter::mWindowRotationTransform;
> +
> +bool
> +MozQOrientationSensorFilter::filter(QOrientationReading* reading)
> +{

Do we need stdio.h here?


> +    r = MozQOrientationSensorFilter::GetRotationTransform().mapRect(r);

should be in #ifdef MOZ_ENABLE_QTMOBILITY



> @@ -3023,9 +3066,8 @@ nsWindow::UserActivity()
>    if (!mIdleService) {
>      mIdleService = do_GetService("@mozilla.org/widget/idleservice;1");
>    }
>  
>    if (mIdleService) {
>      mIdleService->ResetIdleTimeOut();
>    }
>  }
> -

Not really needed.
Attachment #517366 - Flags: feedback?(jeremias.bosch) → feedback+
(Reporter)

Comment 56

7 years ago
I wonder about one other thing - its seems a related bug - but well have not focused on this at all. The website in portrait mode seem to have render bugs (i.e parts disappear, huge part is black and become even bigger when panning / scrolling. 

Any idea where this come from?
Created attachment 517422 [details] [diff] [review]
Fixed minor comments

Have not found any problems in portrait mode, only some black squares in preference page, but that more looks like common problem, possibly cairo related (I see the same problem with HW acceleration enabled), "checkbox" half of item sometime blinking while scrolling... We need separate bug about it.
Attachment #515881 - Attachment is obsolete: true
Attachment #517366 - Attachment is obsolete: true
Attachment #517422 - Flags: review?(doug.turner)
(Reporter)

Comment 58

7 years ago
Comment on attachment 517422 [details] [diff] [review]
Fixed minor comments



+void MozQWidget::orientationChanged()
+{
+    if (!scene() || !scene()->views().size()) {
+        return;
+    }
+
+    NS_ASSERTION(scene()->views().size() == 1, "Not exactly one view for our scene!");
+    QTransform& transform = MozQOrientationSensorFilter::GetRotationTransform();
+    QRect scrTrRect = transform.mapRect(scene()->views()[0]->rect());
+
+    setTransformOriginPoint(scene()->views()[0]->size().width() / 2, scene()->views()[0]->size().height() / 2);
+    scene()->views()[0]->setTransform(transform);
+    int orientation = MozQOrientationSensorFilter::GetWindowRotationAngle();
+    if (orientation == 0 || orientation ==  180) {
+        setPos(0,0);
+    } else {
+        setPos(-(scrTrRect.size().width() - scrTrRect.size().height()) / 2,
+               (scrTrRect.size().width() - scrTrRect.size().height()) / 2);
+    }
+    resize(scrTrRect.size());
+    scene()->setSceneRect(QRectF(QPointF(0, 0), scrTrRect.size()));
+}
+

This should be in qt mobility ifdefs.



I found one issue. With this we do not rotate i.e. the Connection Dialogs. In order to do this you need to set the x window property.
above code should include: 


#ifdef MOZ_X11
        Display *display = QX11Info::display();
        if (!display) {
            return;
        }

        Atom orientationAngleAtom = XInternAtom(display, "_MEEGOTOUCH_ORIENTATION_ANGLE", False);
        XChangeProperty(display, scene()->views()[0]->effectiveWinId(), orientationAngleAtom, XA_CARDINAL, 32,
                        PropModeReplace, (unsigned char*)& orientation, 1);
#endif


and ontop in mozqwidget.cpp you need to add 
#ifdef MOZ_ENABLE_QTMOBILITY
#ifdef MOZ_X11
#include <QX11Info>
#include <X11/Xlib.h>
#include <X11/Xatom.h>
# undef KeyPress
# undef KeyRelease
# undef CursorShape
#endif //MOZ_X11
#endif //MOZ_ENABLE_QTMOBILITY
Attachment #517422 - Flags: review-
Created attachment 517863 [details] [diff] [review]
Fixed dialogs rotation, and ifdefed orientation function
Attachment #517422 - Attachment is obsolete: true
Attachment #517422 - Flags: review?(doug.turner)
Created attachment 517908 [details] [diff] [review]
Fixed nsScreenQt compilation on desktop
Attachment #517863 - Attachment is obsolete: true
Attachment #517908 - Flags: review?(mozilla)
Comment on attachment 517908 [details] [diff] [review]
Fixed nsScreenQt compilation on desktop

>diff --git a/widget/src/qt/mozqorientationsensorfilter.cpp b/widget/src/qt/mozqorientationsensorfilter.cpp
>new file mode 100644
>--- /dev/null
>+++ b/widget/src/qt/mozqorientationsensorfilter.cpp
>@@ -0,0 +1,79 @@
>+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
>+/* vim: set ts=4 et sw=4 tw=80: */
>+/* ***** BEGIN LICENSE BLOCK *****
>+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
>+ *
>+ * The contents of this file are subject to the Mozilla Public License Version
>+ * 1.1 (the "License"); you may not use this file except in compliance with
>+ * the License. You may obtain a copy of the License at
>+ * http://www.mozilla.org/MPL/
>+ *
>+ * Software distributed under the License is distributed on an "AS IS" basis,
>+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
>+ * for the specific language governing rights and limitations under the
>+ * License.
>+ *
>+ * The Original Code is Nokia.
>+ *
>+ * The Initial Developer of the Original Code is Nokia Corporation.
>+ * Portions created by the Initial Developer are Copyright (C) 2010

2011, right? Same for the other new file.

Apart from that I trust you guys that it actually works on hardware ;-)
Attachment #517908 - Flags: review?(mozilla) → review+
Comment on attachment 517908 [details] [diff] [review]
Fixed nsScreenQt compilation on desktop

get ted or another peer to review the build/configure, then request approval on the patch.
Comment on attachment 517908 [details] [diff] [review]
Fixed nsScreenQt compilation on desktop

Ted could you check build/configure part plz?
Attachment #517908 - Flags: review?(ted.mielczarek)
Comment on attachment 517908 [details] [diff] [review]
Fixed nsScreenQt compilation on desktop

>--- a/configure.in
>+++ b/configure.in
>@@ -5505,6 +5505,26 @@ incorrect])
>       MOZ_ENABLE_QTNETWORK=1
>       AC_DEFINE(MOZ_ENABLE_QTNETWORK)
>     fi
>+
>+    MOZ_ENABLE_QTMOBILITY==
>+    PKG_CHECK_MODULES(_QTMOBILITY, QtSensors,

There's an extra equals sign there. r=Mitch with that fixed.
Attachment #517908 - Flags: review?(ted.mielczarek) → review+
Created attachment 518552 [details] [diff] [review]
Fixed config review comment, and license year
Attachment #518552 - Flags: approval2.0?
Created attachment 518818 [details] [diff] [review]
Fixed crash on QSensor destructor, and angle undefined variable on debug build

Found problem with previous patch and qtmobility sensor implementation...
it does not check private variable and crashes on fennec close (QSensor destructor called after QApplication destroy)
Here is the a bit modified version with QSensor allocation on heap. and earlier destruction.
The rest of patch is the same
Attachment #518552 - Attachment is obsolete: true
Attachment #518818 - Flags: review?(doug.turner)
Attachment #518552 - Flags: approval2.0?
Attachment #518818 - Flags: review?(doug.turner) → review?(mozilla)
Comment on attachment 518818 [details] [diff] [review]
Fixed crash on QSensor destructor, and angle undefined variable on debug build

checked interdiff and it looks fine
Attachment #518818 - Flags: review?(mozilla) → review+
Comment on attachment 518818 [details] [diff] [review]
Fixed crash on QSensor destructor, and angle undefined variable on debug build

I think it safe enough for default builds.
Attachment #518818 - Flags: approval2.0?

Updated

7 years ago
Attachment #518818 - Flags: review?(ted.mielczarek)
Comment on attachment 518818 [details] [diff] [review]
Fixed crash on QSensor destructor, and angle undefined variable on debug build

I think Mitch already r+d the relevant bits, but regardless, r=me.
Attachment #518818 - Flags: review?(ted.mielczarek) → review+

Updated

7 years ago
Attachment #518818 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/02e5836b10c2
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.