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

Changing objects upon click #142

Open
rickid123 opened this issue Aug 31, 2023 · 10 comments
Open

Changing objects upon click #142

rickid123 opened this issue Aug 31, 2023 · 10 comments

Comments

@rickid123
Copy link

How do I change an object when it is clicked on? I want satellites to be octahedrons by default then load a glb file to display for the clicked on object once clicked whilst the rest remain as octahedrons.

@vasturiano
Copy link
Owner

You just need to change the value of the objectThreeObject prop. And you can trigger that value change on onObjectClick.

@rickid123
Copy link
Author

rickid123 commented Sep 1, 2023

This may be more of a React thing but when I use useMemo (as used in the satellites example) my default satellite octahedron doesn't work using this method. But when I use a standard function to create a mesh, it works but this creates significant performance issues.

I can make objectThreeObject equal to the useMemo function directly to render the octahedron (as the satellites example). My current code is as follows:

const handleSatelliteClick = satellite => { setSelectedSatellite(satellite); .... }

const objRepresent = object => { if (selectedSatellite && object == selectedSatellite) { return satellitePayloadModel; } else { return defaultSatelliteModel } } }

<Globe .... onObjectClick={satellite => handleSatelliteClick(obj)} objectThreeObject={obj => objRepresent(obj)} />

@vasturiano
Copy link
Owner

I think using useMemo (or useCallback) on these two functions should be fine as long as you set the correct dependencies of each. For objRepresent this should be at minimum [selectedSatellite].
And you should pass these callbacks directly as props, not wrapped in other functions, like:

<Globe ... onObjectClick={handleSatelliteClick} objectThreeObject={objRepresent} />

@MichaelBonnet
Copy link

MichaelBonnet commented Sep 2, 2023

Hey, I've actually done this in my app. Here's a minimal example.

@rickid123
Copy link
Author

Hey, I've actually done this in my app. Here's a minimal example.

Thanks for this, however this is basically where I got to and doesn't resolve the issue i'm having. See edited example here: https://codesandbox.io/s/dreamy-marco-y2r69m?file=/src/Globe.tsx

Basically if I use the useMemo hook to memoize the defaultSat mesh, I can use this for all objects by putting it into objectThreeObject directly. However when I use it in objectRepresent and choose to return a "selected" mesh object, or the memoized defaultSat mesh, it only returns 2 objects. When I return non-memoized meshes, i'm experiencing performance issues

@BangNguyen1992
Copy link

BangNguyen1992 commented Sep 4, 2023

Hello, I just checked your codesandbox and have some feedback, hope it might help

  1. The objectsData useMemo does not help anything in this case since you keep updating its dependencies time in frameTicker
  2. The requestAnimationFrame is not good here since it based on your PC frame rate, you can improve it as commented here Limiting Framerate #143 (comment)
  3. I suggest using a simpler approach, which is setInterval https://codesandbox.io/s/frosty-cdn-52pvjq?file=/src/Globe.tsx
  useEffect(() => {
    setInterval(() => {
      setTime((time) => new Date(+time + TIME_STEP));
    }, 100);
  }, []);

@rickid123
Copy link
Author

Hello, I just checked your codesandbox and have some feedback, hope it might help

  1. The objectsData useMemo does not help anything in this case since you keep updating its dependencies time in frameTicker
  2. The requestAnimationFrame is not good here since it based on your PC frame rate, you can improve it as commented here Limiting Framerate #143 (comment)
  3. I suggest using a simpler approach, which is setInterval https://codesandbox.io/s/frosty-cdn-52pvjq?file=/src/Globe.tsx
  useEffect(() => {
    setInterval(() => {
      setTime((time) => new Date(+time + TIME_STEP));
    }, 100);
  }, []);

These sections were all just copied and pasted from the satellites example to demonstrate my issue with useMemo on optional rendering of satellites but thanks for the tips

@BangNguyen1992
Copy link

Hello @rickid123, Yeah, I just forked yours and replaced the time-ticker with setInterval, nothing else from it 😅

@MichaelBonnet
Copy link

MichaelBonnet commented Sep 5, 2023

@BangNguyen1992 using setInterval seems to either break or rapidly speed up the animation, therefore harming performance. I'm not really sure what's being changed here, or to what end.

@BangNguyen1992
Copy link

Hello @MichaelBonnet, I dont think using setInterval would break or speed up anything. The snippet above is just for demonstration, when using setInterval you will need to cleanup that inside your useEffect, so the full code should look like this:

useEffect(() => {
    const interval = setInterval(() => {
      setTime((time) => new Date(+time + TIME_STEP));
    }, 100);
    return () => {
      clearInterval(interval);
    }
  }, []);

Or if you dont want to use setInterval and keep the requestAnimationFrame, #143 (comment) could be another solution

The requestAnimationFrame is based on your framerate, so if your PC framerate is 166Hz, it will update 166 times per sec, which is unnecessary, that's why I suggest using setInterval for better controlling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants