Code review comment for lp:~gerboland/unity8/orientationLock

> OrientationLock::savedOrientation is not being used anywhere.
I added code to use it, but it's not persistent over logout/in yet so is a touch pointless. I wanted to implement the persistence in a separate MR, but I can do it now if you like.

> ----------------------------------------------------------------
>
> In plugins/Unity/Session/plugin.cpp:
>
> """
> -static QObject *dbusunitysessionservice_provider(QQmlEngine *engine,
> QJSEngine *scriptEngine)
> +static QObject *dbusunitysessionservice_provider(QQmlEngine */*engine*/,
> QJSEngine */*jsEngine*/)
> {
> - Q_UNUSED(engine)
> - Q_UNUSED(scriptEngine)
> -
> """
>
> Why? Is using Q_UNUSED() bad form?

I recall Saviq saying he preferred this to Q_UNUSED. Depends on how clever the compiler is really - if we don't name the argument then the compiler definitely knows not to bother with it. But if we use Q_UNUSED it depends on if the compiler is smart enough to guess our intentions. I prefer this way too really.

> ---------------------------------------------------------------------
>
> In qml/Shell.qml
>
> """
> + readonly property int deviceOrientation:
> Screen.angleBetween(Screen.primaryOrientation, Screen.orientation)
> """
>
> Please call it "orientation angle" or "rotation" instead of "orientation" as
> it's not a Qt.ScreenOrientation type.

Done.

> ---------------------------------------------------------------------
>
> In qml/Stages/TabletStage.qml:
>
> """
> + onInteractiveChanged: {
> + if (interactive) {
> + appDelegate.orientation = Qt.binding( function()
> { return root.orientation; } );
> + } else {
> + appDelegate.orientation = root.orientation; //
> breaks the binding intentionally
> + }
> + }
> """
>
> Could it use the same approach as qml/Stages/PhoneStage.qml?

Done

>
> ---------------------------------------------------------------------
>
> Run "make tryShell", "make tryApplicationWindow" or "make trySpreadDelegate".
> The "SURFACE" word is rotated.
>
> ---------------------------------------------------------------------
>
> Please rotate the whole MirSurfaceItem mock and not just its text. Like this:
> http://paste.ubuntu.com/8292833/

The orientation of the word was my hint to tester the orientation of the mock surface - I thought rotating the image and scaling it was more ugly. But I did it your way, and yeah it's more clear what's going on.

> -----------------------------------------------------------------------
>
> Add a test in SessionContainer that verifies that child sessions rotate along
> with the root one.

Done

> In qml/Stages/ApplicationWindow.qml:
>
> """
> surface.orientation = Qt.binding( function() { return
> root.orientation; } );
> """
>
> Please make it a Binding{} and put it alongside other similar bindings in
> SessionContainer.qml
> Then ApplicationWindow.orientation would be just an alias to its internal
> SessionContainer.orientation.

Done