Make AbstractButton's icon properties win over Action's when both are set

Task-number: QTBUG-65193
Change-Id: Idff23dcc35f3c3fe41406678613b022098149318
Reviewed-by: J-P Nurmi <jpnurmi@qt.io>

4 files changed, 115 insertions, 34 deletions

diff --git a/src/quicktemplates2/qquickabstractbutton.cpp b/src/quicktemplates2/qquickabstractbutton.cpp
index b1522995..768b923b 100644
--- a/src/quicktemplates2/qquickabstractbutton.cpp
+++ b/src/quicktemplates2/qquickabstractbutton.cpp
@@ -306,6 +306,21 @@ void QQuickAbstractButtonPrivate::setText(const QString &newText, bool isExplici
     q->buttonChange(QQuickAbstractButton::ButtonTextChange);
 }
 
+void QQuickAbstractButtonPrivate::updateEffectiveIcon()
+{
+    Q_Q(QQuickAbstractButton);
+    // We store effectiveIcon because we need to be able to tell if the icon has actually changed.
+    // If we only stored our icon and the action's icon, and resolved in the getter, we'd have
+    // no way of knowing what the old value was here. As an added benefit, we only resolve when
+    // something has changed, as opposed to doing it unconditionally in the icon() getter.
+    const QQuickIcon newEffectiveIcon = action ? icon.resolve(action->icon()) : icon;
+    if (newEffectiveIcon == effectiveIcon)
+        return;
+
+    effectiveIcon = newEffectiveIcon;
+    emit q->iconChanged();
+}
+
 void QQuickAbstractButtonPrivate::click()
 {
     Q_Q(QQuickAbstractButton);
@@ -687,17 +702,14 @@ void QQuickAbstractButton::setIndicator(QQuickItem *indicator)
 QQuickIcon QQuickAbstractButton::icon() const
 {
     Q_D(const QQuickAbstractButton);
-    return d->icon;
+    return d->effectiveIcon;
 }
 
 void QQuickAbstractButton::setIcon(const QQuickIcon &icon)
 {
     Q_D(QQuickAbstractButton);
-    if (d->icon == icon)
-        return;
-
     d->icon = icon;
-    emit iconChanged();
+    d->updateEffectiveIcon();
 }
 
 /*!
@@ -759,7 +771,7 @@ void QQuickAbstractButton::setAction(QQuickAction *action)
         QObjectPrivate::disconnect(oldAction, &QQuickAction::triggered, d, &QQuickAbstractButtonPrivate::click);
         QObjectPrivate::disconnect(oldAction, &QQuickAction::textChanged, d, &QQuickAbstractButtonPrivate::actionTextChange);
 
-        disconnect(oldAction, &QQuickAction::iconChanged, this, &QQuickAbstractButton::setIcon);
+        QObjectPrivate::disconnect(oldAction, &QQuickAction::iconChanged, d, &QQuickAbstractButtonPrivate::updateEffectiveIcon);
         disconnect(oldAction, &QQuickAction::checkedChanged, this, &QQuickAbstractButton::setChecked);
         disconnect(oldAction, &QQuickAction::checkableChanged, this, &QQuickAbstractButton::setCheckable);
         disconnect(oldAction, &QQuickAction::enabledChanged, this, &QQuickItem::setEnabled);
@@ -770,33 +782,11 @@ void QQuickAbstractButton::setAction(QQuickAction *action)
         QObjectPrivate::connect(action, &QQuickAction::triggered, d, &QQuickAbstractButtonPrivate::click);
         QObjectPrivate::connect(action, &QQuickAction::textChanged, d, &QQuickAbstractButtonPrivate::actionTextChange);
 
-        connect(action, &QQuickAction::iconChanged, this, &QQuickAbstractButton::setIcon);
+        QObjectPrivate::connect(action, &QQuickAction::iconChanged, d, &QQuickAbstractButtonPrivate::updateEffectiveIcon);
         connect(action, &QQuickAction::checkedChanged, this, &QQuickAbstractButton::setChecked);
         connect(action, &QQuickAction::checkableChanged, this, &QQuickAbstractButton::setCheckable);
         connect(action, &QQuickAction::enabledChanged, this, &QQuickItem::setEnabled);
 
-        QQuickIcon actionIcon = action->icon();
-
-        QString name = actionIcon.name();
-        if (!name.isEmpty())
-            d->icon.setName(name);
-
-        QUrl source = actionIcon.source();
-        if (!source.isEmpty())
-            d->icon.setSource(source);
-
-        int width = actionIcon.width();
-        if (width > 0)
-            d->icon.setWidth(width);
-
-        int height = actionIcon.height();
-        if (height)
-            d->icon.setHeight(height);
-
-        QColor color = actionIcon.color();
-        if (color != Qt::transparent)
-            d->icon.setColor(color);
-
         setChecked(action->isChecked());
         setCheckable(action->isCheckable());
         setEnabled(action->isEnabled());
@@ -807,6 +797,8 @@ void QQuickAbstractButton::setAction(QQuickAction *action)
     if (oldText != text())
         buttonChange(ButtonTextChange);
 
+    d->updateEffectiveIcon();
+
     emit actionChanged();
 }
 
diff --git a/src/quicktemplates2/qquickabstractbutton_p_p.h b/src/quicktemplates2/qquickabstractbutton_p_p.h
index 3b2a87f2..c82f8cf7 100644
--- a/src/quicktemplates2/qquickabstractbutton_p_p.h
+++ b/src/quicktemplates2/qquickabstractbutton_p_p.h
@@ -93,6 +93,8 @@ public:
     void actionTextChange();
     void setText(const QString &text, bool isExplicit);
 
+    void updateEffectiveIcon();
+
     void click();
     void trigger();
     void toggle(bool value);
@@ -119,6 +121,7 @@ public:
     QKeySequence shortcut;
 #endif
     QQuickIcon icon;
+    QQuickIcon effectiveIcon;
     QPointF pressPoint;
     Qt::MouseButtons pressButtons;
     QQuickDeferredPointer<QQuickItem> indicator;
diff --git a/src/quicktemplates2/qquickicon.cpp b/src/quicktemplates2/qquickicon.cpp
index 0b0127d3..79af62b1 100644
--- a/src/quicktemplates2/qquickicon.cpp
+++ b/src/quicktemplates2/qquickicon.cpp
@@ -44,7 +44,8 @@ public:
     QQuickIconPrivate()
         : width(0),
           height(0),
-          color(Qt::transparent)
+          color(Qt::transparent),
+          resolveMask(0)
     {
     }
 
@@ -53,6 +54,18 @@ public:
     int width;
     int height;
     QColor color;
+
+    enum ResolveProperties {
+        NameResolved = 0x0001,
+        SourceResolved = 0x0002,
+        WidthResolved = 0x0004,
+        HeightResolved = 0x0008,
+        ColorResolved = 0x0010,
+        AllPropertiesResolved = 0x1ffff
+    };
+
+    // This is based on QFont's resolve_mask.
+    int resolveMask;
 };
 
 QQuickIcon::QQuickIcon()
@@ -101,7 +114,17 @@ QString QQuickIcon::name() const
 
 void QQuickIcon::setName(const QString &name)
 {
+    if ((d->resolveMask & QQuickIconPrivate::NameResolved) && d->name == name)
+        return;
+
     d->name = name;
+    d->resolveMask |= QQuickIconPrivate::NameResolved;
+}
+
+void QQuickIcon::resetName()
+{
+    d->name = QString();
+    d->resolveMask &= ~QQuickIconPrivate::NameResolved;
 }
 
 QUrl QQuickIcon::source() const
@@ -111,7 +134,17 @@ QUrl QQuickIcon::source() const
 
 void QQuickIcon::setSource(const QUrl &source)
 {
+    if ((d->resolveMask & QQuickIconPrivate::SourceResolved) && d->source == source)
+        return;
+
     d->source = source;
+    d->resolveMask |= QQuickIconPrivate::SourceResolved;
+}
+
+void QQuickIcon::resetSource()
+{
+    d->source = QString();
+    d->resolveMask &= ~QQuickIconPrivate::SourceResolved;
 }
 
 int QQuickIcon::width() const
@@ -121,7 +154,17 @@ int QQuickIcon::width() const
 
 void QQuickIcon::setWidth(int width)
 {
+    if ((d->resolveMask & QQuickIconPrivate::WidthResolved) && d->width == width)
+        return;
+
     d->width = width;
+    d->resolveMask |= QQuickIconPrivate::WidthResolved;
+}
+
+void QQuickIcon::resetWidth()
+{
+    d->width = 0;
+    d->resolveMask &= ~QQuickIconPrivate::WidthResolved;
 }
 
 int QQuickIcon::height() const
@@ -131,7 +174,17 @@ int QQuickIcon::height() const
 
 void QQuickIcon::setHeight(int height)
 {
+    if ((d->resolveMask & QQuickIconPrivate::HeightResolved) && d->height == height)
+        return;
+
     d->height = height;
+    d->resolveMask |= QQuickIconPrivate::HeightResolved;
+}
+
+void QQuickIcon::resetHeight()
+{
+    d->height = 0;
+    d->resolveMask &= ~QQuickIconPrivate::HeightResolved;
 }
 
 QColor QQuickIcon::color() const
@@ -141,12 +194,39 @@ QColor QQuickIcon::color() const
 
 void QQuickIcon::setColor(const QColor &color)
 {
+    if ((d->resolveMask & QQuickIconPrivate::ColorResolved) && d->color == color)
+        return;
+
     d->color = color;
+    d->resolveMask |= QQuickIconPrivate::ColorResolved;
 }
 
 void QQuickIcon::resetColor()
 {
     d->color = Qt::transparent;
+    d->resolveMask &= ~QQuickIconPrivate::ColorResolved;
+}
+
+QQuickIcon QQuickIcon::resolve(const QQuickIcon &other) const
+{
+    QQuickIcon resolved = *this;
+
+    if (!(d->resolveMask & QQuickIconPrivate::NameResolved))
+        resolved.setName(other.name());
+
+    if (!(d->resolveMask & QQuickIconPrivate::SourceResolved))
+        resolved.setSource(other.source());
+
+    if (!(d->resolveMask & QQuickIconPrivate::WidthResolved))
+        resolved.setWidth(other.width());
+
+    if (!(d->resolveMask & QQuickIconPrivate::HeightResolved))
+        resolved.setHeight(other.height());
+
+    if (!(d->resolveMask & QQuickIconPrivate::ColorResolved))
+        resolved.setColor(other.color());
+
+    return resolved;
 }
 
 QT_END_NAMESPACE
diff --git a/src/quicktemplates2/qquickicon_p.h b/src/quicktemplates2/qquickicon_p.h
index 6e28f2a9..5eb18205 100644
--- a/src/quicktemplates2/qquickicon_p.h
+++ b/src/quicktemplates2/qquickicon_p.h
@@ -62,10 +62,10 @@ class QQuickIconPrivate;
 class Q_QUICKTEMPLATES2_PRIVATE_EXPORT QQuickIcon
 {
     Q_GADGET
-    Q_PROPERTY(QString name READ name WRITE setName FINAL)
-    Q_PROPERTY(QUrl source READ source WRITE setSource FINAL)
-    Q_PROPERTY(int width READ width WRITE setWidth FINAL)
-    Q_PROPERTY(int height READ height WRITE setHeight FINAL)
+    Q_PROPERTY(QString name READ name WRITE setName RESET name FINAL)
+    Q_PROPERTY(QUrl source READ source WRITE setSource RESET source FINAL)
+    Q_PROPERTY(int width READ width WRITE setWidth RESET width FINAL)
+    Q_PROPERTY(int height READ height WRITE setHeight RESET height FINAL)
     Q_PROPERTY(QColor color READ color WRITE setColor RESET resetColor FINAL)
 
 public:
@@ -81,20 +81,26 @@ public:
 
     QString name() const;
     void setName(const QString &name);
+    void resetName();
 
     QUrl source() const;
     void setSource(const QUrl &source);
+    void resetSource();
 
     int width() const;
     void setWidth(int width);
+    void resetWidth();
 
     int height() const;
     void setHeight(int height);
+    void resetHeight();
 
     QColor color() const;
     void setColor(const QColor &color);
     void resetColor();
 
+    QQuickIcon resolve(const QQuickIcon &other) const;
+
 private:
     QSharedDataPointer<QQuickIconPrivate> d;
 };