qtubuntu

Merge lp:~nick-dedekind/qtubuntu/shell_chrome into lp:qtubuntu

shell_chrome
Merge into trunk

Proposed by Nick Dedekind on 2016-02-17

Status:

Merged

Approved by:

Gerry Boland on 2016-03-16

Approved revision:

317

Merged at revision:

311

Proposed branch:

lp:~nick-dedekind/qtubuntu/shell_chrome

Merge into:

lp:qtubuntu

Diff against target:

516 lines (+212/-79)

3 files modified

src/ubuntumirclient/input.cpp (+39/-1)
src/ubuntumirclient/window.cpp (+162/-77)
src/ubuntumirclient/window.h (+11/-1)

To merge this branch:

bzr merge lp:~nick-dedekind/qtubuntu/shell_chrome

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Gerry Boland (community)		2016-02-17	Approve on 2016-03-16
Unity8 CI Bot	continuous-integration		Approve on 2016-03-14
PS Jenkins bot	continuous-integration		Needs Fixing on 2016-02-18
Review via email: mp+286308@code.launchpad.net

Commit message

Added support for low shell chrome

Description of the change

Added support for low shell chrome

https://code.launchpad.net/~nick-dedekind/unity-api/shell_chrome/+merge/286309
https://code.launchpad.net/~nick-dedekind/qtmir/shell_chrome/+merge/286307
https://code.launchpad.net/~nick-dedekind/unity8/shell_chrome/+merge/286306

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-17:

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-17:

FAILED: Continuous integration, rev:311
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/306/
Executed test runs:
FAILURE: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/97/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtubuntu-ci/306/rebuild

review: Needs Fixing (continuous-integration)

lp:~nick-dedekind/qtubuntu/shell_chrome updated on 2016-02-18

312. By Nick Dedekind on 2016-02-18: use hidden state

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-18:

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-18:

FAILED: Continuous integration, rev:312
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/308/
Executed test runs:
FAILURE: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/99/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtubuntu-ci/308/rebuild

review: Needs Fixing (continuous-integration)

Gerry Boland (gerboland) wrote on 2016-03-10:

+++ src/ubuntumirclient/input.cpp
at the end of the anonymous namespace, could you please add "// namespace"
I always find
+}
+
+}
hard to parse because you're missing the hint the indent usually gives - since we don't indent namespace contents.

+++ src/ubuntumirclient/window.cpp
+const Qt::WindowType WindowHidesShellDecorations = (Qt::WindowType)0x00800000;
being a pedant, but if mir chose the name "shell chrome" I guess we should use its vocabulary too.

+ void updateSurface();
While it was private, I didn't mind, but now it is public, I do. Function name is terrible considering what it does. Please either rename, or refactor a little to avoid making it public member of UbuntuSurface.

There is a general problem in this file, it's unclear what UbuntuSurface is, and how it relates to UbuntuWindow, and what responsibilities it has. My understanding was that UbuntuSurface should wrap the mir surface api in a Qt way, but as you saw it also is saving state, and referring to state in UbuntuWindow/QWindow when working. So the separation of responsibilities here is totally unclear.

So what you've done is natural, moving some states out of UbuntuSurface into UbuntuWindow. What is annoying is that UbuntuSurface imposes some sanity checks, referring to those states. I suspect we'll try to remove those, and trust that Mir will impose correct behaviour, instead of doing that here.

But
+ MirShellChrome mShellChrome;
does we need to save this state? Can we not just query the value from the mirclient api?

+ const QRect& exposeRect = mWindowVisible ? QRect(QPoint(), geometry().size()) : QRect();
& beside the variable name plz.

review: Needs Fixing

lp:~nick-dedekind/qtubuntu/shell_chrome updated on 2016-03-10

313. By Nick Dedekind on 2016-03-10: renamed flag
314. By Nick Dedekind on 2016-03-10: parent surface changes
315. By Nick Dedekind on 2016-03-10: removed unneeded code

