OpenMfxForBlender icon indicating copy to clipboard operation
OpenMfxForBlender copied to clipboard

Loading .blend file with different plugin version crashes Blender

Open tkarabela opened this issue 3 years ago • 1 comments

How to reproduce (example with MfxVTK):

  • copy mfx_vtk_plugin-0.3.0.ofx to test.ofx
  • open Blender, apply "Volume sampling" effect from test.ofxto default cube, save test.blend, exit Blender
  • copy mfx_vtk_plugin-0.4.0.ofx to test.ofx (note: this no longer has effect named "Volume sampling")
  • open Blender, load test.blend, crash

AFAICT, this particular crash is due to read oveflow in OpenMeshEffectRuntime::get_parameters_from_rna().

There are more possible flavours of this issue (I haven't tested it deeply), like:

  • the blend file specifies plugin path which does not exist now
  • plugin does not have the effect from blend file (should it behave like user didn't select any effect from the plugin?)
  • blend file has extra parameters which the plugin doesn't have (should they be thrown away?)
  • plugin has extra parameters which the blend file doesn't have (should they use defaults?)
  • plugin has different types/min/max for the parameters compared to blend file

In all cases, the host ought to fall back to some default and/or show an error message. Beyond this, I wonder if the plugins should manifest their versions in some way (eg. major/minor versions, like semver?) - using a different plugin version may have unexpected consequences even if the parameters look valid, it seems fair to at least notify the user in such cases.

tkarabela avatar Oct 25 '20 22:10 tkarabela

Ideally ofx plugins should be treated as external ressources the same way e.g. texture images are external resources. It may be a bad idea to pack them into .blend files though for security reasons.

the blend file specifies plugin path which does not exist now

I think currently it displays some "could not load plugin" message but doesn't crash. We should ensure that in such case it remembers the saved parameter values though, so that somebody that does not have the plugin can still interact with other parts of the scene and save it again without breaking it for users who have the plugin.

blend file has extra parameters which the plugin doesn't have (should they be thrown away?) plugin has extra parameters which the blend file doesn't have (should they use defaults?)

These two cases currently crash Blender because it remembers parameters by indices (which may well be a poor choice). Both of these situations are cases of plugin version changes; I should go and read again OpenFX' original doc about plugin versioning. I know that I've spent only little time on this topic in the current implementation so far!

plugin does not have the effect from blend file (should it behave like user didn't select any effect from the plugin?)

Selected plugin is identified by index, so problem as well!

plugin has different types/min/max for the parameters compared to blend file

A change of type is a case of plugin version change I think. For min/max we could still store the original value but apply boundaries in paramGetValue.

eliemichel avatar Oct 27 '20 21:10 eliemichel