search

thriveweb / photoswipe-masonry-v2

PhotoSwipe Masonry takes advantage of the built in gallery features of WordPress. The gallery is built using PhotoSwipe from Dmitry Semenov.
http://thriveweb.com.au
23 stars 6 forks source link

Masonry option is slow for large image galleries #15

Open Jon007 opened 7 years ago

Jon007 commented 7 years ago

Masonry option is relatively slow for large image galleries when compared to eg Robogallery which implements a lazy load masonry style gallery: an initial set of images are loaded into a masonry layout and more are added on scroll so initial load is fully minimized.

thriveweb commented 7 years ago

I'm going to look at Ajax Masonry this when I get a chance

See issue https://github.com/thriveweb/photoswipe-masonry/issues/11

thriveweb commented 7 years ago

Please see the latest commit to see this in action.

Jon007 commented 7 years ago

I did test the progressive loading with twenty-sixteen theme.

It works quite well, however in certain circumstances, where there is text after the gallery, it is possible to entirely scroll past the gallery onto further post text without noticing that there are more pictures to load, or alternatively getting into a battle between attempting the read the text and being frustrated by more pictures jumping up and pushing the text out of the way.

So generally it works well, but you may or may not want to turn it on/off for certain pages. My main issue would be with the use of Settings\Photoswipe for this [and in fact all the other settings on this page].

It would be useful to have all these settings in the [gallery] shortcode itself rather than enforced globally for the whole site:

Jon007 commented 7 years ago

I tested this out a bit further on low bandwidth connection, which you can easily do with Chrome developer tools. On the current checked in version, the code is downloading the full size version of all images, it isn't using any thumbnails. There is a saving in displaying some of the images before all are loaded, however the browser is still spinning dowloading the rest of the images, it is not an on-demand loading.

Current progressive loading test on large gallery (over 50 images and using [gallery size="large"] using "Good 3G" throttling: 91 requests, 41.7MB transferred: Timing: 3.42s - DOMContentLoaded 30s - first renderering of images 50s approx, masonry shuffling images 3.7minutes - load complete

Same page on Jon007 non-progressive version 152 requests, 45.1MB transferred 17.35s - DomContentLoaded 4.2 minutes - load complete

The non-progressive version is clearly a little weightier, however actually loaded faster:

Ultimately, since both versions are loading all of the full size images, there is no real optimization from the progressive version yet. The progressive version has saved a small amount of bandwidth only by not using thumbnails, however this has made the initial display time worse.

If I repeat the test for example using [gallery size="medium"]:

non-progressive version: 151requests, 43.1MB transferred 5.62s - DomContentLoaded - pictures are already visible by this stage 3.8minutes - load complete

Progressive version: 92 requests, 41.7MB downloaded 3.34s DomContentLoaded 50s again it is still redrawing 3.7minutes - load complete

The initial display issues of the progressive version can be solved by merging the codebase so the appropriate size thumbnails are used to speed up the initial display. However this doesn't solve the problem which started this thread: Photoswipe Masonry is expensive and slow in bandwidth as it is always downloading ALL the full size images, even if the user doesn't want to open the lightbox.

Here's the RoboGallery comparison as I mentioned on https://jonmoblog.wordpress.com/2016/12/11/photoswipe/

https://www.inkston.com/stories/people/zhao-mengfu-paintings/ Sorry it's not exactly the same gallery but it has 48 pictures, it's of comparable size. 9.56s DomContentLoaded 102 request, 1.4MB transferred 20.67s - load complete

Jon007 commented 7 years ago

It looks like there’s a simple fix to this, there’s a code block in photoswipe-masonry.js which forces all the full size images to load:

    $.each(items, function(index, value) {
      image[index]     = new Image();
      image[index].src = value['src'];
    });

value[‘src’] has been previously set to the href for the full sized image so this codes is forcing the load of all the full sized images. If you comment this out, the normal and preferable photoswipe behaviour seems to be restored.

If you check http://photoswipe.com/ and watch in debug, you will see that only thumbnails are loaded, then on the first click 3 full size images are loaded, then every advance of 1 image, another image is loaded in the background.

When this code is commented out of photoswipe-masonry, this nice photoswipe lazy loading seems to be restored. And the Ajax Masonry is unnecessary.

I'm not sure how to submit the fix as I already have pull request outstanding with the other changes

thriveweb commented 7 years ago

Please let me know if this is still happening with the new version.

Jon007 commented 7 years ago

well sure... I had already rewritten the javascript and identified the fix...

I can see there's a lot more changes now, but after installing the new version on a couple of sites I get: " Notice: Undefined index: crop_thumbnails in /Users/jon/Sites/waina/wp-content/plugins/photoswipe-masonry/backend/settings-view.php on line 74 /> Crop thumbnails"

also some of the other javascript changes I need I think are not there (eg #10) though the improved structure of the rewrite has obfuscated doing a simple diff comparison.

Jon007 commented 7 years ago

Really all the new stuff should be in a separate branch on GitHub until it is ready. Then you would have been able to get out the quick fixes like this one - commenting out the javascript that forces download of all the full size images - without waiting for the full re-engineering.

thriveweb commented 7 years ago

Agreed. Vincent got a bit carried away! Seems pretty close to a release now do you think?

Jon007 commented 7 years ago

Hmm.. well my version works fine, so it's hard to prioritize testing it all again and re-working my additional changes into the new structure.. I'm not sure there's anything added that I'm missing yet (I don't really want the progressive loading of thumbnails, it was always about the progressive background loading of full images not the thumbnails) .
I do want to get the shortcode options in there, and some of the woo3 goodness. I think I should be a named contributor though if I'm doing more work on it.

thriveweb commented 7 years ago

OK sounds good. Feel free to add your self as a named contributor :) It would be great if you can spend some more time on it.