godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.03k stars 21.18k forks source link

Incorrect Calculation of event.relative for InputEventScreenDrag with Multitouch in Web Version #94346

Open the-asind opened 4 months ago

the-asind commented 4 months ago

Tested versions

Reproducible in:

System Information:

System information

Android 12-13, iOS 17, web, gl_compatible,

Issue description

When running in the web version of Godot Engine 4 (regardless of Thread Support), there are issues with the calculation of event.relative for InputEventScreenDrag during multitouch. Specifically, other touches seem to influence the relative values of different touches, despite the position for each touch index remaining unchanged.

image

In the video example, you can see that the right hand (which controls the camera) I'm barely moving (can be tracked by Position at index 0). And the actions of the second index clearly affect the relative of index 0

https://github.com/user-attachments/assets/f4eacb8d-1692-4f1e-8d1e-8111af22a11e

Steps to reproduce

  1. Run the provided example code in the web version of Godot Engine 4.
  2. Perform multitouch gestures on the screen.
  3. Observe the inconsistent behavior of event.relative values.

Expected Behavior: Each touch should independently have accurate event.relative values based on its movement, unaffected by other touches.

Actual Behavior: The event.relative values are incorrectly influenced by other touches, resulting in erratic and incorrect behavior.

Example Code with Issue:

extends Node

var sensitivity := 0.5

func _input(event):
    if event is InputEventScreenDrag and event.index == 0:
        # Check if the touch event is pressed and in the right half of the screen
        if event.position.x >= get_viewport().size.x / 2:
            %head.rotate_x(-deg_to_rad(event.screen_relative.y * sensitivity))
            rotate_y(-deg_to_rad(event.screen_relative.x * sensitivity))

Example Code with Fix (Reinventing relative):

extends Node

var last_position := Vector2(0, 0)
var is_dragging := false
var relative

const sensitivity := 0.5

func _input(event):
    if event is InputEventScreenDrag:
        # Check if the touch event is pressed and in the right half of the screen
        if event.position.x >= get_viewport().size.x / 2:
            if is_dragging:
                relative = event.position - last_position
                %head.rotate_x(-deg_to_rad(relative.y * sensitivity))
                rotate_y(-deg_to_rad(relative.x * sensitivity))
            last_position = event.position
            is_dragging = true
    elif event is InputEventScreenTouch:
        if not event.pressed:
            is_dragging = false

Minimal reproduction project (MRP)

First Person Controller.zip

bypie5 commented 3 months ago

I just opened up a PR to hopefully address this issue. Here is the root cause I found during my testing - this explanation is taken from the PR.


Description

The bug was being caused because the value of InputEventScreenDrag.relative was sometimes being calculated with an event with a mismatched identifier/index. This happened because relative was being computed without checking if the coordinates of the point stored in the touches array in DisplayServerWeb had a matching identifier/index.

Example

Consider the following example. A user has two fingers dragging on a touch screen. At frame 1, the TouchList of the touchmove event fired would contain two items: [{identifier: 0, clientX: 15, clientY: 110, ...}, {identifier: 1, clientX: 20, clientY: 110, ...}]. Let's say the finger for identifier: 0 stops moving, but is still touching the screen. The next time touchmove fires, the TouchList looks like this: [{identifier: 1, clientX: 5, clientY: 110, ...}]. So when relative for identifier: 1 is computed, it uses the TouchList from frame 1 to compute the value. As mentioned above, the current implementation assumes that the TouchList contains the same touch with the same identifier at the index it was last frame. But, this is not always the case. Therefore, this causes relative to be (10, 0) instead of the expected (15, 0).