<table><tr><td style="">zzag added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D17052">View Revision</a></tr></table><br /><div><div><p>In gneral, +1 for shared pointers.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D17052#inline-92819">View Inline</a><span style="color: #4b4d51; font-weight: bold;">framesvg.cpp:375</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "><span style="color: #74777d">//#define DEBUG_FRAMESVG_CACHE</span>
</div><div style="padding: 0 8px; margin: 0 4px; "><span class="n">FrameSvgPrivate</span><span style="color: #aa2211">::~</span><span class="n">FrameSvgPrivate</span><span class="p">()</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#ifdef DEBUG_FRAMESVG_CACHE</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#ifndef NDEBUG</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #74777d">// qCDebug(LOG_PLASMA) << "*************" << q << q->imagePath() << "****************";</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#endif</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#endif</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #74777d">// we remove all references from this widget to the frame, and delete it if we're the</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #74777d">// last user</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">frame</span> <span style="color: #aa2211">&&</span> <span class="n">frame</span><span style="color: #aa2211">-></span><span class="n">removeRefs</span><span class="p">(</span><span class="n">q</span><span class="p">))</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">const</span> <span class="n">QString</span> <span class="n">key</span> <span style="color: #aa2211">=</span> <span class="n">cacheId</span><span class="p">(</span><span class="n">frame</span><span class="p">,</span> <span class="n">frame</span><span style="color: #aa2211">-></span><span class="n">prefix</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#ifdef DEBUG_FRAMESVG_CACHE</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#ifndef NDEBUG</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #74777d">// qCDebug(LOG_PLASMA) << "2. Removing it" << key << frame << frame->refcount() << s_sharedFrames[theme()->d].contains(key);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#endif</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#endif</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">s_sharedFrames</span><span class="p">[</span><span class="n">frame</span><span style="color: #aa2211">-></span><span class="n">theme</span><span class="p">].</span><span class="n">remove</span><span class="p">(</span><span class="n">key</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">delete</span> <span class="n">frame</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #74777d">//same thing for maskFrame</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">maskFrame</span> <span style="color: #aa2211">&&</span> <span class="n">maskFrame</span><span style="color: #aa2211">-></span><span class="n">removeRefs</span><span class="p">(</span><span class="n">q</span><span class="p">))</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">const</span> <span class="n">QString</span> <span class="n">key</span> <span style="color: #aa2211">=</span> <span class="n">cacheId</span><span class="p">(</span><span class="n">maskFrame</span><span class="p">,</span> <span class="n">maskFrame</span><span style="color: #aa2211">-></span><span class="n">prefix</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">s_sharedFrames</span><span class="p">[</span><span class="n">maskFrame</span><span style="color: #aa2211">-></span><span class="n">theme</span><span class="p">].</span><span class="n">remove</span><span class="p">(</span><span class="n">key</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">delete</span> <span class="n">maskFrame</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#ifdef DEBUG_FRAMESVG_CACHE</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">QHashIterator</span><span style="color: #aa2211"><</span><span class="n">QString</span><span class="p">,</span> <span class="n">FrameData</span> <span style="color: #aa2211">*></span> <span class="n">it2</span><span class="p">(</span><span class="n">s_sharedFrames</span><span class="p">[</span><span class="n">theme</span><span class="p">()</span><span style="color: #aa2211">-></span><span class="n">d</span><span class="p">]);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">int</span> <span class="n">shares</span> <span style="color: #aa2211">=</span> <span style="color: #601200">0</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">while</span> <span class="p">(</span><span class="n">it2</span><span class="p">.</span><span class="n">hasNext</span><span class="p">())</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">it2</span><span class="p">.</span><span class="n">next</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">const</span> <span style="color: #aa4000">int</span> <span class="n">rc</span> <span style="color: #aa2211">=</span> <span class="n">it2</span><span class="p">.</span><span class="n">value</span><span class="p">()</span><span style="color: #aa2211">-></span><span class="n">refcount</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">rc</span> <span style="color: #aa2211">==</span> <span style="color: #601200">0</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#ifndef NDEBUG</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #74777d">// qCDebug(LOG_PLASMA) << " LOST!" << it2.key() << rc << it2.value();// << it2.value()->references;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#endif</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="p">}</span> <span style="color: #aa4000">else</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#ifndef NDEBUG</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #74777d">// qCDebug(LOG_PLASMA) << " " << it2.key() << rc << it2.value();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#endif</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#ifndef NDEBUG</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">foreach</span> <span class="p">(</span><span class="n">FrameSvg</span> <span style="color: #aa2211">*</span><span class="n">data</span><span class="p">,</span> <span class="n">it2</span><span class="p">.</span><span class="n">value</span><span class="p">()</span><span style="color: #aa2211">-></span><span class="n">references</span><span class="p">.</span><span class="n">keys</span><span class="p">())</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">qCDebug</span><span class="p">(</span><span class="n">LOG_PLASMA</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">" "</span> <span style="color: #aa2211"><<</span> <span class="p">(</span><span style="color: #aa4000">void</span> <span style="color: #aa2211">*</span><span class="p">)</span><span class="n">data</span> <span style="color: #aa2211"><<</span> <span class="n">it2</span><span class="p">.</span><span class="n">value</span><span class="p">()</span><span style="color: #aa2211">-></span><span class="n">references</span><span class="p">[</span><span class="n">data</span><span class="p">];</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#endif</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">shares</span> <span style="color: #aa2211">+=</span> <span class="n">rc</span> <span style="color: #aa2211">-</span> <span style="color: #601200">1</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#ifndef NDEBUG</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #74777d">// qCDebug(LOG_PLASMA) << "#####################################" << s_sharedFrames[theme()->d].count() << ", pixmaps saved:" << shares;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#endif</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #304a96">#endif</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">frame</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">nullptr</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">maskFrame</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">nullptr</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">{<span class="bright">}</span></span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p class="remarkup-literal">= default;</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D17052#inline-92827">View Inline</a><span style="color: #4b4d51; font-weight: bold;">framesvg.cpp:569</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="n">QRect</span> <span class="n">FrameSvgPrivate</span><span style="color: #aa2211">::</span><span class="n">contentGeometry</span><span class="p">(</span><span class="n">FrameData<span class="bright"></span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">*</span></span><span class="bright"> </span><span class="n">frame</span><span class="p">,</span> <span style="color: #aa4000">const</span> <span class="n">QSize</span><span style="color: #aa2211">&</span> <span class="n">size</span><span class="p">)</span> <span style="color: #aa4000">const</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="n">QRect</span> <span class="n">FrameSvgPrivate</span><span style="color: #aa2211">::</span><span class="n">contentGeometry</span><span class="p">(<span class="bright"></span></span><span class="bright"></span><span style="color: #aa4000"><span class="bright">const</span></span><span class="bright"> </span><span class="n"><span class="bright">QSharedPointer</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright"><</span></span><span class="n">FrameData<span class="bright"></span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">></span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">&</span></span><span class="n">frame</span><span class="p">,</span> <span style="color: #aa4000">const</span> <span class="n">QSize</span><span style="color: #aa2211">&</span> <span class="n">size</span><span class="p">)</span> <span style="color: #aa4000">const</span>
</div><div style="padding: 0 8px; margin: 0 4px; "><span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Do we really need to pass a shared pointer? Wouldn't it better to just pass a raw pointer? We won't do any ownership-related stuff anyway.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D17052#inline-92821">View Inline</a><span style="color: #4b4d51; font-weight: bold;">framesvg.cpp:618</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #74777d">// we've found a mat<span class="bright">h, so insert</span> that <span class="bright">new one and ref it ..</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">newFd<span class="bright"></span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">ref</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">q</span></span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #74777d">// we've found a mat<span class="bright">ch, use</span> that <span class="bright">one</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="bright"></span><span class="n"><span class="bright">Q_ASSERT</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">newKey</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">==</span></span><span class="bright"> </span><span class="n">newFd<span class="bright"></span></span><span class="bright"></span><span class="p"><span class="bright">.</span></span><span class="bright"></span><span class="n"><span class="bright">data</span></span><span class="bright"></span><span class="p"><span class="bright">()</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">cacheId</span></span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">frame</span> <span style="color: #aa2211">=</span> <span class="n">newFd</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #74777d">//.. then deref the old one and if it's no longer used, get rid of it</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">fd</span><span style="color: #aa2211">-></span><span class="n">deref</span><span class="p">(</span><span class="n">q</span><span class="p">))</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #74777d">//const QString oldKey = cacheId(fd, prefix);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #74777d">//qCDebug(LOG_PLASMA) << "1. Removing it" << oldKey << fd->refcount;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="n">FrameSvgPrivate</span><span style="color: #aa2211">::</span><span class="n">s_sharedFrames</span><span class="p">[</span><span class="n">fd</span><span style="color: #aa2211">-></span><span class="n">theme</span><span class="p">].</span><span class="n">remove</span><span class="p">(</span><span class="n">oldKey</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">delete</span> <span class="n">fd</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Just newFd->cacheId.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R242 Plasma Framework (Library)</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D17052">https://phabricator.kde.org/D17052</a></div></div><br /><div><strong>To: </strong>apol, Plasma, Frameworks<br /><strong>Cc: </strong>zzag, kde-frameworks-devel, michaelh, ngraham, bruns<br /></div>