Merge lp:~alan-griffiths/miral/remove-libmirserver-dev-from-screen.h into lp:miral

[Alan Griffiths]

[miral-qt] Encapsulate class Screen's dependence on libmirserver

[lp:~alan-griffiths/miral/remove-libmirserver-dev-from-screen.h]

426. By Alan Griffiths on 2016-10-28

Can fix customscreenconfiguration.h simply now

[Gerry Boland (gerboland)]

=== added file 'miral-qt/src/platforms/mirserver/screen_configuration.h'
Please remove the underscore in the filename, it's not Qt-ish. It contains the definition of some Screen property types, not what I'd define "configuration" - screentypes.h maybe?

=== modified file 'miral-qt/src/modules/Unity/Screens/qquickscreenwindow.cpp'
-bool QQuickScreenWindow::setScaleAndFormFactor(const float scale, const Screens::FormFactor formFactor)

could you please restore this. It is useful when testing dynamic grid units in applications. (it is a method for QML can call - Q_INVOKABLE)

Rest looks just fine, thank you

review: Needs Fixing

[lp:~alan-griffiths/miral/remove-libmirserver-dev-from-screen.h]

427. By Alan Griffiths on 2016-10-28

screen_configuration.h => screentypes.h

428. By Alan Griffiths on 2016-10-28

Restore code that isn't really unused

[Alan Griffiths (alan-griffiths)]

> === added file 'miral-qt/src/platforms/mirserver/screen_configuration.h'
> Please remove the underscore in the filename, it's not Qt-ish. It contains the
> definition of some Screen property types, not what I'd define "configuration"
> - screentypes.h maybe?

Done
> === modified file 'miral-qt/src/modules/Unity/Screens/qquickscreenwindow.cpp'
> -bool QQuickScreenWindow::setScaleAndFormFactor(const float scale, const
> Screens::FormFactor formFactor)
>
> could you please restore this. It is useful when testing dynamic grid units in
> applications. (it is a method for QML can call - Q_INVOKABLE)

Done

[Gerry Boland (gerboland)]

Thank you!

review: Approve

=== modified file 'miral-qt/src/modules/Unity/Screens/CMakeLists.txt'
--- miral-qt/src/modules/Unity/Screens/CMakeLists.txt	2016-10-21 15:48:36 +0000
+++ miral-qt/src/modules/Unity/Screens/CMakeLists.txt	2016-10-28 14:35:40 +0000
@@ -14,10 +14,6 @@
     qquickscreenwindow.cpp
     )
 
-# screens.h references mir/graphics/display_configuration.h
-set_source_files_properties(screens.cpp             PROPERTIES COMPILE_FLAGS "${CMAKE_CXX_FLAGS} ${FIXME_MIRSERVER_CXXFLAGS}")
-set_source_files_properties(qquickscreenwindow.cpp  PROPERTIES COMPILE_FLAGS "${CMAKE_CXX_FLAGS} ${FIXME_MIRSERVER_CXXFLAGS}")
-
 add_library(unityscreensplugin SHARED
     ${SCREENSPLUGIN_SRC}
 )

=== modified file 'miral-qt/src/modules/Unity/Screens/plugin.cpp'
--- miral-qt/src/modules/Unity/Screens/plugin.cpp	2016-06-01 22:06:51 +0000
+++ miral-qt/src/modules/Unity/Screens/plugin.cpp	2016-10-28 14:35:40 +0000
@@ -36,7 +36,7 @@
         qRegisterMetaType<QScreen*>("QScreen*");
 
         qmlRegisterType<qtmir::Screens>(uri, 0, 1, "Screens");
-        qRegisterMetaType<qtmir::Screens::FormFactor>("Screens::FormFactor");
+        qRegisterMetaType<qtmir::FormFactor>("qtmir::FormFactor");
 
         qmlRegisterType<qtmir::QQuickScreenWindow>(uri, 0, 1, "ScreenWindow");
     }

=== modified file 'miral-qt/src/modules/Unity/Screens/qquickscreenwindow.cpp'
--- miral-qt/src/modules/Unity/Screens/qquickscreenwindow.cpp	2016-08-16 14:15:26 +0000
+++ miral-qt/src/modules/Unity/Screens/qquickscreenwindow.cpp	2016-10-28 14:35:40 +0000
@@ -48,7 +48,7 @@
 QQuickScreenWindow::QQuickScreenWindow(QQuickWindow *parent)
     : QQuickWindow(parent)
     , m_scale(-1.0) // start with invalid initial state, fetch correct value on first invokation
