> On July 24, 2014, 10:55 a.m., David Edmundson wrote: > > src/declarativeimports/core/framesvgitem.cpp, line 45 > > <https://git.reviewboard.kde.org/r/119425/diff/2/?file=292260#file292260line45> > > > > Having spoken to you offline this does make sense, but it needs > > documentation as to why it's doing this. > > Marco Martin wrote: > uhm, why a filter on palette change? > this would work for themes that use system colors but not the other ones. > theme::themechanged should cover both cases (and if it doesn't, it should) > > Aleix Pol Gonzalez wrote: > See SvgPrivate::checkColorHints(). > > I agree it could make sense having it in plasma theme, but this should be > part of another patch. > > Marco Martin wrote: > it does a repaintneeded, that is correct. > but yeah, having it in theme doesn't make much sense, because technically > the theme didn't change and you can't know from there if one of the svgs > actually needs a repaint. > > so, what all of this suggests me is that the thing that would make most > sense is actually use repaintneeded, and either: > a) just remove from the hash the id of this texture (multiple removes > still possible tough) > b) event compress the clear() > c) both of the above > > Aleix Pol Gonzalez wrote: > I'm getting a bit lost, sorry. Why is listening to all svg's better? > RepaintNeeded will emit upon signals we don't need. Note that themeChanged > doesn't clean the cache and it will rather be the textures just stopping to > use the old theme, because the hash is refcounted. That's why I introduced > more elements to the hash key. > > David Edmundson wrote: > Calling clear on an empty hash is an atomic operation. Maybe it should > just go back to the version in revision 1. > It's much simpler and probably much faster than all this extra string > appending and monitoring. > > Marco Martin wrote: > yes. If we can assure that the following scenario will never happen, > better version number one > scenario as in > * clear > * put something back in > * clear again > * repeat for every single instance
I think we can ensure that, because the painting is carried out in a different thread, so it will end up in the next frame painting. - Aleix ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119425/#review63049 ----------------------------------------------------------- On July 24, 2014, 11:14 a.m., Aleix Pol Gonzalez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119425/ > ----------------------------------------------------------- > > (Updated July 24, 2014, 11:14 a.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 > >
_______________________________________________ Plasma-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/plasma-devel
