FrameSvg: Do not wreck shared mask frames

Summary:
When trying to update the maskFrame, there is a chance that it is already
shared by several instances of FrameSvg. In that case, do not wreck maskFrame
and instead act as follows:

* deref it
* try to lookup a matching frame in the shared frames
* if there is the matching frame, use it
* otherwise create a new one

Reviewers: #plasma, #frameworks, mart

Reviewed By: #plasma, mart

Subscribers: mart, kde-frameworks-devel

Tags: #frameworks

Differential Revision: https://phabricator.kde.org/D13384

src/plasma/framesvg.cpp

@@ -450,59 +450,74 @@ QPixmap FrameSvgPrivate::alphaMask()
         if (frame->cachedBackground.isNull()) {
             generateBackground(frame);
         }
-
         return frame->cachedBackground;
-    } else {
+    }
-        // We are setting the prefix only temporary to generate
-        // the needed mask image
-        const QString maskRequestedPrefix = requestedPrefix.isEmpty() ? QStringLiteral("mask") : maskPrefix % requestedPrefix;
-        maskPrefix = maskPrefix % prefix;
 
-        if (!maskFrame) {
+    // We are setting the prefix only temporary to generate
-            const QString key = cacheId(frame, maskPrefix);
+    // the needed mask image
-            // see if we can find a suitable candidate in the shared frames
+    const QString maskRequestedPrefix = requestedPrefix.isEmpty() ? QStringLiteral("mask") : maskPrefix % requestedPrefix;
-            // if successful, ref and insert, otherwise create a new one
+    maskPrefix = maskPrefix % prefix;
-            // and insert that into both the shared frames and our frames.
-            maskFrame = s_sharedFrames[q->theme()->d].value(key);
 
-            if (maskFrame) {
+    if (!maskFrame) {
-                maskFrame->ref(q);
+        maskFrame = lookupOrCreateMaskFrame(frame, maskPrefix, maskRequestedPrefix);
-            } else {
+        if (!maskFrame->cachedBackground.isNull()) {
-                maskFrame = new FrameData(*frame, q);
-                maskFrame->prefix = maskPrefix;
-                maskFrame->requestedPrefix = maskRequestedPrefix;
-                maskFrame->theme = q->theme()->d;
-                maskFrame->imagePath = q->imagePath();
-                s_sharedFrames[q->theme()->d].insert(key, maskFrame);
-            }
-            maskFrame->enabledBorders = frame->enabledBorders;
-
-            updateSizes(maskFrame);
-        }
-
-        const bool shouldUpdate = maskFrame->cachedBackground.isNull()
-            || maskFrame->enabledBorders != frame->enabledBorders
-            || maskFrame->frameSize != frameSize(frame);
-        if (!shouldUpdate) {
             return maskFrame->cachedBackground;
         }
-
-        const QString oldKey = cacheId(maskFrame, maskPrefix);
-        maskFrame->enabledBorders = frame->enabledBorders;
-        maskFrame->frameSize = frameSize(frame).toSize();
-        const QString newKey = cacheId(maskFrame, maskPrefix);
-        if (s_sharedFrames[q->theme()->d].contains(oldKey)) {
-            s_sharedFrames[q->theme()->d].remove(oldKey);
-            s_sharedFrames[q->theme()->d].insert(newKey, maskFrame);
-        }
-
-        maskFrame->cachedBackground = QPixmap();
-
         updateSizes(maskFrame);
         generateBackground(maskFrame);
-
         return maskFrame->cachedBackground;
     }
+
+    const bool shouldUpdate = maskFrame->enabledBorders != frame->enabledBorders
+        || maskFrame->frameSize != frameSize(frame);
+    if (shouldUpdate) {
+        if (maskFrame->refcount() == 1) {
+            const QString oldKey = cacheId(maskFrame, maskPrefix);
+            s_sharedFrames[q->theme()->d].remove(oldKey);
+            maskFrame->enabledBorders = frame->enabledBorders;
+            maskFrame->frameSize = frameSize(frame).toSize();
+            const QString newKey = cacheId(maskFrame, maskPrefix);
+            s_sharedFrames[q->theme()->d].insert(newKey, maskFrame);
+            maskFrame->cachedBackground = QPixmap();
+        } else {
+            maskFrame->deref(q);
+            maskFrame = lookupOrCreateMaskFrame(frame, maskPrefix, maskRequestedPrefix);
+            if (!maskFrame->cachedBackground.isNull()) {
+                return maskFrame->cachedBackground;
+            }
+        }
+        updateSizes(maskFrame);
+    }
+
+    if (maskFrame->cachedBackground.isNull()) {
+        generateBackground(maskFrame);
+    }
+
+    return maskFrame->cachedBackground;
+}
+
+FrameData *FrameSvgPrivate::lookupOrCreateMaskFrame(FrameData *frame, const QString &maskPrefix, const QString &maskRequestedPrefix)
+{
+    const QString key = cacheId(frame, maskPrefix);
+    FrameData *mask = s_sharedFrames[q->theme()->d].value(key);
+
+    // See if we can find a suitable candidate in the shared frames.
+    // If there is one, use it.
+    if (mask) {
+        mask->ref(q);
+        return mask;
+    }
+
+    mask = new FrameData(*frame, q);
+    mask->prefix = maskPrefix;
+    mask->requestedPrefix = maskRequestedPrefix;
+    mask->theme = q->theme()->d;
+    mask->imagePath = frame->imagePath;
+    mask->enabledBorders = frame->enabledBorders;
+    mask->frameSize = frameSize(frame).toSize();
+    s_sharedFrames[q->theme()->d].insert(key, mask);
+
+    return mask;
 }
 
 void FrameSvgPrivate::generateBackground(FrameData *frame)

src/plasma/private/framesvg_p.h

@@ -172,6 +172,7 @@ public:
     void paintCenter(QPainter& p, FrameData* frame, const QRect& contentRect, const QSize& fullSize);
     QRect contentGeometry(FrameData* frame, const QSize& size) const;
     void updateFrameData(UpdateType updateType = UpdateFrameAndMargins);
+    FrameData *lookupOrCreateMaskFrame(FrameData *frame, const QString &maskPrefix, const QString &maskRequestedPrefix);
 
     Types::Location location;
     QString prefix;