Merge lp:~gerboland/qtubuntu/add--more-surface-type-support into lp:qtubuntu

[Gerry Boland]

Add support for more surface types and surface repositioning.

Helps Menus, Dropdowns and Tooltips to work with Mir

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:297
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/145/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/145/rebuild

review: Needs Fixing (continuous-integration)

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:297
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/147/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/147/rebuild

review: Needs Fixing (continuous-integration)

[Lukáš Tinkl (lukas-kde)]

Just a curious question inline :)

review: Needs Information

[Gerry Boland (gerboland)]

> Wouldn't Qt:Tool better be represented by mir_surface_type_satellite?

Maybe, but while Mir has the surface Type in an enum, it doesn't let me create surfaces of type satellite yet (i.e. no mir_connection_create_spec_for_satellite)! So I just chose the nearest type I can create.

This mapping is not final, I expect it to change in future. That's why I put the mapping in an obvious method.

[Gerry Boland (gerboland)]

Hmm, I've broken OSK with this. Marking WIP until I sort that out

[Gerry Boland (gerboland)]

Fixed

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:299
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/148/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/148/rebuild

review: Needs Fixing (continuous-integration)

[Daniel d'Andrada (dandrader)]

"""
 void UbuntuWindow::handleSurfaceVisibilityChanged(bool visible)
493 {
494 - qCDebug(ubuntumirclient, "handleSurfaceFocused(window=%p)", window());
495 + qCDebug(ubuntumirclient, "handleSurfaceVisibilityChanged(window=%p, visible=%d)", window(), visible);
"""

https://code.launchpad.net/~dandrader/qtubuntu/loggingFixes/+merge/305374 already touches this. Branches will conflict.

[lp:~gerboland/qtubuntu/add--more-surface-type-support]

300. By Gerry Boland on 2016-10-18

Fix for dialogs crash

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:300
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/149/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/149/rebuild

review: Needs Fixing (continuous-integration)

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/152/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/3193/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3221
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3077/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3077/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3077/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3077/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3077/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3077/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3077/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3077/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3077/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/152/rebuild

review: Needs Fixing (continuous-integration)

[lp:~gerboland/qtubuntu/add--more-surface-type-support]

301. By Gerry Boland on 2016-10-26

This is not python, fix comment

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:301
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/155/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/155/rebuild

review: Needs Fixing (continuous-integration)

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:301
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/158/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3199
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3227
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3083
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3083/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3083
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3083/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3083
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3083/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3083
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3083/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3083
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3083/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3083
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3083/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3083
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3083/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3083
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3083/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3083
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3083/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/158/rebuild

review: Approve (continuous-integration)

[Nick Dedekind (nick-dedekind)]

Couple of comments.

review: Needs Fixing

[Nick Dedekind (nick-dedekind)]

Ok, if you're confident! LGTM

review: Approve

[lp:~gerboland/qtubuntu/add--more-surface-type-support]

302. By Gerry Boland on 2016-11-03

Only set internal position in setGeometry, not size

303. By Gerry Boland on 2016-11-04

Merge trunk & resolve conflict

[Gerry Boland (gerboland)]

Hmm, I can't land this as it will break Autopilot tests on phones :(

=== modified file 'src/ubuntumirclient/input.cpp'
--- src/ubuntumirclient/input.cpp	2016-10-24 11:31:55 +0000
+++ src/ubuntumirclient/input.cpp	2016-11-04 13:24:44 +0000
@@ -503,6 +503,8 @@
     const auto localPoint = QPointF(mir_pointer_event_axis_value(pev, mir_pointer_axis_x),
                                     mir_pointer_event_axis_value(pev, mir_pointer_axis_y));
 
+    mLastInputWindow = platformWindow;
+
     switch (action) {
     case mir_pointer_action_button_up:
     case mir_pointer_action_button_down:

=== modified file 'src/ubuntumirclient/window.cpp'
--- src/ubuntumirclient/window.cpp	2016-10-24 11:32:31 +0000
+++ src/ubuntumirclient/window.cpp	2016-11-04 13:24:44 +0000
@@ -24,6 +24,7 @@
 #include "logging.h"
 
 #include <mir_toolkit/mir_client_library.h>
+#include <mir_toolkit/version.h>
 
 // Qt
 #include <qpa/qwindowsysteminterface.h>
@@ -123,6 +124,23 @@
     Q_UNREACHABLE();
 }
 
+const char *mirSurfaceTypeToStr(MirSurfaceType type)
+{
+    switch (type) {
+    case mir_surface_type_normal:       return "Normal";        /**< AKA "regular"                   */
+    case mir_surface_type_utility:      return "Utility";       /**< AKA "floating regular"          */
+    case mir_surface_type_dialog:       return "Dialog";
+    case mir_surface_type_gloss:        return "Gloss";
+    case mir_surface_type_freestyle:    return "Freestyle";
+    case mir_surface_type_menu:         return "Menu";
+    case mir_surface_type_inputmethod:  return "Input Method";  /**< AKA "OSK" or handwriting etc.   */
+    case mir_surface_type_satellite:    return "Satellite";     /**< AKA "toolbox"/"toolbar"         */
+    case mir_surface_type_tip:          return "Tip";           /**< AKA "tooltip"                   */
+    case mir_surface_types:             Q_UNREACHABLE();
+    }
+    return "";
+}
+
 MirSurfaceState qtWindowStateToMirSurfaceState(Qt::WindowState state)
 {
     switch (state) {
@@ -139,6 +157,27 @@
     return mir_surface_state_unknown; // should never be reached
 }
 
+MirSurfaceType qtWindowTypeToMirSurfaceType(Qt::WindowType type)
+{
+    switch (type & Qt::WindowType_Mask) {
+    case Qt::Dialog:
+        return mir_surface_type_dialog;
+    case Qt::Sheet:
+    case Qt::Drawer:
+        return mir_surface_type_utility;
+    case Qt::Popup:
+    case Qt::Tool:
+        return mir_surface_type_menu;
+    case Qt::ToolTip:
+        return mir_surface_type_tip;
+    case Qt::SplashScreen:
+        return mir_surface_type_freestyle;
+    case Qt::Window:
+    default:
+        return mir_surface_type_normal;
+    }
+}
+
 WId makeId()
 {
     static int id = 1;
@@ -164,51 +203,93 @@
     return parent ? static_cast<UbuntuWindow *>(parent->handle()) : nullptr;
 }
 
-Spec makeSurfaceSpec(QWindow *window, UbuntuInput *input, MirPixelFormat pixelFormat, MirConnection *connection)
-{
-    const auto geom = window->geometry();
-    const int width = geom.width() > 0 ? geom.width() : 1;
-    const int height = geom.height() > 0 ? geom.height() : 1;
+bool requiresParent(const MirSurfaceType type)
+{
+    switch (type) {
+    case mir_surface_type_dialog: //FIXME - not quite what the specification dictates, but is what Mir's api dictates
+    case mir_surface_type_utility:
+    case mir_surface_type_gloss:
+    case mir_surface_type_menu:
+    case mir_surface_type_satellite:
+    case mir_surface_type_tip:
+        return true;
+    default:
+        return false;
+    }
+}
+
+bool requiresParent(const Qt::WindowType type)
+{
+    return requiresParent(qtWindowTypeToMirSurfaceType(type));
+}
+
+bool isMovable(const Qt::WindowType type)
+{
+    auto mirType = qtWindowTypeToMirSurfaceType(type);
+    switch (mirType) {
+    case mir_surface_type_menu:
+    case mir_surface_type_tip:
+        return true;
+    default:
+        return false;
+    }
+}
+
+Spec makeSurfaceSpec(QWindow *window, MirPixelFormat pixelFormat, UbuntuWindow *parentWindowHandle,
+                     MirConnection *connection)
+{
+    const auto geometry = window->geometry();
+    const int width = geometry.width() > 0 ? geometry.width() : 1;
+    const int height = geometry.height() > 0 ? geometry.height() : 1;
+    auto type = qtWindowTypeToMirSurfaceType(window->type());
 
     if (U_ON_SCREEN_KEYBOARD_ROLE == roleFor(window)) {
-        qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p) - creating input method surface (width=%d, height=%d", window, width, height);
-        return Spec{mir_connection_create_spec_for_input_method(connection, width, height, pixelFormat)};
-    }
-
-    const Qt::WindowType type = window->type();
-    if (type == Qt::Popup) {
-        auto parent = transientParentFor(window);
-        if (parent == nullptr) {
-            //NOTE: We cannot have a parentless popup -
-            //try using the last surface to receive input as that will most likely be
-            //the one that caused this popup to be created
-            parent = input->lastInputWindow();
-        }
-        if (parent) {
-            auto pos = geom.topLeft();
-            pos -= parent->geometry().topLeft();
-            MirRectangle location{pos.x(), pos.y(), 0, 0};
-            qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p) - creating menu surface(width:%d, height:%d)", window, width, height);
-            return Spec{mir_connection_create_spec_for_menu(
-                    connection, width, height, pixelFormat, parent->mirSurface(),
-                    &location, mir_edge_attachment_any)};
-        } else {
-            qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p) - cannot create a menu without a parent!", window);
-        }
-    } else if (type == Qt::Dialog) {
-        auto parent = transientParentFor(window);
-        if (parent) {
-            // Modal dialog
-            qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p) - creating modal dialog (width=%d, height=%d", window, width, height);
-            return Spec{mir_connection_create_spec_for_modal_dialog(connection, width, height, pixelFormat, parent->mirSurface())};
-        } else {
-            // TODO: do Qt parentless dialogs have the same semantics as mir?
-            qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p) - creating parentless dialog (width=%d, height=%d)", window, width, height);
-            return Spec{mir_connection_create_spec_for_dialog(connection, width, height, pixelFormat)};
-        }
-    }
-    qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p) - creating normal surface(type=0x%x, width=%d, height=%d)", window, type, width, height);
-    return Spec{mir_connection_create_spec_for_normal_surface(connection, width, height, pixelFormat)};
+        type = mir_surface_type_inputmethod;
+    }
+
+    MirRectangle location{geometry.x(), geometry.y(), 0, 0};
+    MirSurface *parent = nullptr;
+    if (parentWindowHandle) {
+        parent = parentWindowHandle->mirSurface();
+        // Qt uses absolute positioning, but Mir positions surfaces relative to parent.
+        location.top  -= parentWindowHandle->geometry().top();
+        location.left -= parentWindowHandle->geometry().left();
+    }
+
+    Spec spec;
+
+    switch (type) {
+    case mir_surface_type_menu:
+        spec = Spec{mir_connection_create_spec_for_menu(connection, width, height, pixelFormat, parent,
+                    &location, mir_edge_attachment_any)};
+        break;
+    case mir_surface_type_dialog:
+        spec = Spec{mir_connection_create_spec_for_modal_dialog(connection, width, height, pixelFormat, parent)};
+        break;
+    case mir_surface_type_utility:
+        spec = Spec{mir_connection_create_spec_for_dialog(connection, width, height, pixelFormat)};
+        break;
+    case mir_surface_type_tip:
+#if MIR_CLIENT_VERSION < MIR_VERSION_NUMBER(3, 4, 0)
+        spec = Spec{mir_connection_create_spec_for_tooltip(connection, width, height, pixelFormat, parent,
+                    &location)};
+#else
+        spec = Spec{mir_connection_create_spec_for_tip(connection, width, height, pixelFormat, parent,
+                    &location, mir_edge_attachment_any)};
+#endif
+        break;
+    case mir_surface_type_inputmethod:
+        spec = Spec{mir_connection_create_spec_for_input_method(connection, width, height, pixelFormat)};
+        break;
+    default:
+        spec = Spec{mir_connection_create_spec_for_normal_surface(connection, width, height, pixelFormat)};
+        break;
+    }
+
+    qCDebug(ubuntumirclient, "makeSurfaceSpec(window=%p): %s spec (type=0x%x, position=(%d, %d)px, size=(%dx%d)px)",
+            window, mirSurfaceTypeToStr(type), window->type(), location.left, location.top, width, height);
+
+    return std::move(spec);
 }
 
 void setSizingConstraints(MirSurfaceSpec *spec, const QSize& minSize, const QSize& maxSize, const QSize& increment)
