Review Request 124278: Improve Krita input device switching for Surface Pro 3

[Boudewijn Rempt]

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


I need to build it and test it with various windows tablets, of course. One functional thing I'm wondering about is whether we can also map the sp3 buttons to right-click and middle-click for the popup palette and the pan functionality?

I agree with Dmitry that the readability changes clutter the patch. There are two more problems with them: some of the changes (0->NULL for instance) conflict with the rest of Krita's coding style, and these changes make the regular merge 2.9 to master to the frameworks branch harder. The plan actually is to do a big coding style/consistency fixup commit in one go once we move development over to 3.0, so I'd really prefer not to merge the readability/coding style changes at this point.


krita/ui/input/wintab/kis_screen_size_choice_dialog.cpp (line 38)
<https://git.reviewboard.kde.org/r/124278/#comment56553>

    Our coding style prefers to have the brace where it was.



krita/ui/input/wintab/kis_tablet_support.h (line 43)
<https://git.reviewboard.kde.org/r/124278/#comment56554>

    https://techbase.kde.org/Policies/Kdelibs_Coding_Style#Variable_declaration prefers to have each variable declaration on its own line.



krita/ui/input/wintab/kis_tablet_support_win.cpp (line 89)
<https://git.reviewboard.kde.org/r/124278/#comment56555>

    We use 0, not NULL practically everywhere (where NULL occurs, it's probably an import of 3rd party code).



krita/ui/input/wintab/kis_tablet_support_win.cpp (line 632)
<https://git.reviewboard.kde.org/r/124278/#comment56556>

    I sort of agree with removing the newline, but it does make merging harder.


- Boudewijn Rempt


On July 7, 2015, 6:31 a.m., Michael Abrahams wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124278/
> -----------------------------------------------------------
> 
> (Updated July 7, 2015, 6:31 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Bugs: 341899
>     http://bugs.kde.org/show_bug.cgi?id=341899
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> This patch rewrites most of kis_tablet_support_win.cpp to support basic
> use of the eraser key on the Surface Pro 3.  The basic issue is that the
> SP3 eraser button does not register being held until the stylus is
> touched to the screen. A secondary issue is that the eraser button might
> be pressed in the middle of the stroke, disrupting the ordinary
> schedule of press and release events.
> 
> This patch attempts to handle those issues by watching for "inline"
> cursor changes.  If a packet pops up with a different cursor ID, we will
> dispatch a release event and a tool switch signal.  This breaks some
> layers of abstraction but it seems to work.
> 
> The rest of the rewrite mostly focuses on readability, but adds a few
> assorted fixes elsewhere, in particular allowing the current tool to be
> changed by hovering over the dockers (necessary to allow the SP3 to
> set an alternate alternate tool with the eraser button), an addition to
> the x11 tablet code to give the same functionality, a prescaling feature
> for QTabletEvent, preventing multiple instances of the Qt/Wintab dialog,
> and a few typo corrections.
> 
> 
> Diffs
> -----
> 
>   krita/ui/input/wintab/kis_screen_size_choice_dialog.cpp 364419b 
>   krita/ui/input/wintab/kis_tablet_support.h 8c1b279 
>   krita/ui/input/wintab/kis_tablet_support_win.cpp 5e5d82f 
>   krita/ui/input/wintab/kis_tablet_support_x11.cpp 0e28671 
>   libs/flake/KoToolManager.cpp 731faed 
> 
> Diff: https://git.reviewboard.kde.org/r/124278/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Abrahams
> 
>

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