Code review comment for lp:~gerboland/qtmir/exposeOrientation

> > I think it's a bad idea that there are 2 ways to express orientation, and we
> > should just use one.
>
> But that's what we have in your patch. There's Qt::ScreenOrientation and
> MirOrientation. Ignoring it will not make it go away. :)
>
> So I will more explicitly: I see two ways of fixing it:
> 1 - Doing the proper translation from Qt::ScreenOrientation to MirOrientation
> in MirSurfaceItem::setOrientation

I finally saw what you mean. I'm not such a big fan of it, and it relies other toolkits to deal with orientation in the same way Qt does (i.e. compare supplied orientation with a deduced native orientation), but anyway: it's done.

> > You don't like that I ignore PrimaryOrientation?
>
> It's not about that. But now that you mentioned it, there should be at least a
> warning if unity8 uses it if it's explicitly not supported. Otherwise we would
> be hiding an implementation bug in unity8.

I implemented it properly, PrimaryOrientation = native orientation.

> > > ------------------------
> > >
> > > In src/platforms/mirserver/screen.cpp:
> > >
> > > Who's calling Screen::toggleSensors()?
> > > It was supposed to be called whenever the display goes on and off.
> >
> > Nobody yet. I had hoped to wire it up to the hide/show window event for
> > display off/on, but that event doesn't happen. I'm not sure what signal to
> > wire it up to still. Will be later MR.
>
> If not body uses it then it's dead code. I think it would be better to only
> add it once we have use for it.
>
> Btw, I think we should have a big FIXME somewhere saying that we must turn off
> the orientation sensor when the display is off.

It's not dead if we'll use it soon :) I added a FIXME on top of the method. Will that do?