Add inversion/eraser-end property for tablet pens #62212

hansemro · 2022-06-19T11:06:40Z

Status: Ready for review

As stated in the proposal, this pull request introduces a new InputEventMouseMotion boolean property called pen_inverted which is true when the eraser-end of the pen is detected. This new property makes it possible to map different behavior to both ends of a tablet pen.

Currently, this PR supports Windows (wintab & winink), Linux, and macOS.

Simple demo project: https://github.com/hansemro/InvertedPenGodotTest/tree/godot4

mbrlabs · 2022-06-19T13:34:19Z

What's your OS and tablet model? It's possibly a driver/OS bug...on Linux at least the same kind of flakiness exists for pressure sensitivity (jump from 0.5 to 1.0 and back in 1 frame without any kind of pressure change). I had to filter these out in Lorien.

I can test it later on Windows and Manjaro with a Wacom Intuos Pro (and regular)

fire · 2022-06-19T15:52:11Z

Approved continuous integration testing actions.

fire · 2022-06-19T19:39:52Z

I will suggest that this be put into core, because this type of filtering is hard to design and it's a common usecase of pens.

@JFonS since you are making a pen app, thoughts?

hansemro · 2022-06-20T01:56:23Z

Fixed the intermittent signal issues in x11 by removing a bad assignment that sets the property to false.

Relevant 3.x commit: hansemro@5b7c36c

In Windows, only wintab had the intermittent issue and was fixed in 3.x by leveraging pressure & tilt update logic for wintab API.

Relevant 3.x commit: hansemro@3177c4f

Will update PR branch soon after testing.

Edit: Done

hansemro · 2022-06-20T03:38:47Z

With fixes, we no longer require complicated state machine to filter bad pen_inverted signals in godot projects.

Lorien code from above is so much better now: https://github.com/hansemro/Lorien/blob/77ecb0524bbd3152b9e2fa282efba72d1f125063/lorien/InfiniteCanvas/InfiniteCanvas.gd#L57-L61

hansemro · 2022-06-20T10:22:00Z

godotengine/godot-proposals#4702 (comment)

mbrlabs · 2022-06-20T16:01:22Z

I tested the latest build on Windows 11 with a Wacom Intuos Pro - no problems, works great now! The Linux (Mono) editor CI build failed however...something related to the class reference

Edit: Built it myself on Manjaro Linux - works without problems. I also tried the CI build of the macOS editor on an M1 Mac Mini but it crashed with a NSInvalidArgumentException (not sure if related to this PR...maybe M1 chips are not supported in CI builds?)

hansemro · 2022-06-20T17:47:43Z

@mbrlabs CI builds for just x86_64

% file godot.osx.opt.tools.64 
godot.osx.opt.tools.64: Mach-O 64-bit executable x86_64
% file godot.osx.opt.64
godot.osx.opt.64: Mach-O 64-bit executable x86_64

hansemro · 2022-06-20T17:54:22Z

The Linux (Mono) editor CI build failed however...something related to the class reference

Not sure what to do to fix this. I don't want to make blind attempts to fix this.

akien-mga · 2022-06-20T19:38:09Z

See https://docs.godotengine.org/en/latest/community/contributing/updating_the_class_reference.html to make sure that what you probably added manually actually matches what the doc generator would have done. Here the problem is the alphabetical order not being respected.

hansemro · 2022-06-20T23:17:10Z

Another macOS issue: once pen_inverted is set true, moving mouse (or any non-tablet interface) does not clear it when it should.

Debug print patch

diff --git a/platform/osx/godot_content_view.mm b/platform/osx/godot_content_view.mm
index 739f2d6a1b..f5ea5c455c 100644
--- a/platform/osx/godot_content_view.mm
+++ b/platform/osx/godot_content_view.mm
@@ -387,6 +387,7 @@ - (void)mouseMoved:(NSEvent *)event {
                last_pen_inverted = [event pointingDeviceType] == NSPointingDeviceTypeEraser;
        }
        mm->set_pen_inverted(last_pen_inverted);
+       print_line("subtype: " + rtos(subtype) + ", deviceType: " + rtos([event pointingDeviceType]) + ", pen_inverted: " + rtos(mm->get_pen_inverted()));
        mm->set_global_position(wd.mouse_pos);
        mm->set_velocity(Input::get_singleton()->get_last_mouse_velocity());
        const Vector2i relativeMotion = Vector2i(delta.x, delta.y) * ds->screen_get_max_scale();

Debug build console log

subtype: 2, deviceType: 1, pen_inverted: 0 <- proxmity event: pen-end of stylus (entering)
subtype: 1, deviceType: 0, pen_inverted: 0 <- moving pen and/or pressing down
subtype: 1, deviceType: 0, pen_inverted: 0
subtype: 1, deviceType: 0, pen_inverted: 0
subtype: 1, deviceType: 0, pen_inverted: 0
...
subtype: 1, deviceType: 0, pen_inverted: 0
subtype: 1, deviceType: 0, pen_inverted: 0
subtype: 1, deviceType: 0, pen_inverted: 0
subtype: 2, deviceType: 1, pen_inverted: 0 <- proximity event: pen-end of stylus (exiting)
subtype: 2, deviceType: 3, pen_inverted: 1 <- proxmity event: eraser-end of stylus (entering)
subtype: 1, deviceType: 0, pen_inverted: 1
subtype: 2, deviceType: 3, pen_inverted: 1
subtype: 1, deviceType: 0, pen_inverted: 1 <- moving pen and/or pressing down (inversion retained)
subtype: 1, deviceType: 0, pen_inverted: 1
subtype: 1, deviceType: 0, pen_inverted: 1
subtype: 1, deviceType: 0, pen_inverted: 1
...
subtype: 1, deviceType: 0, pen_inverted: 1
subtype: 1, deviceType: 0, pen_inverted: 1
subtype: 1, deviceType: 0, pen_inverted: 1
subtype: 2, deviceType: 3, pen_inverted: 1 <- proxmity event: eraser-end of stylus (exiting)
subtype: 0, deviceType: 0, pen_inverted: 1 <- moving mouse and/or pressing on mouse buttons
subtype: 0, deviceType: 0, pen_inverted: 1 <-   mouse incorrectly retains pen_inversion
subtype: 0, deviceType: 0, pen_inverted: 1
subtype: 0, deviceType: 0, pen_inverted: 1
subtype: 0, deviceType: 0, pen_inverted: 1
subtype: 0, deviceType: 0, pen_inverted: 1
...
subtype: 0, deviceType: 0, pen_inverted: 1
subtype: 0, deviceType: 0, pen_inverted: 1

One fix is to only retain pen_inversion on tablet subevents and default to false on other event subtypes.

event subtypes: https://developer.apple.com/documentation/appkit/nseventsubtype?language=objc

Edit: Fixed by 367a653

Console log with fix

subtype: 2, deviceType: 1, pen_inverted: 0 <- proxmity event: pen-end of stylus (entering)
subtype: 1, deviceType: 0, pen_inverted: 0 <- moving pen and/or pressing down
subtype: 1, deviceType: 0, pen_inverted: 0
subtype: 1, deviceType: 0, pen_inverted: 0
subtype: 1, deviceType: 0, pen_inverted: 0
subtype: 1, deviceType: 0, pen_inverted: 0
subtype: 1, deviceType: 0, pen_inverted: 0
subtype: 2, deviceType: 1, pen_inverted: 0 <- proximity event: pen-end of stylus (exiting)
subtype: 2, deviceType: 3, pen_inverted: 1 <- proxmity event: eraser-end of stylus (entering)
subtype: 1, deviceType: 0, pen_inverted: 1
subtype: 1, deviceType: 0, pen_inverted: 1
subtype: 1, deviceType: 0, pen_inverted: 1
subtype: 2, deviceType: 3, pen_inverted: 1 <- proximity event: eraser-end of stylus (exiting)
subtype: 0, deviceType: 0, pen_inverted: 0 <- moving mouse and/or pressing on mouse buttons
subtype: 0, deviceType: 0, pen_inverted: 0 <-   mouse (and non-tablet events) reset pen_inverted
subtype: 0, deviceType: 0, pen_inverted: 0
subtype: 0, deviceType: 0, pen_inverted: 0
subtype: 0, deviceType: 0, pen_inverted: 0

core/input/input_event.cpp

hansemro · 2022-06-22T20:51:26Z

Let me know if want me to rebase or squash commits later.

bruvzg

At least macOS and Windows changes seems fine, but I do not have any pan with the eraser to actually test it.

core/input/input_event.cpp

doc/classes/InputEventMouseMotion.xml

platform/osx/godot_content_view.mm

akien-mga

Looks good to me overall. The changes should be squashed before merging, given the scope (+69, -10) I think a single commit is fine for all 3 platform implementations.

InputEventMouseMotion event

hansemro · 2022-07-04T17:44:09Z

Squashed the commits.

akien-mga · 2022-07-04T19:48:57Z

Thanks! And congrats for your first merged Godot contribution 🎉

hansemro changed the title ~~Add inversion/eraser-end detection for tablet pens~~ Add inversion/eraser-end property for tablet pens Jun 19, 2022

