QtMir

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

  1. surviveEmptyTexture
  2. Merge into trunk
Proposed by Daniel d'Andrada
Status: Superseded
Proposed branch: lp:~dandrader/qtmir/surviveEmptyTexture
Merge into: lp:qtmir
Prerequisite: lp:~gerboland/qtmir/multimonitor
Diff against target: 125 lines (+22/-17)
6 files modified
src/modules/Unity/Application/mirbuffersgtexture.cpp (+7/-2)
src/modules/Unity/Application/mirsurface.cpp (+8/-6)
src/modules/Unity/Application/mirsurface.h (+1/-1)
src/modules/Unity/Application/mirsurfaceinterface.h (+1/-1)
src/modules/Unity/Application/mirsurfaceitem.cpp (+4/-6)
tests/modules/common/fake_mirsurface.h (+1/-1)
To merge this branch: bzr merge lp:~dandrader/qtmir/surviveEmptyTexture
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Gerry Boland (community) Abstain
Review via email: mp+273221@code.launchpad.net

This proposal has been superseded by a proposal from 2015-10-15.

Commit message

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.

Description of the change

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

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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)
Revision history for this message
Gerry Boland (gerboland) wrote :

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
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> 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).

Revision history for this message
Gerry Boland (gerboland) wrote :

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
Revision history for this message
Gerry Boland (gerboland) wrote :

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

review: Abstain
lp:~dandrader/qtmir/surviveEmptyTexture updated
387. By Daniel d'Andrada

Merge lp:~gerboland/qtmir/multimonitor once again

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
7676
77bool MirBufferSGTexture::hasAlphaChannel() const77bool MirBufferSGTexture::hasAlphaChannel() const
78{78{
79 return m_mirBuffer->pixel_format() == mir_pixel_format_abgr_888879 if (hasBuffer()) {
80 || m_mirBuffer->pixel_format() == mir_pixel_format_argb_8888;80 return m_mirBuffer->pixel_format() == mir_pixel_format_abgr_8888
81 || m_mirBuffer->pixel_format() == mir_pixel_format_argb_8888;
82 } else {
83 return false;
84 }
81}85}
8286
83void MirBufferSGTexture::bind()87void MirBufferSGTexture::bind()
84{88{
89 Q_ASSERT(hasBuffer());
85 glBindTexture(GL_TEXTURE_2D, m_textureId);90 glBindTexture(GL_TEXTURE_2D, m_textureId);
86 updateBindOptions(true/* force */);91 updateBindOptions(true/* force */);
8792
8893
=== 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 @@
309 }309 }
310}310}
311311
312void MirSurface::updateTexture()312bool MirSurface::updateTexture()
313{313{
314 QMutexLocker locker(&m_mutex);314 QMutexLocker locker(&m_mutex);
315
316 if (m_textureUpdated) {
317 return;
318 }
319
320 Q_ASSERT(!m_texture.isNull());315 Q_ASSERT(!m_texture.isNull());
316
321 MirBufferSGTexture *texture = static_cast<MirBufferSGTexture*>(m_texture.data());317 MirBufferSGTexture *texture = static_cast<MirBufferSGTexture*>(m_texture.data());
322318
319 if (m_textureUpdated) {
320 return texture->hasBuffer();
321 }
322
323 const void* const userId = (void*)123;323 const void* const userId = (void*)123;
324 auto renderables = m_surface->generate_renderables(userId);324 auto renderables = m_surface->generate_renderables(userId);
325325
@@ -345,6 +345,8 @@
345 }345 }
346346
347 m_textureUpdated = true;347 m_textureUpdated = true;
348
349 return texture->hasBuffer();
348}350}
349351
350void MirSurface::onCompositorSwappedBuffers()352void MirSurface::onCompositorSwappedBuffers()
351353
=== 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 @@
87 // methods called from the rendering (scene graph) thread:87 // methods called from the rendering (scene graph) thread:
88 QSharedPointer<QSGTexture> texture() override;88 QSharedPointer<QSGTexture> texture() override;
89 QSGTexture *weakTexture() const override { return m_texture.data(); }89 QSGTexture *weakTexture() const override { return m_texture.data(); }
90 void updateTexture() override;90 bool updateTexture() override;
91 unsigned int currentFrameNumber() const override;91 unsigned int currentFrameNumber() const override;
92 bool numBuffersReadyForCompositor() override;92 bool numBuffersReadyForCompositor() override;
93 // end of methods called from the rendering (scene graph) thread93 // end of methods called from the rendering (scene graph) thread
9494
=== 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 @@
54 // methods called from the rendering (scene graph) thread:54 // methods called from the rendering (scene graph) thread:
55 virtual QSharedPointer<QSGTexture> texture() = 0;55 virtual QSharedPointer<QSGTexture> texture() = 0;
56 virtual QSGTexture *weakTexture() const = 0;56 virtual QSGTexture *weakTexture() const = 0;
57 virtual void updateTexture() = 0;57 virtual bool updateTexture() = 0;
58 virtual unsigned int currentFrameNumber() const = 0;58 virtual unsigned int currentFrameNumber() const = 0;
59 virtual bool numBuffersReadyForCompositor() = 0;59 virtual bool numBuffersReadyForCompositor() = 0;
60 // end of methods called from the rendering (scene graph) thread60 // end of methods called from the rendering (scene graph) thread
6161
=== 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 @@
216216
217 ensureTextureProvider();217 ensureTextureProvider();
218218
219 m_surface->updateTexture();219 if (!m_textureProvider->texture() || !m_surface->updateTexture()) {
220 delete oldNode;
221 return 0;
222 }
220223
221 if (m_surface->numBuffersReadyForCompositor() > 0) {224 if (m_surface->numBuffersReadyForCompositor() > 0) {
222 QTimer::singleShot(0, this, SLOT(update()));225 QTimer::singleShot(0, this, SLOT(update()));
223 }226 }
224227
225 if (!m_textureProvider->texture()) {
226 delete oldNode;
227 return 0;
228 }
229
230 m_textureProvider->smooth = smooth();228 m_textureProvider->smooth = smooth();
231229
232 QSGDefaultImageNode *node = static_cast<QSGDefaultImageNode*>(oldNode);230 QSGDefaultImageNode *node = static_cast<QSGDefaultImageNode*>(oldNode);
233231
=== 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 @@
133 // methods called from the rendering (scene graph) thread:133 // methods called from the rendering (scene graph) thread:
134 QSharedPointer<QSGTexture> texture() override { return QSharedPointer<QSGTexture>(); }134 QSharedPointer<QSGTexture> texture() override { return QSharedPointer<QSGTexture>(); }
135 QSGTexture *weakTexture() const override { return nullptr; }135 QSGTexture *weakTexture() const override { return nullptr; }
136 void updateTexture() override {}136 bool updateTexture() override { return true; }
137 unsigned int currentFrameNumber() const override { return 0; }137 unsigned int currentFrameNumber() const override { return 0; }
138 bool numBuffersReadyForCompositor() override { return 0; }138 bool numBuffersReadyForCompositor() override { return 0; }
139 // end of methods called from the rendering (scene graph) thread139 // end of methods called from the rendering (scene graph) thread

Subscribers

People subscribed via source and target branches