Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EquirectHdrInfoUniform use worker but not disposed in dispose #28

Open
dongho-shin opened this issue Aug 21, 2023 · 3 comments
Open

EquirectHdrInfoUniform use worker but not disposed in dispose #28

dongho-shin opened this issue Aug 21, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@dongho-shin
Copy link
Contributor

I tried to test this pr(pmndrs/react-postprocessing#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
Copy link
Contributor Author

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 dongho-shin reopened this Aug 21, 2023
@dongho-shin dongho-shin changed the title Does realism-effects use worker? EquirectHdrInfoUniform use worker but not disposed in dispose Aug 21, 2023
@dongho-shin
Copy link
Contributor Author

dongho-shin commented Aug 21, 2023

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()
	}

@0beqz 0beqz added the bug Something isn't working label Aug 29, 2023
@0beqz
Copy link
Owner

0beqz commented Aug 29, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants