Merge lp:~mterry/unity8/bleeding-infographic into lp:unity8

[Michael Terry]

Clip the infographic to its bounding box, instead of letting it bleed past the welcome page (visible when dragging the welcome page to the side).

I've also done a bit of cleanup for the mock infographic data:
 - I've forced a very large data bubble on the right side of the screen so that this bug/fix can specifically be seen.
 - I've added data for the has-pin user (so that tryShellWithPin shows infographic data).
 - I've fixed a crasher in our mock libusermetrics when a user has no data but the user double-clicked on the infographic.

Clip the infographic to its bounding box, instead of letting it bleed past the welcome page (visible when dragging the welcome page to the side).

I've also done a bit of cleanup for the mock infographic data:
 - I've forced a very large data bubble on the right side of the screen so that this bug/fix can specifically be seen.
 - I've added data for the has-pin user (so that tryShellWithPin shows infographic data).
 - I've fixed a crasher in our mock libusermetrics when a user has no data but the user double-clicked on the infographic.

= Checklist =

 * Are there any related MPs required for this MP to build/function as expected? Please list.
 No

 * Did you perform an exploratory manual test run of your code change and any related functionality?
 Yes, you can see it clearly with tryShell or tryShellWithPin

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

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
 NA

 * If you changed the UI, has there been a design review?
 NA

[lp:~mterry/unity8/bleeding-infographic]

1597. By Michael Terry on 2015-02-10

More comments

