Review Request 119425: Cache the textures created for the fast path

[Marco Martin]

> On July 23, 2014, 2:38 p.m., David Edmundson wrote:
> > src/declarativeimports/core/framesvgitem.cpp, line 391
> > <https://git.reviewboard.kde.org/r/119425/diff/1/?file=292155#file292155line391>
> >
> >     this means every object performs a method on a singleton.
> >     
> >     Arguably following ones won't do anything, but it's not ideal.
> 
> Marco Martin wrote:
>     kindof calls for the singleton being an object, so it clears itself
> 
> Aleix Pol Gonzalez wrote:
>     I did it like this, because I thought it would be better to just clean the cache whenever FrameSvgItem is requested to rather than covering all the causes explicitly, but yes, I can add another object listening to themeChanged.
> 
> Aleix Pol Gonzalez wrote:
>     Well themeChanged, and colorSchemeChanged and when devicePixelRatio changes.
> 
> Marco Martin wrote:
>     just connect to repaintNeeded()
>     
>     that signal should get emitted in all those cases, and later, ensuring the images are aready updated (while if you connect to themechanged you are not assured you already have the new graphics, since framesvg is using that one to update itself, so depends on the order of slots called)
> 
> Aleix Pol Gonzalez wrote:
>     Isn't connecting to repaintNeeded() escentially the same as calling clear() many times? Or is there any singleton emitting that repaintNeeded?
>     
>     As far as I've seen, they all connect to the Plasma::Theme::themeChanged.

ah, right, true..
so themeChanged it is


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119425/#review62967
-----------------------------------------------------------


On July 23, 2014, 3:35 p.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119425/
> -----------------------------------------------------------
> 
> (Updated July 23, 2014, 3:35 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> Create a cache that has pointers to all the textures that we've generated, so in case we have one already created, we can re-use it.
> 
> 
> Diffs
> -----
> 
>   src/declarativeimports/core/framesvgitem.cpp 323b06b 
>   src/declarativeimports/core/iconitem.cpp 38012cc 
>   src/declarativeimports/core/svgitem.cpp eccff55 
>   src/declarativeimports/core/svgtexturenode.h 21b9b2f 
> 
> Diff: https://git.reviewboard.kde.org/r/119425/diff/
> 
> 
> Testing
> -------
> 
> see the qDebug (to be removed before commit). 
> 
> plasmashell 2> out
> $ grep s_cache out | grep ": miss" | wc -l
> 342
> $ grep s_cache out | grep ": hit" | wc -l
> 126
> 
> So still having 3 times more hits than miss, so there's big room for improvement. Good news is that with this, we get a ~25% of memory and bandwidth save, in a per-item basis.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140723/2a08355b/attachment.html>