Review Request: Add support for dr3d:scene and its children

Thorsten Zachmann t.zachmann at zagge.de
Mon Jun 25 06:10:24 BST 2012



> On June 19, 2012, 4:53 a.m., Thorsten Zachmann wrote:
> > plugins/staging/threedshape/Objects.h, lines 47-48
> > <http://git.reviewboard.kde.org/r/105292/diff/1/?file=69757#file69757line47>
> >
> >     Please move the implementation to the cpp file.
> 
> Inge Wallin wrote:
>     This code is only used inside the 3D shape and nowhere else.  Binary compatibility/incompatibility does not apply.  I think this policy is only used for libraries which are supposed to be binary compatible for a long time.

I think there is no good reason to make this code inline. And the coding standard says all code should be in the cpp file, so please move it to the cpp file. Makes debugging and such things much easier.


> On June 19, 2012, 4:53 a.m., Thorsten Zachmann wrote:
> > plugins/staging/threedshape/Objects.h, line 35
> > <http://git.reviewboard.kde.org/r/105292/diff/1/?file=69757#file69757line35>
> >
> >     Each class should be put into its own cpp file to follow the coding standard.
> 
> Inge Wallin wrote:
>     I'm not sure this applies to non-library code and trivial classes.  But if you insist, I will do it before I merge.

Yes please.


- Thorsten


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


On June 25, 2012, 4:52 a.m., Inge Wallin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105292/
> -----------------------------------------------------------
> 
> (Updated June 25, 2012, 4:52 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> This patch adds rudimentary support for the dr3d:scene element and its children by introducing a 3D shape. The 3D support in ODF is pretty simple: it just defines 4 different object types: cube, sphere, extrude and rotate as well as a scene element that acts a bit like a group (draw:g).
> 
> I implemented all the object types as KoShapes because they can have styling and the scene object is also a KoShapeContainer. None of the shapes except for the scene itself can be modified now, and for the scene it's only the standard shape parameters (size, position, transform, etc).
> 
> My plan is to work in 3 stages:
> 1. Loading and saving - this prevents data loss
> 2. Rendering - this implements viewing
> 3. Scene and object editing
> This patch implements stage 1 only.
> 
> My goal was to load dr3d:scenes and save them back with full compatibility with OOo and LO. Unfortunately I didn't manage to do that yet. 
> 
> I have tested with Karbon during the development and it loads and savs the scenes nicely. But calling KoShape::loadOdfAttributes(..., OdfAllAttributes) in karbon still loses the layer information. This makes OOo/LO not show the shapes when they are loaded again after a roundtrip through Karbon. I have not been able to analyze why Karbon saves back all shapes with layer="" when the infile clearly has layer="layout".
> 
> And if I want to make it work in Words and presumably also Stage (and Sheets?) I have to integrate it with the KoInlineObjectRegistry in kotext, something that I have also not yet managed to do. Help would be appreciated here.
> 
> But until Karbon is fixed and Words integration is done, I'm happy to receive and integrate feedback here.
> 
> 
> Diffs
> -----
> 
>   plugins/staging/CMakeLists.txt f55b316 
>   plugins/staging/threedshape/CMakeLists.txt PRE-CREATION 
>   plugins/staging/threedshape/Messages.sh PRE-CREATION 
>   plugins/staging/threedshape/Object3D.h PRE-CREATION 
>   plugins/staging/threedshape/Object3D.cpp PRE-CREATION 
>   plugins/staging/threedshape/Objects.h PRE-CREATION 
>   plugins/staging/threedshape/Objects.cpp PRE-CREATION 
>   plugins/staging/threedshape/PLAN PRE-CREATION 
>   plugins/staging/threedshape/SceneObject.h PRE-CREATION 
>   plugins/staging/threedshape/SceneObject.cpp PRE-CREATION 
>   plugins/staging/threedshape/ThreedShapeFactory.h PRE-CREATION 
>   plugins/staging/threedshape/ThreedShapeFactory.cpp PRE-CREATION 
>   plugins/staging/threedshape/ThreedShapePlugin.h PRE-CREATION 
>   plugins/staging/threedshape/ThreedShapePlugin.cpp PRE-CREATION 
>   plugins/staging/threedshape/threedshape.desktop PRE-CREATION 
>   plugins/staging/threedshape/utils.h PRE-CREATION 
>   plugins/staging/threedshape/utils.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/105292/diff/
> 
> 
> Testing
> -------
> 
> Tested with a simple ODG file containing 3D objects.
> 
> 
> Thanks,
> 
> Inge Wallin
> 
>

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


More information about the calligra-devel mailing list