Blender 2.92 branch (openmesheffect-nightly) issues

[tkarabela]

I've tried building https://github.com/eliemichel/OpenMeshEffectForBlender/commit/6e1ec285e6c6485927881534edaf08b90e9719a4 on my Linux machine (via make debug lite) and I encountered the following:

• problem 1
  • at runtime, open Blender, add Open Mesh Effect modifier to default cube
  • fails assert in source/blender/blenkernel/intern/modifier.c:414
• problem 2
  • at compile time, lots on undefined stuff in intern/openmesheffect/test/test_plugin_load.cpp
  • PluginRegistry, use_plugin(), init_properties(), PROP_CTX_OTHER, propSetPointer() etc.

[eliemichel]

Oops I had forgotten to push a commit, that fixes problem 1 (it was a stupid typo in reference counting). Regarding problem 2 I'll have a look at it.

[eliemichel]

I pushed a slight fix, tested with gcc 10 on debian. (I should eventually test with the spec from the VFX reference plateform btw)

[tkarabela]

Confirming that both problems still happen on my machine with 9f737c8b6381950e9059ac86d6358c19e44b6373.

More info on problem 1:

• in stars on line 81 here: https://github.com/eliemichel/OpenMeshEffectForBlender/blob/9f737c8b6381950e9059ac86d6358c19e44b6373/source/blender/modifiers/intern/MOD_openmesheffect.c#L73-L86

• inside, it tries to ensure_runtime(): https://github.com/eliemichel/OpenMeshEffectForBlender/blob/9f737c8b6381950e9059ac86d6358c19e44b6373/intern/openmesheffect/blender/intern/mfxModifier.cpp#L68-L75

• inside ensure_runtime(), the is_plugin_valid() call returns false, making the OFX modifier to call BKE_modifier_set_error(): https://github.com/eliemichel/OpenMeshEffectForBlender/blob/9f737c8b6381950e9059ac86d6358c19e44b6373/intern/openmesheffect/blender/intern/mfxModifier.cpp#L60-L62

• then BKE_modifier_set_error() calls BLI_findindex() in assert here (both ob and ob_orig are NULL): https://github.com/eliemichel/OpenMeshEffectForBlender/blob/9f737c8b6381950e9059ac86d6358c19e44b6373/source/blender/blenkernel/intern/modifier.c#L410-L415