@@ -229,11 +310,11 @@
     }
 }
 
-MirSurface *createMirSurface(QWindow *window, int mirOutputId, UbuntuInput *input, MirPixelFormat pixelFormat,
-                             MirConnection *connection, mir_surface_event_callback inputCallback,
-                             void* inputContext)
+MirSurface *createMirSurface(QWindow *window, int mirOutputId, UbuntuWindow *parentWindowHandle,
+                             MirPixelFormat pixelFormat, MirConnection *connection,
+                             mir_surface_event_callback inputCallback, void *inputContext)
 {
-    auto spec = makeSurfaceSpec(window, input, pixelFormat, connection);
+    auto spec = makeSurfaceSpec(window, pixelFormat, parentWindowHandle, connection);
 
     // Install event handler as early as possible
     mir_surface_spec_set_event_handler(spec.get(), inputCallback, inputContext);
@@ -256,6 +337,20 @@
     return surface;
 }
 
+UbuntuWindow *getParentIfNecessary(QWindow *window, UbuntuInput *input)
+{
+    UbuntuWindow *parentWindowHandle = nullptr;
+    if (requiresParent(window->type())) {
+        parentWindowHandle = transientParentFor(window);
+        if (parentWindowHandle == nullptr) {
+            // NOTE: Mir requires this surface have a parent. Try using the last surface to receive input as that will
+            // most likely be the one that caused this surface to be created
+            parentWindowHandle = input->lastInputWindow();
+        }
+    }
+    return parentWindowHandle;
+}
+
 MirPixelFormat disableAlphaBufferIfPossible(MirPixelFormat pixelFormat)
 {
     switch(pixelFormat) {
@@ -287,98 +382,18 @@
 
 } //namespace
 
+
+
 class UbuntuSurface
 {
 public:
-    UbuntuSurface(UbuntuWindow *platformWindow, EGLDisplay display, UbuntuInput *input, MirConnection *connection)
-        : mWindow(platformWindow->window())
-        , mPlatformWindow(platformWindow)
-        , mInput(input)
-        , mConnection(connection)
-        , mEglDisplay(display)
-        , mNeedsRepaint(false)
-        , mParented(mWindow->transientParent() || mWindow->parent())
-        , mFormat(mWindow->requestedFormat())
-        , mShellChrome(mWindow->flags() & LowChromeWindowHint ? mir_shell_chrome_low : mir_shell_chrome_normal)
-    {
-        // Have Qt choose most suitable EGLConfig for the requested surface format, and update format to reflect it
-        EGLConfig config = q_configFromGLFormat(display, mFormat, true);
-        if (config == 0) {
-            // Older Intel Atom-based devices only support OpenGL 1.4 compatibility profile but by default
-            // QML asks for at least OpenGL 2.0. The XCB GLX backend ignores this request and returns a
-            // 1.4 context, but the XCB EGL backend tries to honour it, and fails. The 1.4 context appears to
-            // have sufficient capabilities on MESA (i915) to render correctly however. So reduce the default
-            // requested OpenGL version to 1.0 to ensure EGL will give us a working context (lp:1549455).
-            static const bool isMesa = QString(eglQueryString(display, EGL_VENDOR)).contains(QStringLiteral("Mesa"));
-            if (isMesa) {
-                qCDebug(ubuntumirclientGraphics, "Attempting to choose OpenGL 1.4 context which may suit Mesa");
-                mFormat.setMajorVersion(1);
-                mFormat.setMinorVersion(4);
-                config = q_configFromGLFormat(display, mFormat, true);
-            }
-        }
-        if (config == 0) {
-            qCritical() << "Qt failed to choose a suitable EGLConfig to suit the surface format" << mFormat;
-        }
-
-        mFormat = q_glFormatFromConfig(display, config, mFormat);
-
-        // Have Mir decide the pixel format most suited to the chosen EGLConfig. This is the only way
-        // Mir will know what EGLConfig has been chosen - it cannot deduce it from the buffers.
-        auto pixelFormat = mir_connection_get_egl_pixel_format(connection, display, config);
-        // But the chosen EGLConfig might have an alpha buffer enabled, even if not requested by the client.
-        // If that's the case, try to edit the chosen pixel format in order to disable the alpha buffer.
-        // This is an optimisation for the compositor, as it can avoid blending this surface.
-        if (mWindow->requestedFormat().alphaBufferSize() < 0) {
-            pixelFormat = disableAlphaBufferIfPossible(pixelFormat);
-        }
-
-        const auto outputId = static_cast<UbuntuScreen *>(mWindow->screen()->handle())->mirOutputId();
-
-        mMirSurface = createMirSurface(mWindow, outputId, input, pixelFormat, connection, surfaceEventCallback, this);
-        mEglSurface = eglCreateWindowSurface(mEglDisplay, config, nativeWindowFor(mMirSurface), nullptr);
-
-        mNeedsExposeCatchup = mir_surface_get_visibility(mMirSurface) == mir_surface_visibility_occluded;
-
-        // Window manager can give us a final size different from what we asked for
-        // so let's check what we ended up getting
-        MirSurfaceParameters parameters;
-        mir_surface_get_parameters(mMirSurface, &parameters);
-
-        auto geom = mWindow->geometry();
-        geom.setWidth(parameters.width);
-        geom.setHeight(parameters.height);
-        if (mWindow->windowState() == Qt::WindowFullScreen) {
-            geom.moveTop(0);
-        } else {
-            geom.moveTop(panelHeight());
-        }
-
-        // Assume that the buffer size matches the surface size at creation time
-        mBufferSize = geom.size();
-        platformWindow->QPlatformWindow::setGeometry(geom);
-        QWindowSystemInterface::handleGeometryChange(mWindow, geom);
-
-        qCDebug(ubuntumirclient) << "Created surface with geometry:" << geom << "title:" << mWindow->title()
-                                 << "role:" << roleFor(mWindow);
-        qCDebug(ubuntumirclientGraphics)
-                                 << "Requested format:" << mWindow->requestedFormat()
-                                 << "\nActual format:" << mFormat
-                                 << "with associated Mir pixel format:" << mirPixelFormatToStr(pixelFormat);
-    }
-
-    ~UbuntuSurface()
-    {
-        if (mEglSurface != EGL_NO_SURFACE)
-            eglDestroySurface(mEglDisplay, mEglSurface);
-        if (mMirSurface)
-            mir_surface_release_sync(mMirSurface);
-    }
+    UbuntuSurface(UbuntuWindow *platformWindow, EGLDisplay display, UbuntuInput *input, MirConnection *connection);
+    ~UbuntuSurface();
 
     UbuntuSurface(const UbuntuSurface &) = delete;
     UbuntuSurface& operator=(const UbuntuSurface &) = delete;
 
-    void resize(const QSize& newSize);
+    void updateGeometry(const QRect &newGeometry);
     void updateTitle(const QString& title);
     void setSizingConstraints(const QSize& minSize, const QSize& maxSize, const QSize& increment);
 
@@ -414,6 +429,7 @@
     UbuntuWindow * const mPlatformWindow;
     UbuntuInput * const mInput;
     MirConnection * const mConnection;
+    UbuntuWindow * mParentWindowHandle{nullptr};
 
     MirSurface* mMirSurface;
     const EGLDisplay mEglDisplay;
@@ -423,6 +439,7 @@
     bool mParented;
     QSize mBufferSize;
     QSurfaceFormat mFormat;
+    MirPixelFormat mPixelFormat;
 
     QMutex mTargetSizeMutex;
     QSize mTargetSize;
@@ -430,23 +447,102 @@
     QString mPersistentIdStr;
 };
 
-void UbuntuSurface::resize(const QSize& size)
-{
-    qCDebug(ubuntumirclient,"resize(window=%p, width=%d, height=%d)", mWindow, size.width(), size.height());
-
-    if (mWindow->windowState() == Qt::WindowFullScreen || mWindow->windowState() == Qt::WindowMaximized) {
-        qCDebug(ubuntumirclient, "resize(window=%p) - not resizing, window is maximized or fullscreen", mWindow);
-        return;
-    }
-
-    if (size.isEmpty()) {
-        qCDebug(ubuntumirclient, "resize(window=%p) - not resizing, size is empty", mWindow);
-        return;
-    }
-
-    Spec spec{mir_connection_create_spec_for_changes(mConnection)};
-    mir_surface_spec_set_width(spec.get(), size.width());
-    mir_surface_spec_set_height(spec.get(), size.height());
+UbuntuSurface::UbuntuSurface(UbuntuWindow *platformWindow, EGLDisplay display, UbuntuInput *input, MirConnection *connection)
+    : mWindow(platformWindow->window())
+    , mPlatformWindow(platformWindow)
+    , mInput(input)
+    , mConnection(connection)
+    , mEglDisplay(display)
+    , mNeedsRepaint(false)
+    , mParented(mWindow->transientParent() || mWindow->parent())
+    , mFormat(mWindow->requestedFormat())
+    , mShellChrome(mWindow->flags() & LowChromeWindowHint ? mir_shell_chrome_low : mir_shell_chrome_normal)
+{
+    // Have Qt choose most suitable EGLConfig for the requested surface format, and update format to reflect it
+    EGLConfig config = q_configFromGLFormat(display, mFormat, true);
+    if (config == 0) {
+        // Older Intel Atom-based devices only support OpenGL 1.4 compatibility profile but by default
+        // QML asks for at least OpenGL 2.0. The XCB GLX backend ignores this request and returns a
+        // 1.4 context, but the XCB EGL backend tries to honour it, and fails. The 1.4 context appears to
+        // have sufficient capabilities on MESA (i915) to render correctly however. So reduce the default
+        // requested OpenGL version to 1.0 to ensure EGL will give us a working context (lp:1549455).
+        static const bool isMesa = QString(eglQueryString(display, EGL_VENDOR)).contains(QStringLiteral("Mesa"));
+        if (isMesa) {
+            qCDebug(ubuntumirclientGraphics, "Attempting to choose OpenGL 1.4 context which may suit Mesa");
+            mFormat.setMajorVersion(1);
+            mFormat.setMinorVersion(4);
+            config = q_configFromGLFormat(display, mFormat, true);
+        }
+    }
+    if (config == 0) {
+        qCritical() << "Qt failed to choose a suitable EGLConfig to suit the surface format" << mFormat;
+    }
+
+    mFormat = q_glFormatFromConfig(display, config, mFormat);
+
+    // Have Mir decide the pixel format most suited to the chosen EGLConfig. This is the only way
+    // Mir will know what EGLConfig has been chosen - it cannot deduce it from the buffers.
+    mPixelFormat = mir_connection_get_egl_pixel_format(connection, display, config);
+    // But the chosen EGLConfig might have an alpha buffer enabled, even if not requested by the client.
+    // If that's the case, try to edit the chosen pixel format in order to disable the alpha buffer.
+    // This is an optimisation for the compositor, as it can avoid blending this surface.
+    if (mWindow->requestedFormat().alphaBufferSize() < 0) {
+        mPixelFormat = disableAlphaBufferIfPossible(mPixelFormat);
+    }
+
+    const auto outputId = static_cast<UbuntuScreen *>(mWindow->screen()->handle())->mirOutputId();
+
+    mParentWindowHandle = getParentIfNecessary(mWindow, input);
+
+    mMirSurface = createMirSurface(mWindow, outputId, mParentWindowHandle, mPixelFormat, connection, surfaceEventCallback, this);
+    mEglSurface = eglCreateWindowSurface(mEglDisplay, config, nativeWindowFor(mMirSurface), nullptr);
+
+    mNeedsExposeCatchup = mir_surface_get_visibility(mMirSurface) == mir_surface_visibility_occluded;
+
+    // Window manager can give us a final size different from what we asked for
+    // so let's check what we ended up getting
+    MirSurfaceParameters parameters;
+    mir_surface_get_parameters(mMirSurface, &parameters);
+
+    auto geom = mWindow->geometry();
+    geom.setWidth(parameters.width);
+    geom.setHeight(parameters.height);
+
+    // Assume that the buffer size matches the surface size at creation time
+    mBufferSize = geom.size();
+    platformWindow->QPlatformWindow::setGeometry(geom);
+    QWindowSystemInterface::handleGeometryChange(mWindow, geom);
+
+    qCDebug(ubuntumirclient) << "Created surface with geometry:" << geom << "title:" << mWindow->title()
+                             << "role:" << roleFor(mWindow);
+    qCDebug(ubuntumirclientGraphics)
+                             << "Requested format:" << mWindow->requestedFormat()
+                             << "\nActual format:" << mFormat
+                             << "with associated Mir pixel format:" << mirPixelFormatToStr(mPixelFormat);
+}
+
+UbuntuSurface::~UbuntuSurface()
+{
+    if (mEglSurface != EGL_NO_SURFACE)
+        eglDestroySurface(mEglDisplay, mEglSurface);
+    if (mMirSurface) {
+        mir_surface_release_sync(mMirSurface);
+    }
+}
+
+void UbuntuSurface::updateGeometry(const QRect &newGeometry)
+{
+    qCDebug(ubuntumirclient,"updateGeometry(window=%p, width=%d, height=%d)", mWindow,
+            newGeometry.width(), newGeometry.height());
+
+    Spec spec;
+    if (isMovable(mWindow->type())) {
+        spec = Spec{makeSurfaceSpec(mWindow, mPixelFormat, mParentWindowHandle, mConnection)};
+    } else {
+        spec = Spec{mir_connection_create_spec_for_changes(mConnection)};
+        mir_surface_spec_set_width(spec.get(), newGeometry.width());
+        mir_surface_spec_set_height(spec.get(), newGeometry.height());
+    }
     mir_surface_apply_spec(mMirSurface, spec.get());
 }
 
@@ -690,7 +786,7 @@
 
 void UbuntuWindow::handleSurfaceVisibilityChanged(bool visible)
 {
-    qCDebug(ubuntumirclient, "handleSurfaceVisibilityChanged(visible=%d)", visible);
+    qCDebug(ubuntumirclient, "handleSurfaceVisibilityChanged(window=%p, visible=%d)", window(), visible);
 
     if (mWindowVisible == visible) return;
     mWindowVisible = visible;
@@ -739,6 +835,10 @@
  */
 void UbuntuWindow::updatePanelHeightHack(bool enable)
 {
+    if ((window()->type() & Qt::WindowType_Mask) != Qt::Window) { // only plain windows get the hack
+        return;
+    }
+
     QMutexLocker lock(&mMutex);
 
     QRect newGeometry = geometry();
@@ -758,13 +858,20 @@
 void UbuntuWindow::setGeometry(const QRect &rect)
 {
     QMutexLocker lock(&mMutex);
+
+    if (window()->windowState() == Qt::WindowFullScreen || window()->windowState() == Qt::WindowMaximized) {
+        qCDebug(ubuntumirclient, "setGeometry(window=%p) - not resizing, window is maximized or fullscreen", window());
+        return;
+    }
+
     qCDebug(ubuntumirclient, "setGeometry (window=%p, position=(%d, %d)dp, size=(%dx%d)dp)",
             window(), rect.x(), rect.y(), rect.width(), rect.height());
-
-    //NOTE: mir surfaces cannot be moved by the client so ignore the topLeft coordinates
-    const auto newSize = rect.size();
-
-    mSurface->resize(newSize);
+    // Immediately update internal geometry so Qt believes position updated
+    QRect newPosition(geometry());
+    newPosition.moveTo(rect.topLeft());
+    QPlatformWindow::setGeometry(newPosition);
+
+    mSurface->updateGeometry(rect);
     // Note: don't call handleGeometryChange here, wait to see what Mir replies with.
 }