blasten / turn.js

The page flip effect for HTML5
www.turnjs.com
Other
7.23k stars 2.48k forks source link

Dynamically removing pages is very slow #382

Open davej opened 11 years ago

davej commented 11 years ago

I have a 40 page book, if I dynamically remove every page (except the cover) using $('#book').turn('removePage', i) it is very slow.

Using the Chrome timeline I was able to measure it at ~2.24 seconds on my 2012 Macbook Air. Running the same operation on a new iPad (iOS6) takes about 14 seconds. Looking in the timeline, there is a whole bunch of rendering calls to Recalculate Style and Layout.

screen shot 2013-08-02 at 14 42 58

Can this be done more efficiently by batching the style changes so that the browser isn't forced to do a layout after each one? Or possibly batching the removal of pages by using something like $('#book').turn('removePage', [2,3,4,5,6,7,8..]).

From quickly debugging what's going on it seems that the issue might be the repeated calls to _pageSize() (stack trace: turnMethods.removePage -> turnMethods._movePages -> move -> _pageSize()) which calls the $.fn.width() and $.fn.height() height methods (which force a style recalculation).

Can you advise if there are any hacks that you can think of to make this faster? Also, is this something that you think will be improved in an upcoming release?

By the way I'm using the licensed version of turn.js (4th release)

alQemist commented 11 years ago

Why are you removing pages ? If you have a 40 page book it only has 6 pages loaded at any given time. The way I understand it there is no need to remove a page unless you are changing the source of that page and it is one of the 6 pages that are (pre)loaded. Am I missing something here ?

davej commented 11 years ago

@alQemist: The content of the pages is changing dynamically based on user actions. So when a user makes a change then I need to delete all of the pages and add the updated pages instead.

alQemist commented 11 years ago

Again as I said you should only have to remove the 6 pages that are pre-loaded that should happen in the blink of an eye....are you loading more than 6 pages. I do something similar toggling between sources for pages - for example switching between pages as images and pages as HTML and seen no issue with response times....Do you have an example online ?

davej commented 11 years ago

@alQemist: Sorry I don't have an example online because it is under development and I don't have approval to make it public.

I understand that there are only six pages in the DOM at any one time, but I have 40 pages added to turn (using $.fn.turn("addPage", html, index) and I need to remove them so that I can repopulate the book from scratch.

I'm not sure what you're suggesting? Even though there are only 6 pages in the DOM at any one time, the book still needs to be fully populated so that I can get a page count and the user can use keyboard shortcuts to jump to page x.

alQemist commented 11 years ago

Personally I would not load ALL pages - the 6 loaded pages works great in my case and if readers need to jump beyond the 6 pages they load dynamically as needed - Also I use a separate method for getting the book details such as page count - then I use that data to configure the book with the page count for example. I think what you are experiencing is a "tax" for loading all the pages in the book rather than let turn.js manage page loading. Emmanuel seems to have worked out the optimal model for loading / removing pages. Hope I understand correctly

davej commented 11 years ago

@alQemist I'm not really understanding what you're saying. I am letting turn.js manage page loading. I am using the turn.js methods for adding/removing pages.

alQemist commented 11 years ago

All I am saying is that it is unnecessary to load or remove more than 6 pages unless you have modified the original code.

davej commented 11 years ago

Ok, thanks. @blasten could you comment on whether the approach suggested by @alQemist would be a good one to pursue? And perhaps you might also have some comment about the performance issues with .turn('removePage').

blasten commented 11 years ago

Hi @davej that's definitely a good point. The performance of the upcoming turn.js (Sep, 2013) is way better. Now, I was thinking about a removePages method.

davej commented 11 years ago

@blasten Thanks! Are you taking on any beta testers for the new version? Earlier versions of Turn.js don't seem to be targeted towards dynamically adding/remove pages in a performant way. We'd love to take a look and see if the new version would work for us. I think we could provide some good feedback for.

Just to let you know how we are using turn... We create a book based on a user's activity stream (from Facebook or other sources) that can be dynamically edited inline (within the turn.js book) by the user. The book is then sent to PhantomJS to be rendered as a PDF photobook and then printed + shipped out to users.

Everything works very well but the performance of adding/removing pages is much too slow for production.