Differential D12710

Add tablet tester from Drawpile
ClosedPublic
Actions

Authored by alvinhochun on May 5 2018, 6:25 PM.

Details

Reviewers

dkazakov

Group Reviewers

Krita

Maniphest Tasks

T8652: Krita 4.1.4 Release Task
T8050: WinTab support code enhancement

Commits

R37:e38339f14ca2: Merge branch 'alvin/D12710-tablet-tester'

Summary

This adds the tablet tester from Drawpile (original by Calle Laakkonen) into Krita. I made a few changes to make it compile. It can be accessed from the Tablet page of the config dialog.

With the tablet tester, users with tablet issues can provide a screenshot and copy the log output for support requests, which would be easier than obtaining a tablet log with the existing setup (e.g. not needing to download DebugView on Windows).

However, the log output is a bit less detailed than the existing tablet log. Perhaps we should look into hooking the existing tablet logger to the tablet tester.

Working branch: https://github.com/KDE/krita/compare/master...alvinhochun:alvin/D12710-tablet-tester
Current patch contains changes up to
c0cf285 Improve logging of tablet proximity events

Diff Detail

Repository

R37 Krita

Lint

Lint Skipped

Unit

Unit Tests Skipped

alvinhochun created this revision.May 5 2018, 6:25 PM

Restricted Application added a subscriber: woltherav. · View Herald TranscriptMay 5 2018, 6:25 PM

alvinhochun requested review of this revision.May 5 2018, 6:25 PM

I should also add that this is not a replacement of the tablet log since it bypasses KisInputManager completely.

alvinhochun added a task: T8050: WinTab support code enhancement.May 5 2018, 6:51 PM

alvinhochun updated this revision to Diff 33690.May 5 2018, 7:38 PM

alvinhochun edited the summary of this revision. (Show Details)

We actually used to have a tablet testing application, but that just used Qt's tablet implementation instead of ours.

Hi, @alvinhochun!

I have tested the tester on Linux and for some reason, when I enter/leave proximity of the stylus (not eraser) it says: "Eraser taken away", which seems to be wrong.

Here is the full log:
https://phabricator.kde.org/P206

That is interesting. Is it the same output in Drawpile? Because it should be now that Drawpile borrows the tablet code from Krita.

FWIW the proximity events are separate from the normal tablet hover events, and I suspected that Krita doesn't even use them for it's intended propose at all...

Hi, @alvinhochun!

That is interesting. Is it the same output in Drawpile? Because it should be now that Drawpile borrows the tablet code from Krita.

I have tested Drawpile and all non-proximity events look exactly the same as in your patch. They are correct.

FWIW the proximity events are separate from the normal tablet hover events, and I suspected that Krita doesn't even use them for it's intended propose at all...

We use proximity events quite extensively to block driver-generated mouse events flow when the stylus is painting. Failing proximity events is one of the driver bugs that prevent people from painting in Krita.

I will mark the patch an "needs changes" because of these proximity events issue.

This revision now requires changes to proceed.May 12 2018, 1:58 PM

Should I make improvements based on this specific code from Drawpile, or revive the old tablet tester that @rempt mentioned, or better write a new tablet tester to suit Krita's needs?

In D12710#261443, @alvinhochun wrote:

Should I make improvements based on this specific code from Drawpile, or revive the old tablet tester that @rempt mentioned, or better write a new tablet tester to suit Krita's needs?

I think there are two options:

Just fix this very implementation to handle Proximity events correctly. Looking at the code, the logic in TabletTestDialog::eventFilter() is oversimplified. Just handling the event properties correctly would fix the issue.
Connect our own debugger to this tester widget. This would make the messages look consistent with the main logging framework in KisTabletDebugger, but it would need a bit more work :)

Improved the logging of proximity events.

The patch works perfectly fine now, thank you very much! Please push! :)

This revision is now accepted and ready to land.May 17 2018, 5:39 PM

Closed by commit R37:e38339f14ca2: Merge branch 'alvin/D12710-tablet-tester' (authored by alvinhochun). · Explain WhyMay 17 2018, 5:46 PM

This revision was automatically updated to reflect the committed changes.

alvinhochun mentioned this in R37:e38339f14ca2: Merge branch 'alvin/D12710-tablet-tester'.

woltherav added a task: T8652: Krita 4.1.4 Release Task.May 18 2018, 10:03 AM

Revision Contents
Changeset List

		Path
M		libs/ui/CMakeLists.txt (3 lines)
M		libs/ui/dialogs/kis_dlg_preferences.h (3 lines)
M		libs/ui/dialogs/kis_dlg_preferences.cc (8 lines)
M		libs/ui/forms/wdgtabletsettings.ui (11 lines)
A	M	libs/ui/input/wintab/drawpile_tablettester/README.md (17 lines)
A	M	libs/ui/input/wintab/drawpile_tablettester/tablettest.h (52 lines)
A	M	libs/ui/input/wintab/drawpile_tablettester/tablettest.cpp (131 lines)
A	M	libs/ui/input/wintab/drawpile_tablettester/tablettest.ui (99 lines)
A	M	libs/ui/input/wintab/drawpile_tablettester/tablettester.h (40 lines)
A	M	libs/ui/input/wintab/drawpile_tablettester/tablettester.cpp (53 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	33686			May 5 2018, 5:47 PM	★	★
Diff 2	33690			May 5 2018, 7:34 PM	★	★
Diff 3	34378			May 17 2018, 3:48 PM	★	★
Diff 4	34384	b1d67dd	R37:e38339f14ca20ac2148bed6a15f41f80e6980663	May 17 2018, 5:43 PM	★	★