Merge lp:~nick-dedekind/unity-api/shell_chrome into lp:unity-api

[Nick Dedekind]

Added support for low shell chrome

Added support for low shell chrome

https://code.launchpad.net/~unity-team/qtmir/shell_chrome/+merge/286361
https://code.launchpad.net/~nick-dedekind/unity8/shell_chrome/+merge/286306
https://code.launchpad.net/~nick-dedekind/qtubuntu/shell_chrome/+merge/286308

[lp:~nick-dedekind/unity-api/shell_chrome]

211. By Nick Dedekind on 2016-02-17

whitespace

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:210
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-1-ci/21/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/545/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/568
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/584
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/584
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/580/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/580/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/580/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/580/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/580/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/580/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-1-ci/21/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:210
http://jenkins.qa.ubuntu.com/job/unity-api-ci/391/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-vivid-amd64-ci/79
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-vivid-armhf-ci/80
        deb: http://jenkins.qa.ubuntu.com/job/unity-api-vivid-armhf-ci/80/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-vivid-i386-ci/79
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-wily-amd64-ci/90
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-wily-armhf-ci/90
        deb: http://jenkins.qa.ubuntu.com/job/unity-api-wily-armhf-ci/90/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-wily-i386-ci/90

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-api-ci/391/rebuild

review: Approve (continuous-integration)

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:211
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-1-ci/22/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/548
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/571
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/589
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/589
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/585
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/585/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/585
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/585/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/585
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/585/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/585
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/585/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/585
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/585/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/585
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/585/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-1-ci/22/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:211
http://jenkins.qa.ubuntu.com/job/unity-api-ci/392/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-vivid-amd64-ci/80
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-vivid-armhf-ci/81
        deb: http://jenkins.qa.ubuntu.com/job/unity-api-vivid-armhf-ci/81/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-vivid-i386-ci/80
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-wily-amd64-ci/91
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-wily-armhf-ci/91
        deb: http://jenkins.qa.ubuntu.com/job/unity-api-wily-armhf-ci/91/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-wily-i386-ci/91

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-api-ci/392/rebuild

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

I totally reviewed this, but it got lost somehow. Doing again.

Mainly, I don't understand what is meant by shell chrome here, and what it has to do with the linked bug. You referring to titlebar?

+ * @brief The Shell crhome mode
typo

[Nick Dedekind (nick-dedekind)]

> I totally reviewed this, but it got lost somehow. Doing again.
>
> Mainly, I don't understand what is meant by shell chrome here, and what it has
> to do with the linked bug. You referring to titlebar?
>
> + * @brief The Shell crhome mode
> typo

"Shell chrome" is a mir concept, but my understanding is that it refers to whether "non application" parts of the shell (indicators in our use case) are visible.

For how this relates to the bug; we have changed touch apps to use low chrome mode when wanting "full screen" in phone/tablet form factor, and Window.state = FullScreen when wanting it in desktop mode.

[Gerry Boland (gerboland)]

> "Shell chrome" is a mir concept, but my understanding is that it refers to
> whether "non application" parts of the shell (indicators in our use case) are
> visible.

So, it is a hint from the server to the client, telling the client about a shell visual state, allowing client to decide surface state? Or is it a hint from the client to the server to recommend server to hide its chrome? What if server doesn't support or allow different chrome modes?

Also, are we're implying that "low chrome" is zero chrome?

I can find no documentation for the exact purpose of this thing in Mir. Frankly I'm not a fan of it.

> For how this relates to the bug; we have changed touch apps to use low chrome
> mode when wanting "full screen" in phone/tablet form factor, and Window.state
> = FullScreen when wanting it in desktop mode.

I understand this is now Mir api, but I won't be the first person to find this confusing. If this is indeed a hint from server->client, then (aside from "low chrome" being a vague concept) at least it means the client can decide the suitable surface state. If it a client->server hint, I'm less clear on what happens.

[Alan Griffiths (alan-griffiths)]

> I understand this is now Mir api, but I won't be the first person to find this
> confusing. If this is indeed a hint from server->client, then (aside from "low
> chrome" being a vague concept) at least it means the client can decide the
> suitable surface state. If it a client->server hint, I'm less clear on what
> happens.

This API comes from a request from U8 and discussions at the January sprint.

"low chrome" is a request from the client to the server. What the server does with it is up to the server and can depend upon the surface state. The "chrome" terminology was suggested by Chris as being a "term of art" that covered "indicators and other stuff".

[Nick Dedekind (nick-dedekind)]

> > "Shell chrome" is a mir concept, but my understanding is that it refers to
> > whether "non application" parts of the shell (indicators in our use case)
> are
> > visible.
>
> So, it is a hint from the server to the client, telling the client about a
> shell visual state, allowing client to decide surface state? Or is it a hint
> from the client to the server to recommend server to hide its chrome? What if
> server doesn't support or allow different chrome modes?

It's a hint from client to server to tell the server what level of shell chrome we want visible for the application.
This is essentially the same thing we were using the fullscreen window state for previously.
For now we've hacked in a "well known" window flag into qtubuntu & the clients to represent this "low chrome" mode client side.
Believe me, this is far from a "good solution". I'm not sure this is intended to stay this way or not. There is already a flag in qt 5.5 which may be suitable, but we're not there yet.
I still believe handling form factor changes client side was the way to go with this, but others disagreed. Plus we couldn't get that working in time...

>
> Also, are we're implying that "low chrome" is zero chrome?
>

Chrome state is interpreted by the shell. In our case low chrome means "fullscreen" for phone/tablet.

> I can find no documentation for the exact purpose of this thing in Mir.
> Frankly I'm not a fan of it.
>
> > For how this relates to the bug; we have changed touch apps to use low
> chrome
> > mode when wanting "full screen" in phone/tablet form factor, and
> Window.state
> > = FullScreen when wanting it in desktop mode.
>
> I understand this is now Mir api, but I won't be the first person to find this
> confusing. If this is indeed a hint from server->client, then (aside from "low
> chrome" being a vague concept) at least it means the client can decide the
> suitable surface state. If it a client->server hint, I'm less clear on what
> happens.

Not sure where I can go with this and I agree that it is confusing. Perhaps we can refine it; but this is wanted for OTA 10; so....

[Nick Dedekind (nick-dedekind)]

> > "Shell chrome" is a mir concept, but my understanding is that it refers to
> > whether "non application" parts of the shell (indicators in our use case)
> are
> > visible.
>
> So, it is a hint from the server to the client, telling the client about a
> shell visual state, allowing client to decide surface state? Or is it a hint
> from the client to the server to recommend server to hide its chrome? What if
> server doesn't support or allow different chrome modes?

It's a hint from client to server to tell the server what level of shell chrome we want visible for the application.
This is essentially the same thing we were using the fullscreen window state for previously.
For now we've hacked in a "well known" window flag into qtubuntu & the clients to represent this "low chrome" mode client side.
Believe me, this is far from a "good solution". I'm not sure this is intended to stay this way or not. There is already a flag in qt 5.5 which may be suitable, but we're not there yet.
I still believe handling form factor changes client side was the way to go with this, but others disagreed. Plus we couldn't get that working in time...

>
> Also, are we're implying that "low chrome" is zero chrome?
>

Chrome state is interpreted by the shell. In our case low chrome means "fullscreen" for phone/tablet.

> I can find no documentation for the exact purpose of this thing in Mir.
> Frankly I'm not a fan of it.
>
> > For how this relates to the bug; we have changed touch apps to use low
> chrome
> > mode when wanting "full screen" in phone/tablet form factor, and
> Window.state
> > = FullScreen when wanting it in desktop mode.
>
> I understand this is now Mir api, but I won't be the first person to find this
> confusing. If this is indeed a hint from server->client, then (aside from "low
> chrome" being a vague concept) at least it means the client can decide the
> suitable surface state. If it a client->server hint, I'm less clear on what
> happens.

Not sure where I can go with this and I agree that it is confusing. Perhaps we can refine it; but this is wanted for OTA 10; so....

[Gerry Boland (gerboland)]

Ok. Will accept this is a client to server hint.

But please add more documentation to indicate the expected consequence in the shell of a client setting this.

Now I need to ask about how it will behave (this being as good a place as any to bring up these points):

1. does this only impact a Windowed surface? i.e. what does NormalChrome mode mean for a fullscreen surface? what does LowChrome mean for VerticallyMaximised?

2. I, as a client, ask for a Windowed surface with LowChrome. On phone, this will end up being a fullscreen surface, but windowed on desktop. Do I, as a client, know that I'm fullscreen on the phone? Or do I think I'm windowed? i.e. does shell actually inform the client of the dynamically chosen surface state it has chosen?

review: Needs Information

[Nick Dedekind (nick-dedekind)]

> Ok. Will accept this is a client to server hint.
>
> But please add more documentation to indicate the expected consequence in the
> shell of a client setting this.

Will do.

>
> Now I need to ask about how it will behave (this being as good a place as any
> to bring up these points):
>
> 1. does this only impact a Windowed surface? i.e. what does NormalChrome mode
> mean for a fullscreen surface? what does LowChrome mean for
> VerticallyMaximised?

Chrome only has meaning for phone/tablet stages at the moment (and to determine if fullscreen surfaces are restored when changing to desktop). The logic for treatment of chrome is only in unity8 stages code. All the work in qtmir/unity-api/qtubuntu is just pass-through for the chrome & states info.

>
> 2. I, as a client, ask for a Windowed surface with LowChrome. On phone, this
> will end up being a fullscreen surface, but windowed on desktop. Do I, as a
> client, know that I'm fullscreen on the phone? Or do I think I'm windowed?
> i.e. does shell actually inform the client of the dynamically chosen surface
> state it has chosen?

Here is the logic behind what happens when we change the chrome state and fullscreen.
Clients are informed of surface state changes related to the chrome logic.

In phone/tablet stage:
Flag change to LowChrome -> server sets client window state to "fullscreen"
Flag change to NormalChrome -> server sets client window to "restored" state.
Flag set and state change to restored -> server RESETS client window state to "fullscreen"
Flag not set and state change to restore -> client window stays "restored"
Flag not set and state change to fulscreen -> client window stays "fullscreen"

In desktop stage (only on switching to desktop mode):
Flag set to LowChrome and state is fullscreen -> server sets client window state to "restored"

[lp:~nick-dedekind/unity-api/shell_chrome]

212. By Nick Dedekind on 2016-03-11

typo

[Nick Dedekind (nick-dedekind)]

> I totally reviewed this, but it got lost somehow. Doing again.
>
> Mainly, I don't understand what is meant by shell chrome here, and what it has
> to do with the linked bug. You referring to titlebar?
>
> + * @brief The Shell crhome mode
> typo

Fixed.
My new least favourite word. You have no idea how many times I've misspelled chrome. crome, crhome ...arggg!

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:212
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/32/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/884
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/900
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/900
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/898
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/898/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/898
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/898/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/898
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/898/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/898
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/898/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/898
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/898/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/898
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/898/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/32/rebuild

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

Thanks for patiently answering my queries. This looks fine.

review: Approve

=== modified file 'include/unity/shell/application/CMakeLists.txt'
--- include/unity/shell/application/CMakeLists.txt	2016-02-11 23:38:57 +0000
+++ include/unity/shell/application/CMakeLists.txt	2016-03-11 10:40:24 +0000
@@ -7,7 +7,7 @@
 
 set(UNITY_API_LIB_HDRS ${UNITY_API_LIB_HDRS} ${headers} ${internal_headers} PARENT_SCOPE)
 
-set(VERSION 13)
+set(VERSION 14)
 set(PKGCONFIG_NAME "unity-shell-application")
 set(PKGCONFIG_DESCRIPTION "Unity shell Application APIs")
 set(PKGCONFIG_REQUIRES "Qt5Core")

=== modified file 'include/unity/shell/application/Mir.h'
--- include/unity/shell/application/Mir.h	2015-10-26 18:26:01 +0000
+++ include/unity/shell/application/Mir.h	2016-03-11 10:40:24 +0000
@@ -28,6 +28,8 @@
     Q_ENUMS(Type)
     Q_ENUMS(State)
     Q_ENUMS(OrientationAngle)
+    Q_ENUMS(ShellChrome)
+    Q_ENUMS(FormFactor)
 
     /**
         @brief Name of the mouse cursor to be used. Follows the X Cursor naming convention.
@@ -77,6 +79,26 @@
         Angle270 = 270
     };
 
+    /**
+        @brief Shell chrome
+     */
+    enum ShellChrome {
+        NormalChrome,
+        LowChrome,
+    };
+
+    /**
+        @brief Form Factor
+     */
+    enum FormFactor {
+        FormFactorUnknown,
+        FormFactorPhone,
+        FormFactorTablet,
+        FormFactorMonitor,
+        FormFactorTV,
+        FormFactorProjector,
+    };
+
     /// @cond
     virtual void setCursorName(const QString &cursorName) = 0;
     virtual QString cursorName() const = 0;
@@ -89,5 +111,6 @@
 };
 
 Q_DECLARE_METATYPE(Mir::OrientationAngle)
+Q_DECLARE_METATYPE(Mir::ShellChrome)
 
 #endif // UNITY_SHELL_APPLICATION_MIR_H

=== modified file 'include/unity/shell/application/MirSurfaceInterface.h'
--- include/unity/shell/application/MirSurfaceInterface.h	2016-02-03 11:31:00 +0000
+++ include/unity/shell/application/MirSurfaceInterface.h	2016-03-11 10:40:24 +0000
@@ -114,6 +114,11 @@
      */
     Q_PROPERTY(int heightIncrement READ heightIncrement NOTIFY heightIncrementChanged)
 
+    /**
+     * @brief The Shell chrome mode
+     */
+    Q_PROPERTY(Mir::ShellChrome shellChrome READ shellChrome NOTIFY shellChromeChanged)
+
 public:
     /// @cond
     MirSurfaceInterface(QObject *parent = nullptr) : QObject(parent) {}
@@ -143,6 +148,8 @@
     virtual int maximumHeight() const = 0;
     virtual int widthIncrement() const = 0;
     virtual int heightIncrement() const = 0;
+
+    virtual Mir::ShellChrome shellChrome() const = 0;
     /// @endcond
 
 Q_SIGNALS:
@@ -160,6 +167,7 @@
     void maximumHeightChanged(int value);
     void widthIncrementChanged(int value);
     void heightIncrementChanged(int value);
+    void shellChromeChanged(Mir::ShellChrome value);
     /// @endcond
 };
 

=== modified file 'include/unity/shell/application/MirSurfaceItemInterface.h'
--- include/unity/shell/application/MirSurfaceItemInterface.h	2015-11-30 11:56:02 +0000
+++ include/unity/shell/application/MirSurfaceItemInterface.h	2016-03-11 10:40:24 +0000
@@ -107,6 +107,11 @@
 
     Q_PROPERTY(FillMode fillMode READ fillMode WRITE setFillMode NOTIFY fillModeChanged)
 
+    /**
+     * @brief The Shell chrome mode
+     */
+    Q_PROPERTY(Mir::ShellChrome shellChrome READ shellChrome NOTIFY shellChromeChanged)
+
 public:
 
     enum FillMode {
@@ -142,6 +147,8 @@
 
     virtual FillMode fillMode() const = 0;
     virtual void setFillMode(FillMode value) = 0;
+
+    virtual Mir::ShellChrome shellChrome() const = 0;
     /// @endcond
 
 Q_SIGNALS:
@@ -156,6 +163,7 @@
     void surfaceHeightChanged(int value);
     void nameChanged(const QString &name);
     void fillModeChanged(FillMode value);
+    void shellChromeChanged(Mir::ShellChrome value);
     /// @endcond
 };