Merge into trunk : window-width-height-changes-acted-upon-always : Code : Unity 8

Status:	Superseded
Proposed branch:	lp://staging/~gerboland/unity8/window-width-height-changes-acted-upon-always
Merge into:	lp://staging/unity8
Diff against target:	28 lines (+3/-4) 2 files modified qml/Rotation/NinetyRotationAnimation.qml (+2/-2) qml/Shell.qml (+1/-2)
To merge this branch:	bzr merge lp://staging/~gerboland/unity8/window-width-height-changes-acted-upon-always
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Daniel d'Andrada (community)			Needs Fixing on 2016-04-25
Unity8 CI Bot	continuous-integration		Needs Fixing on 2016-03-24
Albert Astals Cid (community)		2016-02-19	Approve on 2016-03-22
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2016-02-19
Review via email: mp+286665@code.staging.launchpad.net

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-02-19:

#

To repro bug:
1. boot phone with silo10
2. launch browser
3. rotate phone to landscape
4. plug phone into display
You should shell not fullscreen on display

Revision history for this message

Daniel d'Andrada (dandrader) wrote on 2016-02-19:

#

Commit message: add a newline after the short summary?

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-19:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-19:

#

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

FAILED: Continuous integration, rev:2205
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/436/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/590
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay/185
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial/185
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/613
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/633
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/633
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/629
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/629/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/629
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/629/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/629
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/629/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/629
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/629/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/629
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/629/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/629
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/629/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2016-02-23:

#

Why are the changes on qml/Shell.qml needed? I mean what we have is also a binding, no?

review: Needs Information

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-03-08:

#

> Why are the changes on qml/Shell.qml needed? I mean what we have is also a
> binding, no?

Hmm, I can't quite remember now. What is there should not misbehave. I probably switched it since anchors.fill usually better to use.

Revision history for this message

Albert Astals Cid (aacid) wrote on 2016-03-08:

#

> > Why are the changes on qml/Shell.qml needed? I mean what we have is also a
> > binding, no?
>
> Hmm, I can't quite remember now. What is there should not misbehave. I
> probably switched it since anchors.fill usually better to use.

I find it makes code more complicated to understand (and maybe even break?) since with this code we're setting both anchors.fill and width/height bindings for the Shell {} item.

Isn't it better to just set one of those so it's clearer what is applying?

review: Needs Information

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-03-08:

#

Oh, my commit message explained why I did it:
"Use anchors instead of width/height binding for Stages container, avoids a binding loop on width which fixes Stages resizing"
I didn't dig into why a binding loop appeared.

I'm not setting both anchors & width/height for Shell{}. I set width/height of Shell{}, and then ensure the Stages container fills its parent (which is Shell{}'s root item)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2016-03-08:

#

> I'm not setting both anchors & width/height for Shell{}. I set width/height of
> Shell{}, and then ensure the Stages container fills its parent (which is
> Shell{}'s root item)

Ah right, sorry, missed that.

Revision history for this message

Albert Astals Cid (aacid) wrote on 2016-03-08:

#

Is this something we can autotest?

review: Needs Information

Revision history for this message

Albert Astals Cid (aacid) wrote on 2016-03-08:

#

Please add the checklist

review: Needs Fixing

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-03-22:

#

* Are there any related MPs required for this MP to build/function as expected? Please list.
N
* Did you perform an exploratory manual test run of your code change and any related functionality?
Y
* Did you make sure that your branch does not contain spurious tags?
Y
* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A
* If you changed the UI, has there been a design review?
N/A

Revision history for this message

Albert Astals Cid (aacid) wrote on 2016-03-22:

#

* Did you perform an exploratory manual test run of the code change and any related functionality?
Not really a manual test, but it is clear we need the binding

* Did CI run pass? If not, please explain why.
unstable btu doesn't really look related.

* Did you make sure that the branch does not contain spurious tags?
Yes

review: Approve

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-23:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-23:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-23:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-24:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel d'Andrada (dandrader) wrote on 2016-04-25:

#

- The qml/Rotation/NinetyRotationAnimation.qml change is incomplete. It's missing the y binding.
- HalfLoopRotationAnimation still leaves Shell geometry with static assignments.

And it also looks more like a workaround the issue that RotationStates wasn't designed to support geometry changes OrientedShell than a proper fix.

*All* geometry properties (x, y, width, height, transformOriginX and transformOriginY) should have bindings assigned to then while shell is static (ie, not under a rotation animation) in order to properly support OrientedShell resizes.

Let me try to come up with a comprehensive solution for that.

review: Needs Fixing

Unity 8

Merge lp://staging/~gerboland/unity8/window-width-height-changes-acted-upon-always into lp://staging/unity8

Commit message

Description of the change

Unmerged revisions

Preview Diff

Subscribers