----------------------------------------------------------- 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 > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
