Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

Prevent items from being repeatedly redrawn while off screen #3633

Merged
merged 2 commits into from
Nov 4, 2017

Conversation

YoshiWalsh
Copy link
Contributor

This fixes #3249 again, because my previous PR #3250 got overwritten. (Possibly by #3475) Additionally, this time I feel that this change is a bit more robust than my previous one.

The problem

When an item is outside of the currently visible range, it is hidden by Group.prototype._checkIfVisible(). This detaches the item from the DOM. The next time the group containing the item is drawn, Group.prototype._redrawItems calls RangeItem.prototype.redraw(), which in turn calls RangeItem.prototype._appendDomElement(). This function detects that the item is not attached to the DOM and re-attaches it, triggering a reflow event.

In my application I have several thousand items which are offscreen at a given time, and so this unnecessary redrawing degrades performance quite significantly.

My solution

Currently when a group is drawn, each item is shown (and redrawn) if it currently not displayed. With my change, each item is shown if it's currently not displayed AND it either should be visible, or it's never been visible before.

This correctly gets the size of elements when they are first added, doesn't update the size of the element while it's not visible but immediately updates the size once the element becomes visible again.

Performance impact

It's hard to produce a minimal example illustrating this issue because it only really becomes noticeable when you have thousands of items, each one with a non-trivial template. I hope it will be sufficient to provide some performance numbers from my own app:

Before this patch, when I panned/zoomed the timeline in my application I would dip to 7-10fps. Now, my app only dips to 16-17fps. (I'm still working on improving performance in my app)

Joshua Walsh added 2 commits October 30, 2017 12:52
Copy link
Contributor

@yotamberk yotamberk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! I'm sorry your PR has been overwritten by something I did. I guess I missed something.
Thanks again for your contribution!

@yotamberk yotamberk merged commit 658324f into almende:develop Nov 4, 2017
mojoaxel pushed a commit to visjs/vis_original that referenced this pull request Jun 9, 2019
…#3633)

* Prevent items from being repeatedly redrawn while off screen

Fixes almende#3249 again, because my previous PR got overwritten

* Add JSDocs for almende#3633
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants