Merge lp:~dandrader/qtmir/surviveEmptyTexture into lp:qtmir

[Daniel d'Andrada]

MirSurfaceItem: Survive holding a surface with an empty texture

Survive having a surface whose texture holds no mir buffer at all.
Instead of crashing in such situation we simply don't render it.

To solve a reasonably common I crash I get when launching a desktop unity8 session where unity8-dash, for some reason, is messed up.

Now with this patch, when this happens, I just get an empty unity8-dash window instead and closing it (which makes it respawn) solves the problem.

* Are there any related MPs required for this MP to build/function as expected? Please list.
Just the prerequisite.

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:386
http://jenkins.qa.ubuntu.com/job/qtmir-ci/476/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/172/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/172/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/54/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/209/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/209/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/54/console

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

review: Needs Fixing (continuous-integration)

[Gerry Boland (gerboland)]

Thing is, how do we ever end up in a position that a Mir surface has no texture available?? That is an error in my opinion.

review: Needs Information

[Daniel d'Andrada (dandrader)]

> Thing is, how do we ever end up in a position that a Mir surface has no
> texture available?? That is an error in my opinion.

It is. I just haven't spent time to find the root cause.

This patch makes MirSurface[Item] code more robust. So instead of crashing because of this issue it just won't try to render it. And adding robustness is never a bad thing even if this patch doesn't try to deal with the root cause (unity8-dash in a weird state, like it didn't start up correctly or is frozen during init, I don't know).

[Gerry Boland (gerboland)]

It will cause a visual glitch though. If the buffer is missing, something has gone wrong. This is just masking the core problem. I don't like this at all.

review: Disapprove

[Gerry Boland (gerboland)]

This will have to do until we figure out the root cause.

review: Abstain

[lp:~dandrader/qtmir/surviveEmptyTexture]

387. By Daniel d'Andrada on 2015-10-14

Merge lp:~gerboland/qtmir/multimonitor once again

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:387
http://jenkins.qa.ubuntu.com/job/qtmir-ci/513/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/209/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/209/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/91/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/246/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/246/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/91/console

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

review: Needs Fixing (continuous-integration)

Unmerged revisions

=== modified file 'src/modules/Unity/Application/mirbuffersgtexture.cpp'
--- src/modules/Unity/Application/mirbuffersgtexture.cpp	2015-10-14 12:59:18 +0000
+++ src/modules/Unity/Application/mirbuffersgtexture.cpp	2015-10-14 12:59:18 +0000
@@ -76,12 +76,17 @@
 
 bool MirBufferSGTexture::hasAlphaChannel() const
 {
-    return m_mirBuffer->pixel_format() == mir_pixel_format_abgr_8888
-        || m_mirBuffer->pixel_format() == mir_pixel_format_argb_8888;
+    if (hasBuffer()) {
+        return m_mirBuffer->pixel_format() == mir_pixel_format_abgr_8888
+            || m_mirBuffer->pixel_format() == mir_pixel_format_argb_8888;
+    } else {
+        return false;
+    }
 }
 
 void MirBufferSGTexture::bind()
 {
+    Q_ASSERT(hasBuffer());
     glBindTexture(GL_TEXTURE_2D, m_textureId);
     updateBindOptions(true/* force */);
 

=== modified file 'src/modules/Unity/Application/mirsurface.cpp'
--- src/modules/Unity/Application/mirsurface.cpp	2015-10-14 12:59:18 +0000
+++ src/modules/Unity/Application/mirsurface.cpp	2015-10-14 12:59:18 +0000
@@ -309,17 +309,17 @@
     }
 }
 
-void MirSurface::updateTexture()
+bool MirSurface::updateTexture()
 {
     QMutexLocker locker(&m_mutex);
-
-    if (m_textureUpdated) {
-        return;
-    }
-
     Q_ASSERT(!m_texture.isNull());
+
     MirBufferSGTexture *texture = static_cast<MirBufferSGTexture*>(m_texture.data());
 
+    if (m_textureUpdated) {
+        return texture->hasBuffer();
+    }
+
     const void* const userId = (void*)123;
     auto renderables = m_surface->generate_renderables(userId);
 
@@ -345,6 +345,8 @@
     }
 
     m_textureUpdated = true;
+
+    return texture->hasBuffer();
 }
 
 void MirSurface::onCompositorSwappedBuffers()

=== modified file 'src/modules/Unity/Application/mirsurface.h'
--- src/modules/Unity/Application/mirsurface.h	2015-09-25 17:30:26 +0000
+++ src/modules/Unity/Application/mirsurface.h	2015-10-14 12:59:18 +0000
@@ -87,7 +87,7 @@
     // methods called from the rendering (scene graph) thread:
     QSharedPointer<QSGTexture> texture() override;
     QSGTexture *weakTexture() const override { return m_texture.data(); }
-    void updateTexture() override;
+    bool updateTexture() override;
     unsigned int currentFrameNumber() const override;
     bool numBuffersReadyForCompositor() override;
     // end of methods called from the rendering (scene graph) thread

=== modified file 'src/modules/Unity/Application/mirsurfaceinterface.h'
--- src/modules/Unity/Application/mirsurfaceinterface.h	2015-09-25 17:30:26 +0000
+++ src/modules/Unity/Application/mirsurfaceinterface.h	2015-10-14 12:59:18 +0000
@@ -54,7 +54,7 @@
     // methods called from the rendering (scene graph) thread:
     virtual QSharedPointer<QSGTexture> texture() = 0;
     virtual QSGTexture *weakTexture() const = 0;
-    virtual void updateTexture() = 0;
+    virtual bool updateTexture() = 0;
     virtual unsigned int currentFrameNumber() const = 0;
     virtual bool numBuffersReadyForCompositor() = 0;
     // end of methods called from the rendering (scene graph) thread

=== modified file 'src/modules/Unity/Application/mirsurfaceitem.cpp'
--- src/modules/Unity/Application/mirsurfaceitem.cpp	2015-09-28 14:24:48 +0000
+++ src/modules/Unity/Application/mirsurfaceitem.cpp	2015-10-14 12:59:18 +0000
@@ -216,17 +216,15 @@
 
     ensureTextureProvider();
 
-    m_surface->updateTexture();
+    if (!m_textureProvider->texture() || !m_surface->updateTexture()) {
+        delete oldNode;
+        return 0;
+    }
 
     if (m_surface->numBuffersReadyForCompositor() > 0) {
         QTimer::singleShot(0, this, SLOT(update()));
     }
 
-    if (!m_textureProvider->texture()) {
-        delete oldNode;
-        return 0;
-    }
-
     m_textureProvider->smooth = smooth();
 
     QSGDefaultImageNode *node = static_cast<QSGDefaultImageNode*>(oldNode);

=== modified file 'tests/modules/common/fake_mirsurface.h'
--- tests/modules/common/fake_mirsurface.h	2015-09-25 17:30:26 +0000
+++ tests/modules/common/fake_mirsurface.h	2015-10-14 12:59:18 +0000
@@ -133,7 +133,7 @@
     // methods called from the rendering (scene graph) thread:
     QSharedPointer<QSGTexture> texture() override { return QSharedPointer<QSGTexture>(); }
     QSGTexture *weakTexture() const override { return nullptr; }
-    void updateTexture() override {}
+    bool updateTexture() override { return true; }
     unsigned int currentFrameNumber() const override { return 0; }
     bool numBuffersReadyForCompositor() override { return 0; }
     // end of methods called from the rendering (scene graph) thread