-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Improve the performance of remove Snap interaction #14516
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution, @XingGuoZ. Can you please address my suggestions. A bonus would be if you could add a test too.
src/ol/interaction/Snap.js
Outdated
@@ -381,7 +381,8 @@ class Snap extends PointerInteraction { | |||
if (currentMap) { | |||
keys.forEach(unlistenByKey); | |||
keys.length = 0; | |||
features.forEach(this.forEachFeatureRemove_.bind(this)); | |||
this.rBush_.clear(); | |||
features.forEach(this.unlistenFeatureChange_.bind(this)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could speed this up even more by getting rid of the new unlistenFeatureChange_
method and this change:
features.forEach(this.unlistenFeatureChange_.bind(this)); | |
Object.values(this.featureChangeListenerKeys_).forEach(unlistenByKey); | |
this.featureChangeListenerKeys_ = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! It has been modified in this way
631418c
to
b88a1b9
Compare
Just now, I learned about |
📦 Preview the website for this branch here: https://deploy-preview-14516--ol-site.netlify.app/. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @XingGuoZ. I took the liberty to complete the test.
#14515 This commit fixes Issue 14515.
When the Snap Source has 13,000 features,the time of remove Snap Interaction will reduced from 6.5s to 150ms