• BLI_findindex() itself segfaults before it can fail the assert, since the modifier list &ob_orig->modifiers is invalid (curiously, just creating a value &(NULL)->modifiers doesn't segfault, since GCC compiles it as NULL + offsetof(Object, modifiers))

It seems to me that one is not supposed to call BKE_modifier_set_error(NULL, ...).

[eliemichel]

Oh the #ifndef NDEBUG wasn't enabled for me so I only patched the CLOG_ERROR. I was a bit frustrated because BKE_modifier_set_error used not to require the object as a parameter, and it is used only for message logging (and for this assertion as well then). Even though it is not ideal to patch this to handle null argument, it seemed better than pulling a reference to the object holding the modifier at many places in the code just for this.

[tkarabela]

Edit: nevermind, the Hook modifier stores its additional input as Object, not the object it is attached to.

BTW, if the effect instance currently doesn't keep a pointer to its Object, AFAICT it will be needed later anyway, since vertex group names are stored in Object. See the Hook modifier source and MOD_get_vgroup() helper function. Hook keeps its Object pointer in its ModifierData:

https://github.com/eliemichel/OpenMeshEffectForBlender/blob/9f737c8b6381950e9059ac86d6358c19e44b6373/source/blender/modifiers/intern/MOD_hook.c#L287-L295

https://github.com/eliemichel/OpenMeshEffectForBlender/blob/9f737c8b6381950e9059ac86d6358c19e44b6373/source/blender/modifiers/intern/MOD_hook.c#L315-L332

[eliemichel]

I just fixed one remaining linking issue with examples and fixed the issue in BKE_modifier_set_error.

BTW, if the effect instance currently doesn't keep a pointer to its Object, AFAICT it will be needed later anyway, since vertex group names are stored in Object.

Indeed they are on the object but we'll have to be carefull regarding licensing. All the host code shared with the OpenMeshEffect repository must be GPL-free. So far only the blender subdirectory of the intern/openmesheffect project is bound to the GPL and I'd like things to remain so. Leaking GPL code into the shared code would prevent adoption from closed source programs.

[tkarabela]

Indeed they are on the object but we'll have to be carefull regarding licensing. All the host code shared with the OpenMeshEffect repository must be GPL-free. So far only the blender subdirectory of the intern/openmesheffect project is bound to the GPL and I'd like things to remain so. Leaking GPL code into the shared code would prevent adoption from closed source programs.

I understand these concerns. AFAIK, Blender-specific runtime data reside in source/blender/makesdna/DNA_modifier_types.h and are used by code in intern/openmesheffect/blender/intern to communicate with the (potentially closed-source) plug-in through the OFX interface; the plugin itself only has an opaque handle to the OFX host. So, storing some additional runtime data on the Blender host side should not GPL-ifiy any more code that we have now.

https://github.com/eliemichel/OpenMeshEffectForBlender/blob/c74fb98aa80d0f4925dd5985ee68e7bb6f6483ac/source/blender/makesdna/DNA_modifier_types.h#L2350-L2364

I just fixed one remaining linking issue with examples and fixed the issue in BKE_modifier_set_error.

Great, with the latest version I've managed to get a "nightly" build running! :)

After brief testing, I've encountered some issue with parameter UI, here's what I did:

• built c74fb98aa80d0f4925dd5985ee68e7bb6f6483ac with make debug lite (Ubuntu 20.04 LTS, GCC 9.3)
• open Blender, add Suzanne, add modifier, set plugin to MfxVTK 0.4.0 release build
• modifier works, but options UI is broken

Screenshot:

Log:

== mfx_Modifier_do on data 0x61d000956088
== ensure_runtime on data 0x61d000956088
Getting Global Host; reference counter will be set to 2.
Defining input 'OfxMeshMainInput' on OfxMeshEffectHandle 0x6190010e5f80
Defining input 'OfxMeshMainOutput' on OfxMeshEffectHandle 0x6190010e5f80
OfxActionDescribe action returned status 0 (kOfxStatOK)
OfxActionCreateInstance action returned status 0 (kOfxStatOK)
==/ ensure_runtime
VtkEffect::IsIdentity - no
OfxMeshEffectActionIsIdentity action returned status 14 (kOfxStatReplyDefault)
== VtkEffect::Cook
...
==/ VtkEffect::Cook
OfxMeshEffectActionCook action returned status 0 (kOfxStatOK)
==/ mfx_Modifier_do
uiItemR: property not found: OpenMeshEffectModifier.float_value
uiItemR: property not found: OpenMeshEffectModifier.boolean_value
uiItemR: property not found: OpenMeshEffectModifier.boolean_value
python line lookup failed, interpreter inactive
python line lookup failed, interpreter inactive
python line lookup failed, interpreter inactive
python line lookup failed, interpreter inactive
python line lookup failed, interpreter inactive
python line lookup failed, interpreter inactive
uiItemR: property not found: OpenMeshEffectModifier.float_value
uiItemR: property not found: OpenMeshEffectModifier.boolean_value
uiItemR: property not found: OpenMeshEffectModifier.boolean_value

[eliemichel]

I'll look this up, in the meantime I just pushed on this nightly branch a slight refactor of the callback file to pave the way to cleaner code, and also to handle kOfxMeshPropTransformMatrix. Which also means I figured out a way to get access to the object, but still not enough to ensure that we have it at hand when calling BKE_modifier_set_error.

[tkarabela]

in the meantime I just pushed on this nightly branch a slight refactor of the callback file to pave the way to cleaner code, and also to handle kOfxMeshPropTransformMatrix

It looks much more maintainable now :+1:

Not sure if two cachelines worth of data are worth threading here ^^ I would save this for converting attribute buffers. (Looking into Blender's code, it seems that they are moving away from OpenMP and prefer using their own parallel primitives, like in the BKE_mesh_calc_edges() function.)

https://github.com/eliemichel/OpenMeshEffectForBlender/blob/d59d96eb21ce8547d0d770e81fe95e7545d92031/intern/openmesheffect/blender/intern/mfxCallbacks.cpp#L663-L666

[eliemichel]

Ow yeah a leftover of a test I wanted to see if it complains when using openmp ^^ Fixed the parameter issue:

[eliemichel]

@tkarabela Can we consider this solved?

[tkarabela]

@tkarabela Can we consider this solved?

Yes, I've tried building current nightly f579859b365f63131ce16595a8ea92ed37abba6d in "debug lite" and it looks fine with MfxVTK on Linux :)

[eliemichel]

Great news, I'll switch main to being this 2.82-based branch eventually then :)