Review Request 120572: Unbreak Stage: remove duplicated loading of Flake plugins from KoToolRegistry

Boudewijn Rempt boud at valdyas.org
Wed Oct 15 17:22:26 BST 2014


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


Honestly, I have no idea how stuff got this convoluted in flake. It looks like I added those lines, but the git blame is just because of the koffice/calligra rename... And I cannot dig far enough back to figure it out. In any case, if everything still seems to work for you, go ahead and push.

- Boudewijn Rempt


On Oct. 13, 2014, 5:13 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120572/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2014, 5:13 p.m.)
> 
> 
> Review request for Calligra, Boudewijn Rempt and Thorsten Zachmann.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> If you load a file into Stage from current master, you see a lot of "Unknown shapes" on the screen (possibly mapping to some question marks in your head). Seems the plugins for Text shapes and Vector shapes are not available. I tried to find an answer to those questionmarks, and here is my story:
> 
> In Stage these days the singleton KoToolRegistry is created before the KoShapeRegistry one. So the init() method of it starts to load all "Flake" plugins and create their respective plugin objects, of which the first is (for me) the Formula Shape. Its constructor now has
> ```c++
>    KoToolRegistry::instance()->add( new KoFormulaToolFactory() );
> #ifndef _MSC_VER
>     KoToolRegistry::instance()->add( new KoM2MMLFormulaToolFactory());
> #endif
>     KoShapeRegistry::instance()->add( new KoFormulaShapeFactory() );
> ```
> So first gets the KoToolRegistry instance (where we are still in the constructor code actually, doooh) and adds 1-2 factories there, and then... calls for the singleton of KoShapeRegistry. That one does not yet exist, so one is created and in its constructor, yes, goes to load Flake and Shape plugins. Just, when going for the Flake plugins, KoPluginLoader will no longer deliver any Flake plugins to KoShapeRegistry, because
> ```c++
>     // Don't load the same plugins again
>     if (d->loadedServiceTypes.contains(serviceType)) {
>         return;
>     }
> ```
> and in the still ongoing call from KoToolRegistry to `KoPluginLoader::instance()->load(...)` for Flakes the servicetype was registered as first step.
> 
> So when KoShapeRegistry's init method comes to `insertFactory(factories[i]);` it will miss out all Flake plugins, because none yet could register themselves to KoShapeRegistry, as the first trying to reach the singleton of KoShapeRegistry blocked the loading of Flakes until that singleton was done. And when it is done, it has already tried to learn about the factories.
> 
> Somehow this all calls for a redesign. But for now the attached patch, which only loads Flakes in KoShapeRegistry's init, seems to work for me.
> 
> By the commit log it seems like this duplication to KoShapeRegistry which also loads "Flake"s was there forever. So no idea if that is needed perhaps? But I could not see any problem on a quick testing of all apps.
> 
> 
> Diffs
> -----
> 
>   libs/flake/KoToolRegistry.cpp bbb9681 
> 
> Diff: https://git.reviewboard.kde.org/r/120572/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

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


More information about the calligra-devel mailing list