Alembic export: write curve/NURBS as mesh
ClosedPublic
Actions

Authored by Sybren A. Stüvel (sybren) on Jan 15 2019, 4:48 PM.

Details

Reviewers

Commits

rBS3ce9bcee7053: Alembic export: write curve/NURBS as mesh
rBS2bb6028d1a64: Alembic export: separated "writing Alembic mesh" from "writing mesh object"
rB3ce9bcee7053: Alembic export: write curve/NURBS as mesh
rB2bb6028d1a64: Alembic export: separated "writing Alembic mesh" from "writing mesh object"

Summary

This diff consists of two commits:

Separated "writing Alembic mesh" from "writing mesh object"

I moved most of the AbcMeshWriter code to a new class AbcGenericMeshWriter. The latter is an abstract class and does not make any assumptions about the type of Blender object being written. This makes it possible to write metaballs, curves, nurbs surfaces, etc. as mesh to Alembic files.

The AbcMeshWriter class now is the concrete implementation of AbcGenericMeshWriter for Blender meshes.

write curve/NURBS as mesh

It's now possible to export curves and NURBS as mesh data to Alembic. This allows artists to do any crazy thing on curves and export the visible result to Alembic for interoperability with other software (or caching for later use, etc.). It's an often-requested feature.

This works around T60503 and the fixes export part of T51311.

Note that exporting zero-width curves is currently not supported, as exporting a faceless mesh (e.g. just edges and vertices) is not supported by the mesh writer at all.

To test, create a curve with thickness (for example extruded), export to Alembic and check the 'Curves to Mesh' checkbox in the export options.

Diff Detail

Repository

rB Blender

Branch

temp-sybren-curve-as-mesh (branched from master)

Build Status

Buildable 2736
Build 2736: arc lint + arc unit

Event Timeline

Sybren A. Stüvel (sybren) created this revision.Jan 15 2019, 4:48 PM

Harbormaster completed remote builds in B2736: Diff 13225.Jan 15 2019, 4:48 PM

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)Jan 15 2019, 4:48 PM

Code wise seems fine except of some style thingies.

But bigger question is that this seems to be a workaround for a broken curves reader? How much commonly used the feature of curves-as-mesh? Guess the main one is to support curves with constructive modifiers (not sure how those are handled before this patch).

source/blender/alembic/intern/abc_curves.cc
205–206	It's C++ ;) struct Mesh AbcCurveMeshWriter::getEvaluatedMesh(struct Scene /scene_eval/), struct Object *ob_eval, bool &r_needsfree)
source/blender/alembic/intern/abc_mesh.cc
344–350	Should we use `modifier_dependsOnTime` ? Maybe as separate commit?
352	`object->data` is either `NULL` or `ID`. So can do `BKE_animdata_from_id((reinterpret_cast<ID*>(object->data))` to get `adt`. Also, checking for `adt != NULL` is not correct. You can, for example, animate a property (which will allocate `adt` and assign `action`) and then remove curve. Technically, `adt` is still there. It will even be there once you've removed action. P.S. Tricky part are the drivers, those are not in Action. P.P.S. Also NLA, that i donno where it is even.
source/blender/alembic/intern/abc_mesh.h
67	This is still C++. Mesh *getFinalMesh(bool &r_needsfree);
73	C++11 ;) void getGeoGroups(struct Mesh *mesh, std::map<std::string, std::vector<int32_t>> &geoGroups);

Sybren A. Stüvel (sybren) marked 3 inline comments as done.Jan 15 2019, 5:56 PM

FIXUP indents
FIXUP more feedback from Sergey
C++11 doesn't need the space between '> >' in a nested templated declaration

Harbormaster completed remote builds in B2737: Diff 13227.Jan 15 2019, 6:50 PM

Sybren A. Stüvel (sybren) marked 2 inline comments as done.Jan 15 2019, 7:03 PM

Sybren A. Stüvel (sybren) added inline comments.

source/blender/alembic/intern/abc_mesh.cc
344–350	`modifier_dependsOnTime` doesn't work reliably for this use case. For example, the hook modifier declares that it doesn't depend on time, even when the hook object is animated.

Sergey Sharybin (sergey) added inline comments.Jan 15 2019, 7:26 PM

source/blender/alembic/intern/abc_mesh.cc
344–350	See `BKE_object_is_deform_modified` then :)

But bigger question is that this seems to be a workaround for a broken curves reader?

For a small part it is. Also for a very limited curves writer, so Blender Curve → Alembic → Blender is a very lossy operation.

How much commonly used the feature of curves-as-mesh? Guess the main one is to support curves with constructive modifiers (not sure how those are handled before this patch).

It's quite often used, for example with Animation Nodes. Being able to convert to a mesh format like Alembic and then load it in as such and process it further is often requested.

If it's commonly used and requested, then i guess there is nothing against adding this feature.

As for the isAnimated() and friends, keep the logic as-is in this patch, and dig into later. Still think it can use some of the existing utility functions, or be requested via dependency graph. But those changes would need to have own testing and such.

Sybren A. Stüvel (sybren) marked an inline comment as done.Jan 16 2019, 10:18 AM

Sybren A. Stüvel (sybren) added inline comments.

source/blender/alembic/intern/abc_mesh.cc
352	According to the cplusplus.com reference, `static_cast` is stricter than `reinterpret_cast`, so I'll use the former here. Neither of the two create any runtime code, but `static_cast` is safer. Since all the existing casts to `ID *` are now C-style casts, to set an example I'd want to use the strictest and safest of the no-runtime-overhead casts.

Use static_cast instead of reinterpret_cast
Alembic: use BKE_key_from_object() in AbcGenericMeshWriter::isAnimated()

Harbormaster completed remote builds in B2740: Diff 13233.Jan 16 2019, 10:18 AM

Sergey Sharybin (sergey) added inline comments.Jan 16 2019, 10:30 AM

source/blender/alembic/intern/abc_mesh.cc
352	strictest and safest In an environment of heavy use of `void` sounds like a joke :( I do not have strong opinion here. There is just no really correct way here. (Well, there is, to make `data` an `ID`, but that'd require a bunch of casts in other areas..)