Merge lp://qastaging/~loic.molinari/ubuntu-ui-toolkit/ubuntu-ui-toolkit-private-shapes into lp://qastaging/ubuntu-ui-toolkit/staging

PASSED: Continuous integration, rev:1279
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2717/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1440
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1443
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1443/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1439

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2717/rebuild

What are the respective use cases for Stroke and StrokeShape? What is size for?

> What are the respective use cases for Stroke and StrokeShape? What is size
> for?

Looks like Stroke is a rectangle that isn't filled, which we can use for list items. We need better names :-]
Still unclear what size is. It's not the size of the item.

> > What are the respective use cases for Stroke and StrokeShape? What is size
> > for?
>
> Looks like Stroke is a rectangle that isn't filled, which we can use for list
> items. We need better names :-]
> Still unclear what size is. It's not the size of the item.

What if by default fills the Item and has a negative margin of 1-2 DP? Tbh, this could also be set in the components...

> > What are the respective use cases for Stroke and StrokeShape? What is size
> > for?
>
> Looks like Stroke is a rectangle that isn't filled, which we can use for list
> items. We need better names :-]
> Still unclear what size is. It's not the size of the item.

I introduced it during the meeting, I gues I should have also described it here. Stroke is a stroked rectangle, size is the size of the stroke. I could name it StrokeRectangle, and maybe rename size to strokeSize, but I thought it was redundant.

StrokeShape is the shape version with an additional radius.

That makes me think that we should actually rename size to stroke, simply.

> > > What are the respective use cases for Stroke and StrokeShape? What is size
> > > for?
> >
> > Looks like Stroke is a rectangle that isn't filled, which we can use for
> list
> > items. We need better names :-]
> > Still unclear what size is. It's not the size of the item.
>
> What if by default fills the Item and has a negative margin of 1-2 DP? Tbh,
> this could also be set in the components...

Yes, I'd let it for the components ;)

It's not really complexifying the components and items rendering outside of their area is not really standard and causes a few issues with clipping for isntance.

> StrokeShape is the shape version with an additional radius.
>
> That makes me think that we should actually rename size to stroke, simply.

StrokeRectangle and StrokeShape would make sense to me, you can relate them to the respective non-stroke components.

I think stroke should work. Possibly strokeWidth comes to mind.

Remains the property name, what about "weight"?

PASSED: Continuous integration, rev:1282
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2724/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1447
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1450
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1450/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1446

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2724/rebuild

PASSED: Continuous integration, rev:1283
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2725/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1448
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1451
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1451/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1447

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2725/rebuild

As discussed on IRC, I removed the StrokeRectangle (see [1]), renamed the StrokeShape to Frame and renamed the weight property to thickness.

[1] https://bugs.launchpad.net/ubuntu/+source/ubuntu-ui-toolkit/+bug/1526813/comments/1

PASSED: Continuous integration, rev:1286
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2726/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1449
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1452
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1452/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1448

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2726/rebuild

In FrameTest.qml, can you use grid units and Label instead of pixels and Text? On a hi-dpi screen the text is unreadable.

and sync with staging please

262 + // The geometry is made of 8 vertices indexed with a triangle strip mode.

That should be *20* vertices.

285 +const QSGGeometry::AttributeSet& UCFrameNode::attributeSet()

can you add a comment here what the attributes are?

669 + // FMA'd shapeOut * (1.0 - shapeIn)
What is FMA?

    static char const* const attributes[] = {
        "positionAttrib", "texCoord1Attrib", "texCoord2Attrib", "colorAttrib", 0
    };

702 +varying mediump vec2 texCoord1;
703 +varying mediump vec2 texCoord2;

More descriptive names for the texCoord{1,2} would make it more clear what is happening.

It looks like you have a triangle strip with 20 vertices. Did you evaluate whether this is faster than having a simple quad, and discarding all the pixels in the center?

^I guess the triangle strip is fine, since with a large frame that has a thin border there would be a lot of pixels in the center to be discarded.

The properties of Frame are quite straightforward, but still I think in frame.cpp we could have a bit of documentation for the Frame component.

> In FrameTest.qml, can you use grid units and Label instead of pixels and Text?
> On a hi-dpi screen the text is unreadable.

I'd prefer not, the point of using pixels in these tests is to visually check the rendering quality and accuracy at the pixel level. Unless you got a better idea?

> 669 + // FMA'd shapeOut * (1.0 - shapeIn)
> What is FMA?

Fused Multiply-Add [1]. GPUs got that since ages (Intel started including that recently in x86 [2]) and GLSL compilers often don't optimise that correctly, so it's better to help them detect these forms [3].

[1] https://en.wikipedia.org/wiki/Fused_multiply%E2%80%93add
[2] https://en.wikipedia.org/wiki/FMA_instruction_set
[3] http://www.humus.name/Articles/Persson_LowLevelThinking.pdf

> It looks like you have a triangle strip with 20 vertices. Did you evaluate
> whether this is faster than having a simple quad, and discarding all the
> pixels in the center?

It's much much slower. The discard keyword should actually never be used, it prevents GPUs from using early z-cull. See PowerVR's doc [1] on the topic, most constructors provided equivalent recommendations.

[1] Chapter 3.6 "Avoid Using Alpha Test/Discard": http://imgtec.eetrend.com/sites/imgtec.eetrend.com/files/download/201402/1462-2116-powervrdexingnengjianyi.pdf

I'll document the properties and use better names for the texture coordinates.

Should do that too:
- // FMA'd shapeOut * (1.0 - shapeIn)
+ // Fused multiply-add friendly version of (shapeOut * (1.0 - shapeIn))

PASSED: Continuous integration, rev:1286
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64/55/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/134/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64/55/rebuild

PASSED: Continuous integration, rev:1286
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-singlenode/53/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/137/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-singlenode/53/rebuild

I have one request: could you replace the Ubuntu.Components.Themes.Ambiance.FocusFrame implementation with this? Or the entire component eventually...

All good.

Thanks for the references :)

PASSED: Continuous integration, rev:1292
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64/117/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/264/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64/117/rebuild

PASSED: Continuous integration, rev:1292
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-singlenode/112/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/266/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-singlenode/112/rebuild

PASSED: Continuous integration, rev:1292
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64/119/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/269/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64/119/rebuild

PASSED: Continuous integration, rev:1292
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-singlenode/116/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/270/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-singlenode/116/rebuild