Skip to content
This repository was archived by the owner on Aug 8, 2023. It is now read-only.

Conversation

@ChrisLoer
Copy link
Contributor

@ChrisLoer ChrisLoer commented Dec 10, 2018

Fixes issue #13506.

  • Setting the value to zero caused divide-by-zero errors
  • Small non-zero values worked, but were unexpectedly expensive (because of requiring frequent collision detection)

Although this commit reverts the configurable duration, it leaves the "enablePlacementTransitions" flag intact.

cc @ansis @tmpsantos @brunoabinader

@ChrisLoer ChrisLoer requested a review from ansis December 10, 2018 22:02
@ChrisLoer
Copy link
Contributor Author

cc @tobrun

@ChrisLoer
Copy link
Contributor Author

Actually I convinced myself we don't need to revert the configurable duration, we just need to preserve the logic that does placement at >=300ms intervals even if fading is set to happen faster. I think I was misunderstanding what I was seeing running the repro case in the Android debugger.

Fixes issue #13506 -- transition duration of 0 would cause symbol flickering.
@ChrisLoer ChrisLoer changed the title [core] Revert change to use transition duration for collision detection. [core] Handle transition duration of 0ms for symbols Dec 11, 2018
@ChrisLoer
Copy link
Contributor Author

Updated this PR to just do the smaller change of handling the "divide by zero" case. The symbol fade duration still inherits from the map's "transition duration", and I verified that if it's set to less than 300ms, collision detection is still rate limited to every 300ms (although the fade duration itself is still shortened).

@brunoabinader
Copy link
Contributor

Thanks @ChrisLoer 👍

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