Merge lp://staging/~nick-dedekind/unity-api/shell_chrome into lp://staging/unity-api
- shell_chrome
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Michael Terry |
Approved revision: | 212 |
Merged at revision: | 218 |
Proposed branch: | lp://staging/~nick-dedekind/unity-api/shell_chrome |
Merge into: | lp://staging/unity-api |
Diff against target: |
124 lines (+40/-1) 4 files modified
include/unity/shell/application/CMakeLists.txt (+1/-1) include/unity/shell/application/Mir.h (+23/-0) include/unity/shell/application/MirSurfaceInterface.h (+8/-0) include/unity/shell/application/MirSurfaceItemInterface.h (+8/-0) |
To merge this branch: | bzr merge lp://staging/~nick-dedekind/unity-api/shell_chrome |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gerry Boland (community) | Approve | ||
Unity8 CI Bot | continuous-integration | Approve | |
PS Jenkins bot (community) | continuous-integration | Approve | |
Review via email: mp+286309@code.staging.launchpad.net |
Commit message
Added support for low shell chrome
Description of the change
- 211. By Nick Dedekind
-
whitespace
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:210
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:211
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:211
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Gerry Boland (gerboland) wrote : | # |
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) wrote : | # |
> 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) wrote : | # |
> "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) wrote : | # |
> 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) wrote : | # |
> > "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) wrote : | # |
> > "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) wrote : | # |
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 VerticallyMaxim
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?
Nick Dedekind (nick-dedekind) wrote : | # |
> 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
> VerticallyMaxim
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-
>
> 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"
- 212. By Nick Dedekind
-
typo
Nick Dedekind (nick-dedekind) wrote : | # |
> 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) wrote : | # |
PASSED: Continuous integration, rev:212
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Gerry Boland (gerboland) wrote : | # |
Thanks for patiently answering my queries. This looks fine.
FAILED: Continuous integration, rev:210 /unity8- jenkins. ubuntu. com/job/ lp-unity- api-1-ci/ 21/ /unity8- jenkins. ubuntu. com/job/ build/545/ console /unity8- jenkins. ubuntu. com/job/ build-0- fetch/568 /unity8- jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= vivid+overlay/ 584 /unity8- jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= xenial/ 584 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= vivid+overlay/ 580/console /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial/ 580/console /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= vivid+overlay/ 580/console /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial/ 580/console /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= vivid+overlay/ 580/console /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial/ 580/console
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild: /unity8- jenkins. ubuntu. com/job/ lp-unity- api-1-ci/ 21/rebuild
https:/