Review Request 123833: Krita: Add basic modifier key support to selection tools.

[Michael Abrahams]

> On May 19, 2015, 10:15 a.m., Boudewijn Rempt wrote:
> > krita/ui/input/kis_alternate_invocation_action.cpp, line 43
> > <https://git.reviewboard.kde.org/r/123833/diff/1/?file=369772#file369772line43>
> >
> >     These renames seem to be problematic, we need to get rid of the "Alternate" in the string.
> >     
> >     It's weird though, since it suggests we're using translated texts in our config files, which is a big problem.

I don't think the names are the problem, I think it's the fuzzy thinking behind the names.  What is alternate?  What is secondary? What types of actions are appropriate for what sorts of modifier keys? Those terms have rigorous meaning in terms of the KOTool API, but are of no interest to the user in the context of Krita. 

A user cares about: what can I customize and what can't I? Is there anything annoying enough that I'm going to try to tweak it? How straightforward is it to do these customizations?

You'll notice this patch does not hook into the current customization system.  There is one button for each selection mode and they are hard coded.  I plan to bind Ctrl to the "move" tool.  I can't see anybody complaining about this, it is simply the standard used by every other program.


- Michael


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


On May 18, 2015, 8:33 a.m., Michael Abrahams wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123833/
> -----------------------------------------------------------
> 
> (Updated May 18, 2015, 8:33 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> This refactors polygonal, elliptical, and rectangular selection tools to use a basic selection tool template which unifies previously duplicated code. The template overrides the ability to execute alternate actions, but none of those tools supported alternate actions previously and the ellipse and rectangle were already overriding the modifier keys to begin with. 
> 
> Shift: add to selection
> Alt: subtract from selection
> Shift+Alt: intersect current selection
> Ctrl: replace selection
> 
> Certain key combinations allow users the ability to expose the modifier keys to the base tool, i.e. to make proportional / translated / scaled alterations using ctrl/alt/shift.
> 1) If the user *clicks first and then presses modifier keys*, those modifier keys will only alter the selection method.
> 2) If the user *presses modifier keys first and then clicks*, the selection method is locked in, and subsequent modifier keystrokes will change how the selection is drawn.
> 3) If the underlying tool *never takes modifier keys*, modifier keys will always alter the selection method. 
> 
> These rules can be defined systematically by modifying the template.
> 
> Things to do later: 
> + Basic functionality is implemented in KisToolSelectBase, which covers the outline, contiguous, similar color and path selection tools which inherit from KisToolSelectBase.  A more complete refactoring might define KisToolSelectBase via the selection tool template, but it is not simple for the path selection tool which uses a more complicated delegated design pattern.
> + The tools need new icons (e.g. little plus/minus symbols) to give users visual feedback when they activate the modifiers.
> + The Ctrl key should switch temporarily to the move tool, as in Photoshop.
> + Check idiomatic naming conventions, style, etc.
> 
> 
> Diffs
> -----
> 
>   CMakeFiles/2.8.12.1/CMakeDetermineCompilerABI_CXX.bin PRE-CREATION 
>   krita/image/kis_selection.h 6376f874 
>   krita/plugins/tools/selectiontools/kis_tool_select_elliptical.h 7b2cd2f 
>   krita/plugins/tools/selectiontools/kis_tool_select_elliptical.cc 999f1a0 
>   krita/plugins/tools/selectiontools/kis_tool_select_path.cc 9f1a65c 
>   krita/plugins/tools/selectiontools/kis_tool_select_polygonal.h feee9cb 
>   krita/plugins/tools/selectiontools/kis_tool_select_polygonal.cc 9acca50 
>   krita/plugins/tools/selectiontools/kis_tool_select_rectangular.h 5e88766 
>   krita/plugins/tools/selectiontools/kis_tool_select_rectangular.cc 331c6a4 
>   krita/ui/input/kis_alternate_invocation_action.h b47c59e 
>   krita/ui/input/kis_alternate_invocation_action.cpp 48723bf 
>   krita/ui/tool/kis_selection_action_template.h PRE-CREATION 
>   krita/ui/tool/kis_tool_paint.h 48c1f35 
>   krita/ui/tool/kis_tool_polyline_base.h f681fd8 
>   krita/ui/tool/kis_tool_polyline_base.cpp 6071f76 
>   krita/ui/tool/kis_tool_rectangle_base.h a0b470c 
>   krita/ui/tool/kis_tool_rectangle_base.cpp 8e091d0 
>   krita/ui/tool/kis_tool_select_base.h 500d6dd 
>   krita/ui/tool/kis_tool_select_base.cpp 40779ad 
> 
> Diff: https://git.reviewboard.kde.org/r/123833/diff/
> 
> 
> Testing
> -------
> 
> There are no tests targeting the individual selection tools, but the tests for other individual tools passed.
> 
> The ability to add, subtract and intersect complicated shapes quickly exposes some bugs in how the selection marquees are drawn: sometimes extra lines are drawn, sometimes lines fail to be drawn.
> 
> 
> Thanks,
> 
> Michael Abrahams
> 
>

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