----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100809/#review1876 -----------------------------------------------------------
The following things I noted when testing the patch. When I first clip a shape and then resize it somehow does not work correctly. Looks like the clipped area is not correct and therefore I can't see anything anymore. If I then undo the clipping and create a new command then the resizing works as expected. If I clip an object then resize it and then unclip the object I would expect the clipping object to not restore the size it had before it was used for clipping. When you clip an object with a rotated object then move it and unclip the rotated object jumps its position. I also noticed that the code style is not always followed. I marked some of the issues I have seen. It would be nice if you can look over the stuff that goes to libs once more to fix these issues. filters/karbon/svg/SvgParser.h <http://git.reviewboard.kde.org/r/100809/#comment1536> There is additional space not needed. filters/karbon/svg/SvgParser.cpp <http://git.reviewboard.kde.org/r/100809/#comment1537> Looks like the comment is wrong. filters/karbon/svg/SvgParser.cpp <http://git.reviewboard.kde.org/r/100809/#comment1538> I think 0 should be enough. The L is not needed and might not be a good option on different platforms filters/karbon/svg/SvgParser.cpp <http://git.reviewboard.kde.org/r/100809/#comment1545> The comment is wrong. filters/karbon/svg/SvgParser.cpp <http://git.reviewboard.kde.org/r/100809/#comment1546> Comment is wrong filters/karbon/svg/SvgParser.cpp <http://git.reviewboard.kde.org/r/100809/#comment1547> How about initializing n with href then the else part can be removed. filters/karbon/svg/SvgParser.cpp <http://git.reviewboard.kde.org/r/100809/#comment1548> There should be no space after the * filters/karbon/svg/svgexport.cc <http://git.reviewboard.kde.org/r/100809/#comment1549> These includes shoud use <> instead of " karbon/ui/KarbonView.cpp <http://git.reviewboard.kde.org/r/100809/#comment1550> One question about that one. Is it possible as a used to make sure the right shape gets a clip path? karbon/ui/KarbonView.cpp <http://git.reviewboard.kde.org/r/100809/#comment1578> should the command not be named Clip Object as always one objects is clipped libs/flake/KoClipPath.h <http://git.reviewboard.kde.org/r/100809/#comment1551> There should be no space between *,& and the variable libs/flake/KoClipPath.cpp <http://git.reviewboard.kde.org/r/100809/#comment1552> There should be a space after the if libs/flake/KoClipPath.cpp <http://git.reviewboard.kde.org/r/100809/#comment1553> There should be a blank after the if. Can it happen that clipShapes contains a 0? libs/flake/KoClipPath.cpp <http://git.reviewboard.kde.org/r/100809/#comment1554> There should be a blank after the while libs/flake/KoShape.h <http://git.reviewboard.kde.org/r/100809/#comment1555> There should be no blank after (,* and before ) libs/flake/KoShape.cpp <http://git.reviewboard.kde.org/r/100809/#comment1579> There is no need to check clipPath before deleting it. libs/flake/KoShape.cpp <http://git.reviewboard.kde.org/r/100809/#comment1580> There should be no blank after ( and * and before ) - Thorsten On March 6, 2011, 6:01 p.m., Jan Hambrecht wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100809/ > ----------------------------------------------------------- > > (Updated March 6, 2011, 6:01 p.m.) > > > Review request for Calligra. > > > Summary > ------- > > This patch implements clipping for shapes using arbitrary clipping paths. > This is needed to properly support svg clipping. > There is a new class KoClipPath which can be set on a shape. The clipping > path is then used when painting the shape to set the clipping path on the > painter. > There are also two commands to set and to unset a clipping path on a shape. > > > Diffs > ----- > > filters/karbon/svg/CMakeLists.txt 2a89825 > filters/karbon/svg/SvgClipPathHelper.h PRE-CREATION > filters/karbon/svg/SvgClipPathHelper.cpp PRE-CREATION > filters/karbon/svg/SvgGraphicContext.h dbb28ae > filters/karbon/svg/SvgParser.h c9a09b1 > filters/karbon/svg/SvgParser.cpp 0372fde > filters/karbon/svg/svgexport.h 6689299 > filters/karbon/svg/svgexport.cc 7627006 > karbon/data/karbon.rc cf0a238 > karbon/ui/KarbonView.h 235720f > karbon/ui/KarbonView.cpp c4e3603 > libs/flake/CMakeLists.txt e153e14 > libs/flake/KoClipPath.h PRE-CREATION > libs/flake/KoClipPath.cpp PRE-CREATION > libs/flake/KoShape.h b4b3033 > libs/flake/KoShape.cpp c5f0fd9 > libs/flake/KoShapeManager.cpp 7f225f9 > libs/flake/KoShapeManagerPaintingStrategy.cpp 4a267c5 > libs/flake/KoShape_p.h 5874cbd > libs/flake/commands/KoShapeClipCommand.h PRE-CREATION > libs/flake/commands/KoShapeClipCommand.cpp PRE-CREATION > libs/flake/commands/KoShapeUnclipCommand.h PRE-CREATION > libs/flake/commands/KoShapeUnclipCommand.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/100809/diff > > > Testing > ------- > > Tested with svg files from the svg test suite as well as some kde icons like > akregator.svgz (see bug 264411) > > > Thanks, > > Jan > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
