Incorrect behavior when changing opacity for a separate pocket

[hellhorse123]

In React code I display array of pockets on 3d model in scene (with opacity: 0,3). Then, after I check pocket on array of checkbox components in app, this checked pocket must change its opacity to 1. But instead of this smth happens with opacity of all pockets and all array of files are redownloading. Below u can see screen record: https://github.com/nglviewer/ngl/assets/48678379/5a3589c4-0ad4-46ec-8503-f3be69bd0dde

This is my code:

import React, { useState, useEffect, useRef, useContext, useMemo } from "react";
import * as NGL from "ngl";
import { useResizeDetector } from "react-resize-detector";
import { FilesService } from "services";

import { SiteRadarContext } from "pages/site-radar/SiteRadarContext";
import "./style.scss";

interface PocketsViewProps {
  pdbUrl: string;
}

interface PocketData {
  url: string;
  color: string;
}

const PocketsView: React.FC<PocketsViewProps> = ({ pdbUrl }) => {
  const { pockets } = useContext(SiteRadarContext);
  const [pocketData, setPocketData] = useState<PocketData[]>([]);

  const containerRef = useRef();
  const stageRef = useRef();
  const representation = useRef(null);
  const pocketReps = useRef([]);
  const pocketRepMap = useRef(new Map());

  // Function to handle resizing of the container
  const handleResize = () => {
    const stage = stageRef.current;
    if (stage) {
      stage.handleResize();
    }
  };

  // Use the resize detector to trigger resize handling
  const { ref: resizeRef } = useResizeDetector({ onResize: handleResize });

  // Set file IDs of checked pockets
  useEffect(() => {
    const fetchPocketData = async () => {
      const fetchedData = await Promise.all(
        pockets.map(async (pocket) => {
          try {
            const response = await FilesService.getFileLink(pocket.fileId);
            return { url: response.pre_signed_url, color: pocket.color };
          } catch (e) {
            console.error("Error fetching file:", e);
            return null;
          }
        })
      );

      setPocketData(fetchedData.filter((p) => p));
    };

    fetchPocketData();
  }, [pockets]);

  console.log(pockets);

  useEffect(() => {
    const stage = new NGL.Stage(containerRef.current, {
      backgroundColor: "#ededed",
    });
    stageRef.current = stage;

    // Load the main PDB file
    stage
      .loadFile(pdbUrl, { defaultRepresentation: true })
      .then((o) => {
        representation.current = o.addRepresentation("ribbon", {
          defaultRepresentation: true,
        });
        o.autoView();
      })
      .catch((error) => console.error("Error loading file:", error));

    // Cleanup function
    return () => {
      if (stage) {
        stage.viewer.wrapper.remove();
        stage.viewer.renderer.forceContextLoss();
        stage.dispose();
        stage.removeAllComponents()
      }
    };
  }, [pdbUrl]);

  const memoizedPocketData = useMemo(() => pocketData, [pocketData]);

  useEffect(() => {
    const stage = stageRef.current;
    if (stage) {
      // Clear old representations
      pocketReps.current.forEach((rep) => {
        rep.dispose();
      });
      pocketRepMap.current.clear();
      // pocketReps.current = [];

      // Load new pocket data
      memoizedPocketData.forEach((pocket) => {
        if (pocket && !pocketRepMap.current.has(pocket.url)) {
          stage
            .loadFile(pocket.url)
            .then((o) => {
              const rep = o.addRepresentation("surface", {
                color: pocket.color,
                opacity: 0.3,
              });
              pocketRepMap.current.set(pocket.url, rep); // Store representation in the map
            })
            .catch((error) =>
              console.error("Error loading pocket file:", error)
            );
        }
      });

    }

  }, [memoizedPocketData]);

  useEffect(() => {
    pockets.forEach((pocket) => {
      const rep = pocketRepMap.current.get(pocket.fileId);
      if (rep) {
        rep.setParameters({ opacity: pocket.checked ? 1 : 0.3 });
      }
    });
  }, [pockets]);

  return (
    <div ref={resizeRef} className="scene-container">
      <div ref={containerRef} className="scene" />
    </div>
  );
};

export default PocketsView;

[ppillot]

It seems that the opacity is controlled by the property checked of each pocket object in the pockets array. When this value changes, I suspect that the pockets array changes as well. pockets is reactive dependency of 2 different useEffect: one that sets the opacity, and the other one that loads the pocket data.

When pockets changes both useEffect are executed: the second one changes the opacity of representations, and the first one reloads data from server. When the data from server is fetched, setPocketData is executed. This changes pocketData so memoizedPocketData gets a new value (because it has pocketData as a dependency; note that this memoization is never effective there) Next, the useEffect that depends on memoizedPocketData is executed as well, pocket representations are removed and recreated with an opacity of 0.3.

