From 55bd7289af653cd88bb1dd4929842201a51aa39b Mon Sep 17 00:00:00 2001 From: Bela Bohlender Date: Wed, 30 Oct 2024 11:13:02 +0100 Subject: [PATCH] fix leaking geometries and materials --- examples/uikit/src/App.tsx | 423 +++++++++--------- packages/react/src/suspending.tsx | 3 + packages/react/src/video.tsx | 1 + packages/uikit/src/components/icon.ts | 10 +- packages/uikit/src/components/image.ts | 80 ++-- .../uikit/src/panel/instanced-panel-group.ts | 6 + .../uikit/src/panel/instanced-panel-mesh.ts | 1 + .../src/text/render/instanced-glyph-group.ts | 10 + .../src/text/render/instanced-glyph-mesh.ts | 1 + packages/uikit/src/vanilla/custom.ts | 4 +- packages/uikit/src/vanilla/video.ts | 1 + 11 files changed, 292 insertions(+), 248 deletions(-) diff --git a/examples/uikit/src/App.tsx b/examples/uikit/src/App.tsx index a3220487..5f90929a 100644 --- a/examples/uikit/src/App.tsx +++ b/examples/uikit/src/App.tsx @@ -23,6 +23,7 @@ import { import { Texture } from 'three' import { Skeleton } from '../../../packages/kits/default/src/skeleton.js' import { forwardHtmlEvents } from '@pmndrs/pointer-events' +import { Perf } from 'r3f-perf' export default function App() { const texture = useMemo(() => signal(undefined), []) @@ -60,9 +61,9 @@ export default function App() { ({ enabled: false, priority: 0 })} - frameloop="demand" style={{ height: '100dvh', touchAction: 'none' }} gl={{ localClippingEnabled: true }} + onClick={() => setShow((s) => !s)} > @@ -76,243 +77,253 @@ export default function App() { (texture.value = t ?? undefined)}> - - {/* Tests for the Portal component.*/} - - + + {show && ( + + {/* Tests for the Portal component.*/} + + - {/* By default, the Portal should create it's own camera and thus + {/* By default, the Portal should create it's own camera and thus not be affected by the scene camera and orbit controls..*/} - {/* + {/* */} - {/* However, we can provide a camera with custom properties, like + {/* However, we can provide a camera with custom properties, like a different position or field of view. Note that the aspect ratio will be overriden to match with the screens aspect ratio, s.t. resizing the screen would not distort the portal view.*/} - {/* + {/* */} - {/* The resizing should work for the orthographic camera as well.*/} - - - - - - - - - - Escribe algo... - - - - Escribe algo... + {/* The resizing should work for the orthographic camera as well.*/} + + + + + - - - - { - t.value += 'X' - setShow((s) => !s) - }} - flexShrink={0} - width="100%" - backgroundOpacity={0.5} - backgroundColor="black" - fontSize={30} - verticalAlign="bottom" - textAlign="block" - cursor="pointer" - color="white" - > - {t} - more - (x.value = hover ? 'yellow' : undefined)} - backgroundColor={x} - borderColor="white" flexDirection="column" - borderBend={1} - borderWidth={20} - borderRadius={30} - width={300} - height={100} - /> - console.log(w, h)} - keepAspectRatio={false} - borderRightWidth={100} + backgroundColor="blue" + width={100} + positionType="relative" > - - - - - - - - - - - - }> - + + Escribe algo... + + + + Escribe algo... + + + + + { + t.value += 'X' + setShow((s) => !s) + }} flexShrink={0} - hover={{ padding: 30, marginLeft: -30, opacity: 1 }} - objectFit="cover" - borderWidth={20} - ref={ref} - onHoverChange={(hovered) => ref.current?.setStyle({ borderOpacity: hovered ? 1 : 0.5 })} - borderOpacity={0.5} - borderRadius={10} + width="100%" + backgroundOpacity={0.5} + backgroundColor="black" + fontSize={30} + verticalAlign="bottom" + textAlign="block" + cursor="pointer" + color="white" + > + {t} + more + + (x.value = hover ? 'yellow' : undefined)} + backgroundColor={x} + borderColor="white" flexDirection="column" - src="https://picsum.photos/2000/3000" + borderBend={1} + borderWidth={20} + borderRadius={30} width={300} - overflow="scroll" + height={100} + /> + console.log(w, h)} + keepAspectRatio={false} + borderRightWidth={100} > - + + + + + + + + + + + }> + ref.current?.setStyle({ borderOpacity: hovered ? 1 : 0.5 })} + borderOpacity={0.5} + borderRadius={10} + flexDirection="column" + src="https://picsum.photos/2000/3000" + width={300} + overflow="scroll" > - Hello World! - - - Lorem voluptate aliqua est veniam pariatur enim reprehenderit nisi laboris. Tempor sit magna ea - occaecat velit veniam ipsum do deserunt adipisicing labore. Voluptate consectetur Lorem - exercitation laborum do nulla velit sit. Aliqua sit cupidatat excepteur fugiat. Labore proident ea - in in ex ad aute adipisicing ad in occaecat ullamco tempor pariatur. Excepteur consequat ullamco - id est duis elit. Est duis mollit adipisicing labore fugiat duis elit magna. Deserunt nulla dolore - deserunt id sint fugiat cillum cupidatat nulla dolore veniam anim nulla sunt. Excepteur excepteur - nisi officia eiusmod incididunt do. Id reprehenderit aute nulla dolor ut ex veniam aliqua laboris - nisi. Aliqua aute nulla fugiat dolor voluptate quis. Velit sit aliqua eiusmod irure. - - - - - - - input?.focus()} - positionType="absolute" - positionBottom="100%" - positionRight="100%" - marginRight={10} - flexDirection="column" - backgroundColor="red" - > - console.log('focus change', focus)} - backgroundColor="white" - width="100%" - height="100%" - fontSize={100} - color="red" - wordBreak="keep-all" - caretWidth={10} - caretBorderRadius={5} - caretBorderWidth={3} - caretOpacity={0} - caretBorderColor="orange" - selectionOpacity={0} - selectionBorderRadius={5} - selectionBorderWidth={3} - selectionBorderColor="orange" - focus={{ borderRadius: 20 }} - verticalAlign="center" - textAlign="center" - multiline - defaultValue="Hello world" - /> - + + Hello World! + + + Lorem voluptate aliqua est veniam pariatur enim reprehenderit nisi laboris. Tempor sit magna ea + occaecat velit veniam ipsum do deserunt adipisicing labore. Voluptate consectetur Lorem + exercitation laborum do nulla velit sit. Aliqua sit cupidatat excepteur fugiat. Labore proident + ea in in ex ad aute adipisicing ad in occaecat ullamco tempor pariatur. Excepteur consequat + ullamco id est duis elit. Est duis mollit adipisicing labore fugiat duis elit magna. Deserunt + nulla dolore deserunt id sint fugiat cillum cupidatat nulla dolore veniam anim nulla sunt. + Excepteur excepteur nisi officia eiusmod incididunt do. Id reprehenderit aute nulla dolor ut ex + veniam aliqua laboris nisi. Aliqua aute nulla fugiat dolor voluptate quis. Velit sit aliqua + eiusmod irure. + + + + - {show ? ( (s.value += 10)} - backgroundColor="yellow" - width={300} - minHeight={300} - height={300} + width={100} + height={100} + onClick={() => input?.focus()} + positionType="absolute" + positionBottom="100%" + positionRight="100%" + marginRight={10} flexDirection="column" + backgroundColor="red" + > + console.log('focus change', focus)} + backgroundColor="white" + width="100%" + height="100%" + fontSize={100} + color="red" + wordBreak="keep-all" + caretWidth={10} + caretBorderRadius={5} + caretBorderWidth={3} + caretOpacity={0} + caretBorderColor="orange" + selectionOpacity={0} + selectionBorderRadius={5} + selectionBorderWidth={3} + selectionBorderColor="orange" + focus={{ borderRadius: 20 }} + verticalAlign="center" + textAlign="center" + multiline + defaultValue="Hello world" /> - - ) : undefined} - + + {show ? ( + + (s.value += 10)} + backgroundColor="yellow" + width={300} + minHeight={300} + height={300} + flexDirection="column" + /> + + + ) : undefined} + + )} diff --git a/packages/react/src/suspending.tsx b/packages/react/src/suspending.tsx index d11bc4b8..b994a463 100644 --- a/packages/react/src/suspending.tsx +++ b/packages/react/src/suspending.tsx @@ -9,6 +9,9 @@ export type SuspendingImageProperties = ImageProperties & { children?: ReactNode } +/** + * be aware that this component does not dispose the loaded texture + */ export const SuspendingImage: ( props: SuspendingImageProperties & RefAttributes>>, ) => ReactNode = forwardRef(({ src, ...props }, ref) => { diff --git a/packages/react/src/video.tsx b/packages/react/src/video.tsx index 4b093b0f..2a674357 100644 --- a/packages/react/src/video.tsx +++ b/packages/react/src/video.tsx @@ -81,6 +81,7 @@ export const Video: (props: VideoProperties & RefAttributes) => useEffect(() => { const videoTexture = new VideoTexture(element) videoTexture.colorSpace = SRGBColorSpace + videoTexture.needsUpdate = true texture.value = videoTexture return () => videoTexture.dispose() }, [texture, element]) diff --git a/packages/uikit/src/components/icon.ts b/packages/uikit/src/components/icon.ts index 2f868b6a..69e4c61b 100644 --- a/packages/uikit/src/components/icon.ts +++ b/packages/uikit/src/components/icon.ts @@ -1,5 +1,5 @@ import { Signal, effect, signal } from '@preact/signals-core' -import { Color, Group, Mesh, MeshBasicMaterial, Plane, ShapeGeometry } from 'three' +import { BufferGeometry, Color, Group, Material, Mesh, MeshBasicMaterial, Plane, ShapeGeometry } from 'three' import { Listeners } from '../index.js' import { Object3DRef, ParentContext } from '../context.js' import { FlexNode, FlexNodeState, YogaProperties, createFlexNodeState } from '../flex/index.js' @@ -221,6 +221,14 @@ function createIconGroup( group.updateMatrix() parentContext.root.requestRender() }), + () => () => + group.children.forEach((child) => { + if (!(child instanceof Mesh)) { + return + } + ;(child.geometry as BufferGeometry).dispose() + ;(child.material as Material).dispose() + }), () => effect(() => { group.visible = isVisible.value diff --git a/packages/uikit/src/components/image.ts b/packages/uikit/src/components/image.ts index 33dc8896..3fb278b3 100644 --- a/packages/uikit/src/components/image.ts +++ b/packages/uikit/src/components/image.ts @@ -268,48 +268,44 @@ function createImageMesh( mesh.spherecast = makeClippedCast(mesh, makePanelSpherecast(mesh), root.object, parentContext.clippingRect, orderInfo) setupRenderOrder(mesh, root, orderInfo) const objectFit = computedInheritableProperty(propertiesSignal, 'objectFit', defaultImageFit) - initializers.push(() => - effect(() => { - const texture = textureSignal.value - if (texture == null || flexState.size.value == null || flexState.borderInset.value == null) { - return - } - texture.matrix.identity() - root.requestRender() - - if (objectFit.value === 'fill' || texture == null) { - transformInsideBorder(flexState.borderInset, flexState.size, texture) - return - } + initializers.push( + () => + effect(() => { + const texture = textureSignal.value + if (texture == null || flexState.size.value == null || flexState.borderInset.value == null) { + return + } + texture.matrix.identity() + root.requestRender() - const { width: textureWidth, height: textureHeight } = texture.source.data as { width: number; height: number } - const textureRatio = textureWidth / textureHeight - - const [width, height] = flexState.size.value - const [top, right, bottom, left] = flexState.borderInset.value - const boundsRatioValue = (width - left - right) / (height - top - bottom) - - if (textureRatio > boundsRatioValue) { - texture.matrix - .translate(-(0.5 * (boundsRatioValue - textureRatio)) / boundsRatioValue, 0) - .scale(boundsRatioValue / textureRatio, 1) - } else { - texture.matrix - .translate(0, -(0.5 * (textureRatio - boundsRatioValue)) / textureRatio) - .scale(1, textureRatio / boundsRatioValue) - } - transformInsideBorder(flexState.borderInset, flexState.size, texture) - }), - ) + if (objectFit.value === 'fill' || texture == null) { + transformInsideBorder(flexState.borderInset, flexState.size, texture) + return + } - initializers.push(() => - effect(() => { - mesh.visible = isMeshVisible.value - parentContext.root.requestRender() - }), - ) + const { width: textureWidth, height: textureHeight } = texture.source.data as { width: number; height: number } + const textureRatio = textureWidth / textureHeight - initializers.push( + const [width, height] = flexState.size.value + const [top, right, bottom, left] = flexState.borderInset.value + const boundsRatioValue = (width - left - right) / (height - top - bottom) + + if (textureRatio > boundsRatioValue) { + texture.matrix + .translate(-(0.5 * (boundsRatioValue - textureRatio)) / boundsRatioValue, 0) + .scale(boundsRatioValue / textureRatio, 1) + } else { + texture.matrix + .translate(0, -(0.5 * (textureRatio - boundsRatioValue)) / textureRatio) + .scale(1, textureRatio / boundsRatioValue) + } + transformInsideBorder(flexState.borderInset, flexState.size, texture) + }), + () => + effect(() => { + mesh.visible = isMeshVisible.value + parentContext.root.requestRender() + }), () => effect(() => { const map = textureSignal.value ?? null @@ -405,10 +401,14 @@ function setupImageMaterials( const material = createPanelMaterial(panelMaterialClass.value, info) material.clippingPlanes = clippingPlanes target.material = material - return effect(() => { + const cleanupDepthTestEffect = effect(() => { material.depthTest = root.depthTest.value root.requestRender() }) + return () => { + cleanupDepthTestEffect() + material.dispose() + } }), effect(() => { target.renderOrder = root.renderOrder.value diff --git a/packages/uikit/src/panel/instanced-panel-group.ts b/packages/uikit/src/panel/instanced-panel-group.ts index 039426dd..20cef063 100644 --- a/packages/uikit/src/panel/instanced-panel-group.ts +++ b/packages/uikit/src/panel/instanced-panel-group.ts @@ -325,6 +325,12 @@ export class InstancedPanelGroup { destroy() { clearTimeout(this.nextUpdateTimeoutRef) + if (this.mesh == null) { + return + } + this.object.current?.remove(this.mesh) + this.mesh?.dispose() + this.instanceMaterial.dispose() } } diff --git a/packages/uikit/src/panel/instanced-panel-mesh.ts b/packages/uikit/src/panel/instanced-panel-mesh.ts index 48a475fa..34229ac8 100644 --- a/packages/uikit/src/panel/instanced-panel-mesh.ts +++ b/packages/uikit/src/panel/instanced-panel-mesh.ts @@ -65,6 +65,7 @@ export class InstancedPanelMesh extends Mesh { dispose() { this.dispatchEvent({ type: 'dispose' as keyof Object3DEventMap }) + this.geometry.dispose() } copy(): this { diff --git a/packages/uikit/src/text/render/instanced-glyph-group.ts b/packages/uikit/src/text/render/instanced-glyph-group.ts index 378b2d0b..251b13b2 100644 --- a/packages/uikit/src/text/render/instanced-glyph-group.ts +++ b/packages/uikit/src/text/render/instanced-glyph-group.ts @@ -24,6 +24,7 @@ export class GlyphGroupManager { root.onFrameSet.add(onFrame) return () => root.onFrameSet.delete(onFrame) }, + () => () => this.traverse((group) => group.destroy()), () => effect(() => { const ro = renderOrder.value @@ -276,6 +277,15 @@ export class InstancedGlyphGroup { this.mesh.count = this.glyphs.length this.object.current?.add(this.mesh) } + + destroy() { + if (this.mesh == null) { + return + } + this.object.current?.remove(this.mesh) + this.mesh.dispose() + this.instanceMaterial.dispose() + } } function copyBuffer( diff --git a/packages/uikit/src/text/render/instanced-glyph-mesh.ts b/packages/uikit/src/text/render/instanced-glyph-mesh.ts index 61afbfbb..63c5e982 100644 --- a/packages/uikit/src/text/render/instanced-glyph-mesh.ts +++ b/packages/uikit/src/text/render/instanced-glyph-mesh.ts @@ -31,6 +31,7 @@ export class InstancedGlyphMesh extends Mesh { dispose() { this.dispatchEvent({ type: 'dispose' as keyof Object3DEventMap }) + this.geometry.dispose() } //functions not needed because intersection (and morphing) is intenionally disabled diff --git a/packages/uikit/src/vanilla/custom.ts b/packages/uikit/src/vanilla/custom.ts index a14b5f78..67c75b58 100644 --- a/packages/uikit/src/vanilla/custom.ts +++ b/packages/uikit/src/vanilla/custom.ts @@ -14,6 +14,7 @@ export class CustomContainer extends Component { private readonly defaultPropertiesSignal: Signal private readonly parentContextSignal = createParentContextSignal() private readonly unsubscribe: () => void + private readonly material = new MeshBasicMaterial() constructor(properties?: CustomContainerProperties, defaultProperties?: AllOptionalProperties) { super() @@ -22,7 +23,7 @@ export class CustomContainer extends Component { this.propertiesSignal = signal(properties) this.defaultPropertiesSignal = signal(defaultProperties) - const mesh = new Mesh(panelGeometry, new MeshBasicMaterial()) + const mesh = new Mesh(panelGeometry, this.material) super.add(mesh) this.unsubscribe = effect(() => { @@ -79,5 +80,6 @@ export class CustomContainer extends Component { destroy() { this.parent?.remove(this) this.unsubscribe() + this.material.dispose() } } diff --git a/packages/uikit/src/vanilla/video.ts b/packages/uikit/src/vanilla/video.ts index 1cc7b388..87e45823 100644 --- a/packages/uikit/src/vanilla/video.ts +++ b/packages/uikit/src/vanilla/video.ts @@ -20,6 +20,7 @@ export class Video extends Image { const element = props.src instanceof HTMLVideoElement ? props.src : document.createElement('video') updateVideoElement(element, props) const texture = new VideoTexture(element) + texture.needsUpdate = true const aspectRatio = signal(1) super({ aspectRatio, ...props, src: texture }, defaultProperties)