nglviewer / ngl

WebGL protein viewer
http://nglviewer.org/ngl/
MIT License
657 stars 168 forks source link

dispose() method leaves part of the old representation when changing model #1019

Closed hellhorse123 closed 2 months ago

hellhorse123 commented 5 months ago

On the 3D model in React I have several pocket sizes. When I change the size (load different files), the default representation remains on the 3D scene, going beyond the scope of the smaller model (pictures 1/2):

Screenshot 2024-01-26 at 09 59 58 Screenshot 2024-01-26 at 10 00 19

Part of Code:

import React, { useState, useEffect, useRef, useContext, useMemo } from "react";
...
  const stageRef = useRef();
...
  useEffect(() => {
    const stage = stageRef.current;
    if (stage) {
      // Clear old representations
      pocketReps.current.forEach(rep => {
        if (rep) {
          rep.dispose(); // Dispose each representation
        }
      });
      pocketReps.current = [];

      // Load new pocket data
      memoizedPocketData.forEach((pocket) => {
        if (pocket && typeof pocket !== "undefined") {

          stage
            .loadFile(pocket.url, { defaultRepresentation: true })
            .then((o) => {
              const rep = o.addRepresentation("surface", { color: pocket.color });
              pocketReps.current.push(rep); // Track the new representation
              o.autoView();
            })
            .catch((error) => console.error("Error loading pocket file:", error));
        }
      });
    }
  }, [memoizedPocketData]);
fredludlow commented 5 months ago

I think you need to call removeRepresentation on the component (which will call dispose: https://github.com/nglviewer/ngl/blob/master/src/component/component.ts#L308)

ppillot commented 5 months ago

Note also that former components are not removed from the stage with this implementation, and that the default representation (the grey balls around pseudo atoms) remains as well.

A representation is attached to a given components Each time the stage.loadFile promise returns, a new component is created, for which your code creates multiple representations (the default one, apparently only ball+stick representation, and the surface representation). When rep.dispose() is executed, only the surface representation is removed.

      // Clear old representations
      pocketReps.current.forEach(rep => {
        if (rep) {
          stage.removeComponent(rep.component); // Dispose each component and all their representations
        }
      });