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

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

In src/modules/Unity/Application/mirsurfaceitem.cpp:

"""
    case Qt::PortraitOrientation:
        mirOrientation = mir_orientation_normal;
"""

Here you're assuming that the screen aspect-ratio is portrait, which is not always the case.

Qt::ScreenOrientation values do not map directly to MirOrientation ones. The two enums express orientation differently. Trying to shoehorn one into the other is no good.

Either do a proper translation between the two or make MirSurfaceItem::orientation use an enum that does match with MirOrientation.

------------------------

In src/platforms/mirserver/screen.cpp:

Who's calling Screen::toggleSensors()?
It was supposed to be called whenever the display goes on and off.

---------------------------

In src/platforms/mirserver/screen.cpp:

"""
    QCoreApplication::postEvent(this, new OrientationReadingEvent(
                                    OrientationReadingEvent::m_type, m_orientationSensor->reading()->orientation()));
"""

Weird indentation.

review: Needs Fixing

« Back to merge proposal