Handle NoLooseEdge and ConstantFaceCount properties

[eliemichel]

See OpenMeshEffect#6
Assigned to @tkarabela

[tkarabela]

I have the implementation mostly done, just doing more testing to make sure it's correct. Out of curiosity, I did some profiling with Stanford 3D scans. Performance with my branch looks pretty much the same as now, with improvement for edge wireframe meshes (about 5x faster, it was plenty fast already). Below are results for the new branch, release build, Intel i7-7500U (shouldn't matter too much as it's all single threaded). Attributes were copied, not forwarded in the effect.

Dataset	before_mesh_get() [ms]	before_mesh_release(), total [ms]	before_mesh_release(), just BKE_mesh_calc_edges() [ms]
dragon (poly, 1 mil.)	0	244	229
dragon (poly + 1 loose edge)	9	249	~230
dragon (edge wireframe)	7	12	n/a
dragon (point cloud)	0	3	n/a
xyzrgb dragon (poly, 7 mil.)	15	1768	1639

My thoughts:

• for poly meshses, >90% time is spent in Blender's own edge bookkeeping
  • it looks like kOfxMeshEffectPropIsDeformation could be a big perf win
  • not much point in parallelizing the buffer copying on Open Mesh Effect side
• difference between shared buffers vs. copying to meshAlloc() buffers (dragon poly vs dragon poly + 1 loose edge) is almost nothing in terms of runtime, due to the overhead above

[eliemichel]

Very interesting, at some point the project should maintain a benchmarking pipeline, both for its own developpment and to give a baseline to plug-in writers.

I had not realized how much of the time was in effect spent in the BKE_mesh_calc_edges function, I wonder how it affects other modifiers. Indeed kOfxMeshEffectPropIsDeformation would help, but about this btw I think we'll have no choice but to make a second modifier because the "deform only" tag on Blender modifiers is statically defined and cannot change at runtime. We could still display hints to the user when the wrong modifier is used (and maybe add some Python magic to ease this further).

shouldn't matter too much as it's all single threaded

BKE_mesh_calc_edges seems to be parallelized

PS: I've made earlier today an openmesheffect-nightly branch based on current master, rather than on the LTS, because somebody requested it. Which branch do you feel more confortable to work with?

edit: Maybe we can try setting the keep_existing_edges argument of BKE_mesh_calc_edges to false as there is no edge prior to calling this.

[tkarabela]

Good point about the benchmarks! :)

From what I can see, BKE_mesh_calc_edges() is sequential even in current Blender: https://github.com/blender/blender/blob/b87997f59b41471f7f79d9071846b2145589d9b2/source/blender/blenkernel/intern/mesh_validate.c#L1522 (iterate around each face, stuff vertex pairs into hashmap, generate edges from the hashmap). Disabling the update parameter has little effect (still around 1650 ms with xyzrgb dragon); for mesh with no loose edges, there are no pre-existing edges before BKE_mesh_calc_edges(), they all get created by the function.

Re: kOfxMeshEffectPropIsDeformation, I saw that the modifier API is different for deformers vs. general modifiers, though I haven't looked into what that means exactly. I agree with creating a second modifier to take advantage of it.

With attribute forwarding, it should be possible to elide BKE_mesh_calc_edges() even without using the Blender deformer API, it would just have to check that vertices/faces are forwarded from input mesh. Which is internal_data->source_mesh, if I understand correctly?

Re: nightly, I will try compiling it and report back - I don't mind either way and have some trouble compiling Blender with all the optional stuff, so it may even be better for me.

[eliemichel]

From what I can see, BKE_mesh_calc_edges() is sequential even in current Blender

I just realized the parallelization was only in the new c++ version ;) https://github.com/blender/blender/blob/master/source/blender/blenkernel/intern/mesh_validate.cc#L226 (and learn by the occasion that there is some migration to C++ going on)

