----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106283/#review18302 -----------------------------------------------------------
Nice work. I'm impressed with how little code is needed for the stuff you have created. There seems to be a problem generating the outline when the shape generated contains holes. See http://www.zagge.de/files/blobtool.png for a screenshot. The painting started in the bottom right corner. When trying to paint a O there is also a problem generating the outline. plugins/vectortools/KoBlobTool.h <http://git.reviewboard.kde.org/r/106283/#comment14468> If it is not needed please remove. plugins/vectortools/KoBlobTool.cpp <http://git.reviewboard.kde.org/r/106283/#comment14469> This is a very very hacky thing to do. The common functionality should be moved to e.g. flake so it can be reused in both parts without such a hack. plugins/vectortools/KoBlobTool.cpp <http://git.reviewboard.kde.org/r/106283/#comment14470> Please move to the initialization list of the constructor. Also the m_bg should be moved there. plugins/vectortools/KoBlobTool.cpp <http://git.reviewboard.kde.org/r/106283/#comment14471> It seems like m_stroke and m_bg are not deleted when the tool is destructed and therefore leeking memory. plugins/vectortools/KoBlobTool.cpp <http://git.reviewboard.kde.org/r/106283/#comment14472> If the include is needed please move it to the beginning of the file. plugins/vectortools/KoBlobTool.cpp <http://git.reviewboard.kde.org/r/106283/#comment14473> qDebug should only be used for your own debugging and not be committed. Either removed or use kDebug here. plugins/vectortools/KoBlobTool.cpp <http://git.reviewboard.kde.org/r/106283/#comment14475> The indention is wrong here. plugins/vectortools/KoBlobTool.cpp <http://git.reviewboard.kde.org/r/106283/#comment14476> these 2 member need to be deleted to not leak memory before asigning 0. plugins/vectortools/KoBlobTool.cpp <http://git.reviewboard.kde.org/r/106283/#comment14474> This leaks memory as you always generate a new KoPathShape without deleting the old one. plugins/vectortools/KoBlobTool.cpp <http://git.reviewboard.kde.org/r/106283/#comment14478> how about selecting a shape and then when painting it will add to that shape and create one shape when they overlap? plugins/vectortools/KoBlobTool.cpp <http://git.reviewboard.kde.org/r/106283/#comment14477> Please use only one space. - Thorsten Zachmann On Aug. 30, 2012, 10:59 p.m., José Luis Vergara wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106283/ > ----------------------------------------------------------- > > (Updated Aug. 30, 2012, 10:59 p.m.) > > > Review request for Calligra, Boudewijn Rempt, Inge Wallin, C. Boemann, and > Sven Langkamp. > > > Description > ------- > > Blob tool is the name for a vector brush that creates a broad stroke that > fuses with itself and with other shapes that have the same styling, thus > helping to create more organic-looking vector images. > This patch implements a very basic version of this tool, missing the ability > to fuse shapes with other shapes, which I am postponing until we fix bug > 297623 (see thread > http://mail.kde.org/pipermail/calligra-devel/2012-August/006633.html ). > Further details and a link to a video in blog > http://pentalis.org/kritablog/?p=315 > > Note: some dependencies (like KarbonCurveFit) need to be moved, I will post a > separate review for that, because I need to discuss what directory it belongs > to. > > I need an easy way to make clear I commit my code in either license, LGPL, > GPL, or anything we need for it to fit well in our project; it's not easy to > convey since files accumulate more contributors the more time passes by. > Suggestions appreciated. > > > Diffs > ----- > > plugins/CMakeLists.txt 526499f > plugins/vectortools/CMakeLists.txt PRE-CREATION > plugins/vectortools/KoBlobTool.h PRE-CREATION > plugins/vectortools/KoBlobTool.cpp PRE-CREATION > plugins/vectortools/KoBlobToolFactory.h PRE-CREATION > plugins/vectortools/KoBlobToolFactory.cpp PRE-CREATION > plugins/vectortools/KoVectorToolsPlugin.h PRE-CREATION > plugins/vectortools/KoVectorToolsPlugin.cpp PRE-CREATION > plugins/vectortools/calligravectortools.desktop PRE-CREATION > plugins/vectortools/hi22-action-blob-tool.png PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/106283/diff/ > > > Testing > ------- > > Tested compilation, loading in applications Karbon and Krita, and basic > functioning of the tool. It works (incompletely) as intended for a small > group of use cases. Also tested speed, it needs to be faster, but is usable. > > > Thanks, > > José Luis Vergara > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
