Skip to content
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

Fix InputEventScreenDrag on Android #84331

Merged
merged 1 commit into from
May 21, 2024

Conversation

Alex2782
Copy link
Contributor

@Alex2782 Alex2782 commented Nov 1, 2023

Fixes: #84138
Fixes: #84182

I haven't checked the effects on other events yet.
Event onScroll is not fired directly and is not so good for 'ScreenDrag':

androidx.core sources

image
image

Tests

test-project.zip

v4.2.beta2.official [f8818f8]

The "Drag At" and "Motion At" values are higher (the fingers have to be moved more)

beta2-Screen_recording_20231101_233924.mp4

this PR:

PR_Screen_recording_20231101_231239.mp4

Copy link
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 logic looks good.

Can you add comments to the added code to clarify what it does.

Copy link
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.

I'm seeing some scrolling artifacts when testing these changes using the Android editor; can you address them.

Screen_Recording_20240213_154028_Godot.Editor.4.debug.mp4

@Alex2782
Copy link
Contributor Author

I'll try to find time for it again in 2 or 3 days.

@Alex2782
Copy link
Contributor Author

@m4gr3d The new solution should be better, easier to understand and probably also perform better. I tried out various gestures with Godot Editor and checked them in Logcat. Checking 'pointerCount' is not necessary as it was in my old version.

@Alex2782 Alex2782 requested a review from m4gr3d February 21, 2024 23:55
@Alex2782 Alex2782 closed this Mar 27, 2024
@Alex2782 Alex2782 deleted the debug-InputEventScreenDrag branch March 27, 2024 22:01
@AThousandShips AThousandShips removed this from the 4.3 milestone Mar 27, 2024
@akien-mga
Copy link
Member

@Alex2782 Did you intend to close this? Or did you delete the branch by mistake while cleaning up?

@Alex2782 Alex2782 restored the debug-InputEventScreenDrag branch March 27, 2024 23:58
@Alex2782
Copy link
Contributor Author

Alex2782 commented Mar 27, 2024

Sorry, my mistake, I didn't think it would be deleted here too. Can it be reopened or should I create a new PR? @AThousandShips / @akien-mga

@Zireael07
Copy link
Contributor

@Alex2782: IIRC a deleted branch can't be reopened, but check your Git GUI, it might have some undo options...

@AThousandShips AThousandShips added this to the 4.3 milestone Mar 28, 2024
@m4gr3d
Copy link
Contributor

m4gr3d commented May 20, 2024

@Alex2782 Your commit still had a few issues that I addressed in db6f1c9e2f36721d7a7f7fac17f4baebdb31615d. Can you take a look and merge the commit together if the updates look good.

Copy link
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.

Approved with the updates included in db6f1c9e2f36721d7a7f7fac17f4baebdb31615d.

The commits will need to be squashed together prior to merging.

@YeldhamDev YeldhamDev requested review from a team and removed request for a team May 20, 2024 23:28
@akien-mga akien-mga merged commit a720ce3 into godotengine:master May 21, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Not Critical
5 participants