----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123833/#review80559 -----------------------------------------------------------
krita/ui/tool/kis_selection_action_template.h (line 1) <https://git.reviewboard.kde.org/r/123833/#comment55234> We need a copyright header in all new files :-) LGPL v2+ or GPL v2+. krita/ui/tool/kis_selection_action_template.h (line 63) <https://git.reviewboard.kde.org/r/123833/#comment55235> gcc can't handle the extra , here. krita/ui/tool/kis_selection_action_template.h (line 171) <https://git.reviewboard.kde.org/r/123833/#comment55236> m_widgetHelper and m_selectionActionAlternate are initialized in the wrong order: In file included from /home/boud/kde/src/2.9/krita/plugins/tools/selectiontools/kis_tool_select_polygonal.h:28:0, from /home/boud/kde/src/2.9/krita/plugins/tools/selectiontools/kis_tool_select_polygonal.cc:24: /home/boud/kde/src/2.9/krita/ui/tool/kis_selection_action_template.h: In instantiation of ‘SelectionActionHandler<BaseClass>::SelectionActionHandler(KoCanvasBase*, QString) [with BaseClass = __KisToolSelectPolygonalLocal]’: /home/boud/kde/src/2.9/krita/plugins/tools/selectiontools/kis_tool_select_polygonal.cc:94:71: required from here /home/boud/kde/src/2.9/krita/ui/tool/kis_selection_action_template.h:176:21: warning: ‘SelectionActionHandler<__KisToolSelectPolygonalLocal>::m_selectionActionAlternate’ will be initialized after [-Wreorder] SelectionAction m_selectionActionAlternate; ^ /home/boud/kde/src/2.9/krita/ui/tool/kis_selection_action_template.h:171:40: warning: ‘KisSelectionToolConfigWidgetHelper SelectionActionHandler<__KisToolSelectPolygonalLocal>::m_widgetHelper’ [-Wreorder] KisSelectionToolConfigWidgetHelper m_widgetHelper; ^ /home/boud/kde/src/2.9/krita/ui/tool/kis_selection_action_template.h:37:5: warning: when initialized here [-Wreorder] SelectionActionHandler(KoCanvasBase* canvas, QString toolName) - Boudewijn Rempt On May 17, 2015, 9:20 p.m., Michael Abrahams wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123833/ > ----------------------------------------------------------- > > (Updated May 17, 2015, 9:20 p.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 > >
_______________________________________________ calligra-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/calligra-devel
