Add Win8+ "Pointer Input Message" support
ClosedPublic

Authored by alvinhochun on Aug 20 2017, 7:11 PM.

Details

Summary

This adds optional support for Pointer Input Message.

There is an option in the tablet config to enable this (said option is hidden on <=Win7 and non-Windows systems). Enabling this will disable loading WinTab.

It is technically possible to automatically enable this if WinTab cannot be loaded (e.g. wintab32.dll not found), but this is really an experimental function so I didn't add this check.

I tested with both 3.2 (Qt 5.6.1) and master (Qt 5.9.1) inside a Windows 10 VM with my Wacom.

Test Plan

Test pressure sensitivity with option enabled and compare with having it disabled.

Diff Detail

Repository
R37 Krita
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
alvinhochun created this revision.Aug 20 2017, 7:11 PM
Restricted Application added a subscriber: woltherav. · View Herald TranscriptAug 20 2017, 7:11 PM
  • Try to track whether the current hovered QWidget is interested in QTabletMove events (doesn't seem very useful)
  • Force WM_POINTERUPDATE to be processed by DefWindowProc so it generates mouse move events for Qt widgets.

Fix WM_POINTERDOWN and WM_POINTERUP event button handling

  • Disable QWidget interest tracking (QTabletMove events will be sent to widget no matter what), fix pointer getting stuck after right click
  • Reduce logging
alvinhochun retitled this revision from [Not for review] Use Win8+ "Pointer Input Message to Add Win8+ "Pointer Input Message" support.
alvinhochun edited the summary of this revision. (Show Details)
alvinhochun edited the test plan for this revision. (Show Details)
alvinhochun added a reviewer: Krita.
alvinhochun edited projects, added Krita: Next Features; removed Krita.
  • Refactored some code
  • Reduced logging
  • Code cleanups
  • Added an option in the tablet config to enable using Win8 Pointer Input Message (hidden on <=Win7 and non-Windows systems)
  • Fix handling of certain multiple monitor configs with negative coordinates offset
  • Handle change of monitor resolution
alvinhochun planned changes to this revision.Aug 23 2017, 9:34 AM
  • Handle WM_POINTERACTIVATE message
  • Prevent issues if capturing QWidget is destroyed
  • Fix uninitialized field

Hi, @alvinhochun!

I have tested your build. Here are the issues:

  1. Bended lines problem is here again

  1. Rotation sensor rotates into an inverted direction and wraps in two points (looks like it wraps in 2 * M_PI points)
  2. Tilt sensor looks as if it is not normalized: most probably 90 vs 60 degrees issue, since Krita normalizes the stuff by 60 deg
  3. Tilt direction sensor seem to work correctly (at least it doesn't wrap)

And "bended" lines problem is the major one

rempt added a subscriber: rempt.Aug 23 2017, 3:31 PM

That is curious though, since it doesn't seem to happen on my sp3. You're testing on the intel sdp, right?

In D7441#138917, @rempt wrote:

That is curious though, since it doesn't seem to happen on my sp3. You're testing on the intel sdp, right?

I'm testing on "Wacom 5 Touch M". Though connected to the SPD

I have tested the build on the embedded SDP's digitizer. It works even without special wintab drivers (obviously), but has the same bended lines as Intuos connected to this SDP.

  1. Rotation sensor rotates into an inverted direction and wraps in two points (looks like it wraps in 2 * M_PI points)

From https://msdn.microsoft.com/en-us/library/windows/desktop/hh454909(v=vs.85).aspx

rotation
Type: UINT32
The clockwise rotation, or twist, of the pointer normalized in a range of 0 to 359

QTabletEvent's docs doesn't state the direction and range of the rotation, but by taking a look at the WinTab code more carefully it seems that it should be anti-clockwise and in the range of [-180, 180]?

rotation = 360.0 - (packet.pkOrientation.orTwist / 10.0);
            if (rotation > 180.0)
                rotation -= 360.0;

The current calculation used is: penInfo.rotation > 180 ? penInfo.rotation - 360 : penInfo.rotation. It seems like there are two issues:

  1. Flipped direction
  2. I overlooked the fact that penInfo.rotation is unsigned and there is an unsigned underflow in the subtraction :(

The proper calculation would seem to be:

int rotation = 360 - penInfo.rotation; // Flip direction and convert to signed int
if (rotation > 180) {
    rotation -= 360;
}

Not sure what's the wrapping problem you mean, but could it be the underflow that you're seeing?

  1. Tilt sensor looks as if it is not normalized: most probably 90 vs 60 degrees issue, since Krita normalizes the stuff by 60 deg

The end result is still an integer, so I'll just do an integer multiplication and division here: penInfo.tiltX * 2 / 3

  1. Bended lines problem is here again

    And "bended" lines problem is the major one

I have tested the build on the embedded SDP's digitizer. It works even without special wintab drivers (obviously), but has the same bended lines as Intuos connected to this SDP.

It's probably either issues with the driver or the system... @rempt's SP3 works fine and my Intuos Pen and Touch works fine too. @scottpetrovic tried an earlier test build and said it worked fine on the SP4 (don't know how smooth it is though).

This shouldn't be a show-stopper, since the feature is not enabled by default in any case.

(Making a test build with the changes)

Changed the rotation and tilt calculation as stated, I can't test it though.

@dkazakov please check this test build: https://drive.google.com/open?id=0B1i7Q5Isuy2Jd21wU3hJeVpiWEE

Were you testing with no smoothing? I think I always had basic smoothing enabled when testing.

I guess I also tested with the smoothing enabled, I just didn't touch
the default settings. I will check again when testing your new build

victorw added a subscriber: victorw.EditedAug 27 2017, 4:31 PM

I tested this on my Surface Pro 2 (Wacom) to see if there were any regressions. For me it looks like the Win8+ "Pointer Input Message" produces jitter compared to the wintab version. I've attached an image of two reasonably straight lines, drawn in two instances of the 4.0-prealpha_winink build. The first one with the Win8+ Pointer Input Message support enabled, the second without. I merged the images to show the difference (top: "winink", bottom: "wintab"):

Edit: Both are without smoothing enabled

I've also attached two tablet logs for each session, with a "krita.tabletlog" filter:

The only thing I found of note outside of the "krita.tabletlog" lines was that the winink one also says it's using that API:

00000044	10.00228977	[4216] krita.tabletlog: Loaded Windows 8 Pointer Input API functions	
00000045	10.00232697	[4216] Using Win8 Pointer Input for tablet support
alvinhochun added inline comments.Aug 27 2017, 4:59 PM
libs/ui/input/wintab/kis_tablet_support_win8.cpp
505

This probably should be const QPointF localPosF = globalPos + delta; instead? I am seeing some off-by-one rounding error in the above log and this might be the cause.

alvinhochun added inline comments.Aug 27 2017, 5:02 PM
libs/ui/input/wintab/kis_tablet_support_win8.cpp
505

Opps I meant const QPointF localPosF = localPos + delta;

Fixed the incorrect localPos calculation.

New build: https://drive.google.com/open?id=0B1i7Q5Isuy2JeTJpczNvRHV4MlE

alvinhochun marked 2 inline comments as done.Aug 27 2017, 6:07 PM

Fixed the incorrect localPos calculation.

New build: https://drive.google.com/open?id=0B1i7Q5Isuy2JeTJpczNvRHV4MlE

Verified fixed here, nice work! I double checked so it was using Win8+ Pointer Input instead of wintab too.

It should be noted that the above build is 3.2.1-based and not 4.0, though that just means one more data point in favour of the feature going into the 3.2 branch! :)

alvinhochun planned changes to this revision.Sep 1 2017, 11:11 AM
alvinhochun updated this revision to Diff 19116.Sep 3 2017, 8:32 AM
  • Removed a bit of unreachable code
  • Make windows synthesize the mouse events no matter what (Krita will block the mouse events on the canvas when pen is in range), without this some buttons on the right click popup won't work and there are some mishandling with popup widgets
  • Disabled the windows tablet pc feedback (ripple effects), long press and flicking everywhere (can't selectively disable these for the canvas)
  • Updated the config UI

However now right-clicking doesn't work, I'm not sure what the problem is, maybe it's caused by TABLET_DISABLE_PENBARRELFEEDBACK?

alvinhochun updated this revision to Diff 19181.Sep 4 2017, 5:27 PM

(Changes)

  • Don't send tablet events if popup/modal is active and cursor isn't on the popup/modal
  • Revert some changes to not always have windows synthesize mouse events and don't disable the winink feedback (see below)

It appears that tablet press/release events are always accepted when the pen is over the canvas popup palette but it doesn't work properly without mouse press/release events. The brush presets can't be selected and the buttons on the bottom-right corner can't be clicked.

With this pointer input event support, the mouse press/release events are only synthesized (by Windows) if the tablet press/release events aren't accepted. I cannot have Windows synthesize the mouse press/release events unconditionally, because that would cause the annoying Windows Ink ripple effects when drawing on the canvas (and I can't seem to disable them without breaking right clicks).

I can think of two ways of fixing it:

  1. (The proper way:) Find out what code is accepting QTabletEvent even when the pen press should've been blocked by the popup palette, and fix it.
    • I was guessing it's KisInputManager::eventFilter but it's a bit complex and I'm not sure if it's actually the place (the events are supposed to be sent to the popup palette QWidget, not the canvas)
  2. Have the popup palette widgets translate the tablet events to mouse events internally
    • Here is the change I have: P99
    • I tested it with both WinTab and pointer input events and it works fine enough, but no idea about Linux and Mac.

@rempt: Does solving this with P99 seem fine to you?

rempt accepted this revision.Sep 5 2017, 9:41 AM
This revision is now accepted and ready to land.Sep 5 2017, 9:41 AM
This revision was automatically updated to reflect the committed changes.