> On Feb. 20, 2014, 8:13 p.m., Martin Gräßlin wrote: > > src/declarativeimports/core/svgitem.cpp, line 132 > > <https://git.reviewboard.kde.org/r/115923/diff/1/?file=245131#file245131line132> > > > > I never really understood who takes ownership over the nodes: does one > > have to delete the oldNode?
It very much seems so. QQuickText does it for example if d->text is empty. Same for other QQuick classes. > On Feb. 20, 2014, 8:13 p.m., Martin Gräßlin wrote: > > src/declarativeimports/core/svgitem.cpp, line 45 > > <https://git.reviewboard.kde.org/r/115923/diff/1/?file=245131#file245131line45> > > > > are you sure that you can delete the texture here? Is the object being > > destroyed from the rendering thread? It's a good question, but I think it's OK. I would have thought they'd have some very strong guards against deleting a QQuickItem whilst they're still trying to use it. That would cause people other major problems. The texture inherits from QObject so I could call deleteLater() instead? Alternately I can subclass the node and delete the texture in the destructor of the node, which would be quite tidy. Hopefully in a week or so I'll be able to give you a far more concrete answer when I actually understand how this area works. > On Feb. 20, 2014, 8:13 p.m., Martin Gräßlin wrote: > > src/declarativeimports/core/svgitem.cpp, line 127 > > <https://git.reviewboard.kde.org/r/115923/diff/1/?file=245131#file245131line127> > > > > nitpick: looking at the surrounding coding style the * should be moved > > from type to name I'm really not sure which Martin is worse... /me grumbles and fixes it. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115923/#review50402 ----------------------------------------------------------- On Feb. 20, 2014, 7:22 p.m., David Edmundson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115923/ > ----------------------------------------------------------- > > (Updated Feb. 20, 2014, 7:22 p.m.) > > > Review request for Plasma. > > > Repository: plasma-framework > > > Description > ------- > > The rationale behind this patch is on the mailing list in the thread "Minutes > Monday" > > This doesn't boost performance or save memory much, but it paves the way for > texture sharing, faster resizing, and plenty of other things. > > Based on Frederick's comment I have reverted my changes to use QImage > everywhere, otherwise we lose out on the local QPixmap cache in KImageCache. > Changes to plasmacore are minimal. > > I'm currently porting FrameSVG which is where we should see more gains, but I > thought I should get this reviewed/merged in parallel. > > I have only seen one regression which is in the analog clock. > Some odd code in the analog clock (by me apparently!) means the width is > dependent on the current width, which due to some changes in this patch ends > up in a constant spiral getting to infinitely sized and explode. > > > Changelog (in reverse order): > Remove manual isDirty tracking in SvgItem > Always resize the node geometry on resizes > Update to paint to fill the size of the object, not the size of texture > Fix leaking texture > Add convenient QImage image() getter in SVG > Avoid repainting if node is not changed > Render SvgItem natively rather than going through QQuickPaintedItem > > > Diffs > ----- > > src/declarativeimports/core/svgitem.h c8be7cc > src/declarativeimports/core/svgitem.cpp e90751a > src/plasma/svg.h 01d98f8 > src/plasma/svg.cpp 9ec2aa5 > > Diff: https://git.reviewboard.kde.org/r/115923/diff/ > > > Testing > ------- > > > Thanks, > > David Edmundson > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
