----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119336/#review62619 -----------------------------------------------------------
src/declarativeimports/core/framesvgitem.cpp <https://git.reviewboard.kde.org/r/119336/#comment43416> 1) This leaks. if you remove a node from a parent you have to delete it yourself. 2) You're calling this from something which is looping through childCount(),so you'll end up either crashing or skipping items. - David Edmundson On July 17, 2014, 8:17 p.m., Marco Martin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119336/ > ----------------------------------------------------------- > > (Updated July 17, 2014, 8:17 p.m.) > > > Review request for KDE Frameworks and Plasma. > > > Repository: plasma-framework > > > Description > ------- > > This is a derivative of https://git.reviewboard.kde.org/r/119330/ > > It has a much simpler codebase and it doesn't touch framesvg in the library > (and doesn't make things public) > > the important differences are two, that are imo 100% necessary to maintain a > pixel perfect rendering (sacrificing that is a regression simply not > acceptable in any case, even for non default themes, ever). At the same time > avoids duplication of framesvg code in framesvgitem. > > potential issues: > * e037203748 support for tiling still need porting > * yes, it uses a qpainter, that means another copy: but again is necessary as > framesvg knows how to render the end result pixel perfect, since for many > themes the end piece is *not* the simple rendered element id. > * yes, the final scaled texture is still uploaded as a whole, it only avoids > to do it too often (like in animations) with event compression, again, only > way to be sure it's correctly rendered all the time. > > The latter two points can have the following optimizations: > Iff the frame does not have an overlay and does not have composeoverborders > set, the following can be done: > * use Svg::image() to fetch the pixmap of the piece, since in that case would > be valid > * resize the framesvg one single time, at a fixed size , like something > > 256x256 (256 is not random thing, since we have 8 bits per channel, usually > gradients will have no more than 256 stops) and disable the resize timer, > this way we are even sure that only one image per prefix will be stored in > cache > > I should add regarding the last two optimization points: i would like to see > some real benchmarks about them, or i would not consider then necessary until > then. > > > Diffs > ----- > > tests/dialog.qml PRE-CREATION > tests/testborders.qml PRE-CREATION > examples/applets/widgetgallery/contents/ui/Buttons.qml 379585f > examples/applets/widgetgallery/contents/ui/Menu.qml 1336c42 > examples/applets/widgetgallery/contents/ui/standalonemain.qml PRE-CREATION > src/declarativeimports/core/framesvgitem.h e155f6a > src/declarativeimports/core/framesvgitem.cpp 8320212 > src/declarativeimports/core/svgitem.cpp 1ed0631 > > Diff: https://git.reviewboard.kde.org/r/119336/diff/ > > > Testing > ------- > > > Thanks, > > Marco Martin > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