[Daniel d'Andrada (dandrader)]

Are you aware of the performance impact caused by clipping? Isn't there any other way to achieve what you need?

review: Needs Information

[Michael Terry (mterry)]

No, I forgot that clipping was a performance no-no. Hrm. :(

I could scale all bubbles to fit within the welcome page... But that might need Design bye-in.

[Michael Terry (mterry)]

Is there an easy way to measure it's effect?

[Daniel d'Andrada (dandrader)]

> Is there an easy way to measure it's effect?

Nothing comes to mind.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1597
http://jenkins.qa.ubuntu.com/job/unity8-ci/5271/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/1275
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/435
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/436
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/436
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/1128
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1273
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1273/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17937

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

review: Needs Fixing (continuous-integration)

[Andrea Cimitan (cimi)]

> Are you aware of the performance impact caused by clipping? Isn't there any
> other way to achieve what you need?
can't we just clip when needed? just when swiping greeter for example (x coordinate)? then we can make the infographics disappear when is out of screen

[lp:~mterry/unity8/bleeding-infographic]

1598. By Michael Terry on 2015-02-11

Only clip while swiping the greeter

[Michael Terry (mterry)]

> can't we just clip when needed? just when swiping greeter for example (x coordinate)?

Fair point. I've done that and added a comment on why it's more complicated than simply 'clip: true'

> then we can make the infographics disappear when is out of screen

That already happens. This bug is only visible while swiping the greeter.

[Albert Astals Cid (aacid)]

Honestly i am not sure this last change is better than just
  clip: true

Because we have an extra binding to evaluate and we have a clipping change just when the swiping starts, changing the clipping means recalculating the scene graph (i think) so not totally convinced that it's really faster as it is now.

OTOH we may just discussing over something that doesn't really matter much, it's not like the performance in this specific part matters that much i'd say

[Albert Astals Cid (aacid)]

review: Abstain

[Michael Terry (mterry)]

Sounds like I need to get some actual performance numbers here.

On Wed, Feb 11, 2015 at 10:36 AM, Albert Astals Cid <
<email address hidden>> wrote:

> Review: Abstain
>
>
> --
>
> https://code.launchpad.net/~mterry/unity8/bleeding-infographic/+merge/249223
> You are the owner of lp:~mterry/unity8/bleeding-infographic.
>

--
-mt

[Michał Sawicz (saviq)]

Yeah, without numbers there's only so much we can do. But maybe there's better ways? Can we not make it visible: false? Change the z-order? It's not as if we're interested in the contents that just got clipped - they're offscreen after all?

[Michael Terry (mterry)]

Saviq, it's not about the objects being wholly off screen. It's just that the circles bleed past the screen a bit:
http://i.imgur.com/X3TsyC9.jpg

[Gerry Boland (gerboland)]

A one-off clip of an un-rotated rectangular item is not particularly costly. Where clipping really has an impact is for rotated or non-rectangular things, or if you're clipping part of a delegate of a list/grid view (as it breaks batching)

Turning on/off clipping dynamically just adds/removes a ClipNode from the scenegraph at the root of the Infographics' subtree. That usually kicks off a rebatching scan, but as the contents of the Infographics item is pretty unique, I don't expect any batch changes to occur.

Overall I doubt this change mas much impact on rendering speed. To check, simplest is to set QSG_RENDER_TIMING=1, but hard to make accurate comparison that way.

review: Approve

[lp:~mterry/unity8/bleeding-infographic]

1599. By Michael Terry on 2015-02-11

Go back to simple clip: true

[Michael Terry (mterry)]

OK, went back to clip: true based on IRC talk.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1598
http://jenkins.qa.ubuntu.com/job/unity8-ci/5289/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/1296
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/453
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/454
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/454
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/1146
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1294
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1294/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17964

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

review: Approve (continuous-integration)

=== modified file 'qml/Greeter/GreeterContent.qml'
--- qml/Greeter/GreeterContent.qml	2015-01-23 19:47:37 +0000
+++ qml/Greeter/GreeterContent.qml	2015-02-11 16:04:25 +0000
@@ -121,6 +121,7 @@
         objectName: "infographics"
         height: narrowMode ? parent.height : 0.75 * parent.height
         model: greeterContentLoader.infographicModel
+        clip: true // clip large data bubbles
 
         property string selectedUser
         property string infographicUser: AccountsService.statsWelcomeScreen ? selectedUser : ""

=== modified file 'tests/mocks/libusermetrics/UserMetrics.cpp'
--- tests/mocks/libusermetrics/UserMetrics.cpp	2014-10-01 11:09:25 +0000
+++ tests/mocks/libusermetrics/UserMetrics.cpp	2015-02-11 16:04:25 +0000
@@ -289,6 +289,7 @@
                 new UserMetricsData("<b>52km</b> travelled", first, firstMonth,
                         ninth, secondMonth, this));
         m_fakeData.insert("single", data);
+        m_fakeData.insert("has-pin", data);
     }
 
     {
@@ -304,12 +305,14 @@
                 new UserMetricsData("<b>33</b> messages today", second,
                         firstMonth, eighth, secondMonth, this));
         m_fakeData.insert("single", data);
+        m_fakeData.insert("has-pin", data);
     }
 
     {
         QVariantList firstMonth;
         while (firstMonth.size() < 17)
             firstMonth.push_back(QVariant(rand()));
+        firstMonth[8] = QVariant(1.0); // oversized 9th day, to test clipping
         while (firstMonth.size() < 31)
             firstMonth.push_back(QVariant());
         QVariantList secondMonth;
@@ -319,6 +322,7 @@
                 new UserMetricsData("<b>19</b> minutes talk time", eighth,
                         firstMonth, second, secondMonth, this));
         m_fakeData.insert("single", data);
+        m_fakeData.insert("has-pin", data);
         // Also use same data for some tablet users
         m_fakeData.insert("has-password", data);
         m_fakeData.insert("no-password", data);
@@ -386,6 +390,10 @@
     if (m_dataIndex == m_fakeData.constEnd() || m_dataIndex.key() != m_username)
     {
         m_dataIndex = m_fakeData.constFind(m_username);
+        if (m_dataIndex == m_fakeData.constEnd())
+        {
+            m_dataIndex = m_fakeData.constFind("");
+        }
     }
 
     loadFakeData();