Immediate-Mode-UI / Nuklear

A single-header ANSI C immediate mode cross-platform GUI library
https://immediate-mode-ui.github.io/Nuklear/doc/index.html
8.89k stars 533 forks source link

Fix click drag on touch + WebBrowsers #563

Closed FrostKiwi closed 11 months ago

FrostKiwi commented 1 year ago

This one has been driving me insane. Each time I tested my WebApp on a touch device, like a Smartphone, tablet, iPad, what have you, Window resizing and the property click-drag behavior was busted. Touch works a bit differently on Webbrosers and doesn't update the mouse position regularly, which leads to huge deltas, as Nuklear just saves the previous frame's mouse position.

This PR fixes this by setting the drag during the click part of the click-drag to zero. This fixes the breaking behaviour in the video below. Thanks to Chrome's mobile device emulation, I tracked it down. Why this doesn't occur on Scrollbars and Window dragging, I'm not sure, but this seems to be a fix without having a negative effect on other things.

https://github.com/Immediate-Mode-UI/Nuklear/assets/60887273/f52751e8-b481-4430-97f6-ae884fb5a96d

dumblob commented 1 year ago

Just a clarification - the video recording of your setup is before or after applying this patch?

It looks horrendously jumpy - "touch and drag" makes the window resize seemingly in the next frame and first then starts the actuall "resize" operation from there on.

FrostKiwi commented 1 year ago

Just a clarification - the video recording of your setup is before or after applying this patch?

It looks horrendously jumpy - "touch and drag" makes the window resize seemingly in the next frame and first then starts the actuall "resize" operation from there on.

Ohh, should have led with that^^ The video is before the patch. That horrible jumping is exactly what has driving me insane and can also be observed straight in the SDL2 Web demo of Nuklear, when using the browser on a touch enabled device.

dumblob commented 1 year ago

I have to admit I do not have any quick way to test this now. Would you mind if I left this PR opened until someone picks it up and tests it?

It sounds too important to make a mistake - so I would rather want others to test it with several backends on mobile as well as non-mobile devices.

FrostKiwi commented 1 year ago

I have to admit I do not have any quick way to test this now. Would you mind if I left this PR opened until someone picks it up and tests it?

It sounds too important to make a mistake - so I would rather want others to test it with several backends on mobile as well as non-mobile devices.

Sure 👍 I will have the WebApp here if you want to test it with your own: https://mirror.frost.kiwi It will replaced by a pure js rewrite next week (almost done), but the WASM + Nuklear branches will stay online. In that state the touch jumping is fixed via commit https://github.com/FrostKiwi/frostorama/commit/d66f393d50697207e90c67c6dab621c49c0d30a9 And this is the commit prior with all the jumping and stuff: https://github.com/FrostKiwi/frostorama/commit/5ec9aa45dad4ce28af36e4214d64996b039fa2d8

Since the Nuklear WebApp is autocompiled via GitHub actions and auto-published via GitHub pages, you can just make changes to compare right in GitHub and watch it being deployed 5 mins later automatically. https://github.com/FrostKiwi/frostorama/actions/runs/5377023469/jobs/9754955853

The SDL2 demo within this repo should show the same behavior: https://github.com/Immediate-Mode-UI/Nuklear/tree/master/demo/sdl_opengles2

You can test via a mobile, touch enabled device or by going into chrome and turning on mobile device emulation in the dev panel. The crux of the bug stems from the way input is not being updated, even though the event and rendering loop are, which is an optimization that browsers perform, leading to big deltas being computed in this line https://github.com/Immediate-Mode-UI/Nuklear/blob/ca2a26083a5881c376b7fe628662c7898d73afd1/src/nuklear_input.c#L53

RobLoach commented 11 months ago

After a bit more testing, I think this is good to go. Let's merge this, and then revert if there are reports of mouse movement being borked.

dumblob commented 11 months ago

Thanks @RobLoach for spending some time with & merging this (also all the other PRs recently). Really appreciated!