tguerin / newton

An Easy To Use Particle Emitter
https://newton.7omtech.fr
MIT License
25 stars 8 forks source link

[Question] Best way to dispose image rendered for 1 dynamically added effect #40

Open Joshix-1 opened 1 month ago

Joshix-1 commented 1 month ago

I'm dynamically rendering an image and then show it as particle using ImageShape. I'm adding it to the NewtonWIdget with the addEffect method on the state.

What would be the best way to dispose the image after the effect has finished?

The closest thing would be the ParticleConfiguration postEffectBuilder, but that would fail with more than 1 particle (which is common)

With onEffectStateChanged on the Newton it would be hard to know which images to dispose.

Maybe the Effect should have a dispose method that calls a user supplied callable or something similar.

tguerin commented 1 month ago

Hi, you can use the stateCallback to listen to effect state change. When effect is in state killed you can dispose the image. For the image to dispose, you normally know the one used by the effect as it's declared in the configuration. If you don't keep a reference to the image in your widget, it will be disposed automatically when effect is killed and removed.

Joshix-1 commented 1 month ago

Do you mean Effect.stateChangeCallback? That doesn't work as Newton.addEffect overrides it. Newton.onEffectStateChanged would work, but that could cause errors if i in the future use the same Newton with images I do not want to dispose.

void newtonOnEffectStateChangedDisposeImage(Effect effect, EffectState state) {
  if (state == EffectState.killed) {
    final shape = effect.particleConfiguration.shape;
    if (shape is ImageShape) {
      shape.image.dispose();
    }
  }
}

Edit: the snippet above causes assertion errors in dart:ui/painting.dart

tguerin commented 1 month ago

A possible solution is to wrap the existing callback if there is any and forward the state. So you could register your own and wouldn’t be erased.

Regarding image retention there is 2 cases:

All of this makes me wonder, if I shouldn’t change the ImageShape to provide a hook where you can give the image you want to render instead of having it statically configured. It would leverage new possibilities such that dynamically change the image without updating the configuration of the effect.

Joshix-1 commented 1 month ago

effect at runtime, as the effect is removed at the end the image will be garbage collected

https://github.com/flutter/flutter/issues/83274 makes it seem like calling dispose before garbage collection is still be a good idea.

But I'm not sure they even will be garbage collected. NewtonPainter._allImages seems to keep them in storage and uses the images with canvas.drawAtlas. (Which then causes an exception with my code snipped above) (I was using 4b9a8bb5105c0a110907ec5117aaa3068379d54c, it was fixed in the commit after)

tguerin commented 1 month ago

effect at runtime, as the effect is removed at the end the image will be garbage collected

flutter/flutter#83274 makes it seem like calling dispose before garbage collection is still be a good idea.

Good to know, I still think we could let the client handle the image they want to provide by giving them a hook at the right place. I find interesting that we could update the image without changing the configuration of the effect.

~But I'm not sure they even will be garbage collected. NewtonPainter._allImages seems to keep them in storage and uses the images with canvas.drawAtlas. (Which then causes an exception with my code snipped above)~ (I was using 4b9a8bb, it was fixed in the commit after)

Yes there was a leak before