[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: ***UNCHECKED*** DataCenterMap
From:       Thorsten Zachmann <t.zachmann () zagge ! de>
Date:       2010-01-19 20:51:02
Message-ID: 201001192151.03124.t.zachmann () zagge ! de
[Download RAW message or body]

Hello,

> The solution that I'm going to suggest is to have the pattern we have now
> for KoCanvasResourceProvider be reused for a document wide resources
> repository. The current solution of inheriting KoDataCenter doesn't add
> anything and it confuses as it mixes concepts that should really be seen
> as separate things.

I really like this. That solves a problem I already started to write a mail to 
the list on how to get access to the KoCanvasResourceProvider in the shape 
factory.

> I took a look at the places where classes inherit from KoDataCenter *and*
> where the virtual methods will actually be called. This is;
> KoImageCollection
> kpresenter/part/KPrSoundCollection.h
> plugins/videoshape/VideoCollection.h
> Even found a class that has the completeSaving/completeLoading implemented
> but they will never be called;
> kspread/Map.h
> 
> For these classes the concept of dataCenters works and we should keep it.
> For all the others (something like 10 or so) the class just needs to be
> document- shared. The dataCenter concept is irrelevant for them.
> 
> I think the API needs to be improved to make the above gotcha clear;
> dataCenters are great, but they should have nothing to do with the
> dataCenterMap. A dataCenter is 'activated' by calling
> context.addDataCenter(). The two problems that should have different
> solutions.  The mixing we have now is confusing at best.
> 
> So, here is my patchset that essentially does this.
> If you apply these patches you might just want to do something like;
> git show [sha1] libs/flake
> which gives you the patch limited to that directory so you can easier check
> the differences in headers.
> 
Here are my first observations:
I saw the following compiler warning 
/home/tz/develop/kde/svn/koffice-
datacenter/plugins/pictureshape/PictureShape.h:59: warning: 
'PictureShape::m_renderQueue' will be initialized after                               \
                
/home/tz/develop/kde/svn/koffice-
datacenter/plugins/pictureshape/PictureShape.h:58: warning:   
'KoImageCollection* PictureShape::m_imageCollection'                                  \
                
/home/tz/develop/kde/svn/koffice-
datacenter/plugins/pictureshape/PictureShape.cpp:62: warning:   when 
initialized here                    

The enhanced path shapes (funny/arrow category in the add shape docker) now 
all result in an cross when added to the canvas. Looks like this is the 
problem:

KoShape *EnhancedPathShapeFactory::createShape(const KoProperties *params, 
const QMap<QString, KoDataCenter *> &, KoResourceManager *) const

-    KoStyleManager * styleManager = dynamic_cast<KoStyleManager *>(kwDoc()-
> dataCenterMap()["StyleManager"]);
+    KoStyleManager *styleManager = static_cast<KoStyleManager *>(kwDoc()-
> resourceManager()->resource(KoText::StyleManager).value<void*>(

What is the reason to use a static cast instead of a dynamic one?

+        KoResourceManager *resourceprovider = canvas->resourceManager();

How about renaming the variable also to say resourceManger and no longer 
resourceprovice?

I only had a look during the my trainride home. More to come when I find more 
time to look at it.

Thorsten

Thorsten
_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel


[prev in list] [next in list] [prev in thread] [next in thread]