-    , m_formFactor(Screens::FormFactorUnknown)
+    , m_formFactor(FormFactorUnknown)
 {
     if (qGuiApp->platformName() == QLatin1String("mirserver")) {
         connect(qGuiApp->platformNativeInterface(), &QPlatformNativeInterface::windowPropertyChanged,
@@ -91,7 +91,7 @@
     return m_scale;
 }
 
-bool QQuickScreenWindow::setScaleAndFormFactor(const float scale, const Screens::FormFactor formFactor)
+bool QQuickScreenWindow::setScaleAndFormFactor(const float scale, const FormFactor formFactor)
 {
     if (qFuzzyCompare(scale, m_scale) && formFactor == m_formFactor) {
         return true;
@@ -125,9 +125,9 @@
     return controller->setConfiguration(configs);
 }
 
-Screens::FormFactor QQuickScreenWindow::formFactor()
+FormFactor QQuickScreenWindow::formFactor()
 {
-    if (m_formFactor == Screens::FormFactorUnknown) {
+    if (m_formFactor == FormFactorUnknown) {
         m_formFactor = getFormFactorNativeProperty();
     }
     return m_formFactor;
@@ -173,7 +173,7 @@
     return scale;
 }
 
-Screens::FormFactor QQuickScreenWindow::getFormFactorNativeProperty() const
+FormFactor QQuickScreenWindow::getFormFactorNativeProperty() const
 {
     QVariant formFactorVal = qGuiApp->platformNativeInterface()
                                 ->windowProperty(handle(), QStringLiteral("formFactor"));
@@ -181,5 +181,5 @@
         return m_formFactor;
     }
 
-    return static_cast<Screens::FormFactor>(formFactorVal.toInt());
+    return static_cast<FormFactor>(formFactorVal.toInt());
 }

=== modified file 'miral-qt/src/modules/Unity/Screens/qquickscreenwindow.h'
--- miral-qt/src/modules/Unity/Screens/qquickscreenwindow.h	2016-06-01 22:06:51 +0000
+++ miral-qt/src/modules/Unity/Screens/qquickscreenwindow.h	2016-10-28 14:35:40 +0000
@@ -28,7 +28,7 @@
 
     Q_PROPERTY(QScreen *screen READ screen WRITE setScreen NOTIFY screenChanged)
     Q_PROPERTY(float scale READ scale NOTIFY scaleChanged)
-    Q_PROPERTY(Screens::FormFactor formFactor READ formFactor NOTIFY formFactorChanged)
+    Q_PROPERTY(FormFactor formFactor READ formFactor NOTIFY formFactorChanged)
 
 public:
     explicit QQuickScreenWindow(QQuickWindow *parent = 0);
@@ -37,13 +37,13 @@
     void setScreen(QScreen *screen);
 
     qreal scale();
-    Screens::FormFactor formFactor();
-    Q_INVOKABLE bool setScaleAndFormFactor(const float scale, const Screens::FormFactor formFactor);
+    FormFactor formFactor();
+    Q_INVOKABLE bool setScaleAndFormFactor(const float scale, const FormFactor formFactor);
 
 Q_SIGNALS:
     void screenChanged(QScreen *screen);
     void scaleChanged(qreal scale);
-    void formFactorChanged(Screens::FormFactor arg);
+    void formFactorChanged(FormFactor arg);
 
 private Q_SLOTS:
     void nativePropertyChanged(QPlatformWindow *window, const QString &propertyName);
@@ -51,8 +51,8 @@
 private:
     float getScaleNativeProperty() const;
     float m_scale;
-    Screens::FormFactor getFormFactorNativeProperty() const;
-    Screens::FormFactor m_formFactor;
+    FormFactor getFormFactorNativeProperty() const;
+    FormFactor m_formFactor;
 };
 
 } //namespace qtmir

=== modified file 'miral-qt/src/modules/Unity/Screens/screens.cpp'
--- miral-qt/src/modules/Unity/Screens/screens.cpp	2016-06-01 22:06:51 +0000
+++ miral-qt/src/modules/Unity/Screens/screens.cpp	2016-10-28 14:35:40 +0000
@@ -62,7 +62,7 @@
     case OutputTypeRole: {
         auto screen = static_cast<Screen*>(m_screenList.at(index.row())->handle());
         if (screen) {
-            return QVariant(static_cast<OutputTypes>(screen->outputType())); //FIXME: cheeky
+            return QVariant(screen->outputType());
         } else {
             return OutputTypes::Unknown;
         }

=== modified file 'miral-qt/src/modules/Unity/Screens/screens.h'
--- miral-qt/src/modules/Unity/Screens/screens.h	2016-06-01 22:06:51 +0000
+++ miral-qt/src/modules/Unity/Screens/screens.h	2016-10-28 14:35:40 +0000
@@ -17,6 +17,8 @@
 #ifndef SCREENS_H
 #define SCREENS_H
 
+#include "screentypes.h"
+
 #include <QAbstractListModel>
 
 class QScreen;
@@ -39,33 +41,6 @@
         FormFactorRole,
     };
 
-    enum OutputTypes {
-        Unknown,
-        VGA,
-        DVII,
-        DVID,
-        DVIA,
-        Composite,
-        SVideo,
-        LVDS,
-        Component,
-        NinePinDIN,
-        DisplayPort,
-        HDMIA,
-        HDMIB,
-        TV,
-        EDP
-    };
-
-    enum FormFactor {
-        FormFactorUnknown,
-        FormFactorPhone,
-        FormFactorTablet,
-        FormFactorMonitor,
-        FormFactorTV,
-        FormFactorProjector,
-    };
-
     explicit Screens(QObject *parent = 0);
     virtual ~Screens() noexcept = default;
 
@@ -91,6 +66,4 @@
 
 } // namespace qtmir
 
