Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call LoseContext to prevent WARNING: Too many active WebGL contexts. Oldest context will be lost. #1386

Closed
Gykonik opened this issue Sep 27, 2023 · 7 comments
Labels

Comments

@Gykonik
Copy link

Gykonik commented Sep 27, 2023

Similar to #1271, I've ran into WARNING: Too many active WebGL contexts. Oldest context will be lost. because the context is not properly cleaned up even after calling .clear() and .kill.

What I've tried is the following (inside ngOnDestroy):

this._chartRenderer.clear();
this._chartRenderer.kill();
this._chartRenderer = null;

However, this did not solve the issue and I've come up with the following solution. However, the "type cheating" is quite hacky and I feel like this should be the default behaviour of a .kill() function (to properly clean up the WebGL contexts), shouldn't it?

this._chartRenderer.clear();
this._chartRenderer.kill();
Object.values((this._chartRenderer as any).webGLContexts as WebGLRenderingContext[]).forEach(
  context: WebGLRenderingContext) => {
    // Free up all the context
    context.getExtension("WEBGL_lose_context")?.loseContext();
  }
}
this._chartRenderer = null;
@Gykonik Gykonik added the bug label Sep 27, 2023
@jacomyal
Copy link
Owner

Interesting, thanks for this feedback. I'm a bit afraid of just pushing a fix, because of this comment from @Yomguithereal:
#1271 (comment)

Just so that I know what to target: Which version of sigma are you using?

@Gykonik
Copy link
Author

Gykonik commented Sep 27, 2023

Yea,h I've also read through this post. And tbh, I have no idea whether this has any implications or stuff. I can only say that it works for me and I have (a) no warnings in the console and (b) I guess it also frees up the resources (at least, the warnings with "too many WebGL Contexts disappeared).

But sure, this could have bad side-effects :D

I'm using the newest version 3.0.0-alpha3

@jacomyal
Copy link
Owner

OK, good to know, I don't have to push any hot-fix to the v2.4 version, at least :)

@Yomguithereal Any take on that?

@Yomguithereal
Copy link
Collaborator

This kind of problem usually happens when one is leaking renderers somehow. It might be linked to angular's magic on component properties and the fact that ngondestroy happens before or after (can't remember) the DOM nodes of the components are actually removed from the DOM, which means the kill method is not able to work properly.

The loseContext API only simulates that the context was lost and does not cleanup anything at all. So it will definitely get rid of warning in some browsers (in others, it will actually trigger more of them, but this kind of behavior changes fast), but it will not fix the underlying issue, which is GPU resources are leaking (ultimately not an issue because those resources are reclaimed by newer renderers, but still).

I can try some things in ipysigma but I think I can create many cells containing a renderer and clean them up many times and it will never fire this kind of warnings. Also, last time someone has this issue was also using Angular. I think this is why I did not end up adding the loseContext magic because it was not solving the underlying issue. It's also possible we can try writing our kill method differently (e.g. by explicitly nulling our internal references to contexts and dom elements, not only removing them from DOM) to try and avoid this issue. It's possible that Angular somehow retain a reference to a older version of component properties in some global register to enable some of its functionalities and since canvas cleanup is a shitty inexact science we are falling through the seams here wrt garbage collection.

Does it change anything if you use a DestroyRef instead (sorry I am an angular beotian)?

@Gykonik
Copy link
Author

Gykonik commented Oct 2, 2023

Thanks for the explanation! I'm not entirely sure whether this is related to angular or not. I already use the DestroyRef by putting it inside ngOnDestroy that gets automatically called when the component gets destroyed. And as expected, I also see the "DESTROY"-message every time I expect something to be destroyed. Nonetheless, it still doesn't work without the loseContext - with it, it works flawlessly :D

public ngOnDestroy(): void {
    this.graph?.clear();

    console.log("DESTROY!");
    // Proper cleanup to properly delete WebGL context.
    if (this.renderer) {
      this.renderer.clear();
      this.renderer.kill();
      // Free up all the context (see https://github.com/jacomyal/sigma.js/issues/1386)
      // Object.values((this.renderer as any).webGLContexts as WebGLRenderingContext[]).forEach(
      //   (context: WebGLRenderingContext) => {
      //     context.getExtension("WEBGL_lose_context")?.loseContext();
      //   }
      // );
      this._renderer = null;
    }
}

@jacomyal
Copy link
Owner

jacomyal commented Oct 3, 2023

I guess it's not much of a cost to add both those things to the kill method. It will be done for v3.

@jacomyal
Copy link
Owner

jacomyal commented Oct 3, 2023

The patch will be shipped with the next v3 alpha (or beta) release.

@jacomyal jacomyal closed this as completed Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants