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

Proposed by Loïc Molinari on 2015-11-24
Status: Merged
Approved by: Zsombor Egri on 2015-12-03
Approved revision: 1727
Merged at revision: 1736
Proposed branch: lp:~loic.molinari/ubuntu-ui-toolkit/ubuntu-ui-toolkit-ubuntushape-wrong-deprecation-log-fix
Merge into: lp:ubuntu-ui-toolkit/staging
Diff against target: 178 lines (+33/-27)
2 files modified
src/Ubuntu/Components/plugin/ucubuntushape.cpp (+25/-23)
src/Ubuntu/Components/plugin/ucubuntushape.h (+8/-4)
To merge this branch: bzr merge lp:~loic.molinari/ubuntu-ui-toolkit/ubuntu-ui-toolkit-ubuntushape-wrong-deprecation-log-fix
Reviewer Review Type Date Requested Status
Zsombor Egri 2015-11-24 Approve on 2015-12-03
PS Jenkins bot continuous-integration Approve on 2015-12-02
Review via email: mp+278474@code.launchpad.net

Commit message

[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.

Description of the change

[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.

To post a comment you must log in.
Zsombor Egri (zsombi) wrote :

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
1725. By Loïc Molinari on 2015-11-24

[UbuntuShape] Fixed wrong deprecation log info.

Loïc Molinari (loic.molinari) wrote :

> 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) wrote :

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) wrote :

> 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) wrote :

> > 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) wrote :

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

Hehe!

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.

Zsombor Egri (zsombi) wrote :

Ok, finally :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/Ubuntu/Components/plugin/ucubuntushape.cpp'
2--- src/Ubuntu/Components/plugin/ucubuntushape.cpp 2015-10-21 12:54:54 +0000
3+++ src/Ubuntu/Components/plugin/ucubuntushape.cpp 2015-12-02 09:22:46 +0000
4@@ -28,6 +28,7 @@
5 #include "ucubuntushape.h"
6 #include "ucubuntushapetexture.h"
7 #include "ucunits.h"
8+#include "ucnamespace.h"
9 #include "quickutils.h"
10 #include <QtCore/QPointer>
11 #include <QtGui/QGuiApplication>
12@@ -307,6 +308,7 @@
13 , m_sourceFillMode(Stretch)
14 , m_sourceHorizontalWrapMode(Transparent)
15 , m_sourceVerticalWrapMode(Transparent)
16+ , m_version(Version12)
17 , m_sourceOpacity(255)
18 , m_flags(Stretched)
19 {
20@@ -328,6 +330,11 @@
21 || openglContext->hasExtension(QByteArrayLiteral("GL_OES_standard_derivatives")));
22 }
23
24+bool UCUbuntuShape::isVersionGreaterThanOrEqual(Version version)
25+{
26+ return static_cast<int>(m_version) >= static_cast<int>(version);
27+}
28+
29 /*! \qmlproperty string UbuntuShape::radius
30
31 This property defines the corner radius. Three fixed values are supported: \c "small",
32@@ -808,7 +815,7 @@
33 void UCUbuntuShape::setColor(const QColor& color)
34 {
35 static bool loggedOnce = false;
36- if (!loggedOnce) {
37+ if (isVersionGreaterThanOrEqual(Version13) && !loggedOnce) {
38 loggedOnce = true;
39 qmlInfo(this) << "'color' is deprecated. Use 'backgroundColor', 'secondaryBackgroundColor' "
40 "and 'backgroundMode' instead.";
41@@ -841,7 +848,7 @@
42 void UCUbuntuShape::setGradientColor(const QColor& gradientColor)
43 {
44 static bool loggedOnce = false;
45- if (!loggedOnce) {
46+ if (isVersionGreaterThanOrEqual(Version13) && !loggedOnce) {
47 loggedOnce = true;
48 qmlInfo(this) << "'gradientColor' is deprecated. Use 'backgroundColor', "
49 "'secondaryBackgroundColor' and 'backgroundMode' instead.";
50@@ -872,7 +879,7 @@
51 void UCUbuntuShape::setImage(const QVariant& image)
52 {
53 static bool loggedOnce = false;
54- if (!loggedOnce) {
55+ if (isVersionGreaterThanOrEqual(Version13) && !loggedOnce) {
56 loggedOnce = true;
57 qmlInfo(this) << "'image' is deprecated. Use 'source' instead.";
58 }
59@@ -904,12 +911,6 @@
60 // maintain it for a while for compatibility reasons.
61 void UCUbuntuShape::setStretched(bool stretched)
62 {
63- static bool loggedOnce = false;
64- if (!loggedOnce) {
65- loggedOnce = true;
66- qmlInfo(this) << "'stretched' is deprecated. Use 'sourceFillMode' instead";
67- }
68-
69 if (!(m_flags & SourceApiSet)) {
70 if (!!(m_flags & Stretched) != stretched) {
71 if (stretched) {
72@@ -927,13 +928,6 @@
73 // Deprecation layer. Same comment as setStretched().
74 void UCUbuntuShape::setHorizontalAlignment(HAlignment horizontalAlignment)
75 {
76- static bool loggedOnce = false;
77- if (!loggedOnce) {
78- loggedOnce = true;
79- qmlInfo(this) << "'horizontalAlignment' is deprecated. Use 'sourceHorizontalAlignment' "
80- "instead";
81- }
82-
83 if (!(m_flags & SourceApiSet)) {
84 if (m_imageHorizontalAlignment != horizontalAlignment) {
85 m_imageHorizontalAlignment = horizontalAlignment;
86@@ -947,13 +941,6 @@
87 // Deprecation layer. Same comment as setStretched().
88 void UCUbuntuShape::setVerticalAlignment(VAlignment verticalAlignment)
89 {
90- static bool loggedOnce = false;
91- if (!loggedOnce) {
92- loggedOnce = true;
93- qmlInfo(this) << "'horizontalAlignment' is deprecated. Use 'sourceVerticalAlignment' "
94- "instead";
95- }
96-
97 if (!(m_flags & SourceApiSet)) {
98 if (m_imageVerticalAlignment != verticalAlignment) {
99 m_imageVerticalAlignment = verticalAlignment;
100@@ -1047,6 +1034,21 @@
101 update();
102 }
103
104+QString UCUbuntuShape::propertyForVersion(quint16 version) const
105+{
106+ if (MINOR_VERSION(version) == 3) {
107+ return QStringLiteral("relativeRadius");
108+ } else {
109+ return QString();
110+ }
111+}
112+
113+void UCUbuntuShape::componentComplete()
114+{
115+ QQuickItem::componentComplete();
116+ m_version = MINOR_VERSION(importVersion(this)) == 3 ? Version13 : Version12;
117+}
118+
119 void UCUbuntuShape::geometryChanged(const QRectF& newGeometry, const QRectF& oldGeometry)
120 {
121 QQuickItem::geometryChanged(newGeometry, oldGeometry);
122
123=== modified file 'src/Ubuntu/Components/plugin/ucubuntushape.h'
124--- src/Ubuntu/Components/plugin/ucubuntushape.h 2015-09-10 11:04:16 +0000
125+++ src/Ubuntu/Components/plugin/ucubuntushape.h 2015-12-02 09:22:46 +0000
126@@ -24,8 +24,7 @@
127 #include <QtQuick/qsgtexture.h>
128 #include <QtQuick/qsgmaterial.h>
129 #include <QtGui/QOpenGLFunctions>
130-
131-class UCUbuntuShape;
132+#include "ucimportversionchecker_p.h"
133
134 // --- Scene graph shader ---
135
136@@ -121,7 +120,7 @@
137
138 // --- QtQuick item ---
139
140-class UCUbuntuShape : public QQuickItem
141+class UCUbuntuShape : public QQuickItem, public UCImportVersionChecker
142 {
143 Q_OBJECT
144
145@@ -183,6 +182,7 @@
146
147 static bool useDistanceFields(const QOpenGLContext* openglContext);
148
149+ enum Version { Version12 = 0 /* Or lesser */, Version13 = 1 };
150 enum Aspect { Flat = 0, Inset = 1, DropShadow = 2 }; // Don't forget to update private enum.
151 enum BackgroundMode { SolidColor = 0, VerticalGradient = 1 };
152 enum HAlignment { AlignLeft = 0, AlignHCenter = 1, AlignRight = 2 };
153@@ -287,6 +287,8 @@
154 void verticalAlignmentChanged();
155
156 protected:
157+ virtual QString propertyForVersion(quint16 version) const;
158+ virtual void componentComplete();
159 virtual QSGNode* updatePaintNode(QSGNode* oldNode, UpdatePaintNodeData* data);
160 virtual void geometryChanged(const QRectF& newGeometry, const QRectF& oldGeometry);
161
162@@ -305,6 +307,7 @@
163 void _q_textureChanged();
164
165 private:
166+ bool isVersionGreaterThanOrEqual(Version version);
167 void updateFromImageProperties(QQuickItem* image);
168 void connectToPropertyChange(
169 QObject* sender, const char* property, QObject* receiver, const char* slot);
170@@ -344,7 +347,8 @@
171 FillMode m_sourceFillMode : 2;
172 WrapMode m_sourceHorizontalWrapMode : 1;
173 WrapMode m_sourceVerticalWrapMode : 1;
174- quint8 __explicit_padding : 6;
175+ Version m_version : 1;
176+ quint8 __explicit_padding : 5;
177 quint8 m_sourceOpacity;
178 quint8 m_flags;
179

Subscribers

People subscribed via source and target branches