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

Proposed by Nick Dedekind on 2016-06-14
Status: Merged
Approved by: Daniel d'Andrada on 2016-06-21
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) 2016-06-20 Approve on 2016-06-21
Unity8 CI Bot continuous-integration 2016-06-14 Approve on 2016-06-14
Michael Terry 2016-06-14 Pending
PS Jenkins bot continuous-integration 2016-06-14 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.
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
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)
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.

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

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.

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

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?

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/

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/

Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Nick Dedekind (nick-dedekind) wrote :

Crash/Freeze has now been fixed in mir-0.23

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)
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/ubuntumirclient/window.cpp'
2--- src/ubuntumirclient/window.cpp 2016-04-28 14:44:18 +0000
3+++ src/ubuntumirclient/window.cpp 2016-06-14 10:50:07 +0000
4@@ -280,6 +280,8 @@
5 mMirSurface = createMirSurface(mWindow, mirOutputId, input, connection, surfaceEventCallback, this);
6 mEglSurface = eglCreateWindowSurface(mEglDisplay, config, nativeWindowFor(mMirSurface), nullptr);
7
8+ mNeedsExposeCatchup = mir_surface_get_visibility(mMirSurface) == mir_surface_visibility_occluded;
9+
10 // Window manager can give us a final size different from what we asked for
11 // so let's check what we ended up getting
12 MirSurfaceParameters parameters;
13@@ -335,6 +337,8 @@
14 void setSurfaceParent(MirSurface*);
15 bool hasParent() const { return mParented; }
16
17+ bool mNeedsExposeCatchup;
18+
19 private:
20 static void surfaceEventCallback(MirSurface* surface, const MirEvent *event, void* context);
21 void postEvent(const MirEvent *event);
22@@ -562,6 +566,7 @@
23 QMutexLocker lock(&mMutex);
24 qCDebug(ubuntumirclient, "handleSurfaceExposeChange(window=%p, exposed=%s)", window(), exposed ? "true" : "false");
25
26+ mSurface->mNeedsExposeCatchup = false;
27 if (mWindowExposed == exposed) return;
28 mWindowExposed = exposed;
29
30@@ -728,6 +733,14 @@
31 {
32 QMutexLocker lock(&mMutex);
33 mSurface->onSwapBuffersDone();
34+
35+ if (mSurface->mNeedsExposeCatchup) {
36+ mSurface->mNeedsExposeCatchup = false;
37+ mWindowExposed = false;
38+
39+ lock.unlock();
40+ QWindowSystemInterface::handleExposeEvent(window(), QRect(QPoint(), geometry().size()));
41+ }
42 }
43
44 void UbuntuWindow::handleScreenPropertiesChange(MirFormFactor formFactor, float scale)

Subscribers

People subscribed via source and target branches