qtubuntu

Merge lp:~nick-dedekind/qtubuntu/occlusion-2.0 into lp:qtubuntu

  1. occlusion-2.0
  2. Merge into trunk
Proposed by Nick Dedekind
Status: Merged
Approved by: Daniel d'Andrada
Approved revision: 323
Merged at revision: 332
Proposed branch: lp:~nick-dedekind/qtubuntu/occlusion-2.0
Merge into: lp:qtubuntu
Prerequisite: lp:~nick-dedekind/qtubuntu/cross-build-support
Diff against target: 44 lines (+13/-0)
1 file modified
src/ubuntumirclient/window.cpp (+13/-0)
To merge this branch: bzr merge lp:~nick-dedekind/qtubuntu/occlusion-2.0
Reviewer Review Type Date Requested Status
Daniel d'Andrada (community) Approve
Unity8 CI Bot continuous-integration Approve
PS Jenkins bot continuous-integration Pending
Michael Terry Pending
Review via email: mp+297315@code.launchpad.net

This proposal supersedes a proposal from 2016-01-19.

Commit message

Re-added window exposure for occlusion detection

Description of the change

* Are there any related MPs required for this MP to build/function as expected? Please list.
https://code.launchpad.net/~nick-dedekind/qtmir/occlusion-2.0/+merge/283197

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

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

PASSED: Continuous integration, rev:308
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/300/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/91
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/91/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:308
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-1-ci/4/
Executed test runs:

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

review: Approve (continuous-integration)
Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

What's with the mNeedsExposeCatchup usage? If the window starts occluded, we do a one-time handleExposeEvent call? I'm sure it makes sense, I just don't understand the sequencing.

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

This has a merge conflict in src/ubuntumirclient/window.cpp it seems (though LP isn't showing it).

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

I tested this on the phone with the Music app:

QSG_RENDER_TIMING=1 QT_LOGGING_RULES="ubuntumirclient=true" qmlscene -qt5 /usr/share/click/preinstalled/com.ubuntu.music/current/app/music-app.qml --desktop_file_hint=/usr/share/click/preinstalled/com.ubuntu.music/current/com.ubuntu.music_music.desktop

Worked great! I want to try to do some testing with the Desktop stage, but I haven't noticed a regression like bug 1514556 yet.

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

Desktop mode seems to work fine.

But I get unity8 crashes if I flip between windowed and staged mode on my phone. They don't seem to be there without these branches.

gsettings set com.canonical.Unity8 usage-mode Staged
vs
gsettings set com.canonical.Unity8 usage-mode Windowed

I'm not sure what the exact trigger is. I had Music app playing music in the background, trying to render when it can. And some other windows, one was maximized. And I would just move them around, reorder them, and switch usage-modes. Got a crash a couple times that way.

Do you see that?

review: Needs Information
Revision history for this message
Nick Dedekind (nick-dedekind) wrote : Posted in a previous version of this proposal

> Desktop mode seems to work fine.
>
> But I get unity8 crashes if I flip between windowed and staged mode on my
> phone. They don't seem to be there without these branches.
>
> gsettings set com.canonical.Unity8 usage-mode Staged
> vs
> gsettings set com.canonical.Unity8 usage-mode Windowed
>
> I'm not sure what the exact trigger is. I had Music app playing music in the
> background, trying to render when it can. And some other windows, one was
> maximized. And I would just move them around, reorder them, and switch usage-
> modes. Got a crash a couple times that way.
>
> Do you see that?

hm. supprised u8 crashes since this is mostly a qtubuntu change. I'll take a look.

Revision history for this message
Nick Dedekind (nick-dedekind) wrote : Posted in a previous version of this proposal

> > Desktop mode seems to work fine.
> >
> > But I get unity8 crashes if I flip between windowed and staged mode on my
> > phone. They don't seem to be there without these branches.
> >
> > gsettings set com.canonical.Unity8 usage-mode Staged
> > vs
> > gsettings set com.canonical.Unity8 usage-mode Windowed
> >
> > I'm not sure what the exact trigger is. I had Music app playing music in
> the
> > background, trying to render when it can. And some other windows, one was
> > maximized. And I would just move them around, reorder them, and switch
> usage-
> > modes. Got a crash a couple times that way.
> >
> > Do you see that?
>
> hm. supprised u8 crashes since this is mostly a qtubuntu change. I'll take a
> look.

Can't seem to reproduce a crash. Could you please get me a stacktrace?

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

I had mentioned getting crashes earlier on IRC. But I had been using a poorly built version. Tried again with new build, but still get crashes.

I was toggling between Windowed and Staged modes and got lots of crashes in apps. Here are some stacktraces with just qtmir/qtubuntu symbols:
They all look roughly like (this one was from gallery-app) http://paste.ubuntu.com/15096134/

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

And here's one with some more symbols: http://paste.ubuntu.com/15096395/

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:308
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/29/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1131/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

Crash/Freeze has now been fixed in mir-0.23

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:323
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/78/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/1963
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1989
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1923
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1923
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1923
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1914
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1914/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1914
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1914/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1914
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1914/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1914
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1914/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1914
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1914/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1914
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1914/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1914
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1914/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1914
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1914/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1914
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1914/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/ubuntumirclient/window.cpp'
--- src/ubuntumirclient/window.cpp 2016-04-28 14:44:18 +0000
+++ src/ubuntumirclient/window.cpp 2016-06-14 10:50:07 +0000
@@ -280,6 +280,8 @@
280 mMirSurface = createMirSurface(mWindow, mirOutputId, input, connection, surfaceEventCallback, this);280 mMirSurface = createMirSurface(mWindow, mirOutputId, input, connection, surfaceEventCallback, this);
281 mEglSurface = eglCreateWindowSurface(mEglDisplay, config, nativeWindowFor(mMirSurface), nullptr);281 mEglSurface = eglCreateWindowSurface(mEglDisplay, config, nativeWindowFor(mMirSurface), nullptr);
282282
283 mNeedsExposeCatchup = mir_surface_get_visibility(mMirSurface) == mir_surface_visibility_occluded;
284
283 // Window manager can give us a final size different from what we asked for285 // Window manager can give us a final size different from what we asked for
284 // so let's check what we ended up getting286 // so let's check what we ended up getting
285 MirSurfaceParameters parameters;287 MirSurfaceParameters parameters;
@@ -335,6 +337,8 @@
335 void setSurfaceParent(MirSurface*);337 void setSurfaceParent(MirSurface*);
336 bool hasParent() const { return mParented; }338 bool hasParent() const { return mParented; }
337339
340 bool mNeedsExposeCatchup;
341
338private:342private:
339 static void surfaceEventCallback(MirSurface* surface, const MirEvent *event, void* context);343 static void surfaceEventCallback(MirSurface* surface, const MirEvent *event, void* context);
340 void postEvent(const MirEvent *event);344 void postEvent(const MirEvent *event);
@@ -562,6 +566,7 @@
562 QMutexLocker lock(&mMutex);566 QMutexLocker lock(&mMutex);
563 qCDebug(ubuntumirclient, "handleSurfaceExposeChange(window=%p, exposed=%s)", window(), exposed ? "true" : "false");567 qCDebug(ubuntumirclient, "handleSurfaceExposeChange(window=%p, exposed=%s)", window(), exposed ? "true" : "false");
564568
569 mSurface->mNeedsExposeCatchup = false;
565 if (mWindowExposed == exposed) return;570 if (mWindowExposed == exposed) return;
566 mWindowExposed = exposed;571 mWindowExposed = exposed;
567572
@@ -728,6 +733,14 @@
728{733{
729 QMutexLocker lock(&mMutex);734 QMutexLocker lock(&mMutex);
730 mSurface->onSwapBuffersDone();735 mSurface->onSwapBuffersDone();
736
737 if (mSurface->mNeedsExposeCatchup) {
738 mSurface->mNeedsExposeCatchup = false;
739 mWindowExposed = false;
740
741 lock.unlock();
742 QWindowSystemInterface::handleExposeEvent(window(), QRect(QPoint(), geometry().size()));
743 }
731}744}
732745
733void UbuntuWindow::handleScreenPropertiesChange(MirFormFactor formFactor, float scale)746void UbuntuWindow::handleScreenPropertiesChange(MirFormFactor formFactor, float scale)

Subscribers

People subscribed via source and target branches