Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Optimise fling animation #503

Merged
merged 4 commits into from
Jul 29, 2020
Merged

Optimise fling animation #503

merged 4 commits into from
Jul 29, 2020

Conversation

Chaoba
Copy link
Contributor

@Chaoba Chaoba commented Jul 28, 2020

Test on xiaomi 2 pixel ratio 2
Before:

ezgif com-resize (1)

After:
ezgif com-resize (2)

Smaller VELOCITY_THRESHOLD_IGNORE_FLING allows small fling, which makes map pan appear smoother.
Longer ANIMATION_DURATION_FLING_BASE makes small fling appear smoother.
@Chaoba Chaoba self-assigned this Jul 28, 2020
@Chaoba Chaoba requested a review from a team July 28, 2020 11:54
@@ -426,8 +426,8 @@ public boolean onFling(MotionEvent e1, MotionEvent e2, float velocityX, float ve
// tilt results in a bigger translation, limiting input for #5281
double tilt = transform.getTilt();
double tiltFactor = 1.5 + ((tilt != 0) ? (tilt / 10) : 0);
double offsetX = velocityX / tiltFactor / screenDensity;
double offsetY = velocityY / tiltFactor / screenDensity;
double offsetX = velocityX / tiltFactor / 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, why actually 3? 🤔
Will it work OK on non low-density devices?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The pixel ratio is usual 2 for low-density devices and 3 for others. Use 3 here the fling offset will be shorter on low-density devices. But let's make it more robust in case there are higher density devices.

@Chaoba Chaoba merged commit 8d2a86a into master Jul 29, 2020
@Chaoba Chaoba deleted the kl-fast-fling branch July 29, 2020 00:46
@ystsoi
Copy link
Contributor

ystsoi commented Jul 29, 2020

I am afraid that 671f1cb will cause "slow fling issue on devices with high-density screen" in the future.

As mentioned in #340 (comment), offsets will be further adjusted by NativeMapView.moveBy later. So the actual result offset will be "velocityX / tiltFactor / (screenDensity^2)". This will cause issues if the screenDensity increases to a higher value in the future.

The number 3 can be considered as a special factor for tuning. Intuitively, setting it to 3 means to make all devices behave the same as how Mapbox behaves on devices with screen density of 3 currently. Setting it to a lower value will make fling faster on all devices. Setting it to a higher value will make fling slower on all devices.

Chaoba pushed a commit that referenced this pull request Jul 29, 2020
@Chaoba
Copy link
Contributor Author

Chaoba commented Jul 31, 2020

@ystsoi Thanks for your reminder. Can you try it by setting mapbox:mapbox_pixelRatio="xx" in the layout file and see if it will be slower?

@ystsoi
Copy link
Contributor

ystsoi commented Jul 31, 2020

@Chaoba Do you mean setting a high value of mapbox:mapbox_pixelRatio, and see whether fling will be slower? I think that I will try to create an emulator of high-density to test first.

@Chaoba
Copy link
Contributor Author

Chaoba commented Jul 31, 2020

@ystsoi Right. If you set this value, it will overwrite the device density and no need to create an emulator with high-density.

@ystsoi
Copy link
Contributor

ystsoi commented Jul 31, 2020

But I remembered that a smaller mapbox:mapbox_pixelRatio will make all objects smaller, and a larger mapbox:mapbox_pixelRatio will make objects bigger. Although the result should be the same, this may be less convincing.

@ystsoi
Copy link
Contributor

ystsoi commented Aug 1, 2020

I cannot create an emulator with screen density more than 4.

For mapbox:mapbox_pixelRatio, as expected, a higher value will make fling slower, using the committed version. But it seems that it does not feel too bad even if fling is slow.

But I found two other issues:

  1. On a phone with screen density of 3, if I scale up the map 2x by setting mapbox:mapbox_pixelRatio to 6, fling will be slower. But on a phone with screen density of 1.5, if I scale up 2x by setting to 3, fling will not be slower. This is not an issue if fling speed does not depend on screen density.

  2. onFling() uses uiSettings.getPixelRatio() as the screen density. Is it that it should use the actual screen density instead? It seems that the actual one is needed in order to implement either constant or varying fling speed correctly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants