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

A patch version change has broken default camera logic #2795

Closed
helgardferreira opened this issue Mar 4, 2023 · 2 comments · Fixed by #2796
Closed

A patch version change has broken default camera logic #2795

helgardferreira opened this issue Mar 4, 2023 · 2 comments · Fixed by #2796
Labels
bug Something isn't working

Comments

@helgardferreira
Copy link

Patch version 8.11.6 has introduced a breaking change to default camera set logic. The specific commit is 99fef6c27294024b1bda35730a6edc17e32ef390.

When the viewport changes size the default camera changes for some reason.

Here is my custom <OrthographicCamera /> implementation (modification of drei's <OrthographicCamera /> implementation):

import { useLayoutEffect, useRef, FunctionComponent } from 'react';
import { OrthographicCamera as OrthographicCameraImpl, Vector3 } from 'three';
import { useThree } from '@react-three/fiber';

type Props = Omit<JSX.IntrinsicElements['orthographicCamera'], 'children'>;

export const OrthographicCamera: FunctionComponent<Props> = ({ ...props }) => {
  const set = useThree(({ set }) => set);
  const camera = useThree(({ camera }) => camera);
  const size = useThree(({ size }) => size);
  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
  const cameraRef = useRef<OrthographicCameraImpl>(null!);

  useLayoutEffect(() => {
    cameraRef.current.updateProjectionMatrix();
  }, [size]);

  useLayoutEffect(() => {
    cameraRef.current.updateProjectionMatrix();
  });

  useLayoutEffect(() => {
    camera.lookAt(new Vector3(0, 0, 0));
  }, [camera]);

  useLayoutEffect(() => {
    const oldCam = camera;
    set(() => ({ camera: cameraRef.current }));
    return () => set(() => ({ camera: oldCam }));
    // The camera should not be part of the dependency list because this
    // components camera is a stable reference that must exchange the default,
    // and clean up after itself on unmount.
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [set, cameraRef]);

  return (
    <orthographicCamera
      left={size.width / -2}
      right={size.width / 2}
      top={size.height / 2}
      bottom={size.height / -2}
      ref={cameraRef}
      far={2000}
      near={0.1}
      {...props}
    />
  );
};

Now it's entirely possible that my implementation of <OrthographicCamera /> has some flaw but on the other hand I usually don't expect a patch version change to cause breaking changes.

Here is a video showing the bug in action:

broken.mov
@CodyJasonBennett
Copy link
Member

Have a fix out in v8.11.7.

@helgardferreira
Copy link
Author

Thank you!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants