godotengine
Add `pressed_tolerance` to `BaseButton`, to improve accessibility on touch displays
Fixes #95532 Fixes #93612
Test project: base_button_tests.zip
Test on MacOs pressed_tolerance = 2 -> round(DPI / 50)
https://github.com/user-attachments/assets/ef16c387-ba77-4729-ae7f-5f5bbd6aa339
I'm not a fan of this approach as it would require duplicating similar logic for every components where we want a similar behavior.
Instead given that the new behavior was introduced in https://github.com/godotengine/godot/pull/84331, can't we add a pressed_tolerance in the drag logic in GodotGestureHandler based on the dpi as done here?
duplicating similar logic
This PR will not prevent scrolling, but will force pressed events within a tolerance limit.
A desktop app does not prevent pressed events when scrolling with the mouse wheel.
It is not necessary to extend GodotGestureHandler, there is already a solution: the scroll_deadzone property can be increased in the ScrollContainer. I find scrolling better without “deadzone”, it feels much more natural and "responsive", after PR #84331 also on Android.
Maybe more controls are affected, I can only reproduce the pressed event "issue" in the ScrollContainer when Filter is set to Pass.
https://github.com/godotengine/godot/issues/95532#issuecomment-2307631512 https://github.com/godotengine/godot/issues/93612#issuecomment-2299930862
Two confirmations that this PR also works with scroll_deadzone = 0, event pressed is no longer canceled as soon as “scrolling” is started.
Test on Samsung Tab S7 pressed_tolerance = 7 -> round(DPI / 50)
https://github.com/user-attachments/assets/b0f08997-e252-405e-a8ef-54b73c50cf1b
This PR will not prevent scrolling, but will force pressed events within a tolerance limit.
That is my point.
With the updated logic we are turning an InputEventMouseMotion into a pressed event based on the pressed_tolerance; the proper way of doing this would be to not dispatch an InputEventMouseMotion in the first place until we're past the pressed_tolerance and as such this logic belongs in the Java/Kotlin layer which is responsible for dispatching the events.
Moving the logic at the Java/Kotlin layer also provides consistency across all UI elements.
A desktop app does not prevent pressed events when scrolling with the mouse wheel.
A desktop app dispatches InputEventMouseButton events for pressed events, and InputEventMouseMotion events for scroll events hence why this is not an issue for desktop platforms.
It is not necessary to extend GodotGestureHandler, there is already a solution: the scroll_deadzone property can be increased in the ScrollContainer
If there's already a solution, what is the purpose for this PR?
From the linked issues' description, it seems like while the scroll_deadzone works, the new behavior introduced by https://github.com/godotengine/godot/pull/84331 is seen as a regression because it departs from the default behavior.
If that's the case, then the logic added in https://github.com/godotengine/godot/pull/84331 should be placed behind a project settings instead and not be the default.
If that's the case, then the logic added in https://github.com/godotengine/godot/pull/84331 should be placed behind a project settings instead and not be the default.
From reading the comments on the linked issues, I think I'm leaning with the approach of reverting https://github.com/godotengine/godot/pull/84331 and putting that logic behind a project setting. The reason for that is that https://github.com/godotengine/godot/pull/84331 solves an issue where drag latency is important. We cannot assume however this is the desired behavior for all existing projects, and for those existing projects, we are introducing a severe regression.
@Alex2782 So the next step would be to put https://github.com/godotengine/godot/pull/84331 behind a project setting that is disabled by default, and add documentation for the developers enabling that setting.
Yes, a few weeks ago I also wanted to add new configurations for Android. But then I saw that pressed events are also triggered in a desktop app when the mouse wheel is turned. After up to 3 hours of research, I was unable to discover any further regressions.
Issues #95532 and #93612 are very special, several conditions must be fulfilled, it only affects Base Buttons in the ScrollContainer and only if the Mouse Filter has also been switched to Pass.
If there's already a solution, what is the purpose for this PR?
Combine both worlds (DragEvents and BaseButtons), without extra configurations, trigger pressed events for all "Base Buttons" even if something was scrolled over the button. Not only for Android. I think all OS can benefit from this. MacOS, for example, has the Magic Mouse, which also triggers scrolling when you click, similar to the touchscreen.
However, desktop apps are only affected if Emulate Touch From Mouse is active.
Should we perhaps wait a few more weeks?
I check the Android issues at least once a week, so far no further scroll/drag regressions have been reported. In my opinion, gaming elements should be prioritized, and not normal GUI apps. Because scroll_deadzone already exists for GUI, I decided against a configuration. If my arguments are weak, then I could add new configurations in about 2 weeks so that GUI apps on Android work normally again as before with scroll_deadzone = 0. (This would mean: GUI Apps > Gaming Controls)
I tried Magic Mouse on MacOs, what I wrote above is not true, the Magic Mouse is not affected.
If there's already a solution, what is the purpose for this PR?
But here is an example of what is possible on desktop, hold mouse button and scroll at the same time, pressed event is still triggered. This PR also makes this possible (for touch screens), except that the tolerance is not infinite 😃
With a “Magic Mouse” this is easier to demonstrate with one finger.
v4.3.stable.official [77dcf97d8]
https://github.com/user-attachments/assets/5f102a4e-e64a-4165-a197-96a132fccd2d
I have prepared PR #96139.
If this PR is not acceptable, I'll likely have enough time in two weeks to complete everything.
I have tested this PR and can confirm it fixes it on our devices. We were previously testing on a Google Pixel without issue. However, when we tried Samsung, we noticed that we couldn't select our buttons and they were only scrolling. This PR that @Alex2782 has fixed it for us. Thanks Alex!!! 🎉🎉🎉❤️
this PR is no longer necessary: https://github.com/godotengine/godot/pull/96139#issue-2487979073