> On March 29, 2011, 5:06 p.m., Dario Freddi wrote: > > account-button.cpp, lines 102-104 > > <http://git.reviewboard.kde.org/r/100838/diff/4/?file=12726#file12726line102> > > > > It is not very clear to me what you are doing here, but this routine > > can be extremely slow. Can you please tell me what you are trying to > > achieve? > > > > I also see there is more than one occurrence of that, so this should > > indeed optimized if possible. > > Martin Klapetek wrote: > This paints a small overlay over the account icon. It paints the presence > icon (online, offline etc) and I think this is a bit of the original code. > What would be a better way to achieve painting a small emblem over an icon? > > Dario Freddi wrote: > There is a custom KIcon constructor (please see > http://api.kde.org/4.x-api/kdelibs-apidocs/kdeui/html/classKIcon.html#a6057e88b03b32ca70af2da295d626408 > ) which does the dirty job itself when it comes to overlays - it is surely > not much more optimized than this, but when QIcon will support overlays > natively, we'll get the speed bump for free. I advise you to switch to this > method, it will also give more clarity to the code. > > Dario Freddi wrote: > However, I also see that you are trying to create a big overlay, so the > method I suggested might not work (try it anyway though). A possible way to > optimize this routine then can be storing the pixmap (or even the painter) > somewhere to prevent at least one painting cycle to be performed each time > the overlay is painted - this way the same painter will be used to paint all > the icons, which is not really the most lenghty part, but it's still > something. > > Dario Freddi wrote: > Last time I am spamming you: thinking about it twice, that would do. > > class ... { > ... > private: > QPixmap m_errorPixmap; > } > > // > > Constructor::Constructor(Bla *bla) { > > // Load icons and stuff... > m_errorPixmap = KIconLoader::global->loadIcon("dialog-error", > KIconLoader::NoGroup, 16); > > // More stuff > } > > Class::yourMethod() { > // Your routine, which becomes: > QPixmap pixmap = icon().pixmap(32, 32); > QPainter painter(&pixmap); > painter->drawPixmap(15, 15, 16, 16); > } > > Unfortunately we can cache only the overlayed pixmap - the other one will > need to be loaded and drawn every time. If the routine is used VERY > frequently, I advise you to load the pixmap through KIconLoader instead of > getting it straight from the icon and see how it goes - IIRC, KIconLoader > performs slightly better than straight KIcon conversion when talking about > pixmap, but this is really dependent on how many times you are calling such a > routine and how it is critical for it to be fast. Surely such a modification > should give you a good speed bump: now you are loading one pixmap less per > cycle, which is quite an expensive operation. Ideally, you should never load > pixmaps in the drawing phase, but it looks like you can't avoid loading at > least one in this case.
I obviously fail at copying&pasting: of course I meant painter->drawPixmap(15, 15, m_errorPixmap); - Dario ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100838/#review2218 ----------------------------------------------------------- On March 23, 2011, 9:32 p.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100838/ > ----------------------------------------------------------- > > (Updated March 23, 2011, 9:32 p.m.) > > > Review request for Telepathy. > > > Summary > ------- > > This is the work that begun in my clone repo and has matured enough now to be > merged back to 'upstream', where the work should continue now. > > > Diffs > ----- > > CMakeLists.txt 2c62ea10556cdba38d1bca0fe63603df97414336 > account-button.h PRE-CREATION > account-button.cpp PRE-CREATION > account-filter-model.h PRE-CREATION > account-filter-model.cpp PRE-CREATION > accounts-model-item.h PRE-CREATION > accounts-model-item.cpp PRE-CREATION > accounts-model.h PRE-CREATION > accounts-model.cpp PRE-CREATION > contact-delegate-overlay.h PRE-CREATION > contact-delegate-overlay.cpp PRE-CREATION > contact-delegate.h PRE-CREATION > contact-delegate.cpp PRE-CREATION > contact-model-item.h PRE-CREATION > contact-model-item.cpp PRE-CREATION > contact-overlays.h PRE-CREATION > contact-overlays.cpp PRE-CREATION > contact-view-hover-button.h PRE-CREATION > contact-view-hover-button.cpp PRE-CREATION > filter-bar.h PRE-CREATION > filter-bar.cpp PRE-CREATION > main-widget.h aed6f7c8336321bc8a6ffb3b9af6b1d493f85790 > main-widget.cpp 6146b62eec17b54be63b594200613931673ac5fe > main-widget.ui 5b0d5aaaf3a4e2f49eb030b98b15fcae3a86170c > main.cpp bba0e4175e8853afe603f26ea4707f4974192d0f > ontologies/CMakeLists.txt 3e0bbe8c634908f689dcd360409aeddcc6fc0d23 > tree-node.h PRE-CREATION > tree-node.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/100838/diff > > > Testing > ------- > > We did a lot of thourough testing on #kde-telepathy, also some people emailed > their reports which all has been fixed. > > > Thanks, > > Martin > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
