Review Request 122584: Use shared pointers for markers.

[Inge Wallin]

> On Feb. 16, 2015, 11:11 a.m., Inge Wallin wrote:
> > Wouldn't a much better solution be to make the Private class of the KoMarker shared? If so, we could have the collection store KoMarkers instead of a collection of KoMarker*s. Making KoMarker itself a QSharedData feels very strange and is different than anything else in Calligra.
> 
> Sven Langkamp wrote:
>     That would not work. There is only one marker object in the marker collection and everything else just refers by pointers. Internal reference count would always be 1.
> 
> Inge Wallin wrote:
>     Yes, but then we wouldn't have to refer by pointer anymore. Actually using the objects themselves instead would be the way to do it. 
>     
>     I still say it feels very strange to make this class alone a QSharedObject and no other object in Flake or libs/odf (which I think is where this class actually belongs).
> 
> Boudewijn Rempt wrote:
>     KoMarker doesn't belong in libs/odf, it's something that paints. Which also means that KoMarkerCollection doesn't beloing in libs/odf. And QSharedData is used in another places in flake: KoClipData.

Ah, so *that's* the distinction between libs/odf and libs/flake. I never really saw the logical border between them.

But did you actually look at KoClipData?  It's just the exposed internal implementation of KoClipPath (which also belongs in libs/odf, btw, since it doesn't paint). And it strengthens my point since I propose that we make the internal class the shared object.


- Inge


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122584/#review76108
-----------------------------------------------------------


On Feb. 16, 2015, 2:26 a.m., Sven Langkamp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122584/
> -----------------------------------------------------------
> 
> (Updated Feb. 16, 2015, 2:26 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Bugs: 343719
>     http://bugs.kde.org/show_bug.cgi?id=343719
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> Use shared pointers for markers.
> Currently markers crash Krita on close (bug 343719). The problem is that existing path shapes point to the marker even after it's deleted. The patch solves that by using shared pointers for the marker.
> 
> 
> Diffs
> -----
> 
>   libs/flake/KoMarkerData.cpp 247e151 
>   libs/flake/KoMarker.h fa2adfd 
>   libs/flake/KoMarkerCollection.h 1555363 
>   libs/flake/KoMarkerCollection.cpp 98e80f8 
> 
> Diff: https://git.reviewboard.kde.org/r/122584/diff/
> 
> 
> Testing
> -------
> 
> Tested with Krita.
> 
> 
> Thanks,
> 
> Sven Langkamp
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20150216/c6b5c878/attachment.htm>