FarazzShaikh / THREE-CustomShaderMaterial

🧩 Extend Three.js standard materials with your own shaders!
https://farazzshaikh.github.io/THREE-CustomShaderMaterial/
MIT License
891 stars 56 forks source link

Material gets recreated on every render loop #12

Closed gromgull closed 2 years ago

gromgull commented 2 years ago

This thing is amazing - but I've noticed the material gets recreated each loop - even if the props are the same :(

if you add a console.log(material.current) to your water example it should happen there too.

I am not sure how, but I believe the problem is here: https://github.com/FarazzShaikh/THREE-CustomShaderMaterial/blob/master/src/index.tsx#L18-L22

If I add the material to my tree as customerShaderMaterial and pass the args array the way the constructor expects it works.

FarazzShaikh commented 2 years ago

Huh this sucks, I will look into this

gromgull commented 2 years ago

I solved it by creating the material in a useAsset block - caching it "globally" - then passing it in as an attribute <mesh material={myMaterial} ... and not inline as a jsx element. But it's not as nice.

FarazzShaikh commented 2 years ago

Yeah you could do the same with useMemo, I’ll be refactoring this library again with the patterns I learnt working on other projects to make it solid and easy to work with.

FarazzShaikh commented 2 years ago

Im not sure where to add console.log(material.current) to test this. Do you mean to useFrame?

I added it outside useFrame and It's only recreated every time the component is re-rendered. Adding it inside useFrame shows that the UUID of the material remains unchanged, which implies it is not re-initialized on each renader

gromgull commented 2 years ago

Sorry - unclear language :D

It's recreated every time react calls the render function (not for every threejs render!)

That is still not great though - and not the way other react three fiber components work - the reconcilliation part should only recreate three objects when their properties change. Look at this slightly contrived example: https://codesandbox.io/s/getting-started-01-forked-j5d2r?file=/src/App.js

The material remains constant, although the state changes and the App is re-rendered.

I think the problem happens because you do {...rest} - this creates a new props object each time, and react uses === for matching them.

At the same time, I am confused, since the using {...rest} to pull out the props you care about and passing the rest on seems like a common pattern :shrug:

FarazzShaikh commented 2 years ago

OK so I added a log to the constructor of the CustomShaderMaterial class to see how many times its initialized.

The water example currently initializes it twice, however, moving the <Water /> component outside of <Suspense /> makes it only initialize once. I have pushed the "fixed" example to the repo and sandbox.

Suspense might be forcing a re-render of everything within it. Does this solve it?

FarazzShaikh commented 2 years ago

Closing as solved, reopen if necessary