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