Skip to content
This repository has been archived by the owner on Apr 5, 2023. It is now read-only.

fix: children should refresh the parent #27

Closed
wants to merge 1 commit into from
Closed

fix: children should refresh the parent #27

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 47 additions & 33 deletions src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
MeshLambertMaterialProps,
MeshStandardMaterialProps,
} from '@react-three/fiber'
import React, { useMemo } from 'react'
import React, { useMemo, useRef, useEffect, useLayoutEffect } from 'react'
import mergeRefs from 'react-merge-refs'
import {
DepthProps,
Expand Down Expand Up @@ -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!)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the _needsRefresh or whatever to the LayerMaterial itself? Might be generally useful to have it there, typed and all

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a needsupdate was added, when and where exactly will the material be updated? I never understood how 3JS’s needsupdate works

const [args, otherProps] = useMemo(() => getLayerMaterialArgs(props), [props])

React.useLayoutEffect(() => {
useLayoutEffect(() => {
ref.current.layers = (ref.current as any).__r3f.objects
ref.current.refresh()
}, [children])

const [args, otherProps] = useMemo(() => getLayerMaterialArgs(props), [props])
// This will fire before all children, allow possible children to refresh again
ref.current.__hasRefreshed = false
})

return (
<layerMaterial args={[args]} ref={mergeRefs([ref, forwardRef])} {...otherProps}>
Expand All @@ -98,49 +98,63 @@ function getNonUniformArgs(props: any) {
] as any
}

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
}
Comment on lines +101 to +113
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smells like .needsUpdate 😛

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a serious note, maybe consider inverting the logic so that it's __needsRefresh, makes the code less clumsy


const Depth = React.forwardRef<LAYERS.Depth, DepthProps>((props, forwardRef) => {
//@ts-ignore
return <depth_ args={getNonUniformArgs(props)} ref={forwardRef} {...props} />
const ref = useRefresh<LAYERS.Depth>(props)
return <depth_ args={getNonUniformArgs(props)} ref={mergeRefs([ref, forwardRef])} {...props} />
}) as React.ForwardRefExoticComponent<DepthProps & React.RefAttributes<LAYERS.Depth>>

const Color = React.forwardRef<LAYERS.Color, ColorProps>((props, ref) => {
//@ts-ignore
return <color_ ref={ref} args={getNonUniformArgs(props)} {...props} />
const Color = React.forwardRef<LAYERS.Color, ColorProps>((props, forwardRef) => {
const ref = useRefresh<LAYERS.Color>(props)
return <color_ ref={mergeRefs([ref, forwardRef])} args={getNonUniformArgs(props)} {...props} />
}) as React.ForwardRefExoticComponent<ColorProps & React.RefAttributes<LAYERS.Color>>

const Noise = React.forwardRef<LAYERS.Noise, NoiseProps>((props, ref) => {
//@ts-ignore
return <noise_ ref={ref} args={getNonUniformArgs(props)} {...props} />
const Noise = React.forwardRef<LAYERS.Noise, NoiseProps>((props, forwardRef) => {
const ref = useRefresh<LAYERS.Noise>(props)
return <noise_ ref={mergeRefs([ref, forwardRef])} args={getNonUniformArgs(props)} {...props} />
}) as React.ForwardRefExoticComponent<NoiseProps & React.RefAttributes<LAYERS.Noise>>

const Fresnel = React.forwardRef<LAYERS.Fresnel, FresnelProps>((props, ref) => {
//@ts-ignore
return <fresnel_ ref={ref} args={getNonUniformArgs(props)} {...props} />
const Fresnel = React.forwardRef<LAYERS.Fresnel, FresnelProps>((props, forwardRef) => {
const ref = useRefresh<LAYERS.Fresnel>(props)
return <fresnel_ ref={mergeRefs([ref, forwardRef])} args={getNonUniformArgs(props)} {...props} />
}) as React.ForwardRefExoticComponent<FresnelProps & React.RefAttributes<LAYERS.Fresnel>>

const Gradient = React.forwardRef<LAYERS.Gradient, GradientProps>((props, ref) => {
//@ts-ignore
return <gradient_ ref={ref} args={getNonUniformArgs(props)} {...props} />
const Gradient = React.forwardRef<LAYERS.Gradient, GradientProps>((props, forwardRef) => {
const ref = useRefresh<LAYERS.Gradient>(props)
return <gradient_ ref={mergeRefs([ref, forwardRef])} args={getNonUniformArgs(props)} {...props} />
}) as React.ForwardRefExoticComponent<GradientProps & React.RefAttributes<LAYERS.Gradient>>

const Matcap = React.forwardRef<LAYERS.Matcap, MatcapProps>((props, ref) => {
//@ts-ignore
return <matcap_ ref={ref} args={getNonUniformArgs(props)} {...props} />
const Matcap = React.forwardRef<LAYERS.Matcap, MatcapProps>((props, forwardRef) => {
const ref = useRefresh<LAYERS.Matcap>(props)
return <matcap_ ref={mergeRefs([ref, forwardRef])} args={getNonUniformArgs(props)} {...props} />
}) as React.ForwardRefExoticComponent<MatcapProps & React.RefAttributes<LAYERS.Matcap>>

const Texture = React.forwardRef<LAYERS.Texture, TextureProps>((props, ref) => {
//@ts-ignore
return <texture_ ref={ref} args={getNonUniformArgs(props)} {...props} />
const Texture = React.forwardRef<LAYERS.Texture, TextureProps>((props, forwardRef) => {
const ref = useRefresh<LAYERS.Texture>(props)
return <texture_ ref={mergeRefs([ref, forwardRef])} args={getNonUniformArgs(props)} {...props} />
}) as React.ForwardRefExoticComponent<TextureProps & React.RefAttributes<LAYERS.Texture>>

const Displace = React.forwardRef<LAYERS.Displace, DisplaceProps>((props, ref) => {
//@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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could avoid this double ref by just passing the ref to useRefresh, no?

useRefresh(forwardedRef, props)

return <displace_ ref={mergeRefs([ref, forwardRef])} args={getNonUniformArgs(props)} {...props} />
}) as React.ForwardRefExoticComponent<DisplaceProps & React.RefAttributes<LAYERS.Displace>>

const Normal = React.forwardRef<LAYERS.Normal, NormalProps>((props, ref) => {
//@ts-ignore
return <normal_ ref={ref} args={getNonUniformArgs(props)} {...props} />
const Normal = React.forwardRef<LAYERS.Normal, NormalProps>((props, forwardRef) => {
const ref = useRefresh<LAYERS.Normal>(props)
return <normal_ ref={mergeRefs([ref, forwardRef])} args={getNonUniformArgs(props)} {...props} />
}) as React.ForwardRefExoticComponent<NormalProps & React.RefAttributes<LAYERS.Normal>>

export { DebugLayerMaterial, LayerMaterial, Depth, Color, Noise, Fresnel, Gradient, Matcap, Texture, Displace, Normal }