Nick Dedekind (nick-dedekind) wrote on 2016-03-10:

> +++ src/ubuntumirclient/input.cpp
> at the end of the anonymous namespace, could you please add "// namespace"
> I always find
> +}
> +
> +}
> hard to parse because you're missing the hint the indent usually gives - since
> we don't indent namespace contents.

Done

>
>
> +++ src/ubuntumirclient/window.cpp
> +const Qt::WindowType WindowHidesShellDecorations =
> (Qt::WindowType)0x00800000;
> being a pedant, but if mir chose the name "shell chrome" I guess we should use
> its vocabulary too.

Done

>
> + void updateSurface();
> While it was private, I didn't mind, but now it is public, I do. Function name
> is terrible considering what it does. Please either rename, or refactor a
> little to avoid making it public member of UbuntuSurface.

I've refactored, but not sure it's the best way. I'm wondering if we can mode the code to QPlatformWindow::setParent.
We should probably be documenting everything we need to refactor!

>
> There is a general problem in this file, it's unclear what UbuntuSurface is,
> and how it relates to UbuntuWindow, and what responsibilities it has. My
> understanding was that UbuntuSurface should wrap the mir surface api in a Qt
> way, but as you saw it also is saving state, and referring to state in
> UbuntuWindow/QWindow when working. So the separation of responsibilities here
> is totally unclear.
>
> So what you've done is natural, moving some states out of UbuntuSurface into
> UbuntuWindow. What is annoying is that UbuntuSurface imposes some sanity
> checks, referring to those states. I suspect we'll try to remove those, and
> trust that Mir will impose correct behaviour, instead of doing that here.
>
> But
> + MirShellChrome mShellChrome;
> does we need to save this state? Can we not just query the value from the
> mirclient api?

There doesnt seem to be a client query api for this. It's pushed through the surface spec.

>
> + const QRect& exposeRect = mWindowVisible ? QRect(QPoint(),
> geometry().size()) : QRect();
> & beside the variable name plz.

Not needed. removed.

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-10:

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

review: Needs Fixing (continuous-integration)

lp:~nick-dedekind/qtubuntu/shell_chrome updated on 2016-03-10

316. By Nick Dedekind on 2016-03-10: log

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-10:

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

review: Approve (continuous-integration)

lp:~nick-dedekind/qtubuntu/shell_chrome updated on 2016-03-14

317. By Nick Dedekind on 2016-03-14: removed check for input method

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-14:

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

review: Approve (continuous-integration)

Gerry Boland (gerboland) wrote on 2016-03-16:

Ok, looks fine. Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Daniel d'Andrada

Nick Dedekind

Ubuntu Phablet Team

 === modified file 'src/ubuntumirclient/input.cpp'
 --- src/ubuntumirclient/input.cpp	2016-02-02 11:07:38 +0000
 +++ src/ubuntumirclient/input.cpp	2016-03-14 17:37:49 +0000
@@ -38,6 +38,9 @@
  Q_LOGGING_CATEGORY(ubuntumirclientInput, "ubuntumirclient.input", QtWarningMsg)
++namespace
++{
++
  // XKB Keysyms which do not map directly to Qt types (i.e. Unicode points)
  static const uint32_t KeyTable[] = {
      XKB_KEY_Escape,                  Qt::Key_Escape,
@@ -118,6 +121,29 @@
 ,                          0
  };
++Qt::WindowState mirSurfaceStateToWindowState(MirSurfaceState state)
++{
++    switch (state) {
++    case mir_surface_state_fullscreen:
++        return Qt::WindowFullScreen;
++    case mir_surface_state_maximized:
++    case mir_surface_state_vertmaximized:
++    case mir_surface_state_horizmaximized:
++        return Qt::WindowMaximized;
++    case mir_surface_state_minimized:
++        return Qt::WindowMinimized;
++    case mir_surface_state_hidden:
++        // We should be handling this state separately.
++        Q_ASSERT(false);
++    case mir_surface_state_restored:
++    case mir_surface_state_unknown:
++    default:
++        return Qt::WindowNoState;
++    }
++}
++
++} // namespace
++
  class UbuntuEvent : public QEvent
+ {
  public:
@@ -226,7 +252,9 @@
      case mir_event_type_surface:
+     {
          auto surfaceEvent = mir_event_get_surface_event(nativeEvent);
--        if (mir_surface_event_get_attribute(surfaceEvent) == mir_surface_attrib_focus) {
++        auto surfaceEventAttribute = mir_surface_event_get_attribute(surfaceEvent);
++
++        if (surfaceEventAttribute == mir_surface_attrib_focus) {
              const bool focused = mir_surface_event_get_attribute_value(surfaceEvent) == mir_surface_focused;
              // Mir may have sent a pair of focus lost/gained events, so we need to "peek" into the queue
              // so that we don't deactivate windows prematurely.
@@ -245,6 +273,16 @@
                  QWindowSystemInterface::handleWindowActivated(nullptr, Qt::ActiveWindowFocusReason);
                  QWindowSystemInterface::handleApplicationStateChanged(Qt::ApplicationInactive);
+             }
++        } else if (surfaceEventAttribute == mir_surface_attrib_state) {
++            MirSurfaceState state = static_cast<MirSurfaceState>(mir_surface_event_get_attribute_value(surfaceEvent));
++
++            if (state == mir_surface_state_hidden) {
++                ubuntuEvent->window->handleSurfaceVisibilityChanged(false);
++            } else {
++                // it's visible!
++                ubuntuEvent->window->handleSurfaceVisibilityChanged(true);
++                ubuntuEvent->window->handleSurfaceStateChanged(mirSurfaceStateToWindowState(state));
++            }
+         }
          break;
+     }
 === modified file 'src/ubuntumirclient/window.cpp'
 --- src/ubuntumirclient/window.cpp	2016-02-02 11:07:38 +0000
 +++ src/ubuntumirclient/window.cpp	2016-03-14 17:37:49 +0000
@@ -36,6 +36,8 @@
  Q_LOGGING_CATEGORY(ubuntumirclientBufferSwap, "ubuntumirclient.bufferSwap", QtWarningMsg)
++const Qt::WindowType LowChromeWindowHint = (Qt::WindowType)0x00800000;
++
  namespace
+ {
@@ -65,23 +67,6 @@
      return reinterpret_cast<EGLNativeWindowType>(mir_buffer_stream_get_egl_native_window(stream));
+ }
--MirSurfaceState qtWindowStateToMirSurfaceState(Qt::WindowState state)
--{
--    switch (state) {
--    case Qt::WindowNoState:
--        return mir_surface_state_restored;
--    case Qt::WindowFullScreen:
--        return mir_surface_state_fullscreen;
--    case Qt::WindowMaximized:
--        return mir_surface_state_maximized;
--    case Qt::WindowMinimized:
--        return mir_surface_state_minimized;
--    default:
--        qCWarning(ubuntumirclient, "Unexpected Qt::WindowState: %d", state);
--        return mir_surface_state_restored;
--    }
--}
--
  const char *qtWindowStateToStr(Qt::WindowState state)
+ {
      switch (state) {
@@ -93,11 +78,45 @@
          return "Maximized";
      case Qt::WindowMinimized:
          return "Minimized";
++    case Qt::WindowActive:
++        return "Active";
      default:
          return "!?";
+     }
+ }
++const char *mirSurfaceStateToStr(MirSurfaceState surfaceState)
++{
++    switch (surfaceState) {
++    case mir_surface_state_unknown: return "unknown";
++    case mir_surface_state_restored: return "restored";
++    case mir_surface_state_minimized: return "minimized";
++    case mir_surface_state_maximized: return "vertmaximized";
++    case mir_surface_state_vertmaximized: return "vertmaximized";
++    case mir_surface_state_fullscreen: return "fullscreen";
++    case mir_surface_state_horizmaximized: return "horizmaximized";
++    case mir_surface_state_hidden: return "hidden";
++    default: return "!?";
++    }
++}
++
++MirSurfaceState qtWindowStateToMirSurfaceState(Qt::WindowState state)
++{
++    switch (state) {
++    case Qt::WindowNoState:
++        return mir_surface_state_restored;
++    case Qt::WindowFullScreen:
++        return mir_surface_state_fullscreen;
++    case Qt::WindowMaximized:
++        return mir_surface_state_maximized;
++    case Qt::WindowMinimized:
++        return mir_surface_state_minimized;
++    default:
++        qCWarning(ubuntumirclient, "Unexpected Qt::WindowState: %d", state);
++        return mir_surface_state_restored;
++    }
++}
++
  WId makeId()
+ {
      static int id = 1;
@@ -209,6 +228,10 @@
          mir_surface_spec_set_fullscreen_on_output(spec.get(), screen->mirOutputId());
+     }
++    if (window->flags() & LowChromeWindowHint) {
++        mir_surface_spec_set_shell_chrome(spec.get(), mir_shell_chrome_low);
++    }
++
      auto surface = mir_surface_create_sync(spec.get());
      Q_ASSERT(mir_surface_is_valid(surface));
      return surface;
@@ -244,11 +267,9 @@
          , mMirSurface(createMirSurface(mWindow, screen, input, connection))
          , mEglDisplay(screen->eglDisplay())
          , mEglSurface(eglCreateWindowSurface(mEglDisplay, screen->eglConfig(), nativeWindowFor(mMirSurface), nullptr))
--        , mVisible(false)
          , mNeedsRepaint(false)
          , mParented(mWindow->transientParent() || mWindow->parent())
--        , mWindowState(mWindow->windowState())
--
++        , mShellChrome(mWindow->flags() & LowChromeWindowHint ? mir_shell_chrome_low : mir_shell_chrome_normal)
+     {
          mir_surface_set_event_handler(mMirSurface, surfaceEventCallback, this);
@@ -260,7 +281,7 @@
          auto geom = mWindow->geometry();
          geom.setWidth(parameters.width);
          geom.setHeight(parameters.height);
--        if (mWindowState == Qt::WindowFullScreen) {
++        if (mWindow->windowState() == Qt::WindowFullScreen) {
              geom.setY(0);
          } else {
              geom.setY(panelHeight());
@@ -287,8 +308,6 @@
      UbuntuSurface& operator=(UbuntuSurface const&) = delete;
      void resize(const QSize& newSize);
--    void setState(Qt::WindowState newState);
--    void setVisible(bool state);
      void updateTitle(const QString& title);
      void setSizingConstraints(const QSize& minSize, const QSize& maxSize, const QSize& increment);
@@ -296,13 +315,22 @@
      void handleSurfaceResized(int width, int height);
      int needsRepaint() const;
++    MirSurfaceState state() const { return mir_surface_get_state(mMirSurface); }
++    void setState(MirSurfaceState state);
++
++    MirSurfaceType type() const { return mir_surface_get_type(mMirSurface); }
++
++    void setShellChrome(MirShellChrome shellChrome);
++
      EGLSurface eglSurface() const { return mEglSurface; }
      MirSurface *mirSurface() const { return mMirSurface; }
++    void setSurfaceParent(MirSurface*);
++    bool hasParent() const { return mParented; }
++
  private:
      static void surfaceEventCallback(MirSurface* surface, const MirEvent *event, void* context);
      void postEvent(const MirEvent *event);
--    void updateSurface();
      QWindow * const mWindow;
      UbuntuWindow * const mPlatformWindow;
@@ -313,21 +341,20 @@
      const EGLDisplay mEglDisplay;
      const EGLSurface mEglSurface;
--    bool mVisible;
      bool mNeedsRepaint;
      bool mParented;
--    Qt::WindowState mWindowState;
      QSize mBufferSize;
      QMutex mTargetSizeMutex;
      QSize mTargetSize;
++    MirShellChrome mShellChrome;
  };
  void UbuntuSurface::resize(const QSize& size)
+ {
      qCDebug(ubuntumirclient,"resize(window=%p, width=%d, height=%d)", mWindow, size.width(), size.height());
--    if (mWindowState == Qt::WindowFullScreen || mWindowState == Qt::WindowMaximized) {
++    if (mWindow->windowState() == Qt::WindowFullScreen || mWindow->windowState() == Qt::WindowMaximized) {
          qCDebug(ubuntumirclient, "resize(window=%p) - not resizing, window is maximized or fullscreen", mWindow);
          return;
+     }
@@ -343,28 +370,6 @@
      mir_surface_apply_spec(mMirSurface, spec.get());
+ }
--void UbuntuSurface::setState(Qt::WindowState newState)
--{
--    mir_wait_for(mir_surface_set_state(mMirSurface, qtWindowStateToMirSurfaceState(newState)));
--    mWindowState = newState;
--}
--
--void UbuntuSurface::setVisible(bool visible)
--{
--    if (mVisible == visible)
--        return;
--
--    mVisible = visible;
--
--    if (mVisible)
--        updateSurface();
--
--    // TODO: Use the new mir_surface_state_hidden state instead of mir_surface_state_minimized.
--    //       Will have to change qtmir and unity8 for that.
--    const auto newState = visible ? qtWindowStateToMirSurfaceState(mWindowState) : mir_surface_state_minimized;
--    mir_wait_for(mir_surface_set_state(mMirSurface, newState));
--}
--
  void UbuntuSurface::updateTitle(const QString& newTitle)
+ {
      const auto title = newTitle.toUtf8();
@@ -410,6 +415,22 @@
      return 0;
+ }
++void UbuntuSurface::setState(MirSurfaceState state)
++{
++    mir_wait_for(mir_surface_set_state(mMirSurface, state));
++}
++
++void UbuntuSurface::setShellChrome(MirShellChrome chrome)
++{
++    if (chrome != mShellChrome) {
++        auto spec = Spec{mir_connection_create_spec_for_changes(mConnection)};
++        mir_surface_spec_set_shell_chrome(spec.get(), chrome);
++        mir_surface_apply_spec(mMirSurface, spec.get());
++
++        mShellChrome = chrome;
++    }
++}
++
  void UbuntuSurface::onSwapBuffersDone()
+ {
      static int sFrameNumber = 0;
@@ -470,22 +491,14 @@
      mInput->postEvent(mPlatformWindow, event);
+ }
--void UbuntuSurface::updateSurface()
++void UbuntuSurface::setSurfaceParent(MirSurface* parent)
+ {
--    qCDebug(ubuntumirclient, "updateSurface(window=%p)", mWindow);
++    qCDebug(ubuntumirclient, "setSurfaceParent(window=%p)", mWindow);
--    if (!mParented && mWindow->type() == Qt::Dialog) {
--        // The dialog may have been parented after creation time
--        // so morph it into a modal dialog
--        auto parent = transientParentFor(mWindow);
--        if (parent) {
--            qCDebug(ubuntumirclient, "updateSurface(window=%p) dialog now parented", mWindow);
--            mParented = true;
--            Spec spec{mir_connection_create_spec_for_changes(mConnection)};
--            mir_surface_spec_set_parent(spec.get(), parent->mirSurface());
--            mir_surface_apply_spec(mMirSurface, spec.get());
--        }
--    }
++    mParented = true;
++    Spec spec{mir_connection_create_spec_for_changes(mConnection)};
++    mir_surface_spec_set_parent(spec.get(), parent);
++    mir_surface_apply_spec(mMirSurface, spec.get());
+ }
  UbuntuWindow::UbuntuWindow(QWindow *w, const QSharedPointer<UbuntuClipboard> &clipboard, UbuntuScreen *screen,
@@ -494,6 +507,9 @@
      , QPlatformWindow(w)
      , mId(makeId())
      , mClipboard(clipboard)
++    , mWindowState(w->windowState())
++    , mWindowFlags(w->flags())
++    , mWindowVisible(false)
      , mSurface(new UbuntuSurface{this, screen, input, connection})
+ {
      qCDebug(ubuntumirclient, "UbuntuWindow(window=%p, screen=%p, input=%p, surf=%p)", w, screen, input, mSurface.get());
@@ -517,6 +533,7 @@
      // updated size but it still needs re-rendering so another redraw may be needed.
      // A mir API to drop the currently held buffer would help here, so that we wouldn't have to redraw twice
      auto const numRepaints = mSurface->needsRepaint();
++    lock.unlock();
      qCDebug(ubuntumirclient, "handleSurfaceResize(window=%p) redraw %d times", window(), numRepaints);
      for (int i = 0; i < numRepaints; i++) {
          qCDebug(ubuntumirclient, "handleSurfaceResize(window=%p) repainting width=%d, height=%d", window(), geometry().size().width(), geometry().size().height());
@@ -537,13 +554,47 @@
      mClipboard->requestDBusClipboardContents();
+ }
++void UbuntuWindow::handleSurfaceVisibilityChanged(bool visible)
++{
++    qCDebug(ubuntumirclient, "handleSurfaceFocused(window=%p)", window());
++
++    if (mWindowVisible == visible) return;
++    mWindowVisible = visible;
++
++    QWindowSystemInterface::handleExposeEvent(window(), QRect(QPoint(), geometry().size()));
++}
++
++void UbuntuWindow::handleSurfaceStateChanged(Qt::WindowState state)
++{
++    qCDebug(ubuntumirclient, "handleSurfaceStateChanged(window=%p, %s)", window(), qtWindowStateToStr(state));
++
++    if (mWindowState == state) return;
++    mWindowState = state;
++
++    QWindowSystemInterface::handleWindowStateChanged(window(), state);
++}
++
  void UbuntuWindow::setWindowState(Qt::WindowState state)
+ {
      QMutexLocker lock(&mMutex);
      qCDebug(ubuntumirclient, "setWindowState(window=%p, %s)", this, qtWindowStateToStr(state));
--    mSurface->setState(state);
--
--    updatePanelHeightHack(state);
++
++    if (mWindowState == state) return;
++    mWindowState = state;
++
++    lock.unlock();
++    updateSurfaceState();
++}
++
++void UbuntuWindow::setWindowFlags(Qt::WindowFlags flags)
++{
++    QMutexLocker lock(&mMutex);
++    qCDebug(ubuntumirclient, "setWindowFlags(window=%p, 0x%x)", this, (int)flags);
++
++    if (mWindowFlags == flags) return;
++    mWindowFlags = flags;
++
++    mSurface->setShellChrome(mWindowFlags & LowChromeWindowHint ? mir_shell_chrome_low : mir_shell_chrome_normal);
+ }
  /*
@@ -552,16 +603,19 @@
      window is always on the top-left corner, right below the indicators panel if not
      in fullscreen.
   */
--void UbuntuWindow::updatePanelHeightHack(Qt::WindowState state)
++void UbuntuWindow::enablePanelHeightHack(bool enable)
+ {
--    if (state == Qt::WindowFullScreen && geometry().y() != 0) {
--        QRect newGeometry = geometry();
++    QMutexLocker lock(&mMutex);
++
++    QRect newGeometry = geometry();
++    if (enable) {
++        newGeometry.setY(panelHeight());
++    } else {
          newGeometry.setY(0);
--        QPlatformWindow::setGeometry(newGeometry);
--        QWindowSystemInterface::handleGeometryChange(window(), newGeometry);
--    } else if (geometry().y() == 0) {
--        QRect newGeometry = geometry();
--        newGeometry.setY(panelHeight());
++    }
++
++    if (newGeometry != geometry()) {
++        lock.unlock();
          QPlatformWindow::setGeometry(newGeometry);
          QWindowSystemInterface::handleGeometryChange(window(), newGeometry);
+     }
@@ -587,11 +641,23 @@
      QMutexLocker lock(&mMutex);
      qCDebug(ubuntumirclient, "setVisible (window=%p, visible=%s)", window(), visible ? "true" : "false");
--    mSurface->setVisible(visible);
--    const QRect& exposeRect = visible ? QRect(QPoint(), geometry().size()) : QRect();
++    if (mWindowVisible == visible) return;
++    mWindowVisible = visible;
++
++    if (visible) {
++        if (!mSurface->hasParent() && window()->type() == Qt::Dialog) {
++            // The dialog may have been parented after creation time
++            // so morph it into a modal dialog
++            auto parent = transientParentFor(window());
++            if (parent) {
++                mSurface->setSurfaceParent(parent->mirSurface());
++            }
++        }
++    }
      lock.unlock();
--    QWindowSystemInterface::handleExposeEvent(window(), exposeRect);
++    updateSurfaceState();
++    QWindowSystemInterface::handleExposeEvent(window(), QRect(QPoint(), geometry().size()));
      QWindowSystemInterface::flushWindowSystemEvents();
+ }
@@ -613,6 +679,11 @@
      mSurface->setSizingConstraints(win->minimumSize(), win->maximumSize(), win->sizeIncrement());
+ }
++bool UbuntuWindow::isExposed() const
++{
++    return mWindowVisible;
++}
++
  void* UbuntuWindow::eglSurface() const
+ {
      return mSurface->eglSurface();
@@ -633,3 +704,17 @@
      QMutexLocker lock(&mMutex);
      mSurface->onSwapBuffersDone();
+ }
++
++void UbuntuWindow::updateSurfaceState()
++{
++    QMutexLocker lock(&mMutex);
++    MirSurfaceState newState = mWindowVisible ? qtWindowStateToMirSurfaceState(mWindowState) :
++                                                mir_surface_state_hidden;
++    qCDebug(ubuntumirclient, "updateSurfaceState (window=%p, surfaceState=%s)", window(), mirSurfaceStateToStr(newState));
++    if (newState != mSurface->state()) {
++        mSurface->setState(newState);
++
++        lock.unlock();
++        enablePanelHeightHack(newState != mir_surface_state_fullscreen);
++    }
++}
 === modified file 'src/ubuntumirclient/window.h'
 --- src/ubuntumirclient/window.h	2015-12-16 17:55:47 +0000
 +++ src/ubuntumirclient/window.h	2016-03-14 17:37:49 +0000
@@ -43,22 +43,32 @@
      WId winId() const override;
      void setGeometry(const QRect&) override;
      void setWindowState(Qt::WindowState state) override;
++    void setWindowFlags(Qt::WindowFlags flags) override;
      void setVisible(bool visible) override;
      void setWindowTitle(const QString &title) override;
      void propagateSizeHints() override;
++    bool isExposed() const override;
      // New methods.
      void *eglSurface() const;
      MirSurface *mirSurface() const;
      void handleSurfaceResized(int width, int height);
      void handleSurfaceFocused();
++    void handleSurfaceVisibilityChanged(bool visible);
++    void handleSurfaceStateChanged(Qt::WindowState state);
      void onSwapBuffersDone();
  private:
--    void updatePanelHeightHack(Qt::WindowState);
++    void enablePanelHeightHack(bool enable);
++    void updateSurfaceState();
++
      mutable QMutex mMutex;
      const WId mId;
      const QSharedPointer<UbuntuClipboard> mClipboard;
++    Qt::WindowState mWindowState;
++    Qt::WindowFlags mWindowFlags;
++    bool mWindowVisible;
++
      std::unique_ptr<UbuntuSurface> mSurface;
  };