Merge lp:~loic.molinari/ubuntu-ui-toolkit/ubuntu-ui-toolkit-ubuntushape-wrong-deprecation-log-fix into lp:ubuntu-ui-toolkit/staging

[Loïc Molinari]

[UbuntuShape] Fixed deprecation logging issues.

This fix prevents logging a deprecation warning for "image", "color" and "gradientColor" properties when the import version is less than 1.3. The logging of properties used internally (through the old image wrapper) have been removed too since the user might not even have used them.

[UbuntuShape] Fixed deprecation logging issues.

This fix prevents logging a deprecation warning for "image", "color" and "gradientColor" properties when the import version is less than 1.3. The logging of properties used internally (through the old image wrapper) have been removed too since the user might not even have used them.

[Zsombor Egri (zsombi)]

You may want to use http://bazaar.launchpad.net/~ubuntu-sdk-team/ubuntu-ui-toolkit/staging/view/head:/src/Ubuntu/Components/plugin/ucimportversionchecker_p.h to check in what version the UbuntuShape instance is used. Check UCListItem on how it is used.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1725
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2529/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1257/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1259/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1256/console

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

review: Needs Fixing (continuous-integration)

[lp:~loic.molinari/ubuntu-ui-toolkit/ubuntu-ui-toolkit-ubuntushape-wrong-deprecation-log-fix]

1725. By Loïc Molinari on 2015-11-24

[UbuntuShape] Fixed wrong deprecation log info.

[Loïc Molinari (loic.molinari)]

> You may want to use http://bazaar.launchpad.net/~ubuntu-sdk-team/ubuntu-ui-too
> lkit/staging/view/head:/src/Ubuntu/Components/plugin/ucimportversionchecker_p.
> h to check in what version the UbuntuShape instance is used. Check UCListItem
> on how it is used.

Done, thanks.

[Zsombor Egri (zsombi)]

One more thing... I do not feel you'd need to introduce a different versioning definition just for this, you could use the version encoding we have right now, I do not think there is much performance implications using BUILD_VERSION(major, minor) vs. enums. Or even better, declaring constants with V13 = BUILD_VERSION(1, 3) then using that in the tests, i.e if (m_version - V13 && !loggedOnce) would even exclude the eventual overhead provided by the BUILD_VERSION (i.e. bit-shift + bit-or)

See comments inline.

[Loïc Molinari (loic.molinari)]

> One more thing... I do not feel you'd need to introduce a different versioning
> definition just for this, you could use the version encoding we have right
> now, I do not think there is much performance implications using
> BUILD_VERSION(major, minor) vs. enums. Or even better, declaring constants
> with V13 = BUILD_VERSION(1, 3) then using that in the tests, i.e if (m_version
> - V13 && !loggedOnce) would even exclude the eventual overhead provided by the
> BUILD_VERSION (i.e. bit-shift + bit-or)
>
> See comments inline.

Not that I don't like the encoding (and I don't force anyone to do the same) but I did that to avoid adding 16 bits (very likely to be 32 bits if the 4 bytes boundary is reached) per instance just for that, instead of 1 bit in the already allocated padding space. The shape being the item that's used the most by apps I try to make it as compact as possible. But I can store it as U16 if you think it's not worth it.

Yes, I'm a data torturer :)

[Zsombor Egri (zsombi)]

> > One more thing... I do not feel you'd need to introduce a different
> versioning
> > definition just for this, you could use the version encoding we have right
> > now, I do not think there is much performance implications using
> > BUILD_VERSION(major, minor) vs. enums. Or even better, declaring constants
> > with V13 = BUILD_VERSION(1, 3) then using that in the tests, i.e if
> (m_version
> > - V13 && !loggedOnce) would even exclude the eventual overhead provided by
> the
> > BUILD_VERSION (i.e. bit-shift + bit-or)
> >
> > See comments inline.
>
> Not that I don't like the encoding (and I don't force anyone to do the same)
> but I did that to avoid adding 16 bits (very likely to be 32 bits if the 4
> bytes boundary is reached) per instance just for that, instead of 1 bit in the
> already allocated padding space. The shape being the item that's used the most
> by apps I try to make it as compact as possible. But I can store it as U16 if
> you think it's not worth it.

Fair point!

>
> Yes, I'm a data torturer :)

I am glad you are, at least I can find my pair in this company ;)

[Loïc Molinari (loic.molinari)]

> > Yes, I'm a data torturer :)
>
> I am glad you are, at least I can find my pair in this company ;)

Hehe!

[lp:~loic.molinari/ubuntu-ui-toolkit/ubuntu-ui-toolkit-ubuntushape-wrong-deprecation-log-fix]

1726. By Loïc Molinari on 2015-12-02

trigger new jenkins pass.

1727. By Loïc Molinari on 2015-12-02

trigger new jenkins pass.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1727
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2557/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1285
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1287
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1287/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1284

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

review: Approve (continuous-integration)

[Zsombor Egri (zsombi)]

Ok, finally :)

review: Approve

=== modified file 'src/Ubuntu/Components/plugin/ucubuntushape.cpp'
--- src/Ubuntu/Components/plugin/ucubuntushape.cpp	2015-10-21 12:54:54 +0000
+++ src/Ubuntu/Components/plugin/ucubuntushape.cpp	2015-12-02 09:22:46 +0000
@@ -28,6 +28,7 @@
 #include "ucubuntushape.h"
 #include "ucubuntushapetexture.h"
 #include "ucunits.h"
+#include "ucnamespace.h"
 #include "quickutils.h"
 #include <QtCore/QPointer>
 #include <QtGui/QGuiApplication>
@@ -307,6 +308,7 @@
     , m_sourceFillMode(Stretch)
     , m_sourceHorizontalWrapMode(Transparent)
     , m_sourceVerticalWrapMode(Transparent)
+    , m_version(Version12)
     , m_sourceOpacity(255)
     , m_flags(Stretched)
 {
@@ -328,6 +330,11 @@
          || openglContext->hasExtension(QByteArrayLiteral("GL_OES_standard_derivatives")));
 }
 
