realism-effects icon indicating copy to clipboard operation
realism-effects copied to clipboard

EquirectHdrInfoUniform use worker but not disposed in dispose

Open dongho-shin opened this issue 1 year ago • 3 comments

I tried to test this pr(https://github.com/pmndrs/react-postprocessing/pull/211) for test in react-postprocessing

image
I found when SSGIEffect instance created, Javascript VM instance has added so I think it is kind of workers but I can't find where the worker created in chrome Mac OS

dongho-shin avatar Aug 21 '23 11:08 dongho-shin

oh I found it... https://github.com/0beqz/realism-effects/blob/main/src/ssgi/utils/EquirectHdrInfoUniform.js

updateFrom(map) {
		map = map.clone()
		const { width, height, data } = map.image
		const { type } = map

		this.size.set(width, height)

		return new Promise(resolve => {
			this.worker?.terminate()

			this.worker = new Worker(workerUrl)

			this.worker.postMessage({ width, height, isFloatType: type === FloatType, flipY: map.flipY, data })
			this.worker.onmessage = ({ data: { data, totalSumValue, marginalDataArray, conditionalDataArray } }) => {
				this.dispose()

dongho-shin avatar Aug 21 '23 11:08 dongho-shin

I think function dispose in EquirectHdrInfoUniform should terminate workers I'll make a PR for this

	dispose() {
		this.marginalWeights.dispose()
		this.conditionalWeights.dispose()
		this.map.dispose()
	}

dongho-shin avatar Aug 21 '23 11:08 dongho-shin

Oh thanks for seeing that, it can indeed be problematic for memory. I should fix that before releasing the new version.

0beqz avatar Aug 29 '23 18:08 0beqz