fix: children should refresh the parent #27
Conversation
| let hasRefreshed = false | ||
| function useRefresh<T>({ mode, visible, type, mapping, map, axes }: any) { | ||
| const ref = useRef<T>() | ||
| useEffect(() => { | ||
| const parent: LAYERS.LayerMaterial & { __hasRefreshed: boolean } = (ref.current as any)?.__r3f?.parent | ||
| // Refresh parent material on mount | ||
| if (parent && parent.__hasRefreshed == false) { | ||
| parent.refresh() | ||
| parent.__hasRefreshed = true | ||
| } | ||
| }, [mode, visible, type, mapping, map, axes]) | ||
| return ref | ||
| } |
There was a problem hiding this comment.
On a serious note, maybe consider inverting the logic so that it's __needsRefresh, makes the code less clumsy
| //@ts-ignore | ||
| return <displace_ ref={ref} args={getNonUniformArgs(props)} {...props} /> | ||
| const Displace = React.forwardRef<LAYERS.Displace, DisplaceProps>((props, forwardRef) => { | ||
| const ref = useRefresh<LAYERS.Displace>(props) |
There was a problem hiding this comment.
You could avoid this double ref by just passing the ref to useRefresh, no?
useRefresh(forwardedRef, props)
| @@ -69,14 +69,14 @@ const LayerMaterial = React.forwardRef< | |||
| LAYERS.LayerMaterial, | |||
| React.PropsWithChildren<LayerMaterialProps & Omit<AllMaterialProps, 'color'>> | |||
| >(({ children, ...props }, forwardRef) => { | |||
| const ref = React.useRef<LAYERS.LayerMaterial>(null!) | |||
| const ref = React.useRef<LAYERS.LayerMaterial & { __hasRefreshed: boolean }>(null!) | |||
There was a problem hiding this comment.
Add the _needsRefresh or whatever to the LayerMaterial itself? Might be generally useful to have it there, typed and all
There was a problem hiding this comment.
If a needsupdate was added, when and where exactly will the material be updated? I never understood how 3JS’s needsupdate works
|
This should be fixed in #29 No more Also removed |
|
I'm having problems with the refresh of the LayoutMaterial, I suppose that PR solves it, when will it be available? |
|
@Kevinparra535 Due to deeper underlying issues this PR has been incorporated into the upcoming Will be available as time openes up for me to complete the release. |
lamina will re-create the shader on every render because "children" is not a stable reference. children should rather inform lamina on mount, and when certain props have changed that it must refresh. this of course would duplicate refresh multiple times once you're dealing with multiple layers — this draft would try to work against that.