From 11c9206bf68f6e8f00d9bc41601f492d38d67f4d Mon Sep 17 00:00:00 2001
From: Aleix Pol <aleixpol@kde.org>
Date: Tue, 14 Nov 2017 03:13:07 +0100
Subject: [PATCH] Reduce spurious signal emissions

Summary:
We were reporting that the margins were changing whenever anything changed
in a FrameSvg, this patch makes sure we are only reporting the changes
when it actually changes.
This also fixes a binding loop in Desktop.qml from plasma-desktop.

BUG: 382233

Reviewers: #plasma, davidedmundson

Reviewed By: #plasma, davidedmundson

Subscribers: broulik, davidedmundson, plasma-devel, #frameworks

Tags: #plasma, #frameworks

Differential Revision: https://phabricator.kde.org/D8809
---
 src/declarativeimports/core/framesvgitem.cpp | 60 +++++++++++++-------
 src/declarativeimports/core/framesvgitem.h   |  3 +
 src/plasma/framesvg.cpp                      |  4 +-
 3 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/src/declarativeimports/core/framesvgitem.cpp b/src/declarativeimports/core/framesvgitem.cpp
index 6f528459e..922b347bc 100644
--- a/src/declarativeimports/core/framesvgitem.cpp
+++ b/src/declarativeimports/core/framesvgitem.cpp
@@ -232,6 +232,13 @@ qreal FrameSvgItemMargins::vertical() const
     return top() + bottom();
 }
 
+QVector<qreal> FrameSvgItemMargins::margins() const
+{
+    qreal left, top, right, bottom;
+    m_frameSvg->getMargins(left, top, right, bottom);
+    return {left, top, right, bottom};
+}
+
 void FrameSvgItemMargins::update()
 {
     emit marginsChanged();
@@ -272,12 +279,32 @@ FrameSvgItem::~FrameSvgItem()
 {
 }
 
+class CheckMarginsChange
+{
+public:
+    CheckMarginsChange(FrameSvgItemMargins *margins)
+        : m_oldMargins(margins ? margins->margins() : QVector<qreal>()), m_margins(margins)
+    {}
+
+    ~CheckMarginsChange() {
+        if (m_margins && m_margins->margins() != m_oldMargins) {
+            m_margins->update();
+        }
+    }
+
+    const QVector<qreal> m_oldMargins;
+    FrameSvgItemMargins *const m_margins;
+};
+
 void FrameSvgItem::setImagePath(const QString &path)
 {
     if (m_frameSvg->imagePath() == path) {
         return;
     }
 
+    CheckMarginsChange checkMargins(m_margins);
+    CheckMarginsChange checkFixedMargins(m_fixedMargins);
+
     updateDevicePixelRatio();
     m_frameSvg->setImagePath(path);
 
@@ -290,12 +317,6 @@ void FrameSvgItem::setImagePath(const QString &path)
     }
 
     emit imagePathChanged();
-    if (m_margins) {
-        m_margins->update();
-    }
-    if (m_fixedMargins) {
-        m_fixedMargins->update();
-    }
 
     if (isComponentComplete()) {
         applyPrefixes();
@@ -325,6 +346,9 @@ void FrameSvgItem::setPrefix(const QVariant &prefixes)
         return;
     }
 
+    CheckMarginsChange checkMargins(m_margins);
+    CheckMarginsChange checkFixedMargins(m_fixedMargins);
+
     m_prefixes = prefixList;
     applyPrefixes();
 
@@ -337,12 +361,6 @@ void FrameSvgItem::setPrefix(const QVariant &prefixes)
     }
 
     emit prefixChanged();
-    if (m_margins) {
-        m_margins->update();
-    }
-    if (m_fixedMargins) {
-        m_fixedMargins->update();
-    }
 
     if (isComponentComplete()) {
         m_frameSvg->resizeFrame(QSizeF(width(), height()));
@@ -415,12 +433,11 @@ void FrameSvgItem::setEnabledBorders(const Plasma::FrameSvg::EnabledBorders bord
         return;
     }
 
+    CheckMarginsChange checkMargins(m_margins);
+
     m_frameSvg->setEnabledBorders(borders);
     emit enabledBordersChanged();
     m_textureChanged = true;
-    if (m_margins) {
-        m_margins->update();
-    }
     update();
 }
 
@@ -446,6 +463,9 @@ void FrameSvgItem::geometryChanged(const QRectF &newGeometry,
 
 void FrameSvgItem::doUpdate()
 {
+    CheckMarginsChange checkMargins(m_margins);
+    CheckMarginsChange checkFixedMargins(m_fixedMargins);
+
     //if the theme changed, the available prefix may have changed as well
     applyPrefixes();
 
@@ -473,12 +493,7 @@ void FrameSvgItem::doUpdate()
     m_textureChanged = true;
 
     update();
-    if (m_margins) {
-        m_margins->update();
-    }
-    if (m_fixedMargins) {
-        m_fixedMargins->update();
-    }
+
     emit repaintNeeded();
 }
 
@@ -569,6 +584,9 @@ void FrameSvgItem::classBegin()
 
 void FrameSvgItem::componentComplete()
 {
+    CheckMarginsChange checkMargins(m_margins);
+    CheckMarginsChange checkFixedMargins(m_fixedMargins);
+
     QQuickItem::componentComplete();
     m_frameSvg->resizeFrame(QSize(width(), height()));
     m_frameSvg->setRepaintBlocked(false);
diff --git a/src/declarativeimports/core/framesvgitem.h b/src/declarativeimports/core/framesvgitem.h
index c5bcdad57..82d91d583 100644
--- a/src/declarativeimports/core/framesvgitem.h
+++ b/src/declarativeimports/core/framesvgitem.h
@@ -83,6 +83,9 @@ public:
     qreal horizontal() const;
     qreal vertical() const;
 
+    /// returns a vector with left, top, right, bottom
+    QVector<qreal> margins() const;
+
     void setFixed(bool fixed);
     bool isFixed() const;
 
diff --git a/src/plasma/framesvg.cpp b/src/plasma/framesvg.cpp
index 3696b09ce..6c1b24145 100644
--- a/src/plasma/framesvg.cpp
+++ b/src/plasma/framesvg.cpp
@@ -275,7 +275,7 @@ qreal FrameSvg::fixedMarginSize(const Plasma::Types::MarginEdge edge) const
 
 void FrameSvg::getMargins(qreal &left, qreal &top, qreal &right, qreal &bottom) const
 {
-    if (d->frame->noBorderPadding) {
+    if (!d->frame || d->frame->noBorderPadding) {
         left = top = right = bottom = 0;
         return;
     }
@@ -288,7 +288,7 @@ void FrameSvg::getMargins(qreal &left, qreal &top, qreal &right, qreal &bottom)
 
 void FrameSvg::getFixedMargins(qreal &left, qreal &top, qreal &right, qreal &bottom) const
 {
-    if (d->frame->noBorderPadding) {
+    if (!d->frame || d->frame->noBorderPadding) {
         left = top = right = bottom = 0;
         return;
     }