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

React Three Fiber integration #262

Closed
marcelpi opened this issue May 2, 2022 · 19 comments
Closed

React Three Fiber integration #262

marcelpi opened this issue May 2, 2022 · 19 comments

Comments

@marcelpi
Copy link

marcelpi commented May 2, 2022

Is your feature request related to a problem? Please describe.
Please make this library easy to use in React Three Fiber. That would be a major time-saver as a lot of people are using R3F nowadays and there are plenty of projects developed in React.

Describe the solution you'd like
A component one can import and use right away, like .

I tried using it in R3F using the example you provided, but didn't manage to get it going.
Still, I would love to use it.

@yomotsu
Copy link
Owner

yomotsu commented May 3, 2022

Thanks for using and your suggestion.

but didn't manage to get it going.

I think it works. Could you double-check that you have made a wrapper component of the original camera-controls like the example?
https://codesandbox.io/s/react-three-fiber-camera-controls-4jjor?file=/src/camera-controls.tsx

or would you mind providing a demo that does not work as expected?

Please make this library easy to use in React Three Fiber

That would be nice and we could make another package with camera-control, however, I think just providing the example code is more flexible to use

@marcelpi
Copy link
Author

marcelpi commented May 3, 2022

Yes, I got it to work eventually. I just had to install TypeScript since my code is Vanilla.
Would you be so kind as to include the wrapper directly in the library so we don't have to create a separate file?
Just installing camera-controls and importing the component out of the box would be awesome.

I'm currently using OrbitControls, but it has some limitations and would really love to give this a try.

Thank you for your effort!

@DennisSmolek
Copy link

DennisSmolek commented May 4, 2022

So here's Pauls (creator of r3f) simple wrapper using the r3f extend https://codesandbox.io/s/yomotsu-camera-controls-febus

After that is on you to make it work for your project.
I'm surprised though because the wrapper they used is fairly solid too, if a bit more verbose..

What are you trying to do that you couldnt?

I'm ALSO writing a r3f variant...

@marcelpi
Copy link
Author

marcelpi commented May 4, 2022

So here's Pauls (creator of r3f) simple wrapper using the r3f extend https://codesandbox.io/s/yomotsu-camera-controls-febus

After that is on you to make it work for your project. I'm surprised though because the wrapper they used is fairly solid too, if a bit more verbose..

What are you trying to do that you couldnt?

I'm ALSO writing a r3f variant...

I managed to get it to work. I was suggesting it would be nice to have this wrapper included in the library by default, instead of having to set it up. I'm not an expert in Three and React, quite contrary, so I'm having a difficult time trying to wrap my hand around this stuff and how it works.

If I could just import a component and add it to my scene out of the box, like OrbitControls, that would be awesome.

Anyway, thanks again for your feedback.

@yomotsu
Copy link
Owner

yomotsu commented May 4, 2022

First of all, I'm happy to see this conversation. Thank you for your suggestions again.
I think I should prepare a jsx version(not tsx) example of it. Also, I could add some usage of that for r3f users.

If we provide r3f ready camera-controls in this repository, we also need to add additional dependencies which are React and r3f. but I think that is too much. So, I would make another repository, but I'm not sure that I could keep it updated.

I personally encourage you to use our example
https://codesandbox.io/s/react-three-fiber-camera-controls-4jjor?file=/src/camera-controls.tsx
over https://codesandbox.io/s/yomotsu-camera-controls-febus

Because ours can use more instance methods via the Ref!

@marcelpi
Copy link
Author

marcelpi commented May 4, 2022

Alright, then a .jsx version would be fine. I would use that one instead of .tsx.
I understand the part about dependencies, if it's too much overhead then skip it for now.

In the future I think more and more people will migrate towards r3f though, since it's so much easier to use compared to vanilla Three.

But I also understand it's a lot of work for just one person to keep this updated.
Thank you again for your work, pulling something like this off must've been a challenge.

@DennisSmolek
Copy link

First of all, I'm happy to see this conversation. Thank you for your suggestions again. I think I should prepare a jsx version(not tsx) example of it. Also, I could add some usage of that for r3f users.

If we provide r3f ready camera-controls in this repository, we also need to add additional dependencies which are React and r3f. but I think that is too much. So, I would make another repository, but I'm not sure that I could keep it updated.

I personally encourage you to use our example https://codesandbox.io/s/react-three-fiber-camera-controls-4jjor?file=/src/camera-controls.tsx over https://codesandbox.io/s/yomotsu-camera-controls-febus

Because ours can use more instance methods via the Ref!

Can you clarify this point?

I saw the instance mentioned in your wrapper but Im not sure when you would be utilizing it.

Pauls version is very basic but also has the ref exposed so I believe most generic methods would work

@yomotsu
Copy link
Owner

yomotsu commented May 5, 2022

@marcelpi Thanks for understanding.

@DennisSmolek
My wrapper component exposes Ref with forwardRef. So, you can call methods from out of <Canvas>.

As you can see there are <button>s to call camera-controls methods in my App.tsx.
I think mine is more controllable.

image

Also, provide types in ts.

@yomotsu
Copy link
Owner

yomotsu commented May 5, 2022

@marcelpi
Copy link
Author

marcelpi commented May 6, 2022

@marcelpi jsx version is available here https://codesandbox.io/s/react-three-fiber-camera-controls-forked-6hclkb

Thank you!

@yomotsu yomotsu closed this as completed Jun 27, 2022
@abernier
Copy link
Collaborator

abernier commented Jan 21, 2023

I have mimicked the implementation of drei's OrbitControls into that CameraControls (TS) impl: https://codesandbox.io/s/yomotsu-camera-controls-forked-ts-version-forked-vbgq3p?file=/src/CameraControls.tsx

It supports currently:

  • native intellisense auto-completion on <CameraControls /> props / types-checking
  • passing a ref
  • passing a custom camera prop
  • passing a custom domElement prop
  • calling .dispose() when unmounting
  • only .update()ing when camera .enabled

@CodyJasonBennett
Copy link

No need, this is why we have Drei. Happy to continue in pmndrs/drei#1237.

@abernier
Copy link
Collaborator

abernier commented Jan 21, 2023

<CameraControls /> component was shipped in @react-three/drei (>= v9.54.0)

@yomotsu
Copy link
Owner

yomotsu commented Jan 22, 2023

@abernier @CodyJasonBennett That's big news for me! Thank you very much!!
I was always wondering if someone could handle r3f-ready camera-controls, because I'm not really confident using r3f.

Now it's sort of official! Really appreciate it.
I will also add "for r3f users section" in our readme when drei v9.54.0 is shipped.

@abernier
Copy link
Collaborator

abernier commented Jan 22, 2023

Thank you @yomotsu it'a nice to see your enthusiasm about it :)

After the release, @drcmda one of the author of r3f, pointed me out a small issue though...

In react, in dev mode (with strict-mode) a component can be rendered twice(or more).

As a consequence, it is important we can disconnect and re-connect all events, once the component is re-rendered. In other words, it is particularly important the CameraControls class's constructor does not have any side-effects (like binding events). Otherwise, events for example, will be bound multiple times.

I think we could quite easily create a dedicated .connect method and move any events binding/side-effects into it1.

We could then have the following flow in React:

new CameraControls()
.connect()
.disconnect()
.connect()
.disconnect()
...

I will try submitting a PR for this, but will for sure need help from you ;)

Footnotes

  1. CameraControls class seems to already have a such "disconnect" method, called .dispose()

@drcmda
Copy link

drcmda commented Jan 22, 2023

@yomotsu so happy to see you like it, i hope it will drive more users to this amazing library.

btw here's how we handle the constructor side effect issue with all other controls:

class OrbitControls extends EventDispatcher {
  domElement: HTMLElement | undefined
  constructor(object: Camera, domElement?: HTMLElement) {
    this.connect = (domElement: HTMLElement): void => {
      scope.domElement = domElement
      scope.domElement.style.touchAction = 'none'
      scope.domElement.addEventListener('contextmenu', onContextMenu)
      ...
    }
    this.dispose = (): void => {
      scope.domElement?.removeEventListener('contextmenu', onContextMenu)
      scope.domElement?.removeEventListener('pointerdown', onPointerDown)
      ...
    }
    // connect events
    if (domElement !== undefined) this.connect(domElement)

it's entirely optional and backward compatible.

this standard way:

const ctrl = new Controls(cam, domNode)

and optionally

const ctrl = new Controls(cam)
ctrl.connect(domNode)
// later ...
ctrl.dispose()

this allows us to connect and re-connect to dom layers, but also takes care of this issue: mrdoob/three.js#20575

@abernier
Copy link
Collaborator

Here is my try with a new .connect method @yomotsu #338 :)

@yomotsu
Copy link
Owner

yomotsu commented Jan 22, 2023

@abernier @drcmda Thank you for pointing it out, and suggestions. (now it's ongoing!)

@abernier
Copy link
Collaborator

abernier commented Jan 23, 2023

for the record, <CameraControls /> was finalized (thanks to @yomotsu) and is now part of pmndrs/drei components

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

6 participants