Skip to content

BaseButton: multitouch support#110893

Merged
Repiteo merged 1 commit into
godotengine:masterfrom
Kazox61:basebutton-multitouch
Apr 9, 2026
Merged

BaseButton: multitouch support#110893
Repiteo merged 1 commit into
godotengine:masterfrom
Kazox61:basebutton-multitouch

Conversation

@Kazox61
Copy link
Copy Markdown
Contributor

@Kazox61 Kazox61 commented Sep 25, 2025

Closes Partially implements godotengine/godot-proposals#3976

I added support for multitouch by using screen events instead of mouse events when touch screen is available.

Add a Touchscreen Only boolean property to BaseButton (similar to TouchScreenButton's existing property).

I couldn't figure out how this boolean should work.

Do we want to have a hover if the touch started outside and dragged inside the button?

@Kazox61 Kazox61 requested a review from a team as a code owner September 25, 2025 12:27
@AThousandShips AThousandShips changed the title Basebutton: multitouch support BaseButton: multitouch support Sep 25, 2025
@AThousandShips AThousandShips added this to the 4.x milestone Sep 25, 2025
@Calinou
Copy link
Copy Markdown
Member

Calinou commented Sep 25, 2025

I couldn't figure out how this boolean should work.

Something like this should be at the beginning of NOTIFICATION_DRAW:

if (!Engine::get_singleton()->is_editor_hint() && !DisplayServer::get_singleton()->is_touchscreen_available() && visibility == VISIBILITY_TOUCHSCREEN_ONLY) {
return;
}

This should also be done where input is processed, so that input is skipped if the button is invisible because the user doesn't have a touchscreen currently.

@Kazox61 Kazox61 requested review from a team as code owners September 25, 2025 12:54
@Kazox61 Kazox61 requested review from a team September 25, 2025 12:54
@Kazox61 Kazox61 requested a review from a team as a code owner September 25, 2025 12:54
@Kazox61 Kazox61 requested a review from a team September 25, 2025 12:54
@Kazox61 Kazox61 requested review from a team as code owners September 25, 2025 12:54
@Kazox61
Copy link
Copy Markdown
Contributor Author

Kazox61 commented Sep 25, 2025

I couldn't figure out how this boolean should work.

Something like this should be at the beginning of NOTIFICATION_DRAW:

if (!Engine::get_singleton()->is_editor_hint() && !DisplayServer::get_singleton()->is_touchscreen_available() && visibility == VISIBILITY_TOUCHSCREEN_ONLY) {
return;
}

This should also be done where input is processed, so that input is skipped if the button is invisible because the user doesn't have a touchscreen currently.

Thanks for the instructions. Added the boolean touchscreen_only.

@AThousandShips AThousandShips marked this pull request as draft September 25, 2025 14:56
@AThousandShips
Copy link
Copy Markdown
Member

You've accidentally merged the master branch into your branch incorrectly taking ownership over the changes, to prevent this make sure you rebase to update your branch, to fix this you need to remove the merge commit with master and rebase instead, see here

@Kazox61 Kazox61 force-pushed the basebutton-multitouch branch from 71ead7f to 885808d Compare September 25, 2025 15:11
@Kazox61
Copy link
Copy Markdown
Contributor Author

Kazox61 commented Sep 25, 2025

Is it now fixed?

@AThousandShips AThousandShips marked this pull request as ready for review September 25, 2025 15:20
@AThousandShips AThousandShips removed request for a team September 25, 2025 15:20
Copy link
Copy Markdown
Member

@Alex2782 Alex2782 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it on Android and didn't find any critical regressions. We can still optimize some minor things in the code.

Comment thread scene/gui/base_button.h Outdated
Comment thread scene/gui/base_button.cpp Outdated
Comment thread scene/gui/base_button.cpp
@Alex2782
Copy link
Copy Markdown
Member

Alex2782 commented Mar 24, 2026

Do we want to have a hover if the touch started outside and dragged inside the button?

Hover test, Video
Screen_recording_20260324_041445.mp4

I believe the following bug fix is ​​needed to solve it properly.

Only after that is it possible to receive InputEventScreenTouch, when the finger inside the button is no longer touching the touchscreen.

			if (!over) {
				over = gui_find_control(pos);
			}

here in the Viewport: https://github.com/godotengine/godot/blob/master/scene/main/viewport.cpp#L2238-L2251

It still needs to be checked whether this change leads to regressions.
EDIT: A safer/better solution would probably be to force an InputEventScreenDragevent with pressed = false (if possible) instead.

What also doesn't work yet is the multitouch hover effect; a maximum of 1 button changes the hover status.

Bildschirmfoto 2026-03-24 um 03 59 12

@Kazox61
Copy link
Copy Markdown
Contributor Author

Kazox61 commented Mar 27, 2026

Do we even want to have a hovering for touch input, since hovering is technically not possible with a touch input.

@AdriaandeJongh
Copy link
Copy Markdown
Contributor

I agree that it makes little sense to give touches a hovering state.

@Kazox61 Kazox61 force-pushed the basebutton-multitouch branch from 03d76c2 to fffa9f0 Compare March 27, 2026 09:19
@lyuma
Copy link
Copy Markdown
Contributor

lyuma commented Mar 27, 2026

I just want to offer a rebuttal that there are touch inputs that support hovering.

For example, I plan to use multitouch support for XR finger input, and in XR you have full 6 degree of freedom information about the finger locations at all times when using hand tracking.

Also, tablets with a pen and stylus often show the hover location of the stylus. it's not multi-touch, but it is a form of touch input.

I'm not here to block this on hover support for touch input, just trying to make a case that there are applications (such as my VR framework) that may have a use for hover.

@Kazox61
Copy link
Copy Markdown
Contributor Author

Kazox61 commented Mar 30, 2026

Maybe we could add hovering for touch input in a follow up pull request. Currently i don't have a device to test it.

@Kazox61 Kazox61 requested a review from Alex2782 March 30, 2026 09:02
@syntaxerror247 syntaxerror247 self-requested a review March 30, 2026 11:32
Copy link
Copy Markdown
Member

@Alex2782 Alex2782 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retested on Android: Touch input and Bluetooth mouse.

No critical regressions detected, this PR behaves like version 4.6-official (with input emulation enabled).

I believe a lot more work is needed before proposal #3976 can be closed / before TouchScreenButton can be marked as deprecated.

Copy link
Copy Markdown
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good and I wasn't able to detect any regressions from a quick round of testing, so the PR looks good to go!

@m4gr3d
Copy link
Copy Markdown
Contributor

m4gr3d commented Apr 9, 2026

@Kazox61 Can you squash the commits.

@Kazox61 Kazox61 force-pushed the basebutton-multitouch branch 2 times, most recently from d961128 to 7be8ea9 Compare April 9, 2026 09:18
@Kazox61 Kazox61 force-pushed the basebutton-multitouch branch from 7be8ea9 to 13b347b Compare April 9, 2026 09:19
@Repiteo Repiteo modified the milestones: 4.x, 4.7 Apr 9, 2026
@Repiteo
Copy link
Copy Markdown
Contributor

Repiteo commented Apr 9, 2026

Thanks!

@StockerGaming
Copy link
Copy Markdown

Awesome!

@Alex2782
Copy link
Copy Markdown
Member

@Kazox61 Do you still have the motivation and time to fix this issue? #118581
(Otherwise, I'll try my luck, maybe next week)

@shiena
Copy link
Copy Markdown
Contributor

shiena commented Apr 15, 2026

Hi @Alex2782, I took a shot at this in #118608 — turned out on_action_event() was gating button_down emission on status.hovering, which isn't set on the touch path. Feel free to review whenever convenient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.