Review Request 120505: Adding a activation-toggle and preview to assistants.

Sven Langkamp sven.langkamp at gmail.com
Fri Oct 10 22:36:53 BST 2014


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120505/#review68238
-----------------------------------------------------------



krita/plugins/assistants/RulerAssistant/PerspectiveAssistant.h
<https://git.reviewboard.kde.org/r/120505/#comment47548>

    _ is only used for m_. Should use camel case.



krita/plugins/assistants/RulerAssistant/PerspectiveAssistant.cc
<https://git.reviewboard.kde.org/r/120505/#comment47549>

    indention missing



krita/plugins/assistants/RulerAssistant/PerspectiveAssistant.cc
<https://git.reviewboard.kde.org/r/120505/#comment47550>

    no spaces in empty lines.



krita/plugins/assistants/RulerAssistant/PerspectiveAssistant.cc
<https://git.reviewboard.kde.org/r/120505/#comment47551>

    indention one level too deep



krita/plugins/assistants/RulerAssistant/PerspectiveAssistant.cc
<https://git.reviewboard.kde.org/r/120505/#comment47552>

    return should be in it's own line



krita/ui/canvas/kis_canvas_decoration.cc
<https://git.reviewboard.kde.org/r/120505/#comment47555>

    kDebug instead of qDebug



krita/ui/canvas/kis_canvas_widget_base.cpp
<https://git.reviewboard.kde.org/r/120505/#comment47556>

    own line and kDebug



krita/ui/kis_painting_assistant.cc
<https://git.reviewboard.kde.org/r/120505/#comment47553>

    no m_ if d pointer is used



krita/ui/kis_painting_assistants_decoration.cpp
<https://git.reviewboard.kde.org/r/120505/#comment47554>

    no empty braces here


- Sven Langkamp


On Okt. 10, 2014, 9:32 nachm., Wolthera van Hövell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120505/
> -----------------------------------------------------------
> 
> (Updated Okt. 10, 2014, 9:32 nachm.)
> 
> 
> Review request for Calligra, Dmitry Kazakov and Boudewijn Rempt.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> This patch adds a little eye icon underneath the deletion icon which can be used to activate/deactivate snapping and in the case of the perspective assistant, add a preview for where the snapping will happen/head to.
> 
> This last bit of code may be refactored a little more, if I want to allow to option to see this preview without having the basic assistant visible. This would allow these previews to make sense on normal assistants as well.
> (Right now, the 'hide assistants' button in view just makes the whole widget invisible).[done]
> Preview will be important if we're gonna have parallel rulers and the like.
> 
> I also fixed a minor bug where the canvas wasn't passed to the drawAssistant function in the painting assistant by the PaintingAssistantDecorations class, because whoever wrote the caching system had added a boolean for it in the arguments in a rather random place. Both caching and me accessing the canvas for cursor conversion work now.
> 
> Update: Added a whole framework so that the previews and main assistant can be toggled seperately. I also implemented preview for Ellipse, Ruler and Spline based on intersection with the bounding box. This won't help much when there's only one ruler on the canvas, but with multiple insersecting rulers this gives tremendous feedback.
> I replaced the 'show painting assistants' action internally with the option to toggle the assistants themselves, instead of turning of visiblity for the whole widget.
> I also added a new action 'Show Assistant previews'. This allows the user to disable all previews globally. Because I added this action, the kritarc file is set to version 53 from 52.
> 
> 
> Diffs
> -----
> 
>   krita/plugins/assistants/RulerAssistant/PerspectiveAssistant.cc e3ec525 
>   krita/plugins/assistants/RulerAssistant/RulerAssistant.h 388b18a 
>   krita/plugins/assistants/RulerAssistant/RulerAssistant.cc dd173c0 
>   krita/plugins/assistants/RulerAssistant/SplineAssistant.h 92ccb49 
>   krita/plugins/assistants/RulerAssistant/kis_ruler_assistant_tool.h 11e92f3 
>   krita/plugins/assistants/RulerAssistant/kis_ruler_assistant_tool.cc bbfe5cd 
>   krita/ui/canvas/kis_canvas_decoration.cc 9179d3f 
>   krita/ui/canvas/kis_canvas_widget_base.cpp 02f0ab5 
>   krita/ui/kis_painting_assistant.h 6f315e8 
>   krita/krita.rc 50faa31 
>   krita/plugins/assistants/RulerAssistant/EllipseAssistant.h e36bd9d 
>   krita/plugins/assistants/RulerAssistant/EllipseAssistant.cc ec6ee06 
>   krita/plugins/assistants/RulerAssistant/PerspectiveAssistant.h c4fd3b2 
>   krita/ui/kis_painting_assistant.cc 81c6069 
>   krita/ui/kis_painting_assistants_decoration.h a2c1263 
>   krita/ui/kis_painting_assistants_decoration.cpp dd65ec0 
>   krita/plugins/assistants/RulerAssistant/SplineAssistant.cc 8391f65 
> 
> Diff: https://git.reviewboard.kde.org/r/120505/diff/
> 
> 
> Testing
> -------
> 
> Lots, I can't find any crashes or bugs here.
> 
> 
> Thanks,
> 
> Wolthera van Hövell
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20141010/af193d0e/attachment.htm>


More information about the calligra-devel mailing list