> On May 30, 2016, 11:34 a.m., Hugo Pereira Da Costa wrote: > > Ship It!
err. Wait ... There are rendering issues here once the patch is applied. See http://wstaw.org/m/2016/05/30/plasma-desktopY12228.png (left is "before", right is "after"). So something seems wrong with the background gradient. I'll investigate a bit ... - Hugo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127866/#review96029 ----------------------------------------------------------- On May 22, 2016, 4:20 a.m., Michael Pyne wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127866/ > ----------------------------------------------------------- > > (Updated May 22, 2016, 4:20 a.m.) > > > Review request for kde-workspace and Hugo Pereira Da Costa. > > > Repository: oxygen > > > Description > ------- > > This should mostly complete the QCache fixes I kicked off in a previous RR, > 127837. Hugo noted there were many other similar usages, and boy he wasn't > kidding! ;) The long story short is that these usages can theoretically cause > use-after-free behavior (which can lead to crashes and even undefined > behavior if the compiler ever gets smart). > > *NOTE* It is -much- easier to review if you download the diff to your git > repository for oxygen and then run "git diff -b" to ignore whitespace > changes, particularly for the QPixmap changes. > > For QPixmaps we return values instead of pointers, so we simply make a > separate copy to be cached when we do insert. For QColor we return references > to values so we *must* return pointers, and those have to be owned by a > QCache to avoid memleaks. So I added a helper function to loop until the > cache accepts the new entry. TileSets are a similar concern, except those > have manual loops since I was uncertain about whether TileSet's copy > constructor was the best idea or not. > > This fixes a ton of Coverity issues (59717 - 259733, 259739, 259742 - 259752, > 1336154, 1336155) and might be associated with Qt bug 38142 and KDE bug > 219055 (which doesn't actually appear to be a dupe of a different bug to > me...). > > > Diffs > ----- > > kdecoration/oxygendecohelper.cpp aa75eca > kstyle/oxygenstyle.cpp e428606 > kstyle/oxygenstylehelper.h 9510a60 > kstyle/oxygenstylehelper.cpp 612ba37 > liboxygen/oxygenhelper.h a6453a0 > liboxygen/oxygenhelper.cpp 4843604 > liboxygen/oxygenshadowcache.cpp 907e586 > > Diff: https://git.reviewboard.kde.org/r/127866/diff/ > > > Testing > ------- > > Compiled without warnings, installed and ran `oxygen-demo5 -style oxygen`. > Used the GUI Benchmark feature to automatically cycle through all the listed > features -- no crashes or obvious rendering errors. > > > Thanks, > > Michael Pyne > >
