brianzinn / react-babylonjs

React for Babylon 3D engine
https://brianzinn.github.io/react-babylonjs/
812 stars 102 forks source link

Demos #200

Closed benallfree closed 2 years ago

benallfree commented 2 years ago

New PR stacked on #198. Just leaving it here for visibility, still a work in progress. Keeping in draft mode while WIP.

The main thing I'm finding as I'm writing demos is that I want to create a package @react-babylonjs/extra that contains all the reusable components used in demos. I think it will eventually hold a lot of HOC because encapsulation and abstraction seems to be a point of emphasis.

Anyway, if you think it should be in the base package we can move it there but I thought maybe keeping it as a separate package would be a good approach for now.

Please keep this one open til I give the green light :)

benallfree commented 2 years ago

@brianzinn One thing to notice is I added a useEffect dependency array to useOnBeforeRender. It's a little awkward that the params list is getting so long, but at least it's backward compatible :)

brianzinn commented 2 years ago

I think that would be useful and was already thinking of one. I’d move some hooks over as well too make this core project more lightweight

brianzinn commented 2 years ago

@benallfree - I needed to remove the yarn.lock, since it's referencing colors, which has been removed and yarn won't install the lockfile versions. what version of yarn are you using? i'm on 1.22.17. Just running yarn and yarn build in the root folder.

yarn build
yarn run v1.22.17
$ yarn workspaces run build

> @devtool/loader
$  microbundle build -i src/index.ts -o dist/index.js -f cjs --target node
Build "@devtool/loader" to dist:
      2.34 kB: index.js.gz
      2.01 kB: index.js.br

> @devtool/react
$ parcel build
⠋ Building index.ts...
error Command failed with exit code 3221225477.

I can run tsc no problem, so it's building OK (tsc needs a specific import). Looks like parcel uses something faster by default. I can't see how to find any errors... it looks like an access violation. I'm learning lots of new tooling at same time, so a bit of a learning curve for me. 😄

benallfree commented 2 years ago

https://www.npmjs.com/package/colors is alive and well at v1.4.0 published in 2019. They must have removed the updated versions from npm. Are you experiencing something different? I checked the lockfile and it only references safe versions of colors.

Yarn 1.22.17 here.

Everything does build for me, but in hindsight Parcel was a mistake. I converted those to just use tsc rather than needlessly bundling.

Please try again:

rm -rf node_modules
yarn
yarn build

✅ @devtool/loader ✅ @devtool/react ✅ @devtool/gatsby-mdx-plugin ✅ react-babylon-js ✅ @react-babylon-js/docs (new static site) ✅ react-babylonjs-storybook (old storybook docs)

As an aside, I'm going to pitch another PR to you where the main lib is built using rollup but also as es6 modules so it can be tree shaken by downstream bundlers.

brianzinn commented 2 years ago

I'll check out all the new goodies tonight - as an aside in my learning/research - I came across this article: https://maxrohde.com/2021/11/20/the-ultimate-guide-to-typescript-monorepos/

It goes into linking the repos with composite/incremental and extending base tsconfig.

benallfree commented 2 years ago

That link refers to Yarn 2 workspaces. I've not had great luck with Yarn 2 but you might!

brianzinn commented 2 years ago

The website looks really great! 🎉 I got your branch up and running. The only issue I have so far is that it's pegging CPU on my laptop and Firefox/Chrome become unresponsive - I think just the pages with multiple scenes. I have a newish Ryzen9 16GB RAM and NVidia RTX 3060 - which is far above what most people have. I think it's when there are too many scenes on a single page. I am going to look adding an IntersectionObserver opt-in to skip render as I think that will address the issue.

Also, there is a Vector3.FromArray([x, y, z]) method. All the math stuff has creation/factory methods like that or de/serialize method. The thing about newing up on every frame render is that it creates GC pressure when done in higher numbers. That's one advantage of setting outside of React render loop, but you are right in that you lose control of state going to objects - also if a React render occurs while you are rendering on your own then it would create some unexpected behaviour. It may also be the react reconciler diffing on the 3d frame rate having high CPU, but I am not familiar enough with the scheduling algorithm how that would impact performance. I'm going to try the IntersectionObserver before profiling cpu/memory.

benallfree commented 2 years ago

Hmm I hadn't noticed any CPU issues, but then again I'm on the new M1 haha. The demos are so simple I would imagine it would take hundreds of them to bog things down unless there is a memory leak like the one I stumbled across with onBeforeRender. Let me know how your testing goes, I'll keep my eye out too.

And yes I'm definitely out of my element with all 3D primitives 🙄

Regarding state update propagation - I've been playing with react-three-fiber and they say the same thing as you: changing any input will cause a new object to be created. On the surface it doesn't seem like it needs to be the case, especially if the object can have a unique name. If I were writing the rendering logic and I knew the guid for an object, I wouldn't need to create a new instance. I could keep a shadow state, run a comparison to incoming state, and make imperative alterations to the object props as needed.

But I can imagine type-specific territory where updating an object's state couldn't easily be codegen'd...so maybe not.

benallfree commented 2 years ago

Oh I can tell you easily that it's bogging down by setting state in onBeforeRender. Ok, let's look into fixing this...

brianzinn commented 2 years ago

What this project renderer does is copy the Vector3 from props into the same object reference:

(target[update.propertyName] as Vector3).copyFrom(update.value)

If you were to use the same object in the React world (ie: with useRef() instead of new Vector3() or Vector3.From...()) then it would not be creating any new instances on each render (same 2 instances always used). So, it really doesn't need to be the case. Now if you are talking about changing constructor parameters that is a different story (args in r3f world) - typically you need to trick the reconciler by using a key prop. ie:

  // changing radius or subdivisions will change smoothing and you can alter with state and new meshes are created
  const IcoSphere = ({radius, subdivisions}) => <icoSphere key={`ico-${radius}-${subdivisions}`} radius={radius} subdivions={subdivisions} />

HTH - and yes onBeforeRender causes a lot of work, but I think if we paused the scenes that aren't viewable with IntersectionObserver then we would be OK. I'm working on a new Engine FC that does so and will have it ready soon - I will be testing it on this branch - good place to test.

benallfree commented 2 years ago

I think onBeforeRender is fine. The CPU pegging happens when altering React state from within the onBeforeRender callback. I think that's just too much. As soon as I disabled the state management, I could run multiple instances on a page with no problem. It appears, at least for now, that managing Babylon objects via pure react state is not feasible.

brianzinn commented 2 years ago

hi @benallfree - lots of cool changes. I noticed that you added:

"react-three-gui": "../react-three-gui"

Is that a custom build on your machine? I didn't think his latest version would work with babylonjs, but I did do a PR in 2020 that at least worked at the time (it is still pending! LOL). Maybe we can link to a github repo or commit? I didn't try, but maybe a direct github link would work in package.json to here (https://github.com/brianzinn/react-three-gui) or otherwise let me know if I can help there at all.

Additionally, also for debugging there is a Scene Explorer as part of babylonjs project. It has a lot of capabilities. It can be brought in like this:

// this imports the side-effects
import "@babylonjs/inspector";
// can add a 'show:boolean' prop and also hide, if needed
const Inspector = () => {
  const scene = useScene();
  scene.debugLayer.show();
  return null;
};

const MyScene = () => {
   return (
     <Scene ...>
        <Inspector />
     </Scene>
  )
}

Also, for the useBeforeRender hook to add the deps is a good idea. Since it is an optional parameter - you can specify a default value in TypeScript:

deps?: React.DependencyList = []
benallfree commented 2 years ago

Hey Brian thanks for all your continued help, I'm learning so much!

Oops! Sorry about react-three-gui, I was experimenting to see how tightly coupled it was to THREE (answer: not very) and whether it could be split into @react-gui/core, @react-gui/three, and @react-gui/babylon. I think it actually could without too much effort, especially if I had some help from the maintainers. But since then, I learned more about Babylon and how they have their own GUIs included, so I'll probably abandon this idea. Anyway, I fixed that.

Updated useBeforeRender 👍 I forgot to include [scene, ...deps] in the useEffect anyway :)

I'm really struggling to understand how to use React. It just ends up being ref's to everything and imperative updates. In normal React programming, that would be considered a huge antipattern. I think the declarative definition of the scene or subscenes is cool, but without propagating changes through state and props it's not reactive at all 😂 So I'm experimenting with how to implement state management, especially in a case like a multiplayer game where there is shared state. Maybe https://github.com/pmndrs/zustand#using-zustand-without-react or something.

Am I missing something super fundamental? For example, why does every call to <box> re-create the object? I assume at the render level there is some way to know whether that work needs to be repeated. Sorry for the ignorance.

