-
-
Notifications
You must be signed in to change notification settings - Fork 21.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wayland: Restore tablet support and handle multiple tools #88744
Conversation
I think it makes sense to submit to https://github.com/godotengine/godot-demo-projects, so it can be used as a troubleshooting tool like we do with the Joypads demo. |
d6384fe
to
b872e40
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good to me overall. Haven't tested yet (I have an older laptop with a touchscreen and pen, I'll see if it gets recognized as a tablet). Edit: Pen is out of battery and I don't have any AAAA batteries laying around, so I can't test now :P
I didn't check in detail but the memnew
/memdelete
stuff around TabletToolState
probably needs some caution to avoid double-free or memory leaks (it's good to remember that if it needs to be freed after use, any ERR_FAIL
kind of early return need to ensure it gets freed first too).
Should be good to go already I think, as if there's no godot state set (e.g. libdecor or whatever messing around with our display) it should return We also I am trying to be as careful as possible on keeping state memory handling as easy and simple as possible without needing full RAII wrappers and whatnot. |
b872e40
to
8d7a1cb
Compare
Is |
@akien-mga Oof it might not be safe :( Just gave a closer look at the template, and we have this: if constexpr (!std::is_trivially_destructible_v<T>) {
p_class->~T();
} I'll add a null check. Edit: wait, what about |
Since it was never used in the codebase, I think for now we should just do the We could refactor the whole codebase to use |
8d7a1cb
to
af273a1
Compare
This code was already partially there, although heavily incomplete and nowadays commented out. It got broken after the `WaylandThread` refactor and I didn't bother to bring it over, preferring to `#if 0` it into oblivion for the time being as I don't have a tablet/pen which support an eraser and tilt reporting. This commit brings it back and adds proper multi-tool support (needed for eraser detection) thanks to winston-yallow, who could test this code with their more capable tablet.
af273a1
to
b01a36b
Compare
Should be ready. I can't build it locally right now (low on battery), so a last test would probably be wise. |
I built and tested af273a1efde392b4ca479b285df5d3047d9504a0 + the hotfix to bring back Also out of battery (AAAA, who uses those anymore aside from Ubisoft...) for my touchscreen pen so I couldn't actually check that the feature works :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested the most recent version (b01a36b), everything seems to work perfectly.
Device
Wacom Intuos M Pro (2017)
Tilted backwards / right, medium pressure, normal tip:
Tilted backwards / slightly left, high pressure, eraser tip:
Thanks! |
Fixes #88333.
This code was already partially there, although heavily incomplete and nowadays commented out.
It got broken after the
WaylandThread
refactor and I didn't bother to bring it over, preferring to#if 0
it into oblivion for the time being as I don't have a tablet/pen which support an eraser and tilt reporting.This commit brings it back and adds proper multi-tool support (needed for eraser detection) thanks to @winston-yallow, who could test this code with their more capable tablet.
I also noticed some occasional weird segfaults even without even having a tablet installed which I haven't been able to isolate/replicate reliably, so I'm not able to tell if they're a new thing or not.
Although it has been confirmed that eraser and tilt are properly reported now, considering the above, some further testing would be wise.
Here's a nice lil' visualizer I quickly made for this PR, please give it a spin if you have a drawing tablet: https://github.com/riteo/godot-tablet-visualizer
(I wonder if we could make this a demo or something, it's real nifty :D )