pmndrs / postprocessing

A post processing library for three.js.
zlib License
2.33k stars 211 forks source link

Sobel messes up Environment in 6.24.1 #353

Closed LorenzWieseke closed 2 years ago

LorenzWieseke commented 2 years ago

Description of the bug

When I update from 6.23.3 to 6.24.1 somehow after activating the Sobel effect my environment reflection is messed up.

Changing the Environment will work but when I change it back it's still not working right.

To Reproduce

With 6.23.3 https://platform.govie.de/share/draft/F9ALTRzy3uRsfpNAo?Datsun-GT

With 6.24.1 https://beta.govie.de/share/draft/F9ALTRzy3uRsfpNAo?Datsun-GT

Expected behavior

A clear and concise description of what you expected to happen.

Screenshots

24 1_before_Sobel 24 1_Sobel 24 1_after_Sobel

Library versions used

Desktop

Mobile

vanruesc commented 2 years ago

Does the effect work when you're using postprocessing@6.23.3 in combination with three r135, r136, r137 and r138? If not, does it break when you use postprocessing@6.23.4? Or does it only break when you use postprocessing@6.24.x? It's important to identify the exact point where it breaks to figure out what could cause it and if it's exclusively related to three.

The error could be in the Environment setup of this particular app or in the mentioned Sobel effect which is not included in postprocessing. It would be easier to debug this if you could provide a minimal reproduction of the issue.

LorenzWieseke commented 2 years ago

So we spend some time and figured out:

  1. its not about the sobel, it happens with any of the effects
  2. it was the pass.dispose() that somehow cleared something
  3. tried to set pass.renderer = null before so its not getting cleared but that didn't help, exclude clearPass or renderTarget from dispose didn't work cause we use _composer.removeAllPasses()
  4. no we just stick with removing pass.dispose hoping the GPU memory doesn't get to full, but using the chrome task manger shows me the size is staying the same
vanruesc commented 2 years ago

Thanks for investigating!

This looks like an unintended side-effect of the Pass.dispose and Effect.dispose methods that went unnoticed during the recent changes. Since postprocessing@6.24.0, passes and effects keep a reference to the renderer and since it has a dispose method, it will automatically be disposed whenever a pass or effect is disposed.

I'll release a fix in a bit.

vanruesc commented 2 years ago

Should be fixed in postprocessing@6.25.0. Sorry for the delay!