brianzinn commented 2 years ago

Hi @benallfree no worries at all. This is fun for me too - I’m happy to be learning about gatsby, but also super excited to be able to present this learning experience in the step by step walk through fashion you have built.

I think in order to fully understand what is going on would require knowing more of what the reconciler is doing and how the diffs are pushed. The renderer has a “HostConfig” and is told when there is a new element and when there are prop changes - that is the same as the DOM renderer, so should not be any different. ie: when you create a ‘div’ and change the ‘className’ it will not create a new div, but instead will update an existing div. That is handled by the reconciler and the references are maintained there. That is why the ‘<box ../>’ shouldn’t be recreated and props should be passed through to the renderer. The renderer (this project) is told when there is a new element or property changes. If you put console logs in the host config and load a basic scene you can follow through object creation and prop diffs. If objects aren’t reactive then it may be due to unexpected hook behaviour. Happy to explain more - I learned through trial and error by debugging the host config create and update methods.

benallfree commented 2 years ago

Ok thank you, so this example will eventually crash https://codesandbox.io/s/codesandbox-react-tsx-forked-kb0xq

Can you think of why? I thought you said it was because was being created over and over, but I think I misunderstood.

benallfree commented 2 years ago

Clarification... we thought multiple engines was crashing on the docs pages, but it appears to be running fine in Codesandbox. Hmm. Well it absolutely was crashing with the old useBeforeRender.

brianzinn commented 2 years ago

The Vector3 will be created on each render. Consider this change which would not create a new Vector3 x frames/second x scenes:

const StatefulMovingBox: FC = () => {
  ...
  // https://reactjs.org/docs/hooks-reference.html#lazy-initial-state
  // note: direct changes to `position` without a React render will not propagate
  const [position] = useState(() => new Vector3(0, 1, 0));
 ...
 return (
    <box
      name="box"
      size={2}
      position={positionRef}
      rotation={new Vector3(0, y, 0)}
    />
  ) 
}

I think when it gets to high performance if you look at operations that are happening on each frame render - code tends to re-use temporary variables to prevent GC pressure. I don't think React is best suited state updates 120 times/second/scene. If you look at libraries that do animations like react-spring or otherwise there is nothing wrong with updating CSS or rotations outside of the react render loop and actually it is more efficient. That's why in my demos I would typically not assign a rotation as a prop from state, but instead update directly (or even using a babylonjs animation, which alters object properties (ie: Vector3, Quaternion, number) directly over a lifetime of frames) - you can use state to create and cancel animations. If the state updates are not on every frame render then absolutely I would always go from props to make the application more intuitive and easier to track. I know that there can be optimisations made in this renderer and at some point I will go there.

That codesanbox runs fine for me too on the code sandbox demo - must be something else going on. 😕

benallfree commented 2 years ago

That's an amazing explanation. So any kind of react state manipulation inside a Babylon render loop is going to be bad news. Got it!

I'm cooking up something really fun for you to take a look at. Multiplayer, firebase, realtime stuff. Will drop soon.

benallfree commented 2 years ago

Here's a little demo I cooked up today. It uses the Firebase realtime database for synchronization. Try opening the app in multiple browsers and letting each one start a few bots. Use the keyboard w a s d to steer your player around and see the movements echoed pretty accurately in the other browser windows using an optimistic movement approach.

One area you can maybe help me understand, it seems like it is rendering a second copy of the player on initial movement. You'll see a little jump at first and then every 100ms as it updates the position state to match the mesh's position that gets updated in the render loop.

https://codesandbox.io/s/codesandbox-react-tsx-forked-2yeec

brianzinn commented 2 years ago

It worked briefly - not I'm getting these: Player is invalid {"exp":1643053647778} [{},{},{},{},{},{},{},{},{}]

I did see the ghosting though - looks like it is rendering twice - I saw that same also when moving. You could try LERP to the position it is meant to be as well to make it maybe appear more realistic, but it would lag slightly more to accurate position.

benallfree commented 2 years ago

Yeah I was thinking about LERP or maybe some kind of curve-based animation. All fun stuff to try!

brianzinn commented 2 years ago

@benallfree is this ready to merge? I have this branch running locally and if you are done here I can take over. Let me know - thanks!

benallfree commented 2 years ago

It's down to just updating demo mdx's and their corresponding code.

I need to remove the setState stuff from the Babylon render loop in the demos and update the explanation, but yes the core is stable.