hansemro marked this pull request as ready for review June 19, 2022 11:37

hansemro requested review from a team as code owners June 19, 2022 11:37

hansemro marked this pull request as draft June 19, 2022 11:38

This comment was marked as resolved.

Sign in to view

akien-mga added enhancement platform:windows platform:linuxbsd platform:macos topic:input labels Jun 19, 2022

akien-mga added this to the 4.0 milestone Jun 19, 2022

Calinou added needs testing topic:porting labels Jun 19, 2022

This comment was marked as outdated.

Sign in to view

hansemro marked this pull request as ready for review June 22, 2022 20:39

hansemro requested a review from a team as a code owner June 22, 2022 20:39

hansemro commented Jun 22, 2022

View reviewed changes

core/input/input_event.cpp Show resolved Hide resolved

bruvzg reviewed Jul 4, 2022

View reviewed changes

core/input/input_event.cpp Show resolved Hide resolved

akien-mga reviewed Jul 4, 2022

View reviewed changes

doc/classes/InputEventMouseMotion.xml Outdated Show resolved Hide resolved

akien-mga reviewed Jul 4, 2022

View reviewed changes

platform/osx/godot_content_view.mm Outdated Show resolved Hide resolved

akien-mga approved these changes Jul 4, 2022

View reviewed changes

[macOS, Windows, X11] Add stylus inverted/eraser support to

6dcc9d1

InputEventMouseMotion event

hansemro force-pushed the eraser-detect-4 branch from fe884a3 to 6dcc9d1 Compare July 4, 2022 17:42

akien-mga approved these changes Jul 4, 2022

View reviewed changes

akien-mga merged commit 344b427 into godotengine:master Jul 4, 2022

akien-mga mentioned this pull request Jul 4, 2022

Add inversion/eraser-end detection for tablet pens godotengine/godot-proposals#4702

Open

hansemro mentioned this pull request Jul 4, 2022

[3.x, macOS, Windows, X11] Add stylus inverted/eraser support to InputEventMouseMotion event #62723

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add inversion/eraser-end property for tablet pens #62212

Add inversion/eraser-end property for tablet pens #62212

hansemro commented Jun 19, 2022 •

edited

Loading

This comment was marked as resolved.

mbrlabs commented Jun 19, 2022 •

edited

Loading

fire commented Jun 19, 2022

This comment was marked as outdated.

fire commented Jun 19, 2022 •

edited

Loading

hansemro commented Jun 20, 2022 •

edited

Loading

hansemro commented Jun 20, 2022 •

edited

Loading

hansemro commented Jun 20, 2022

mbrlabs commented Jun 20, 2022 •

edited

Loading

hansemro commented Jun 20, 2022

hansemro commented Jun 20, 2022 •

edited

Loading

akien-mga commented Jun 20, 2022

hansemro commented Jun 20, 2022 •

edited

Loading

hansemro commented Jun 22, 2022

bruvzg left a comment

akien-mga left a comment

hansemro commented Jul 4, 2022

akien-mga commented Jul 4, 2022

Add inversion/eraser-end property for tablet pens #62212

Add inversion/eraser-end property for tablet pens #62212

Conversation

hansemro commented Jun 19, 2022 • edited Loading

This comment was marked as resolved.

mbrlabs commented Jun 19, 2022 • edited Loading

fire commented Jun 19, 2022

This comment was marked as outdated.

fire commented Jun 19, 2022 • edited Loading

hansemro commented Jun 20, 2022 • edited Loading

hansemro commented Jun 20, 2022 • edited Loading

hansemro commented Jun 20, 2022

mbrlabs commented Jun 20, 2022 • edited Loading

hansemro commented Jun 20, 2022

hansemro commented Jun 20, 2022 • edited Loading

akien-mga commented Jun 20, 2022

hansemro commented Jun 20, 2022 • edited Loading

hansemro commented Jun 22, 2022

bruvzg left a comment

Choose a reason for hiding this comment

akien-mga left a comment

Choose a reason for hiding this comment

hansemro commented Jul 4, 2022

akien-mga commented Jul 4, 2022

hansemro commented Jun 19, 2022 •

edited

Loading

mbrlabs commented Jun 19, 2022 •

edited

Loading

fire commented Jun 19, 2022 •

edited

Loading

hansemro commented Jun 20, 2022 •

edited

Loading

hansemro commented Jun 20, 2022 •

edited

Loading

mbrlabs commented Jun 20, 2022 •

edited

Loading

hansemro commented Jun 20, 2022 •

edited

Loading

hansemro commented Jun 20, 2022 •

edited

Loading