Fix T65052: "Convert to mesh from curve" fail if the curve has a bevel
ClosedPublic
Actions

Authored by Sergey Sharybin (sergey) on May 27 2019, 12:08 PM.

Details

Reviewers

Brecht Van Lommel (brecht)
Bastien Montagne (mont29)

Commits

rBSa096fbb32de8: Fix T65052: "Convert to mesh from curve" fail if the curve has a bevel
rBa096fbb32de8: Fix T65052: "Convert to mesh from curve" fail if the curve has a bevel

Summary

Use evaluated object as an input for mesh construction. This ensures
all dependencies are ready.

Possibly can benefit from library remap functions, but i am not sure
about most clean approach for that since remapping should also change
object type.

Diff Detail

Repository

rB Blender

Branch

fix_T65052 (branched from master)

Build Status

Buildable 3741
Build 3741: arc lint + arc unit

Event Timeline

Sergey Sharybin (sergey) created this revision.May 27 2019, 12:08 PM

Harbormaster completed remote builds in B3741: Diff 15626.May 27 2019, 12:08 PM

After this change, BKE_mesh_from_nurbs and usage of bmain in BKE_mesh_from_nurbs_displist can be removed. That makes it more clear this is existing behavior, not a new hack introduced by this change.

source/blender/editors/object/object_add.c
1979	Why is it no freeing modifiers anymore? I can see how it's weak though if you have multiple objects using the same data.

Brecht Van Lommel (brecht) requested changes to this revision.May 27 2019, 1:49 PM

This revision now requires changes to proceed.May 27 2019, 1:49 PM

Sergey Sharybin (sergey) added inline comments.May 27 2019, 2:12 PM

source/blender/editors/object/object_add.c
1979	That is a very great question.

Clear modifiers after conversion.

One thing which is unclear to me is that old code did not clear
modifiers for "other" objects. This is somewhat confusing.

The cleanup is indeed possible, can even remove all the remapping
logic there. And, ideally, make it to NOT free original data. But
all this i'd prefer to do separately.

Harbormaster completed remote builds in B3743: Diff 15629.May 27 2019, 2:13 PM

Fine to do the cleanup in a separate commit. It wasn't obvious to me that this object loop was copying existing code, knowing that it seems not worth looking into better library remapping now.

This revision is now accepted and ready to land.May 27 2019, 2:19 PM

LGTM too.

About not freeing modifiers of other objects using same curve ID: I think this is a bug… Fairly sure that wan lead to some nasty errors and crashes. But agree it is better fixed in a separate commit.

Regarding remapping, don’t think we should use generic ID remap code here, this is a rather special case, and we only ever affect one pointer (ob->data)… Though in practice obdata datablocks are not commonly used by anything else I think…

Closed by commit rBa096fbb32de8: Fix T65052: "Convert to mesh from curve" fail if the curve has a bevel (authored by Sergey Sharybin (sergey)). · Explain WhyMay 27 2019, 2:40 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

source/

blender/

editors/

object/

object_add.c

30 lines

Diff 15626

View Options

source/blender/editors/object/object_add.c

Fix T65052: "Convert to mesh from curve" fail if the curve has a bevelClosedPublicActions