----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104688/#review12773 -----------------------------------------------------------
I think the patch should also include the parts to use the class so we can see the used API is actually useful for the aim you are trying to archive. libs/odf/Ko3dScene.h <http://git.reviewboard.kde.org/r/104688/#comment9983> You should pass the KoShapeLoadingContext and the KoShapeSavingContext to this methods so in case it is needed later we don't need to change the signature. That is true for all those methods. libs/odf/Ko3dScene.h <http://git.reviewboard.kde.org/r/104688/#comment9984> Please move the implementation to the cpp file. Please remove the additional spaces. libs/odf/Ko3dScene.h <http://git.reviewboard.kde.org/r/104688/#comment9985> This will break binary compatibility later. I think we should do the right stuff for new classes. libs/odf/Ko3dScene.h <http://git.reviewboard.kde.org/r/104688/#comment9987> As this is not a shape I'm not sure it makes sense to split those into two different methods. libs/odf/Ko3dScene.h <http://git.reviewboard.kde.org/r/104688/#comment9986> Implementation should be moved to cpp file libs/odf/Ko3dScene.cpp <http://git.reviewboard.kde.org/r/104688/#comment9988> Please don't give a default argument of "". There is already a default value of an empty QString so need to pass an empty string that needs to be converted to a QString. That is true for more place in the code. libs/odf/Ko3dScene.cpp <http://git.reviewboard.kde.org/r/104688/#comment9989> The save method of the Ko3dScene should also save the tag drd3:screen tag. - Thorsten Zachmann On April 21, 2012, 12:39 p.m., Inge Wallin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/104688/ > ----------------------------------------------------------- > > (Updated April 21, 2012, 12:39 p.m.) > > > Review request for Calligra. > > > Description > ------- > > This new code adds support for 3D scenes in libs/odf. It was ported from my > threedscene branch when I found out that exactly the same attributes are used > in 3D charts. > > The only tricky thing with this code is that the attributes are not stored in > a style but in the actual elements (dr3d:scene and chart:plot-area > respectively). This means that we have to save the attributes and the > children separately since the saving code of these objects may want to add > other attributes *and* children. > > You may note that I didn't d-pointer-ify the class. Right now the odf > library is not binary compatibility frozen so I don't strictly need to but if > there is a strong wish for it I can do it. > > > Diffs > ----- > > libs/odf/CMakeLists.txt c411a8c > libs/odf/Ko3dScene.h PRE-CREATION > libs/odf/Ko3dScene.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/104688/diff/ > > > Testing > ------- > > I have tested this code while working on the 3D Scene shape. > > > Thanks, > > Inge Wallin > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
