Fix: Bmesh from_object applies modifiers twice
ClosedPublic
Actions

Authored by Omar Emara (OmarSquircleArt) on Jan 8 2021, 10:22 PM.

Details

Reviewers

Brecht Van Lommel (brecht)
Philipp Oeser (lichtwerk)

Commits

rBc56da67716d9: Fix: Bmesh from_object applies modifiers twice

Summary

The Bmesh from_object method applies modifiers twice when the input
Deform is enabled and the input Depsgraph is a render one.

The evaluated object is passed to mesh_create_eval_final_render
incorrectly, instead the original object should be passed.

Diff Detail

Repository: rB Blender

Event Timeline

Omar Emara (OmarSquircleArt) created this revision.Jan 8 2021, 10:22 PM

Harbormaster completed remote builds in B12059: Diff 32560.Jan 8 2021, 10:22 PM

Omar Emara (OmarSquircleArt) requested review of this revision.Jan 8 2021, 10:22 PM

Animation Nodes report:
https://github.com/JacquesLucke/animation_nodes/issues/1599

Brecht Van Lommel (brecht) requested changes to this revision.Jan 11 2021, 12:52 PM

Brecht Van Lommel (brecht) added inline comments.

source/blender/python/bmesh/bmesh_py_types.c
1138–1139	An unevaluated object should not be used for modifier evaluation, it will not generally give the correct result when there are e.g. constraints, animation or drivers. I think you can use `BKE_mesh_new_from_object` instead and pass it the evaluated object. That's what is used for `object.to_mesh()` in the Python API. Note that the returned mesh needs to be freed.

This revision now requires changes to proceed.Jan 11 2021, 12:52 PM

@Brecht Van Lommel (brecht) Makes sense, but isn't that inconsistent with how the rest of the cases are computed? In particular, if use_deform is false and the user provided an unevaluated object, that unevaluated object will be used in modifier evaluation.

I just realized that those two cases also suffer from the same problem, if use_deform is false and the user provided an evaluated object, the output mesh will have constructive modifiers applied twice. So what should be done about those cases as well?

That code path looks wrong, does it even work? I guess it leaks memory as well by not freeing the mesh. It's only used by the Python API, Blender itself doesn't use it.

Really this parameter should have been removed in 2.80. We should probably ignore the parameter for now with a warning and then remove it in a major release.

@Brecht Van Lommel (brecht) I see. Alright. So, to clarify, this patch should be updated to use BKE_mesh_new_from_object and a new patch should be submitted to deprecate the use_deform parameter?