Re: kOfxMeshEffectPropIsDeformation, I saw that the modifier API is different for deformers vs. general modifiers, though I haven't looked into what that means exactly. I agree with creating a second modifier to take advantage of it.

Yes, the entry points are different, and there is a "tag" field that tells which of the APIs to use

With attribute forwarding, it should be possible to elide BKE_mesh_calc_edges() even without using the Blender deformer API, it would just have to check that vertices/faces are forwarded from input mesh. Which is internal_data->source_mesh, if I understand correctly?

Oh sure, we should totally do this. I openned #42 to remember it.

Good point about the benchmarks! :)

Ok, so I'll open an issue for this ass well actually. #43

Re: nightly, I will try compiling it and report back - I don't mind either way and have some trouble compiling Blender with all the optional stuff, so it may even be better for me.

I think it make more sense to follow more closely the last versions of Blender since we are still largely wip so it is not consistent with the notion of "Long Term Support". Well it's also that I had not taken the time to sync with master since 2.83. ^^ Juan from BoneMaster asked for it, hopefully they're going to release OpenMeshEffect in their package. :)

[tkarabela]

I just realized the parallelization was only in the new c++ version ;) https://github.com/blender/blender/blob/master/source/blender/blenkernel/intern/mesh_validate.cc#L226 (and learn by the occasion that there is some migration to C++ going on)

It was implemented just this month; what are the odds? ^^ It looks nice, I will try it once I get around #44.

I think it make more sense to follow more closely the last versions of Blender since we are still largely wip so it is not consistent with the notion of "Long Term Support". Well it's also that I had not taken the time to sync with master since 2.83. ^^ Juan from BoneMaster asked for it, hopefully they're going to release OpenMeshEffect in their package. :)

Glad to hear that OpenMeshEffect is getting more attention! I agree, it would be good to keep up with upstream releases.

I've sent the pull request with the implementation and I did some more testing and benchmarking. I actually had to implement everything on MfxVTK side as well, to have any confidence that it works, so I have some more insight into how these can help on plugin side.

Here are some tests, this time from my desktop (Intel i5-7400 quadcore, 24GB RAM so I was able to edit the big dragon in Blender). The two results are from MfxVTK 0.4.0 (before optimization) and MfxVTK optimized branch (feature-mesh-optimization-attributes ):

XYZRGB Dragon dataset (point cloud, 3 million points), optimized input/output path in MfxVTK

MfxVTK 0.4.0 - VtkEffect cooked in 0 ms (+ 83 ms = 19/32 ms VTK, 0/31 ms MFX prologue/epilogue)
MfxVTK opt.  - VtkEffect cooked in 0 ms (+ 36 ms = 6/0 ms VTK, 0/29 ms MFX prologue/epilogue)

XYZRGB Dragon dataset (polygonal mesh, 7 million triangles), optimized input/output path in MfxVTK

poly
MfxVTK 0.4.0 - VtkEffect cooked in 0 ms (+ 1967 ms = 158/94 ms VTK, 16/1698 ms MFX prologue/epilogue)
MfxVTK opt.  - VtkEffect cooked in 0 ms (+ 1749 ms = 45/1 ms VTK, 16/1686 ms MFX prologue/epilogue)

XYZRGB Dragon dataset (polygonal mesh + 1 loose edge), generic input/output path in MfxVTK

MfxVTK 0.4.0 - VtkEffect cooked in 0 ms (+ 2114 ms = 221/93 ms VTK, 92/1706 ms MFX prologue/epilogue)
MfxVTK opt.  - VtkEffect cooked in 0 ms (+ 2040 ms = 194/68 ms VTK, 92/1685 ms MFX prologue/epilogue)

5x improvement on plugin side for a big polygonal mesh doesn't look bad. With BKE_mesh_calc_edges() addressed, it looks like OFX <-> Blender roundtrip can be pretty lightweight.

(The MfxVTK optimizations are attribute forwarding of VTK buffers to output mesh, OpenMP for copying, and using specialized routines based on input/output type, so parts of processing can be skipped.)