Ubuntu UI Toolkit

Merge lp://qastaging/~tpeeters/ubuntu-ui-toolkit/crossFadeImage_SourceSize_fix into lp://qastaging/ubuntu-ui-toolkit

crossFadeImage_SourceSize_fix
Merge into trunk

Proposed by Tim Peeters on 2013-09-25

Status:

Merged

Approved by:

Tim Peeters on 2013-10-18

Approved revision:

781

Merged at revision:

798

Proposed branch:

lp://qastaging/~tpeeters/ubuntu-ui-toolkit/crossFadeImage_SourceSize_fix

Merge into:

lp://qastaging/ubuntu-ui-toolkit

Diff against target:

143 lines (+71/-2)

4 files modified

CHANGES (+1/-0)
components.api (+1/-1)
modules/Ubuntu/Components/CrossFadeImage.qml (+43/-1)
tests/unit/tst_components/tst_CrossFadeImage.qml (+26/-0)

To merge this branch:

bzr merge lp://qastaging/~tpeeters/ubuntu-ui-toolkit/crossFadeImage_SourceSize_fix

Critical

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Approve on 2013-10-17
Tim Peeters			Approve on 2013-10-17
Zsombor Egri			Needs Fixing on 2013-10-04
Günter Schwann (community)			Approve on 2013-09-25
Michael Zanetti (community)		2013-09-25	Needs Information on 2013-09-25
Review via email: mp+187454@code.qastaging.launchpad.net

Commit message

Update CrossFadeImage API so that sourceSize can be set by the applications.

* CHANGED in CrossFadeImage: readonly property size sourceSize TO property size sourceSize

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-09-25:

FAILED: Continuous integration, rev:764
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/748/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy-vm/67/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/1568/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-amd64-ci/605/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-armhf-ci/605/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/4049/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/1570/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-ui-toolkit-ci/748/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2013-09-25:

59 + } else if (internals.loadingImage && (sourceSize != Qt.size(internals.loadingImage.sourceSize.width, internals.loadingImage.sourceSize.height))) {

Hmm... Can it be that internals.loadingImage is null at some point? In that case setting the sourceSize would get lost if setting during that period. If it can't happen we can probably drop this check either. In any case I think the "internals.loadingImage &&" should not be here as the sourceSize gets bound to currentImage and nextImage directly anyways.

No tests?

review: Needs Information

Revision history for this message

Günter Schwann (schwann) wrote on 2013-09-25:

looks good, and helps fixing the issue decscribed in the bug

review: Approve

Revision history for this message

Tim Peeters (tpeeters) wrote on 2013-09-25:

> 59 + } else if (internals.loadingImage && (sourceSize !=
> Qt.size(internals.loadingImage.sourceSize.width,
> internals.loadingImage.sourceSize.height))) {
>
> Hmm... Can it be that internals.loadingImage is null at some point? In that
> case setting the sourceSize would get lost if setting during that period. If
> it can't happen we can probably drop this check either. In any case I think
> the "internals.loadingImage &&" should not be here as the sourceSize gets
> bound to currentImage and nextImage directly anyways.

apparently it can be null/undefined when the object is being created and image pointers are not yet set.

>
> No tests?

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-09-25:

FAILED: Continuous integration, rev:769
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/754/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy-vm/73/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/1579/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-amd64-ci/611/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-armhf-ci/611/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/4060/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/1581/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-ui-toolkit-ci/754/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-09-25:

FAILED: Continuous integration, rev:770
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/755/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy-vm/74/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/1588/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-amd64-ci/612/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-armhf-ci/612/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/4068/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/1590/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-ui-toolkit-ci/755/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-09-26:

FAILED: Continuous integration, rev:771
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/760/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy-vm/81/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/1632/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-amd64-ci/617/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-armhf-ci/617/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/4115/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/1634/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-ui-toolkit-ci/760/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-09-26:

FAILED: Continuous integration, rev:772
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/764/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy-vm/85/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/1650/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-amd64-ci/621/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-armhf-ci/621/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/4133/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/1652/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-ui-toolkit-ci/764/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2013-09-27:

> > 59 + } else if (internals.loadingImage && (sourceSize !=
> > Qt.size(internals.loadingImage.sourceSize.width,
> > internals.loadingImage.sourceSize.height))) {
> >
> > Hmm... Can it be that internals.loadingImage is null at some point? In that
> > case setting the sourceSize would get lost if setting during that period. If
> > it can't happen we can probably drop this check either. In any case I think
> > the "internals.loadingImage &&" should not be here as the sourceSize gets
> > bound to currentImage and nextImage directly anyways.
>
> apparently it can be null/undefined when the object is being created and image
> pointers are not yet set.

Yeah, this is what I meant. So doesn't this fail in that case?

I mean, if you're using it like this:

CrossFadeImage {
   source: "foo.png"
   sourceSize.height: 500
   sourceSize.width: 500
}

Given that the whole thing is built up and sourceSize is set immediately, when loadingImage is still null. The sourceSize setting would just be discarded afaics. Unless the constructions happens before the property setting. In that case the check doesn't break anything, but is still useles..

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-01:

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-autolanding/313/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy-vm/182/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/1995/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-amd64-autolanding/239/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-saucy-armhf-autolanding/239/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/4330/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/1997/console

review: Needs Fixing (continuous-integration)

Revision history for this message

Zsombor Egri (zsombi) wrote on 2013-10-04:

52 +
53 + onSourceSizeChanged: {

qdoc failure given on this line. Also remember to merge trunk, there were issues with autopilot tests.

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-08:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-ui-toolkit-ci/904/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-08:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-ui-toolkit-ci/908/rebuild

review: Approve (continuous-integration)

Revision history for this message

Tim Peeters (tpeeters) wrote on 2013-10-09:

> > > 59 + } else if (internals.loadingImage && (sourceSize !=
> > > Qt.size(internals.loadingImage.sourceSize.width,
> > > internals.loadingImage.sourceSize.height))) {
> > >
> > > Hmm... Can it be that internals.loadingImage is null at some point? In
> that
> > > case setting the sourceSize would get lost if setting during that period.
> If
> > > it can't happen we can probably drop this check either. In any case I
> think
> > > the "internals.loadingImage &&" should not be here as the sourceSize gets
> > > bound to currentImage and nextImage directly anyways.
> >
> > apparently it can be null/undefined when the object is being created and
> image
> > pointers are not yet set.
>
> Yeah, this is what I meant. So doesn't this fail in that case?
>
> I mean, if you're using it like this:
>
> CrossFadeImage {
> source: "foo.png"
> sourceSize.height: 500
> sourceSize.width: 500
> }
>
> Given that the whole thing is built up and sourceSize is set immediately, when
> loadingImage is still null. The sourceSize setting would just be discarded
> afaics. Unless the constructions happens before the property setting. In that
> case the check doesn't break anything, but is still useles..

I added a test that checks for explicitly defined source sizes.

If no image is loaded yet, sourceSize is set to 0,0, and setting it implicitly will trigger a changed and will set forcedSourceSize, and as a result the image sourceSize will be ignored in the future. So it all seems good.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-09: