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

Effect and EffectPass are not completely cleaned up after calling the dispose method #648

Closed
Steve245270533 opened this issue Aug 12, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@Steve245270533
Copy link

Description of the bug

When destroying the entire three context, two methods removeToneMappingPass, removeSceneFadePass are called in the destroy method of the PostProcessingEffect class, both methods call the Effect and EffectPass's dispose method, but by printing renderer.info to see that there are still uncleaned programs

To reproduce

export class PostProcessingEffect implements IPostProcessingEffect {
  private composer: EffectComposer
  private renderer: ICore["renderer"]
  private camera: ICore["camera"]
  private scene: ICore["scene"]
  private sizes: ISizes

  constructor(core: ICore, sizes: ISizes) {
    this.renderer = core.renderer
    this.camera = core.camera
    this.scene = core.scene
    this.sizes = sizes

    this.composer = new EffectComposer(this.renderer)
    this.composer.setSize(this.sizes.width, this.sizes.height)
    this.setupRenderPass()
    this.setupToneMappingEffectPass(core.config.tone_mapping)
    this.setupSceneFadePass()
  }

  private render_pass!: RenderPass
  private setupRenderPass() {
    this.render_pass = new RenderPass(this.scene, this.camera)
    this.composer.addPass(this.render_pass)
  }

  private removeRenderPass() {
    this.removePass(this.render_pass)
    this.render_pass.dispose()
  }

  addPass(pass: Pass, index?: number) {
    this.composer.addPass(pass, index)
    this.ensureToneMappingLast()
  }

  removePass(pass: Pass) {
    this.composer.removePass(pass)
  }

  /**
   * ToneMappingEffect
   */
  private tone_mapping_effect_pass!: EffectPass
  private tone_mapping_effect!: ToneMappingEffect

  private setupToneMappingEffectPass(tone_mapping_mode: ToneMapping) {
    this.tone_mapping_effect = new ToneMappingEffect({ mode: tone_mapping_mode })
    this.tone_mapping_effect_pass = new EffectPass(this.camera, this.tone_mapping_effect)
  }

  private ensureToneMappingLast() {
    const tone_mapping_effect_pass_index = this.composer.passes.findIndex(pass => (pass as EffectPass)?.effects?.some(e => e instanceof ToneMappingEffect))
    if (tone_mapping_effect_pass_index === -1) {
      this.composer.addPass(this.tone_mapping_effect_pass)
    }
    else if (tone_mapping_effect_pass_index !== this.composer.passes.length - 1) {
      this.composer.removePass(this.tone_mapping_effect_pass)
      this.composer.addPass(this.tone_mapping_effect_pass)
    }
  }

  private removeToneMappingPass() {
    this.removePass(this.tone_mapping_effect_pass)
    this.tone_mapping_effect.dispose()
    this.tone_mapping_effect_pass.dispose()
  }

  /**
   * FadeEffect
   */
  private scene_fade_effect!: IFadeEffect
  private scene_fade_pass!: EffectPass
  private scene_fade_animation: ReturnType<typeof gsap.to> | undefined
  private setupSceneFadePass() {
    this.scene_fade_effect = new FadeEffect()
    this.scene_fade_pass = new EffectPass(this.camera, this.scene_fade_effect)
    this.addPass(this.scene_fade_pass)
  }

  setSceneFadeIn(duration: number = 2): Promise<void> {
    return new Promise((resolve) => {
      this.scene_fade_animation?.kill()
      this.scene_fade_animation = gsap.to(this.scene_fade_effect, {
        opacity: 0,
        duration,
        ease: "linear",
        onComplete: () => {
          resolve()
        },
      })
    })
  }

  setSceneFadeOut(duration: number = 2): Promise<void> {
    return new Promise((resolve) => {
      this.scene_fade_animation = gsap.to(this.scene_fade_effect, {
        opacity: 1,
        duration,
        ease: "linear",
        onComplete: () => {
          resolve()
        },
      })
    })
  }

