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: prevent unnecessary rerendering of clusters when nothing has changed #802

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ixam1
Copy link

@ixam1 ixam1 commented Nov 17, 2023

This PR fixes the unnecessary rerendering of clusters when nothing has changed.

Fixes #801 🦕

The fix is pretty simple, just compare the amount of clusters from before and after and if its the same -> dont render.

Correct me if I am wrong I am no expert in the algorithms used here, but this should cover all cases right? Since whenever clusters are changing they will either combine or decluster and in either way the amount of clusters will decrease or increase, so this simple check should work.

without fix with fix
fractional zoom with animation
fractional.not.fixed.mov
fractional.fixed.mov
normal zoom with animation
normal.not.fixed.mov
normal.fixed.mov
fractional zoom without animation
fractional.no.animation.not.fixed.mov

Copy link

google-cla bot commented Nov 17, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@wangela
Copy link
Member

wangela commented Dec 5, 2023

Thanks @ixam1 for offering this fix; please make sure to sign the CLA so that doesn't block merging of your pull request.

@usefulthink
Copy link
Contributor

This doesn't quite sit right with me. It seems to work, but I feel like checking the number of clusters as a proxy for "has anything changed" isn't accurate enough.

Why don't we fix the "changed" flag being returned instead, since obviously the algorithms don't properly signal changes to the markerclusterer?

@gopinath-sixt
Copy link

Thank you for these fixes, @ixam1 . Could you please let me know when we can expect to release them?

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

Successfully merging this pull request may close these issues.

MarkerClusterer: unnecessary rerendering of clusters when nothing has changed
4 participants