Review Request: Fix regression in size of shape groups with a lot of shapes

Jan Hambrecht jaham at gmx.net
Thu Sep 22 21:55:52 BST 2011


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102645/#review6744
-----------------------------------------------------------

Ship it!


All in all I think you can commit. However while looking at the code I had another idea. If the calculation of the size and the sizeCached member is moved to group shape model, we do not have to expose the invalidateCacheSize function at all. And you would not need the shape group private anymore. But this is just cosmetics.


libs/flake/KoShapeContainerModel.cpp
<http://git.reviewboard.kde.org/r/102645/#comment5954>

    I am not sure about that. Probably best to check with Caspar if it is ok with him.



libs/flake/KoShapeGroup.h
<http://git.reviewboard.kde.org/r/102645/#comment5955>

    Could you please add some documentation here?


- Jan


On Sept. 22, 2011, 2:52 a.m., Thorsten Zachmann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102645/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2011, 2:52 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Summary
> -------
> 
> With the fix for the group size now every time the function is called it calculates the size from its children.
> 
> When testing this with cstester I found problmes in the documents I uploaded to.
> 
> http://www.zagge.de/anchor/groupsize/
> 
> The problem there is that due the very huge amount of shapes in the group it takes now quite a very very long time to render the pages.
> 
> The patch fixes that problem by adding a cache to the group size and only recalculates the size when there was a change.
> 
> It additionally don't forward the ChildChanged when a CollisionDetection is encountered. The CollisonChanged is not needed to forward to any parent as if the results in a change this change will be probagated.
> 
> The patch as it is now is ready for inclusion. The comments provided have been implemented.
> 
> Looking forward to your feedback.
> 
> 
> Diffs
> -----
> 
>   libs/flake/KoShapeContainerModel.cpp 7995236 
>   libs/flake/KoShapeGroup.h a53d2ad 
>   libs/flake/KoShapeGroup.cpp 409b18c 
>   libs/flake/KoShape_p.h d055056 
> 
> Diff: http://git.reviewboard.kde.org/r/102645/diff
> 
> 
> Testing
> -------
> 
> Before the patch switching between slides in the www.renaissance.co.il%2Fivbug%2Fmeeting87%2FArchitecture%20and%20design%20patterns.ppt file takes more then 10 seconds, with the patch it is instantly.
> 
> Tested resizing of groups of groups, resizing/modifying of shapes in groups. 
> 
> 
> Thanks,
> 
> Thorsten
> 
>

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


More information about the calligra-devel mailing list