Skip to content

Commit

Permalink
fix: fix array of reused primitives
Browse files Browse the repository at this point in the history
  • Loading branch information
StefanSchmutz committed Jan 6, 2024
1 parent b72f32e commit ebddb72
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 2 deletions.
10 changes: 8 additions & 2 deletions packages/fiber/src/core/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export type LocalState = {
previousAttach: any
memoizedProps: { [key: string]: any }
autoRemovedBeforeAppend?: boolean
isCreatedBeforeAttach?: boolean
}

export type AttachFnType = (parent: Instance, self: Instance) => () => void
Expand Down Expand Up @@ -96,7 +97,7 @@ function createRenderer<TCanvas>(_roots: Map<TCanvas, Root>, _getEventPriority?:
if (type === 'primitive') {
if (props.object === undefined) throw new Error("R3F: Primitives without 'object' are invalid!")
const object = props.object as Instance
instance = prepare<Instance>(object, { type, root, attach, primitive: true })
instance = prepare<Instance>(object, { type, root, attach, primitive: true, isCreatedBeforeAttach: true })
} else {
const target = catalogue[name]
if (!target) {
Expand Down Expand Up @@ -149,6 +150,7 @@ function createRenderer<TCanvas>(_roots: Map<TCanvas, Root>, _getEventPriority?:
if (!added) parentInstance.__r3f?.objects.push(child)
if (!child.__r3f) prepare(child, {})
child.__r3f.parent = parentInstance
child.__r3f.isCreatedBeforeAttach = false
updateInstance(child)
invalidateInstance(child)
}
Expand All @@ -175,6 +177,7 @@ function createRenderer<TCanvas>(_roots: Map<TCanvas, Root>, _getEventPriority?:
if (!added) parentInstance.__r3f?.objects.push(child)
if (!child.__r3f) prepare(child, {})
child.__r3f.parent = parentInstance
child.__r3f.isCreatedBeforeAttach = false
updateInstance(child)
invalidateInstance(child)
}
Expand Down Expand Up @@ -222,7 +225,10 @@ function createRenderer<TCanvas>(_roots: Map<TCanvas, Root>, _getEventPriority?:
}

// Remove references
delete (child as Partial<Instance>).__r3f
// only remove primitive reference if it is not freshly created
if (!(child as Partial<Instance>).__r3f?.isCreatedBeforeAttach) {
delete (child as Partial<Instance>).__r3f
}

// Dispose item whenever the reconciler feels like it
if (shouldDispose && child.dispose && child.type !== 'Scene') {
Expand Down
51 changes: 51 additions & 0 deletions packages/fiber/tests/core/renderer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1015,4 +1015,55 @@ describe('renderer', () => {
expect(meshDispose).toBeCalledTimes(1)
expect(primitiveDispose).not.toBeCalled()
})

it('should not clear reattached primitive', async () => {
const primitive1 = new THREE.Mesh()
const primitive2 = new THREE.Mesh()
/* This test still fails
await act(async () =>
root.render(
<group>
<primitive object={primitive1} dispose={null} />
</group>,
),
)
expect((primitive1 as any).__r3f?.type).toBe('primitive')
await act(async () =>
root.render(
<group>
<primitive object={primitive2} dispose={null} />
<primitive object={primitive1} dispose={null} />
</group>,
),
)
expect((primitive1 as any).__r3f?.type).toBe('primitive')
expect((primitive2 as any).__r3f?.type).toBe('primitive')
*/

// Initialize a list of primitives
await act(async () =>
root.render(
<group>
<primitive key={'a'} object={primitive1} name="p2" dispose={null} />
</group>,
),
)

expect((primitive1 as any).__r3f?.type).toBe('primitive')

// New list of primitives while reusing primitive object
await act(async () =>
root.render(
<group>
<primitive key={'b'} object={primitive1} name="p1" dispose={null} />
<primitive key={'c'} object={primitive2} name="p2" dispose={null} />
</group>,
),
)

expect((primitive1 as any).__r3f?.type).toBe('primitive')
expect((primitive2 as any).__r3f?.type).toBe('primitive')
})
})

0 comments on commit ebddb72

Please sign in to comment.