[Bug/Feature]: updateViewsAsync keeps being invoked for no reason (?)

[alexandernst]

What happened?

I noticed that if I create a Paper with async: false option and a dummy viewport function, the function will get called just a couple of times right after loading a graph. Further invocations of viewport will happen only when I interact in some way with the diagram (when I click on something, when I drag something, etc...).

However, if I create a Paper with async: true option, the viewport function won't stop being called, even if I'm not interacting at all with the diagram.

I searched for anything that might be related to viewport here as well as in the old google groups, but I couldn't find anything, so I'm not really sure if this is an expected behaviour or not.

So I did what I usually do: debug the code and find out what's actually going on. It turns out updateViewsAsync (Paper.mjs) is being invoked whenever JS has access to a new tick (nextFrame). This function calls checkViewport, which then calls checkMountedView, which then runs viewportFn.call, which is my viewport function.

I have a couple of questions:

1. Does it make sense for updateViewsAsync to have some sort of check / sleep / pause that will invoke it only when it's actually required, as in, when there are actual changes that might require to trigger a re-render. Is that even feasible? This code looks quite expensive, running it on each tick might be affecting negatively the performance (and battery life / power consumption?)

2. Is it possible to, somehow, avoid invoking the viewport callback if nothing changed?

Version

3.6.x

What browsers are you seeing the problem on?

Chrome, Safari

What operating system are you seeing the problem on?

Mac

[slavede]

This also happens for me, even though I have called

paper.remove() at the end

It causes big performance issues if you are keep navigating to that paper. You just can't destroy it/stop it.

Could you have some property on the paper (which indicates if it's removed/destroyed) and if so, don't call nextFrame anymore?

[kumilingus]

@alexandernst

It's not a bug. It is by design.

There is a plan to extend it so the paper will put itself to sleep when idle or do some chores e.g. cleaning the paper z-index pivots. And then wake up when there are updates scheduled.

Is it possible to, somehow, avoid invoking the viewport callback if nothing changed?

Not sure, I need to investigate. Please bear with me (I'll probably look into it after the release).

---

@slavede

I can see there is paper.freeze() called when the paper is removed.

Are you sure this is happening? Could you create a demo showing the issue?

[kumilingus]

Hi @alexandernst, this might work for the time being.

const defaultOnViewUpdate = paper.options.onViewUpdate;

paper.options.onViewUpdate = function(view, flag, priority, opt, paper) {
    if (paper.isFrozen()) {
        console.log('unfrozen');
        paper.unfreeze();
    }
    defaultOnViewUpdate(view, flag, priority, opt, paper);
},

paper.on('render:done', () => {
    console.log('frozen');
    paper.freeze();
});

[slavede]

@kumilingus I have trouble reproducing it as a simple paper as in my project there is a lot of async things happening in parallel with the paper, but I'm sure .remove is being called (and it seems that some other parallel action caused another recursive call of updateViewsAsync).

What I have added on my side as an override is this:

    const targetId = this.el?.getAttribute('id');
    if (targetId && document.getElementById(targetId)) {
      updates.id = joint.util.nextFrame(this.updateViewsAsync, this, opt, data);
    }

Maybe it wouldn't harm to have this check if the element is still in the DOM.

[kumilingus]

@slavede I see. You are probably calling paper.unfreeze() after the paper was removed.

If you want to check if the paper element is in the DOM, I think it is safe to use isConnected property now.

if (this.el.isConnected) {
  updates.id = joint.util.nextFrame(this.updateViewsAsync, this, opt, data);
}

But I am afraid this is a breaking change. The first animation frame is requested on paper initialization (and the paper element does not have to be in the DOM yet).

We could add some info to internal this._updates object in onRemove() method though.

if (!updates.disabled) {
  updates.id = joint.util.nextFrame(this.updateViewsAsync, this, opt, data);
}

Or perhaps just throw an exception and let the users handle it on the application level (I kind of like this one).

if (updates.disabled) throw new Error('dia.Paper: can not unfreeze the paper after it was removed');
updates.id = joint.util.nextFrame(this.updateViewsAsync, this, opt, data);

[kumilingus]

@slavede it's implemented here: #2130

Please note that if the paper is removed, it is removed along with all the cell views. This means that it is not possible that the viewport function is triggered after removal for any of the views.

[kumilingus]

Fixed by #2134 .