revolunet / angular-carousel

Mobile friendly AngularJS carousel
http://revolunet.github.io/angular-carousel
MIT License
1.55k stars 708 forks source link

Poor performance and crashing on Galaxy S5, Nexus 5 #175

Open marcsyp opened 10 years ago

marcsyp commented 10 years ago

Hi there. I'm writing an Ionic app and managed to integrate angular-carousel reasonably well by removing the ngClick directive from angular-touch (to avoid conflicts with Ionic).

It works really well on a carousel of 100 slides with 1-5 thumbnails each. However, a carousel with full resolution images (3000x5000) is choking quite a bit. Doesn't matter if it's 10 images or 300. Longer load time, and a jittery/flashy first swipe, followed by laggy swipes, followed by crashes. My markup is as follows.

HTML

ion-content has-header="false"> ul rn-carousel rn-carousel-buffered="3" rn-carousel-index="currentSlide" class="my-slider ng-cloak"> li ng-repeat="selectedPhoto in selectedPhotos track by $index"> div class="photoViewDiv"> div style="background-image: url('{{selectedPhoto}}');" class="photoViewImg"> /div> /li> /ul> /ion-content>

CSS

.photoViewDiv { background: black; max-width: 100%; max-height: 100%; }

.photoViewImg { border: 1px solid #666; /float: left;/ width: 100%; max-height: 100%; padding-bottom: 100%; background-position: center; cursor: pointer;

background-size: cover;

}

JS

Feel free to ask, not sure it's important here.

Any ideas?

marcsyp commented 10 years ago

I'm thinking of implemeting a Canvas approach inside a Flexbox. Any thoughts?

revolunet commented 10 years ago

is 3000x5000 the device resolution ? its huge ! resizing images in the browser have a big impact on perfs

marcsyp commented 10 years ago

Yep.

5312x2988, full resolution captured by the Galaxy S5. I understand that resizing in the browser is memory-intensive. But how to improve performance in the angular-carousel? Will implementing an html canvas for each slide, with a single image in each canvas increase performance dramatically?

Will the carousel work with canvas objects?

On Tue, Jun 24, 2014 at 5:05 PM, Julien Bouquillon <notifications@github.com

wrote:

is 3000x5000 the device resolution ? its huge ! resizing images in the browser have a big impact on perfs

— Reply to this email directly or view it on GitHub https://github.com/revolunet/angular-carousel/issues/175#issuecomment-47046224 .

Marc Syp, Application Engineer San Francisco, CA

WARNING: The information contained in this message is legally privileged and proprietary information intended only for the use of the individual or entity named above. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution, or copy of this message is strictly prohibited. If you have received this message in error, please immediately delete the original message and notify the sender. Communication of the information contained in this message to any unauthorized persons may be a violation of state or federal laws.

revolunet commented 10 years ago

Yep carousel works with any kind of content but i dont get why canvas perf could be better; Its already GPU accelerated with the CSS3d transformations. Not using "cover" should help increase perfo. Maybe try in a empty page outside of the ionic context too

marcsyp commented 10 years ago

Having a hard time getting the CSS right without using "cover", at least with divs. I was under the impression that divs with bg images are faster than img tags. Either way, none of the above helped. Not sure I got fully outside the Ionic context, as it is still within the app but not within ion-content tags. Anyway... Still pretty unmanageable performance.

On Tue, Jun 24, 2014 at 6:05 PM, Julien Bouquillon <notifications@github.com

wrote:

Yep carousel works with any kind of content but i dont get why canvas perf could be better; Its already GPU accelerated with the CSS3d transformations. Not using "cover" should help increase perfo. Maybe try in a empty page outside of the ionic context too

— Reply to this email directly or view it on GitHub https://github.com/revolunet/angular-carousel/issues/175#issuecomment-47049699 .

Marc Syp, Application Engineer San Francisco, CA

WARNING: The information contained in this message is legally privileged and proprietary information intended only for the use of the individual or entity named above. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution, or copy of this message is strictly prohibited. If you have received this message in error, please immediately delete the original message and notify the sender. Communication of the information contained in this message to any unauthorized persons may be a violation of state or federal laws.

