Merge lp:~gerboland/miral/qt-distinguish-visible-exposure into lp:miral

[Gerry Boland]

[miral-qt] Distinguish visible from exposure: miral sets surface visibility, qml should apply that and set the exposure

visible = miral has decided the surface can be visible
exposed = surface is not occluded, so compositor will draw the surface, therefore client should keep generating frames

[Daniel d'Andrada (dandrader)]

What's the motivation (use case) for this division?

review: Needs Information

[Daniel d'Andrada (dandrader)]

I see. WindowInfo::is_visible() is defined by mir_surface_state (hidden || minimized).

Looks like redundant API to me since we have state already. Unless there are future plans for "visible" that involves more than surface state.

unity8/qml/Components/InputMethod.qml could make use of this helper property.

review: Approve

[Gerry Boland (gerboland)]

They have different purposes.

Clients set their surface state, and can use those states to inform Mir/Shell if a surface should be visible not. That is what WindowInfo::is_visible decides for us.

But the mir_surface_attrib_visibility is how Mir/Shell tells the client that its surface is occluded or not. Then client can decide to stop drawing useless frames.

They're different concepts, but with similar names, so I separated them and renamed the later to "exposed" to hopefully reduce the confusion.

=== modified file 'miral-qt/demos/qml-demo-shell/windowModel.qml'
--- miral-qt/demos/qml-demo-shell/windowModel.qml	2016-09-20 12:01:17 +0000
+++ miral-qt/demos/qml-demo-shell/windowModel.qml	2016-09-20 13:41:40 +0000
@@ -26,6 +26,7 @@
                 width: surface.size.width
                 height: surface.size.height
                 focus: surface.focused
+                visible: surface.visible
 
                 Rectangle {
                     anchors { top: parent.bottom; right: parent.right }

=== modified file 'miral-qt/src/modules/Unity/Application/mirsurface.cpp'
--- miral-qt/src/modules/Unity/Application/mirsurface.cpp	2016-09-16 10:12:17 +0000
+++ miral-qt/src/modules/Unity/Application/mirsurface.cpp	2016-09-20 13:41:40 +0000
@@ -623,7 +623,7 @@
 
 bool MirSurface::visible() const
 {
-    return m_surface->query(mir_surface_attrib_visibility) == mir_surface_visibility_exposed;
+    return m_windowInfo.is_visible();
 }
 #include <mir_toolkit/event.h>
 void MirSurface::mousePressEvent(QMouseEvent *event)
@@ -741,32 +741,34 @@
             deleteLater();
         }
     }
-    updateVisibility();
+    updateExposure();
     setViewActiveFocus(viewId, false);
 }
 
-void MirSurface::setViewVisibility(qintptr viewId, bool visible)
+void MirSurface::setViewExposure(qintptr viewId, bool exposed)
 {
     if (!m_views.contains(viewId)) return;
 
-    m_views[viewId].visible = visible;
-    updateVisibility();
+    m_views[viewId].exposed = exposed;
+    updateExposure();
 }
 
-void MirSurface::updateVisibility()
+void MirSurface::updateExposure()
 {
-    bool newVisible = false;
+    bool newExposed = false;
     QHashIterator<qintptr, View> i(m_views);
     while (i.hasNext()) {
         i.next();
-        newVisible |= i.value().visible;
+        newExposed |= i.value().exposed;
     }
 
-    if (newVisible != visible()) {
-        DEBUG_MSG << "(" << newVisible << ")";
+    const bool oldExposed = (m_surface->query(mir_surface_attrib_visibility) == mir_surface_visibility_exposed);
+
+    if (newExposed != oldExposed) {
+        DEBUG_MSG << "(" << newExposed << ")";
 
         m_surface->configure(mir_surface_attrib_visibility,
-                             newVisible ? mir_surface_visibility_exposed : mir_surface_visibility_occluded);
+                             newExposed ? mir_surface_visibility_exposed : mir_surface_visibility_occluded);
     }
 }
 

=== modified file 'miral-qt/src/modules/Unity/Application/mirsurface.h'
--- miral-qt/src/modules/Unity/Application/mirsurface.h	2016-09-16 10:12:17 +0000
+++ miral-qt/src/modules/Unity/Application/mirsurface.h	2016-09-20 13:41:40 +0000
@@ -108,7 +108,7 @@
 
     void registerView(qintptr viewId) override;
     void unregisterView(qintptr viewId) override;
-    void setViewVisibility(qintptr viewId, bool visible) override;
+    void setViewExposure(qintptr viewId, bool exposed) override;
 
     // methods called from the rendering (scene graph) thread:
     QSharedPointer<QSGTexture> texture() override;
@@ -181,7 +181,7 @@
 private:
     void syncSurfaceSizeWithItemSize();
     bool clientIsRunning() const;
-    void updateVisibility();
+    void updateExposure();
     void applyKeymap();
     void updateActiveFocus();
 
@@ -206,7 +206,7 @@
 
     bool m_live;
     struct View {
-        bool visible;
+        bool exposed;
     };
     QHash<qintptr, View> m_views;
 

=== modified file 'miral-qt/src/modules/Unity/Application/mirsurfaceinterface.h'
--- miral-qt/src/modules/Unity/Application/mirsurfaceinterface.h	2016-09-15 16:18:13 +0000
+++ miral-qt/src/modules/Unity/Application/mirsurfaceinterface.h	2016-09-20 13:41:40 +0000
@@ -64,7 +64,7 @@
 
     virtual void registerView(qintptr viewId) = 0;
     virtual void unregisterView(qintptr viewId) = 0;
-    virtual void setViewVisibility(qintptr viewId, bool visible) = 0;
+    virtual void setViewExposure(qintptr viewId, bool exposed) = 0;
 
     // methods called from the rendering (scene graph) thread:
     virtual QSharedPointer<QSGTexture> texture() = 0;

=== modified file 'miral-qt/src/modules/Unity/Application/mirsurfaceitem.cpp'
--- miral-qt/src/modules/Unity/Application/mirsurfaceitem.cpp	2016-08-16 14:15:26 +0000
+++ miral-qt/src/modules/Unity/Application/mirsurfaceitem.cpp	2016-09-20 13:41:40 +0000
@@ -104,7 +104,7 @@
     connect(&m_updateMirSurfaceSizeTimer, &QTimer::timeout, this, &MirSurfaceItem::updateMirSurfaceSize);
 
     connect(this, &QQuickItem::activeFocusChanged, this, &MirSurfaceItem::updateMirSurfaceActiveFocus);
-    connect(this, &QQuickItem::visibleChanged, this, &MirSurfaceItem::updateMirSurfaceVisibility);
+    connect(this, &QQuickItem::visibleChanged, this, &MirSurfaceItem::updateMirSurfaceExposure);
     connect(this, &QQuickItem::windowChanged, this, &MirSurfaceItem::onWindowChanged);
 }
 
@@ -511,13 +511,13 @@
     m_surface->resize(width, height);
 }
 
-void MirSurfaceItem::updateMirSurfaceVisibility()
+void MirSurfaceItem::updateMirSurfaceExposure()
 {
     if (!m_surface || !m_surface->live()) {
         return;
     }
 
-    m_surface->setViewVisibility((qintptr)this, isVisible());
+    m_surface->setViewExposure((qintptr)this, isVisible());
 }
 
 void MirSurfaceItem::updateMirSurfaceActiveFocus()
@@ -620,7 +620,7 @@
 
         updateMirSurfaceSize();
         setImplicitSize(m_surface->size().width(), m_surface->size().height());
-        updateMirSurfaceVisibility();
+        updateMirSurfaceExposure();
 
         // Qt::ArrowCursor is the default when no cursor has been explicitly set, so no point forwarding it.
         if (m_surface->cursor().shape() != Qt::ArrowCursor) {

=== modified file 'miral-qt/src/modules/Unity/Application/mirsurfaceitem.h'
--- miral-qt/src/modules/Unity/Application/mirsurfaceitem.h	2016-09-08 23:57:22 +0000
+++ miral-qt/src/modules/Unity/Application/mirsurfaceitem.h	2016-09-20 13:41:40 +0000
@@ -115,7 +115,7 @@
     void updateMirSurfaceSize();
 
     void updateMirSurfaceActiveFocus();
-    void updateMirSurfaceVisibility();
+    void updateMirSurfaceExposure();
 
     void onActualSurfaceSizeChanged(QSize size);
     void onCompositorSwappedBuffers();

=== modified file 'miral-qt/tests/framework/fake_mirsurface.cpp'
--- miral-qt/tests/framework/fake_mirsurface.cpp	2016-09-15 16:18:13 +0000
+++ miral-qt/tests/framework/fake_mirsurface.cpp	2016-09-20 13:41:40 +0000
@@ -131,7 +131,7 @@
     }
 }
 
-void FakeMirSurface::setViewVisibility(qintptr viewId, bool visible) {
+void FakeMirSurface::setViewExposure(qintptr viewId, bool visible) {
     if (!m_views.contains(viewId)) return;
 
     m_views[viewId] = visible;

=== modified file 'miral-qt/tests/framework/fake_mirsurface.h'
--- miral-qt/tests/framework/fake_mirsurface.h	2016-09-15 16:18:13 +0000
+++ miral-qt/tests/framework/fake_mirsurface.h	2016-09-20 13:41:40 +0000
@@ -100,7 +100,7 @@
     void stopFrameDropper() override;
     void startFrameDropper() override;
     void setLive(bool value) override;
-    void setViewVisibility(qintptr viewId, bool visible) override;
+    void setViewExposure(qintptr viewId, bool visible) override;
     bool isBeingDisplayed() const override;
     void registerView(qintptr viewId) override;
     void unregisterView(qintptr viewId) override;