Review Request: Implement the basics of the Canvas Interaction proposal

Thu Jun 14 14:20:34 BST 2012

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105250/#review14735
-----------------------------------------------------------

Yeah... It's great progress, but definitely needs more work. Here are a couple of preliminary notes.

* that a single click doesn't paint really is a _big_ problem. We need to find a solution here.
* pan with space seems to put the canvas in the wrong place.
* did we lose zoom with the mouse wheel?
* right cursor moves the canvas left -- here I'd also expect autorepeat to work.
* let's not forget the level jump when zooming with the keyboard.
* there's a problem with the brush resizing: it is less precise than it used to be and the brush tends to snap back to a bigger size on mouseup/shift up. It feels a bit more like it is on windows, in fact, with not enough feedback between stages.
* applying a gradient with the tablet doesn't work: the action doesn't get started 
* then applying a gradient with the mouse causes a crash:

#6  KisToolInvocationAction::end (this=0x5de8df0) at /home/boud/kde/src/calligra/krita/ui/input/kis_tool_invocation_action.cpp:80
#7  0x00007f9e967fb12a in KisInputManager::Private::clearState (this=0x5de8d80) at /home/boud/kde/src/calligra/krita/ui/input/kis_input_manager.cpp:376
#8  0x00007f9e967fb921 in KisInputManager::eventFilter (this=0x5de8ca0, object=<optimized out>, event=0x7fff94927e50) at /home/boud/kde/src/calligra/krita/ui/input/kis_input_manager.cpp:137
#9  0x00007f9e96e9e3b8 in QCoreApplicationPrivate::sendThroughObjectEventFilters (this=<optimized out>, receiver=0x69ef730, event=0x7fff94927e50) at kernel/qcoreapplication.cpp:986
#10 0x00007f9e8fbec09f in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib64/libQtGui.so.4
#11 0x00007f9e8fbf1823 in QApplication::notify(QObject*, QEvent*) () from /usr/lib64/libQtGui.so.4
#12 0x00007f9e909308f6 in KApplication::notify(QObject*, QEvent*) () from /usr/lib64/libkdeui.so.5
#13 0x00007f9e96e9e22c in QCoreApplication::notifyInternal (this=0x7fff949289b0, receiver=0x69ef730, event=0x7fff94927e50) at kernel/qcoreapplication.cpp:876
#14 0x00007f9e8fbed0a2 in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool) () from /usr/lib64/libQtGui.so.4
#15 0x00007f9e8fc68e85 in ?? () from /usr/lib64/libQtGui.so.4
#16 0x00007f9e8fc67d4a in QApplication::x11ProcessEvent(_XEvent*) () from /usr/lib64/libQtGui.so.4
#17 0x00007f9e8fc904da in ?? () from /usr/lib64/libQtGui.so.4
#18 0x00007f9e96e9d012 in QEventLoop::processEvents (this=<optimized out>, flags=...) at kernel/qeventloop.cpp:149
#19 0x00007f9e96e9d267 in QEventLoop::exec (this=0x7fff94928970, flags=...) at kernel/qeventloop.cpp:204
#20 0x00007f9e96ea1dc5 in QCoreApplication::exec () at kernel/qcoreapplication.cpp:1148
#21 0x00007f9e973c74ed in kdemain (argc=<optimized out>, argv=<optimized out>) at /home/boud/kde/src/calligra/krita/main.cc:71
#22 0x00007f9e8e57223d in __libc_start_main () from /lib64/libc.so.6
#23 0x0000000000400921 in _start () at ../sysdeps/x86_64/elf/start.S:113

* rotating with shift-space is way too fast, at least with the opengl canvas, it's less smooth than with the pan tool in the qpainter canvas. The steps seem too big, too. Also, I got the same crash as above.

krita/ui/input/kis_pan_action.cpp
<http://git.reviewboard.kde.org/r/105250/#comment11636>

    This fails to compile:

    /home/boud/kde/src/calligra/krita/ui/input/kis_pan_action.cpp:40:29: error: ISO C++ forbids initialization of member ‘panDistance’ [-fpermissive]
    /home/boud/kde/src/calligra/krita/ui/input/kis_pan_action.cpp:40:29: error: making ‘panDistance’ static [-fpermissive]

- Boudewijn Rempt

On June 14, 2012, 1:18 p.m., Arjen Hiemstra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105250/
> -----------------------------------------------------------
> 
> (Updated June 14, 2012, 1:18 p.m.)
> 
> 
> Review request for Calligra and Boudewijn Rempt.
> 
> 
> Description
> -------
> 
> This patch is a squashed patch of all the current changes in the krita-canvasinteraction-ahiemstra branch. It implements the basics of my canvas interaction proposal. 
> 
> With these changes, the input to the canvas is now handled by the class KisInputManager. This class installs an event filter and uses the events to try and match certain shortcuts. When a match is found, an action associated with that shortcut gets activated and perfoms whatever the action is meant to do. These changes implement 7 different actions that cover most of what used to be performed by the canvas.
> 
> Known Issues:
> - The recognition code is not yet very robust. Especially the shortcuts configured as Control + Some Button or Key seem hard to trigger.
> - The "Alternate Invocation" action with the normal brush tool somehow does not get deactivated correctly.
> - A KAction is added for setting up the mirror axis, but it fails. I do not know why since the code is, as far as I can see, the same as the old code.
> - Due to the way recognition is currently done, a single click on the canvas will not paint.
> - The Pan, Rotate and Zoom actions may be a little jumpy, due to them using incorrect mouse positions.
> 
> Still To Do:
> - Loading shortcut configuration from a configuration file instead of hardcoding them.
> - Create a configuration UI for shortcut configuration.
> - Implement overlay widgets for the Pan, Rotate and Zoom actions.
> 
> 
> Diffs
> -----
> 
>   krita/ui/CMakeLists.txt 212f515 
>   krita/ui/canvas/kis_canvas2.cpp 8f22f46 
>   krita/ui/canvas/kis_canvas_widget_base.h d9a92fd 
>   krita/ui/canvas/kis_canvas_widget_base.cpp 4ee3bb6 
>   krita/ui/canvas/kis_qpainter_canvas.h a70f463 
>   krita/ui/canvas/kis_qpainter_canvas.cpp c2cd5f6 
>   krita/ui/input/kis_abstract_input_action.h PRE-CREATION 
>   krita/ui/input/kis_abstract_input_action.cpp PRE-CREATION 
>   krita/ui/input/kis_alternate_invocation_action.h PRE-CREATION 
>   krita/ui/input/kis_alternate_invocation_action.cpp PRE-CREATION 
>   krita/ui/input/kis_change_primary_setting_action.h PRE-CREATION 
>   krita/ui/input/kis_change_primary_setting_action.cpp PRE-CREATION 
>   krita/ui/input/kis_input_manager.h PRE-CREATION 
>   krita/ui/input/kis_input_manager.cpp PRE-CREATION 
>   krita/ui/input/kis_pan_action.h PRE-CREATION 
>   krita/ui/input/kis_pan_action.cpp PRE-CREATION 
>   krita/ui/input/kis_rotate_canvas_action.h PRE-CREATION 
>   krita/ui/input/kis_rotate_canvas_action.cpp PRE-CREATION 
>   krita/ui/input/kis_shortcut.h PRE-CREATION 
>   krita/ui/input/kis_shortcut.cpp PRE-CREATION 
>   krita/ui/input/kis_show_palette_action.h PRE-CREATION 
>   krita/ui/input/kis_show_palette_action.cpp PRE-CREATION 
>   krita/ui/input/kis_tool_invocation_action.h PRE-CREATION 
>   krita/ui/input/kis_tool_invocation_action.cpp PRE-CREATION 
>   krita/ui/input/kis_zoom_action.h PRE-CREATION 
>   krita/ui/input/kis_zoom_action.cpp PRE-CREATION 
>   krita/ui/opengl/kis_opengl_canvas2.h 7523e92 
>   krita/ui/opengl/kis_opengl_canvas2.cpp d125f5a 
>   krita/ui/tool/kis_tool.cc eca749a 
>   krita/ui/tool/kis_tool_paint.cc 61be742 
> 
> Diff: http://git.reviewboard.kde.org/r/105250/diff/
> 
> 
> Testing
> -------
> 
> I have performed quite some drawing using this code. The basic concept works very well and most of the actions are fairly intuitive, but the recognition code needs improvement.
> 
> 
> Thanks,
> 
> Arjen Hiemstra
> 
>

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