+bool UCUbuntuShape::isVersionGreaterThanOrEqual(Version version)
+{
+    return static_cast<int>(m_version) >= static_cast<int>(version);
+}
+
 /*! \qmlproperty string UbuntuShape::radius
 
     This property defines the corner radius. Three fixed values are supported: \c "small",
@@ -808,7 +815,7 @@
 void UCUbuntuShape::setColor(const QColor& color)
 {
     static bool loggedOnce = false;
-    if (!loggedOnce) {
+    if (isVersionGreaterThanOrEqual(Version13) && !loggedOnce) {
         loggedOnce = true;
         qmlInfo(this) << "'color' is deprecated. Use 'backgroundColor', 'secondaryBackgroundColor' "
             "and 'backgroundMode' instead.";
@@ -841,7 +848,7 @@
 void UCUbuntuShape::setGradientColor(const QColor& gradientColor)
 {
     static bool loggedOnce = false;
-    if (!loggedOnce) {
+    if (isVersionGreaterThanOrEqual(Version13) && !loggedOnce) {
         loggedOnce = true;
         qmlInfo(this) << "'gradientColor' is deprecated. Use 'backgroundColor', "
             "'secondaryBackgroundColor' and 'backgroundMode' instead.";
@@ -872,7 +879,7 @@
 void UCUbuntuShape::setImage(const QVariant& image)
 {
     static bool loggedOnce = false;
-    if (!loggedOnce) {
+    if (isVersionGreaterThanOrEqual(Version13) && !loggedOnce) {
         loggedOnce = true;
         qmlInfo(this) << "'image' is deprecated. Use 'source' instead.";
     }
@@ -904,12 +911,6 @@
 // maintain it for a while for compatibility reasons.
 void UCUbuntuShape::setStretched(bool stretched)
 {
-    static bool loggedOnce = false;
-    if (!loggedOnce) {
-        loggedOnce = true;
-        qmlInfo(this) << "'stretched' is deprecated. Use 'sourceFillMode' instead";
-    }
-
     if (!(m_flags & SourceApiSet)) {
         if (!!(m_flags & Stretched) != stretched) {
             if (stretched) {
@@ -927,13 +928,6 @@
 // Deprecation layer. Same comment as setStretched().
 void UCUbuntuShape::setHorizontalAlignment(HAlignment horizontalAlignment)
 {
-    static bool loggedOnce = false;
-    if (!loggedOnce) {
-        loggedOnce = true;
-        qmlInfo(this) << "'horizontalAlignment' is deprecated. Use 'sourceHorizontalAlignment' "
-            "instead";
-    }
-
     if (!(m_flags & SourceApiSet)) {
         if (m_imageHorizontalAlignment != horizontalAlignment) {
             m_imageHorizontalAlignment = horizontalAlignment;
@@ -947,13 +941,6 @@
 // Deprecation layer. Same comment as setStretched().
 void UCUbuntuShape::setVerticalAlignment(VAlignment verticalAlignment)
 {
-    static bool loggedOnce = false;
-    if (!loggedOnce) {
-        loggedOnce = true;
-        qmlInfo(this) << "'horizontalAlignment' is deprecated. Use 'sourceVerticalAlignment' "
-            "instead";
-    }
-
     if (!(m_flags & SourceApiSet)) {
         if (m_imageVerticalAlignment != verticalAlignment) {
             m_imageVerticalAlignment = verticalAlignment;
@@ -1047,6 +1034,21 @@
     update();
 }
 
+QString UCUbuntuShape::propertyForVersion(quint16 version) const
+{
+    if (MINOR_VERSION(version) == 3) {
+        return QStringLiteral("relativeRadius");
+    } else {
+        return QString();
+    }
+}
+
+void UCUbuntuShape::componentComplete()
+{
+    QQuickItem::componentComplete();
+    m_version = MINOR_VERSION(importVersion(this)) == 3 ? Version13 : Version12;
+}
+
 void UCUbuntuShape::geometryChanged(const QRectF& newGeometry, const QRectF& oldGeometry)
 {
     QQuickItem::geometryChanged(newGeometry, oldGeometry);

=== modified file 'src/Ubuntu/Components/plugin/ucubuntushape.h'
--- src/Ubuntu/Components/plugin/ucubuntushape.h	2015-09-10 11:04:16 +0000
+++ src/Ubuntu/Components/plugin/ucubuntushape.h	2015-12-02 09:22:46 +0000
@@ -24,8 +24,7 @@
 #include <QtQuick/qsgtexture.h>
 #include <QtQuick/qsgmaterial.h>
 #include <QtGui/QOpenGLFunctions>
-
-class UCUbuntuShape;
+#include "ucimportversionchecker_p.h"
 
 // --- Scene graph shader ---
 
@@ -121,7 +120,7 @@
 
 // --- QtQuick item ---
 
-class UCUbuntuShape : public QQuickItem
+class UCUbuntuShape : public QQuickItem, public UCImportVersionChecker
 {
     Q_OBJECT
 
@@ -183,6 +182,7 @@
 
     static bool useDistanceFields(const QOpenGLContext* openglContext);
 
+    enum Version { Version12 = 0 /* Or lesser */, Version13 = 1 };
     enum Aspect { Flat = 0, Inset = 1, DropShadow = 2 };  // Don't forget to update private enum.
     enum BackgroundMode { SolidColor = 0, VerticalGradient = 1 };
     enum HAlignment { AlignLeft = 0, AlignHCenter = 1, AlignRight = 2 };
@@ -287,6 +287,8 @@
     void verticalAlignmentChanged();
 
 protected:
+    virtual QString propertyForVersion(quint16 version) const;
+    virtual void componentComplete();
     virtual QSGNode* updatePaintNode(QSGNode* oldNode, UpdatePaintNodeData* data);
     virtual void geometryChanged(const QRectF& newGeometry, const QRectF& oldGeometry);
 
@@ -305,6 +307,7 @@
     void _q_textureChanged();
 
 private:
+    bool isVersionGreaterThanOrEqual(Version version);
     void updateFromImageProperties(QQuickItem* image);
     void connectToPropertyChange(
         QObject* sender, const char* property, QObject* receiver, const char* slot);
@@ -344,7 +347,8 @@
     FillMode m_sourceFillMode : 2;
     WrapMode m_sourceHorizontalWrapMode : 1;
     WrapMode m_sourceVerticalWrapMode : 1;
-    quint8 __explicit_padding : 6;
+    Version m_version : 1;
+    quint8 __explicit_padding : 5;
     quint8 m_sourceOpacity;
     quint8 m_flags;