Review Request: GSoC: Add support of Anim:Formula tag in Stage
Thorsten Zachmann
t.zachmann at zagge.de
Sat May 26 06:26:23 BST 2012
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105030/#review14167
-----------------------------------------------------------
The new class KPrFormulaParser and the class KPrFormulaParser are nearly the same. I think they should be merged into one class. If needed to have a little bit different functionality (without the new one you added) maybe a enum giving the type of parser can be added to differenciate them. It will reduce the code quite a bit.
When using the same parser you might need only one member in the KPrSmilValues holding the data. (We can discuss per mail if you need more information.)
stage/part/animations/KPrAnimate.cpp
<http://git.reviewboard.kde.org/r/105030/#comment11204>
Maybe remove the empty vaiable again and just pass a QString() to the method.
stage/part/animations/strategy/KPrFormulaParser.h
<http://git.reviewboard.kde.org/r/105030/#comment11206>
As this function does not modify any members it can be made const.
stage/part/animations/strategy/KPrFormulaParser.h
<http://git.reviewboard.kde.org/r/105030/#comment11205>
Having this function const seems to be wrong as it is setting the values. Can the mutable below be removed when the const for this function is removed?
stage/part/animations/strategy/KPrSmilValues.h
<http://git.reviewboard.kde.org/r/105030/#comment11208>
There can be only one KPrFormulaParser. Please remove the overhead of using a QList here.
- Thorsten Zachmann
On May 24, 2012, 3:55 p.m., Paul Mendez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105030/
> -----------------------------------------------------------
>
> (Updated May 24, 2012, 3:55 p.m.)
>
>
> Review request for Calligra.
>
>
> Description
> -------
>
> Add support for Anim:Formula tag in Stage. (That key is part of ODF animations specification).
>
> Note: animations tested don't run as smooth as in LibreOffice because KeySplines tag is not implemented.
>
>
> Diffs
> -----
>
> stage/part/CMakeLists.txt 3c7916ef7496af21e65d9a5441d5cb924829c347
> stage/part/animations/KPrAnimate.cpp dddd1fa401d596e7e23688f950428cd0ea76b639
> stage/part/animations/strategy/KPrFormulaParser.h PRE-CREATION
> stage/part/animations/strategy/KPrFormulaParser.cpp PRE-CREATION
> stage/part/animations/strategy/KPrSmilValues.h 163d78b830a151ce150192000890a395f9e273dd
> stage/part/animations/strategy/KPrSmilValues.cpp 3faafc4eb1c8783224f9f32c38106407cc219096
> stage/part/animations/strategy/KPrValueParser.h 8f3c6ebcdf7ae9f5d938d97d518f21977b572940
>
> Diff: http://git.reviewboard.kde.org/r/105030/diff/
>
>
> Testing
> -------
>
> Test some animations of documents created in Libre Office (One test document is also uploaded)
>
>
> Thanks,
>
> Paul Mendez
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120526/64067455/attachment.htm>
More information about the calligra-devel
mailing list