revolunet commented 10 years ago

could be interesting to have an isolated example with these giant images to benchmark/analyse

marcsyp commented 10 years ago

I will try to create an isolated example for these purposes when I have a chance. In the meantime, the idea behind using html canvas was that performance is supposedly better than div background rendering, and also I was thinking I could use canvas to resize with JS:

http://stackoverflow.com/questions/18922880/html5-canvas-resize-downscale-image-high-quality

However, I'm not sure whether the scaling would result in any performance increase, or how to integrate these techniques with the angular-carousel so that only the relevant images get scaled as part of the lazy loading process. I was thinking that I could use a $scope.watch() to get the current index and trigger scaling of the current set of slides iteratively. Seems like a big commitment for an amateur such as myself to take on just on a hunch, which is why I brought the question to this forum. Either way, I'm kind of blocked on my app development until I can resolve this issue...

Another note re: html canvas. Due to the Galaxy S5's lack of rotation of the actual file, there is a good chance that I will have to rotate images based on EXIF data, and my first thought was to use canvas to do this...

On Wed, Jun 25, 2014 at 12:25 AM, Julien Bouquillon < notifications@github.com> wrote:

could be interesting to have an isolated example with these giant images to benchmark/analyse

— Reply to this email directly or view it on GitHub https://github.com/revolunet/angular-carousel/issues/175#issuecomment-47068407 .

Marc Syp, Application Engineer San Francisco, CA

WARNING: The information contained in this message is legally privileged and proprietary information intended only for the use of the individual or entity named above. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution, or copy of this message is strictly prohibited. If you have received this message in error, please immediately delete the original message and notify the sender. Communication of the information contained in this message to any unauthorized persons may be a violation of state or federal laws.

marcsyp commented 10 years ago

Note: I implemented an image scaling directive in Angular, but I'm having some problems. Here is the directive:

app.directive("imageResize", [ "$parse", function($parse) { return { link: function(scope, elm, attrs) { var imagePercent; imagePercent = $parse(attrs.imagePercent)(scope); elm.bind("load", function(e) { elm.unbind("load"); //Hack to ensure load is called only once var canvas, ctx, neededHeight, neededWidth; neededHeight = elm[0].naturalHeight * imagePercent / 100; neededWidth = elm[0].naturalWidth * imagePercent / 100; canvas = document.createElement("canvas"); canvas.width = neededWidth; canvas.height = neededHeight; ctx = canvas.getContext("2d"); ctx.drawImage(elm[0], 0, 0, neededWidth, neededHeight); elm.attr('src', canvas.toDataURL("image/jpeg")); }); } }; } ]);

The directive works on the first set of slides in the carousel, but as soon as the first swipe is performed, the images revert to their full, non-scaled versions.

Also, the performance gain for the swiping carousel is good but load time is not any better, in fact it's probably a bit worse.

Any thoughts?

marcsyp commented 10 years ago

HMTL markup that I'm using:

ul rn-carousel rn-carousel-buffered rn-carousel-index="currentSlide" class="my-slider ng-cloak"> li ng-repeat="selectedPhoto in selectedPhotos track by $index"> div on-hold="openPhotoModal($index)" > img ng-src="{{selectedPhoto.path}}" image-resize image-percent="10" />
/div> /li> /ul>

revolunet commented 9 years ago

btw, this document could be useful if you report other issues somewhere : https://guides.github.com/features/mastering-markdown/

rlaurente commented 9 years ago

Hi revolunet,

Is there any updates on this issue? I still having this error with 1080x1830 resolution.

thanks

marksy commented 9 years ago

@rlaurente (i'm a bit late to your reply but) did you try using the option "rn-carousel-buffered"? It loads a maximum of 5 items into the DOM at a time so should work better with lots of carousel items.

sunshineo commented 9 years ago

ng-if ? https://docs.angularjs.org/api/ng/directive/ngIf