waynegm / imgNotes

Extension of the jQuery imgViewer plugin to add markers and notes to the image
MIT License
99 stars 26 forks source link

Destroy is time consuming with a large(r) amount of markers #16

Closed ProcSIMus closed 8 years ago

ProcSIMus commented 8 years ago

If you place a few markers on an image the time to destroy/clear the widget is increasing rapidly. Please see this screencast that shows the behavior with 10 markers: http://screencast.com/t/raedvhLPU

Is there something I can do about this myself? Would be nice to get some advice/assistance because this plugin is very helpful to me except for this issue.

Thanks and best regards, Richard

waynegm commented 8 years ago

Hi Richard, Have you actually tried this on hardware as opposed to an emulator? On my desktop there are no issues destroying a widget with 100 markers.

Looking at the "clear" method code I can't see any scope for a significant speedup - its just a for loop that removes elements from the DOM. As an alternative to the destroy method could you test $(".viewport").remove() This removes the zoomable image widget and all attached markers directly using a single jQuery call.

Regards, Wayne

ProcSIMus commented 8 years ago

Hi Wayne,

I replaced the for loop with the remove function and that improves the clearing time significantly.

When using the plugin on a desktop I also do not experience problems. The situation occurs when the plugin is used in the DevExtreme HTML/JS framework for building hybrid apps. They suggested another possible solution:

Replace: $.each(self.notes, function() { var $elem = $(this); $elem.remove(); }); By: $(self.img.parentNode).remove('.marker .black');

This also reduces the time to clear a lot...

Could you comment on this? Which way would have you preference? I could reproduce the behavior native on a iPhone 5S and iPad Mini 4 with iOS 9...

Thanks and best regards, Richard

waynegm commented 8 years ago

Richard, It seems you are using release 0.7.5 from March last year. Since that release another user submitted a commit that replaced the $.each loop in the imgNotes clear function with a for loop to improve the speed of clearing note markers. The jQuery $.each construct tends to be a performance bottleneck. Suggest you download the current (unreleased but working) code base from the master branch of the GitHub repository and give that a try. It should solve your performance problem without having to resort to using selectors to remove the widget and markers.

The other approaches of directly removing elements from the DOM using class selectors are OK but there would be an issue if you have multiple imgNote widgets on a page. In that case using the class selector to trigger the remove would cause all the widgets and markers on the page to disappear which may not be what you want.

Regards, Wayne

ProcSIMus commented 8 years ago

Thanks Wayne! Found the new version and implemented it. Works like a charm...

Best regards, Richard