-Q_DECLARE_METATYPE(qtmir::Screens::FormFactor)
-
 #endif // SCREENS_H

=== modified file 'miral-qt/src/platforms/mirserver/CMakeLists.txt'
--- miral-qt/src/platforms/mirserver/CMakeLists.txt	2016-10-27 07:30:45 +0000
+++ miral-qt/src/platforms/mirserver/CMakeLists.txt	2016-10-28 14:35:40 +0000
@@ -74,12 +74,14 @@
     windowmanagementpolicy.cpp
     mirserverhooks.cpp mirserverhooks.h
     setqtcompositor.cpp setqtcompositor.h
+    tracepoints.c
 # We need to run moc on these headers
     ${APPLICATION_API_INCLUDEDIR}/unity/shell/application/Mir.h
     ${MIRAL_QT_SOURCE_DIR}/src/common/appnotifier.h
     ${MIRAL_QT_SOURCE_DIR}/src/common/windowcontrollerinterface.h
     ${MIRAL_QT_SOURCE_DIR}/src/common/windowmodelnotifier.h
 )
+set_source_files_properties(tracepoints.c PROPERTIES COMPILE_FLAGS "${CMAKE_CFLAGS} -fPIC")
 
 include_directories(SYSTEM ${MIRSERVER_INCLUDE_DIRS})
 
@@ -97,7 +99,6 @@
     mirserverintegration.cpp
     miropenglcontext.cpp
     offscreensurface.cpp
-    tracepoints.c
     promptsessionmanager.cpp promptsessionmanager.h promptsession.h
 # We need to run moc on these headers
     ${APPLICATION_API_INCLUDEDIR}/unity/shell/application/MirMousePointerInterface.h

=== modified file 'miral-qt/src/platforms/mirserver/customscreenconfiguration.h'
--- miral-qt/src/platforms/mirserver/customscreenconfiguration.h	2016-06-01 22:06:51 +0000
+++ miral-qt/src/platforms/mirserver/customscreenconfiguration.h	2016-10-28 14:35:40 +0000
@@ -20,11 +20,13 @@
 #include <QPoint>
 #include <QVector>
 
-#include <mir/graphics/display_configuration.h>
+#include "screentypes.h"
+#include <mir_toolkit/common.h>
+
 
 struct CustomScreenConfiguration
 {
-    mir::graphics::DisplayConfigurationOutputId id;
+    qtmir::OutputId id;
 
     QPoint topLeft;
     uint32_t currentModeIndex;

=== modified file 'miral-qt/src/platforms/mirserver/screen.cpp'
--- miral-qt/src/platforms/mirserver/screen.cpp	2016-09-27 15:11:04 +0000
+++ miral-qt/src/platforms/mirserver/screen.cpp	2016-10-28 14:35:40 +0000
@@ -24,6 +24,7 @@
 #include "mir/graphics/buffer.h"
 #include "mir/graphics/display_buffer.h"
 #include "mir/graphics/display.h"
+#include <mir/graphics/display_configuration.h>
 #include <mir/renderer/gl/render_target.h>
 
 // Qt
@@ -96,25 +97,25 @@
     }
 }
 
-QString displayTypeToString(enum mir::graphics::DisplayConfigurationOutputType type)
+QString displayTypeToString(qtmir::OutputTypes type)
 {
-    typedef mir::graphics::DisplayConfigurationOutputType Type;
+    typedef qtmir::OutputTypes Type;
     switch (type) {
-    case Type::vga:           return QStringLiteral("VGP");
-    case Type::dvii:          return QStringLiteral("DVI-I");
-    case Type::dvid:          return QStringLiteral("DVI-D");
-    case Type::dvia:          return QStringLiteral("DVI-A");
-    case Type::composite:     return QStringLiteral("Composite");
-    case Type::svideo:        return QStringLiteral("S-Video");
-    case Type::lvds:          return QStringLiteral("LVDS");
-    case Type::component:     return QStringLiteral("Component");
-    case Type::ninepindin:    return QStringLiteral("9 Pin DIN");
-    case Type::displayport:   return QStringLiteral("DisplayPort");
-    case Type::hdmia:         return QStringLiteral("HDMI-A");
-    case Type::hdmib:         return QStringLiteral("HDMI-B");
-    case Type::tv:            return QStringLiteral("TV");
-    case Type::edp:           return QStringLiteral("EDP");
-    case Type::unknown:
+    case Type::VGA:           return QStringLiteral("VGP");
+    case Type::DVII:          return QStringLiteral("DVI-I");
+    case Type::DVID:          return QStringLiteral("DVI-D");
+    case Type::DVIA:          return QStringLiteral("DVI-A");
+    case Type::Composite:     return QStringLiteral("Composite");
+    case Type::SVideo:        return QStringLiteral("S-Video");
+    case Type::LVDS:          return QStringLiteral("LVDS");
+    case Type::Component:     return QStringLiteral("Component");
+    case Type::NinePinDIN:    return QStringLiteral("9 Pin DIN");
+    case Type::DisplayPort:   return QStringLiteral("DisplayPort");
+    case Type::HDMIA:         return QStringLiteral("HDMI-A");
+    case Type::HDMIB:         return QStringLiteral("HDMI-B");
+    case Type::TV:            return QStringLiteral("TV");
+    case Type::EDP:           return QStringLiteral("EDP");
+    case Type::Unknown:
     default:
         return QStringLiteral("Unknown");
     } //switch
@@ -211,7 +212,7 @@
 
     // Output data - each output has a unique id and corresponding type. Can be multiple cards.
     m_outputId = screen.id;
-    m_type = screen.type;
+    m_type = static_cast<qtmir::OutputTypes>(screen.type); //FIXME: need compile time check these are equivalent
 
     // Physical screen size
     m_physicalSize.setWidth(screen.physical_size_mm.width.as_int());
@@ -412,7 +413,7 @@
 bool Screen::internalDisplay() const
 {
     using namespace mir::graphics;
-    if (m_type == DisplayConfigurationOutputType::lvds || m_type == DisplayConfigurationOutputType::edp) {
+    if (m_type == qtmir::OutputTypes::LVDS || m_type == qtmir::OutputTypes::EDP) {
         return true;
     }
     return false;

=== modified file 'miral-qt/src/platforms/mirserver/screen.h'
--- miral-qt/src/platforms/mirserver/screen.h	2016-09-27 15:11:04 +0000
+++ miral-qt/src/platforms/mirserver/screen.h	2016-10-28 14:35:40 +0000
@@ -25,15 +25,16 @@
 #include <qpa/qplatformscreen.h>
 
 // Mir
-#include <mir/graphics/display_configuration.h>
+#include <mir_toolkit/common.h>
 
 // local
 #include "cursor.h"
 #include "screenwindow.h"
+#include "screentypes.h"
 
 class QOrientationSensor;
 namespace mir {
-    namespace graphics { class DisplayBuffer; class DisplaySyncGroup; }
+    namespace graphics { class DisplayBuffer; class DisplaySyncGroup; class DisplayConfigurationOutput; }
     namespace renderer { namespace gl { class RenderTarget; }}
 }
 
@@ -59,8 +60,8 @@
     float scale() const { return m_scale; }
     MirFormFactor formFactor() const { return m_formFactor; }
     MirPowerMode powerMode() const { return m_powerMode; }
-    mir::graphics::DisplayConfigurationOutputId outputId() const { return m_outputId; }
-    mir::graphics::DisplayConfigurationOutputType outputType() const { return m_type; }
+    qtmir::OutputId outputId() const { return m_outputId; }
+    qtmir::OutputTypes outputType() const { return m_type; }
     uint32_t currentModeIndex() const { return m_currentModeIndex; }
 
     ScreenWindow* window() const;
@@ -101,8 +102,8 @@
 
     mir::renderer::gl::RenderTarget *m_renderTarget;
     mir::graphics::DisplaySyncGroup *m_displayGroup;
-    mir::graphics::DisplayConfigurationOutputId m_outputId;
-    mir::graphics::DisplayConfigurationOutputType m_type;
+    qtmir::OutputId m_outputId;
+    qtmir::OutputTypes m_type;
     MirPowerMode m_powerMode;
 
     Qt::ScreenOrientation m_nativeOrientation;

=== modified file 'miral-qt/src/platforms/mirserver/screenscontroller.cpp'
--- miral-qt/src/platforms/mirserver/screenscontroller.cpp	2016-08-16 14:15:26 +0000
+++ miral-qt/src/platforms/mirserver/screenscontroller.cpp	2016-10-28 14:35:40 +0000
@@ -16,6 +16,7 @@
 
 #include "screenscontroller.h"
 #include "screen.h"
+#include "screensmodel.h"
 
 // Mir
 #include <mir/graphics/display.h>

=== modified file 'miral-qt/src/platforms/mirserver/screenscontroller.h'
--- miral-qt/src/platforms/mirserver/screenscontroller.h	2016-08-16 14:15:26 +0000
+++ miral-qt/src/platforms/mirserver/screenscontroller.h	2016-10-28 14:35:40 +0000
@@ -24,7 +24,10 @@
 
 // local
 #include "customscreenconfiguration.h"
-#include "screensmodel.h"
+
+#include <memory>
+
+class ScreensModel;
 
 namespace mir {
     namespace graphics { class Display; }

=== added file 'miral-qt/src/platforms/mirserver/screentypes.h'
--- miral-qt/src/platforms/mirserver/screentypes.h	1970-01-01 00:00:00 +0000
+++ miral-qt/src/platforms/mirserver/screentypes.h	2016-10-28 14:35:40 +0000
@@ -0,0 +1,60 @@
+/*
+ * Copyright © 2016 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU Lesser General Public License version 3, as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+ * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QTMIR_SCREEN_TYPES_H
+#define QTMIR_SCREEN_TYPES_H
+
+#include <mir/int_wrapper.h>
+
+#include <QtCore/qmetatype.h>
+
+namespace mir { namespace graphics { namespace detail { struct GraphicsConfOutputIdTag; } } }
+
+namespace qtmir
+{
+using OutputId = mir::IntWrapper<mir::graphics::detail::GraphicsConfOutputIdTag>;
+
+enum OutputTypes {
+    Unknown,
+    VGA,
+    DVII,
+    DVID,
+    DVIA,
+    Composite,
+    SVideo,
+    LVDS,
+    Component,
+    NinePinDIN,
+    DisplayPort,
+    HDMIA,
+    HDMIB,
+    TV,
+    EDP
+};
+
+enum FormFactor {
+    FormFactorUnknown,
+    FormFactorPhone,
+    FormFactorTablet,
+    FormFactorMonitor,
+    FormFactorTV,
+    FormFactorProjector,
+};
+}
+
+Q_DECLARE_METATYPE(qtmir::FormFactor)
+
+#endif //QTMIR_SCREEN_TYPES_H

=== modified file 'miral-qt/tests/mirserver/Screen/screen_test.cpp'
--- miral-qt/tests/mirserver/Screen/screen_test.cpp	2016-06-01 22:06:51 +0000
+++ miral-qt/tests/mirserver/Screen/screen_test.cpp	2016-10-28 14:35:40 +0000
@@ -80,7 +80,7 @@
     EXPECT_EQ(screen->format(), QImage::Format_RGBA8888);
     EXPECT_EQ(screen->refreshRate(), 59);
     EXPECT_EQ(screen->physicalSize(), QSize(1111, 2222));
-    EXPECT_EQ(screen->outputType(), mg::DisplayConfigurationOutputType::dvid);
+    EXPECT_EQ(screen->outputType(), qtmir::OutputTypes::DVID);
 }
 
 TEST_F(ScreenTest, ReadDifferentConfigurationFromDisplayConfig)
@@ -93,5 +93,5 @@
     EXPECT_EQ(screen->format(), QImage::Format_RGBX8888);
     EXPECT_EQ(screen->refreshRate(), 75);
     EXPECT_EQ(screen->physicalSize(), QSize(1000, 2000));
-    EXPECT_EQ(screen->outputType(), mg::DisplayConfigurationOutputType::lvds);
+    EXPECT_EQ(screen->outputType(), qtmir::OutputTypes::LVDS);
 }