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

Prevent camera from resetting when shapes change #181

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

North101
Copy link

This prevents the camera from resetting when the rendered shapes change.

@North101 North101 changed the title Fix camera resetting when shapes change Prevent camera from resetting when shapes change Jan 15, 2025
@@ -33,7 +33,7 @@ export default function Stage({ children, center, ...props }) {

set({ radius: sphere.radius, previousRadius: radius, top: box3.max.z });
//eslint-disable-next-line react-hooks/exhaustive-deps
}, [children]);
}, []);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I need to think about this one. I has been a while since I wrote this, but it was probably here on purpose (i.e. my guess is to have a relatively good first render).

I will try to have a look at it by the end of the week.

In any case thanks for the PRs!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I'm sure there was a reason for this but it also caused the camera to reset when the rendered shape changed. In testing so far, everything looks good.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this code does not work as expected from the get go. Once the controls are initialized, this is where we should change the camera position.

That said, in theory, the camera should never change even now, which is a bug to fix (but not related to your changes I guess).

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

Successfully merging this pull request may close these issues.

2 participants