  private removeSceneFadePass() {
    this.removePass(this.scene_fade_pass)
    this.scene_fade_effect.dispose()
    this.scene_fade_pass.dispose()
  }

  /**
   * Vignette Effect
   */
  private vignette_effect: IVignetteEffect | undefined
  private vignette_pass: EffectPass | undefined
  addVignettePass(params?: VignetteEffectParams) {
    this.removeVignettePass()
    this.vignette_effect = new VignetteEffect(params)
    this.vignette_pass = new EffectPass(this.camera, this.vignette_effect)
    this.addPass(this.vignette_pass)
  }

  updateVignettePass(params: Omit<VignetteEffectParams, "blendFunction">) {
    if (!this.vignette_effect)
      return

    const { technique, offset, darkness, center } = params

    this.vignette_effect.technique = technique ?? this.vignette_effect.technique
    this.vignette_effect.offset = offset ?? this.vignette_effect.offset
    this.vignette_effect.darkness = darkness ?? this.vignette_effect.darkness
    this.vignette_effect.center = center ?? this.vignette_effect.center
  }

  removeVignettePass() {
    if (this.vignette_effect && this.vignette_pass) {
      this.removePass(this.vignette_pass)
      this.vignette_effect.dispose()
      this.vignette_pass.dispose()
    }
  }

  resize() {
    this.camera.aspect = this.sizes.aspect
    this.camera.updateProjectionMatrix()
    this.composer.setSize(this.sizes.width, this.sizes.height)
    this.renderer.setPixelRatio(this.sizes.device_pixel_ratio)
  }

  update() {
    this.composer.render()
  }

  destroy() {
    this.removeToneMappingPass()
    this.removeSceneFadePass()
    this.removeVignettePass()
    this.removeRenderPass()
    this.composer.removeAllPasses()
    this.composer.dispose()
  }
}

Expected behavior

renderer.info.programs should be cleared

Screenshots

image

Library versions used

  • Three: [0.164.1]
  • Post Processing: [6.36.0]

Desktop

  • OS: [Windows]
  • Browser [Chrome 127.0.6533.89]
  • Graphics hardware: [NVIDIA]

Mobile

  • Device: [e.g. iPhone]
  • OS: [e.g. Android]
  • Browser [e.g. Firefox X.X.X]
@vanruesc
Copy link
Member

renderer.info.programs should be cleared

Programs and other disposable resources will be recreated automatically if you call render after calling dispose.

Your reproduction steps are incomplete. How exactly are you disposing things? Have you debugged your code and checked whether dispose is being called on the respective resources? Please provide a complete and minimal reproduction of the issue instead of code snippets.

@vanruesc vanruesc added the troubleshooting Root problem must be identified label Aug 14, 2024
@Steve245270533
Copy link
Author

renderer.info.programs should be cleared

Programs and other disposable resources will be recreated automatically if you call render after calling dispose.

Your reproduction steps are incomplete. How exactly are you disposing things? Have you debugged your code and checked whether dispose is being called on the respective resources? Please provide a complete and minimal reproduction of the issue instead of code snippets.

This is the smallest scene I restored. It seems that any Effect added cannot be cleaned up normally.
stackblitz-demo

@vanruesc
Copy link
Member

Thanks for the example 👍

This does indeed look like a bug. The dispose method in Pass doesn't dispose the fullscreen material because that property is defined as a getter/setter which doesn't get picked up by Object.keys. I'll work on a fix.

In the meantime, you can manually dispose those materials as shown here: vitejs-vite-als1ht

Note that the remaining geometry is the fullscreen triangle that is shared by all postprocessing passes. The lifetime of that mesh is basically tied to the WebGL context and it's being reused when needed so it shouldn't do any harm. That being said, I'll check if it can be disposed somehow.

@vanruesc vanruesc added bug Something isn't working and removed troubleshooting Root problem must be identified labels Aug 15, 2024
@Steve245270533
Copy link
Author

Thank you very much. Currently, manually disposing of these passes materials is effective.

vanruesc added a commit that referenced this issue Sep 2, 2024
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