Review Request: implement deferred plugin loading for shape plugins

Sat Apr 23 05:04:46 BST 2011

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

When starting stage I can see that it still loads the spreadsheetshape even for an empty document.

Looks like this is not working as wanted.

lsof gives me:

kpresente 3920 ko2t  mem    REG                8,5 20088594 3949070 /opt/ko2o/lib/libcalligratablesodf.so.8.0.0
kpresente 3920 ko2t  mem    REG                8,5 23856453 3949071 /opt/ko2o/lib/libcalligratablescommon.so.8.0.0
kpresente 3920 ko2t  mem    REG                8,5  1485240 3949076 /opt/ko2o/lib/kde4/spreadsheetshape.so

LD_DEGUG=files shows that the plugin is already loaded before even a document is loaded.

libs/flake/KoShapeFactoryBase.cpp
<http://git.reviewboard.kde.org/r/101111/#comment2460>

    Can the variable be moved next to each other as they belong to each other.

libs/flake/KoShapeFactoryBase.cpp
<http://git.reviewboard.kde.org/r/101111/#comment2461>

    How about adding the deferredPluginName to the constructor of the Plugin.

libs/flake/KoShapeFactoryBase.cpp
<http://git.reviewboard.kde.org/r/101111/#comment2462>

    should there be a warning in case the deferred plugin is not found?

libs/flake/KoToolRegistry.h
<http://git.reviewboard.kde.org/r/101111/#comment2463>

    Please remove the line

libs/flake/KoToolRegistry.cpp
<http://git.reviewboard.kde.org/r/101111/#comment2459>

    The qDebug should be replaced by a kDebug

libs/main/KoToolBoxLayout_p.h
<http://git.reviewboard.kde.org/r/101111/#comment2465>

    Should the class be renamed to match the filename?

plugins/CMakeLists.txt
<http://git.reviewboard.kde.org/r/101111/#comment2474>

    Please commit it it like that.

tables/shape/TableShapeFactory.cpp
<http://git.reviewboard.kde.org/r/101111/#comment2475>

    Should the plugin not be named TableShapeDeferred to match more the name of the TableShape plugin instead of a very different name. If the name already says deferred in it I think the stuff is clearer.

- Thorsten

On April 13, 2011, 1:02 p.m., Boudewijn Rempt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101111/
> -----------------------------------------------------------
> 
> (Updated April 13, 2011, 1:02 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Summary
> -------
> 
> This patch implements three things:
> 
> * deferred plugin loading for shape plugins. Shapes (even with tools) can now be loaded in two stages: a stub factory which defers to the second, real plugin. The advantage is that we do not load all libraries of all Calligra applications when starting a calligra application. The disadvantage is slightly more complexity. For unported plugins, nothing changes at all.
> * dynamic tool insertion. Until now, tool factories were loaded at at startup and tools were created on canvas creation. Now it is possible to dynamically add a tool factory. Doing so will create a new tool for all canvases.
> * split the spreadsheet shape plugin as an example.
> 
> The text shape plugin is next, in this patch it's still disabled.
> 
> 
> Diffs
> -----
> 
>   libs/flake/KoDeferredShapeFactoryBase.h PRE-CREATION 
>   libs/flake/CMakeLists.txt e4a16f3a34b5e4645a0bb60898926ccc0f7bfb0d 
>   krita/plugins/assistants/RulerAssistant/SplineAssistant.cc 7c212d94379a73d68255109e55e7641f4d3a33de 
>   krita/image/tiles3/kis_tile_data.h d00a4afc11952c3df16abe5ff89f85be92dfa4d5 
>   krita/plugins/assistants/RulerAssistant/PerspectiveAssistant.cc f49842c90fc712165520e588fd26c71126795ec5 
>   libs/flake/KoDeferredShapeFactoryBase.cpp PRE-CREATION 
>   libs/flake/KoShapeFactoryBase.h 8ff17e4d27a4c87db280bdbfa484c81dafffe66d 
>   libs/flake/KoShapeFactoryBase.cpp 536b0baef75cc3de5073ae90cbcb9eedeabae0fe 
>   libs/flake/KoToolManager.h 00333248644cf4a1021d55623c5061a580229ad6 
>   libs/flake/KoToolManager.cpp 3b6554a41dc4910b238473477b59f02b7c0c52de 
>   libs/flake/KoToolRegistry.h bcdb5de89e089e850e7d93eb6fd95a42e5b3b110 
>   libs/flake/KoToolRegistry.cpp f83072db104144035c350b6056bdc02081e1fb25 
>   libs/main/CMakeLists.txt a7f3077db8db5be3cf9a77f77e87c09fc84f17a0 
>   libs/main/KoToolBox.cpp 092920b3f1b5a25f63df81125df2d3565bb63b46 
>   libs/main/KoToolBoxDocker.cpp PRE-CREATION 
>   libs/main/KoToolBoxDocker_p.h PRE-CREATION 
>   libs/main/KoToolBoxFactory.h 8269759d5ccdf728007a6e613ce18feeda527c20 
>   libs/main/KoToolBoxFactory.cpp 4af151dcdfe373ae2697f65940b223d811ea8f53 
>   libs/main/KoToolBoxLayout_p.h PRE-CREATION 
>   libs/main/KoToolBox_p.h 81e8178a8f62b40f5a6bb4201b45e34bd4a95021 
>   libs/main/KoToolDocker_p.h 3f965dd29523b73fcf1f7b2133366c8e514db627 
>   plugins/CMakeLists.txt 6f3b538850909c6bb34be7845531582f46df125c 
>   plugins/artistictextshape/MoveStartOffsetStrategy.cpp 6a47031ddd0dd33b89897cfcde6df67bf4865787 
>   plugins/textshape/CMakeLists.txt 3eb805bee8e4b81faae4b767380b65770d4ce52a 
>   plugins/textshape/TextShapeDeferredFactory.h PRE-CREATION 
>   plugins/textshape/TextShapeDeferredFactory.cpp PRE-CREATION 
>   plugins/textshape/TextShapeFactory.h 91202a01534cfe93fff4abaf573ec32b009335c2 
>   plugins/textshape/TextShapeFactory.cpp d65c2c87f1441c672a0f3378d6e170d79cd05ae7 
>   plugins/textshape/textshape-deferred.desktop PRE-CREATION 
>   servicetypes/CMakeLists.txt 3db70a8fc61b638b33cf4c7bca3cc3b5d30a88cd 
>   servicetypes/README.txt PRE-CREATION 
>   servicetypes/calligra_deferred_plugin.desktop PRE-CREATION 
>   tables/shape/CMakeLists.txt 07a2109a2b1b1d50b06c51c94b6f7c39162419f5 
>   tables/shape/TableShapeDeferredFactory.h PRE-CREATION 
>   tables/shape/TableShapeDeferredFactory.cpp PRE-CREATION 
>   tables/shape/TableShapeFactory.h effadbae553233c55cf1c2bd1142973e87010504 
>   tables/shape/TableShapeFactory.cpp a62c48acac2c1ef8c027e370107bcb58bf9c8fbf 
>   tables/shape/spreadsheetshape-deferred.desktop PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/101111/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Boudewijn
> 
>

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