IMO, this bug is not related to NGL, but is related to React intricacies. Cascades of useEffect make the execution of the code difficult to reason about. To circumvent this, I would suggest 2 options:

• regarding the pocket objects: split the data part from the controls part. Like that a change in the cheched property would not cause the refetch of data and re-creation of the pockets
• second option would be to keep the same data structure, but to control the whole pocket management in a single useEffect. To avoid refetch/rerenders, you can use the name property for the representation and check wether they have already been created or not. You'd have to manage the internal state of the NGL stage by yourself to avoid unneeded recomputations. I personally prefer doing that, rather than let React logics take care of the NGL state because React philosophy is to re-render components multiple times to sync with state which is acceptable for the DOM, but not suitable for 3D rendering and scientific computation

[hellhorse123]

I created a separate component without any additional logic to try to implement the desired behavior. The screen recording shows that with all attempts to trigger only one mutable pocket object, I have a loading file on both pocket files:

https://github.com/nglviewer/ngl/assets/48678379/d6f44777-2c42-4fd2-a1b3-3d2f7e36cd82

Code:

import { useState, useEffect, useRef } from "react";
import * as NGL from "ngl";
import pdbUrl from "../../assets/test-pockets/3hi7.pdb";
import pocket1Url from "../../assets/test-pockets/pocket1.sdf";
import pocket2Url from "../../assets/test-pockets/pocket2.sdf";

const TestOpacity = () => {
  const containerRef = useRef();
  const stageRef = useRef();
  const pocketReps = useRef([]); // To store pocket representations

  const [pocketsArray, setPocketsArray] = useState([
    { url: pocket1Url, checked: false, color: "red", name: 'red' },
    { url: pocket2Url, checked: false, color: 'blue', name: 'blue' },
  ]);

  const prevPocketsRef = useRef();

  // Initialize NGL Stage
  useEffect(() => {
    const stage = new NGL.Stage(containerRef.current, {
      backgroundColor: "#ededed",
    });
    stageRef.current = stage;

    // Cleanup function
    return () => {
      if (stage) {
        stage.viewer.wrapper.remove();
        stage.viewer.renderer.forceContextLoss();
        stage.dispose();
      }
    };
  }, []);

  // Load main PDB file
  useEffect(() => {
    if (pdbUrl && stageRef.current) {
      stageRef.current
        .loadFile(pdbUrl, {
          defaultRepresentation: true,
        })
        .then((o) => {
          console.log("Loaded object:", o);
          o.addRepresentation("cartoon");
          o.autoView();
        })
        .catch((error) => {
          console.error("Error loading file:", error);
        });
    }
  }, [pdbUrl]);

 // Load pockets only once or when URLs change
  useEffect(() => {
    if (stageRef.current) {
      pocketsArray.forEach((pocket, index) => {
        stageRef.current
          .loadFile(pocket.url)
          .then((o) => {
            const rep = o.addRepresentation("surface", {
              color: pocket.color,
              opacity: pocket.checked ? 1 : 0.3,
            });
            pocketReps.current[index] = rep;
          })
          .catch((error) => {
            console.error("Error loading pocket:", error);
          });
      });
    }

    const currentPocketReps = pocketReps.current;

    // Cleanup function
    return () => {
        currentPocketReps.forEach(rep => rep && rep.dispose());
    };
  }, [pocketsArray.map(pocket => pocket.url)]);

  // Update opacity when checked state changes
  useEffect(() => {
    pocketsArray.forEach((pocket, index) => {
      const rep = pocketReps.current[index];
      if (rep && prevPocketsRef.current && prevPocketsRef.current[index].checked !== pocket.checked) {
        rep.setParameters({ opacity: pocket.checked ? 1 : 0.3 });
      }
    });

    prevPocketsRef.current = pocketsArray;
  }, [pocketsArray.map(pocket => pocket.checked)]); 

  // Toggle checked state
  const toggleChecked = (index) => {
    setPocketsArray((currentPockets) =>
      currentPockets.map((pocket, idx) =>
        idx === index ? { ...pocket, checked: !pocket.checked } : pocket
      )
    );
  };

  return (
    <div>
      <div>
        {pocketsArray.map((pocket, index) => (
          <label key={index}>
            <input
              type="checkbox"
              checked={pocket.checked}
              onChange={() => toggleChecked(index)}
            />
            Pocket {pocket.name}
          </label>
        ))}
      </div>
      <div ref={containerRef} style={{ width: "90vw", height: "90vh" }} />
    </div>
  );
};

export default TestOpacity;

[ppillot]

This line of code in the dependency list of the useEffect creates a new reference at each rerender because it always returns a new array: pocketsArray.map(pocket => pocket.url)

These questions have nothing to do with bugs or features in NGL. I can't dedicate time to reviewing third party code