Review Request: implement deferred plugin loading for shape plugins

Boudewijn Rempt boud at valdyas.org
Tue Apr 26 13:16:29 BST 2011 


> On April 23, 2011, 4:04 a.m., Thorsten Zachmann wrote:
> > 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.

spreadsheetshape is the stub plugin, spreadsheetshape-deferred the real plugin. And when I run lsof on my stage process, libcalligratablesodf.so and libcalligratablescommon.so are not loaded. So I think you actually tested with an old, non-stub plugin.

I've fixed the code for the other remarks and restored the text shape to its old situation for now. Since there aren't any major issues, is it ok to merge or do you want an updated diff?


> On April 23, 2011, 4:04 a.m., Thorsten Zachmann wrote:
> > libs/flake/KoShapeFactoryBase.cpp, lines 72-74
> > <http://git.reviewboard.kde.org/r/101111/diff/1/?file=14279#file14279line72>
> >
> >     How about adding the deferredPluginName to the constructor of the Plugin.

I can do that, but in general, I don't like long lists of parameters. For a Private class (I assume you mean that), it doesn't matter much though.


> On April 23, 2011, 4:04 a.m., Thorsten Zachmann wrote:
> > libs/flake/KoShapeFactoryBase.cpp, line 223
> > <http://git.reviewboard.kde.org/r/101111/diff/1/?file=14279#file14279line223>
> >
> >     should there be a warning in case the deferred plugin is not found?

There is a warning on loading the deferred plugin. I don't think it should warn everywhere where the plugin is used.


> On April 23, 2011, 4:04 a.m., Thorsten Zachmann wrote:
> > plugins/CMakeLists.txt, line 38
> > <http://git.reviewboard.kde.org/r/101111/diff/1/?file=14293#file14293line38>
> >
> >     Please commit it it like that.

"don't" commit it, right?


> On April 23, 2011, 4:04 a.m., Thorsten Zachmann wrote:
> > tables/shape/TableShapeFactory.cpp, line 55
> > <http://git.reviewboard.kde.org/r/101111/diff/1/?file=14308#file14308line55>
> >
> >     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.

No, this plugin isn't the deferred plugin, it's the stub plugin. I didn't rename it because it isn't a generic table component.


- Boudewijn


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


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/20110426/1c1a1595/attachment.htm>

More information about the calligra-devel mailing list