Initial implementation of Alembic for basic import/export and data streaming.
ClosedPublic

Authored by Kévin Dietrich (kevindietrich) on Jun 20 2016, 2:21 PM.

Details

Summary

For user support and questions or custom builds please refer to the dedicated thread on blenderartists.org.

Firstly, this patch adds an import and an export operator to convert data between Alembic and Blender. This is pretty straightforward. The exporter creates an Alembic archive based on the Blender object(s) to be exported, the importer creates Blender objects and parents them if needed based on the content of a given Alembic archive.

For animated data streaming support, a new modifier (Mesh Sequence Cache) was added to be able to support file sequences and animated objects with a varying number of vertices/points. Reason was that it was easier to write a new modifier than refactoring the Mesh Cache modifier, but eventually it will be merged with the Mesh Cache modifier if nothing prevents that.

To be able to stream animations made at the transformation level (rotation, scale and/or translation), e.g. rigid bodies, a new constraint was added, dubbed Transform Cache, which works in the same way as the mesh cache modifier. Since an Alembic archive can store an arbitrary number of objects, and multiple cache constraints and modifiers added to objects created from the same Alembic archive can reference the said archive, and to help managing and synchronizing data related to said archive (frame offset, scale...), a new data-block is introduced. Although this data-block, called CacheFile, is introduced is this Alembic-centric patch, it shall stay generic and simple enough to be reused in other contexts, for example in some possible future external OpenVDB file importer, or as part of a bigger, scene level, cache system.

The importer automatically adds a cache modifier and/or cache constraint to the imported objects if the caches are split in file sequences or either the data, the parent transform or both are not constant in the Alembic archive.

NOTE: import and streaming are read-only, there is no cache invalidation/overwrite.

Some more (still WIP) documentation is available on the wiki.

For the reviewers' attention, so that there is no surprise if/while testing the patch, here is a list of the current known bugs and todos:

bugs

  • crash when importing cyclic NURBS from another software (this is a buffer overflow due to the fact that cyclic NURBS in other software have duplicated points).
  • Sintel IK has bugs, might be an export issue. (Alembic does not really take care of armatures and baked vertices data is correct, so low priority).
  • motion blur only affects parts of the mesh.

features/todos/ideas/others:

  • allow for custom axis for import/export (in tests in a local branch).
  • animated camera focal length (in tests in a local branch, using the mesh cache modifier...).
  • create new objects when refreshing/reloading a cache file if necessary.
  • expose the Alembic properties that are read to the UI (uvs, normals, vertices, ...) so the user can choose what to import/stream.
  • point cache workflow (e.g. only export vertices)

Diff Detail

Repository
rB Blender
Kévin Dietrich (kevindietrich) retitled this revision from to Initial implementation of Alembic for basic import/export and data streaming..Jun 20 2016, 2:21 PM
Campbell Barton (campbellbarton) requested changes to this revision.EditedJun 20 2016, 4:25 PM

Currently orientation and scale are stored per in every constraint.

Are there many cases you would want to use the same cache file with different up/forward axis per object?

If not, think this could be moved into BlendDataCacheFiles (we could still have a local scale, applied ontop - if thats needed).


Something else I noticed is there are many properties being stored in the object's ID-properties.
Is the intended workflow for using this explained anywhere?
Currently nested ID-properties aren't so nice for user editing, possibly the workflow here needs to be checked on.

build_files/cmake/macros.cmake
618

Prefer to call this bf_alembic, in general context abc often means abstract-base-class.

source/blender/alembic/intern/abc_exporter.cc
466–467

Would prefer to have a single alembic property-group where all these settings can be stored (eg, object["alembic"]).

source/blender/alembic/intern/abc_mesh.cc
148

Other code in this file is writing MPoly data, seems like this should too.

Tessface data won't be aligned when ngons are used.

source/blender/blenkernel/intern/cachefile.c
56

This could take a frame argument instead of scene
callers may want to select the time later on (with some offset of adjustments).
And current callers have the frame anyway.

source/blender/makesrna/intern/rna_cachefile.c
49–51

"is_" prefix is typically used for read-only variables.

Better call "use_sequence" or "use_file_sequence" ?

source/blender/makesrna/intern/rna_scene_api.c
49
This revision now requires changes to proceed.Jun 20 2016, 4:25 PM
source/blender/alembic/intern/abc_util.cc
270–414

Looks like this could be extracted into a generic function for reuse in object.c

Kévin Dietrich (kevindietrich) marked 4 inline comments as done.Jun 20 2016, 6:52 PM

Are there many cases you would want to use the same cache file with different up/forward axis per object?

Moved to the data-block, though note for now Alembic up/forward axis transform is still hard coded, so it's not exposed to the UI just yet.

Something else I noticed is there are many properties being stored in the object's ID-properties.
Is the intended workflow for using this explained anywhere?
Currently nested ID-properties aren't so nice for user editing, possibly the workflow here needs to be checked on.

I forgot to mention that the exporter code comes from the DwarfLabs patch (D1783), and I haven't really touched it besides cleanups and smalls refactor. Then the ID props part of the code can be traced to the original patch by Esteban Tovagliari, and I really don't know what was the intended workflow here. I removed the code for the time being since it is not clear what workflow to use here, how to read back such properties (what about other software?)... We can revisit this later down the road if need be, see what kind of request emanates from studios and such about custom properties read/write.

source/blender/alembic/intern/abc_util.cc
270–414

Can do but it is relying on up/forward axis conversions (through the call to create_rotation_matrix), so need to check on that.

Kévin Dietrich (kevindietrich) updated this object.
  • Only compute normals if we haven't read normals from alembic.
  • Add support to stream vertex colors for dynamic meshes.
  • From review: rename bf_abc -> bl_alembic.
  • From review: use MPoly instead of MFace for poly data.
  • From review: pass a frame instead of scene to
  • Cleanup: 2-spaces indent for pre-processor.
  • Move scale and axis cnversion properties into the CacheFile data-block.
  • Fix for possible null pointer dereferencing.
  • Remove custom ID properties writing code for now.
  • Store a pointer to an open Alembic archive in the CacheFile data-block.
  • Avoid trying to read unsupported/unknown data.
  • Fix for missing property.
  • Don't set smooth flag on polys when reading non-smooth normals.
  • Add support to write normals in the right scope.
  • Merge branch 'master' into alembic_basic_io
  • Merge branch 'master' into alembic_basic_io
  • Cleanups:
  • Fix crash accessing arrays out of bounds.
  • Initial support to export curves.
  • Support animation of CacheFile properties.
  • Properly ref-count CacheFile block usage.
  • Various cleanups and refactor.

I think the patch is nearly done (if you don't include bugs ;) ). The features/todos listed in the summary are not really influencing the overall design and can be done anytime (now in the branch or later in master).

There is still the case of the new added modifier. I think the code should be merged with the Mesh Cache modifier. But few things to note:

  • the Mesh Cache modifier only deforms the input mesh, whilst this new modifier might create new DerivedMeshes, so the Mesh Cache will need to be able to use a DerivedMesh (through ModifierType.applyModifier).
  • most of the properties of the mesh cache modifier would need to be (or already are) part of the CacheFile datablock, but then I'm not how to handle versioning if there were some animation on some properties.
  • Fix various NURBS bugs.
  • Add support to read edge crease data.
  • Modify the way time offset and scaling works.
  • Curves: fix corrupted displist.
  • Lookup velocity using the right times.
  • Support motion blur in Cycles.
  • Various fixes and refactors.
  • Delete objects when cancelling an import.
  • Make sure progress bar is updated.
  • Merge branch 'master' into alembic_basic_io
  • Implement granular data reading.
  • Make use of the SubD reader.
  • Depsgraph: add relations between cache files and transform cache
  • Avoid updating the whole scene when updating cache file dependencies.
  • Merge branch 'master' into alembic_basic_io
  • Fix typo in comment.
source/blender/alembic/intern/abc_customdata.cc
69–80

In this case it makes sense to reserve the array size for uvidx and uvs, since we know the number of loops.

source/blender/alembic/intern/abc_mesh.cc
734

Why is the name lookup case insensitive here?

746

This is quite in-efficient, object_remove_material_slot loops over all objects and updates object data, noticed this is already being done in Blender.

We should have a object_clear_material_slots function for this.

770

May be worth storing a {name: material} map, to avoid full material searches every time.

1173

This is a slow and intensive operation, think it may be good to have it as optional. While it allows for invalid data - the performance over-head of having to check mesh data on loading goes against the goal of fast data streaming.

(same for other usage of mesh-validate).

source/blender/blenloader/intern/readfile.c
2709

This seems over zealous?

Wouldn't it be better to have this work like most other data (image, sound, font... etc) types which load when they're needed?

source/blender/editors/space_time/space_time.c
417–426

Since its common to have multiple scenes, think showing all caches keyframes is likely to be confusing and not so useful.

I think it would be better to loop over all objects in the scene and show caches used by those objects.

Campbell Barton (campbellbarton) requested changes to this revision.Jul 20 2016, 9:42 PM
This revision now requires changes to proceed.Jul 20 2016, 9:42 PM
source/blender/alembic/intern/abc_mesh.cc
746

Note, this will clear previously set material indices. If we 'own' this object it would be better to set the array directly.

Kévin Dietrich (kevindietrich) marked 7 inline comments as done.
  • Minor edits for alembic branch
  • Merge branch 'master' into alembic_basic_io
  • Fix compiling after recent cleanup
  • Fix for library cache paths not using library as relative basis
  • Use regular include guards
  • Fix over-allocation of exported normals.
  • Pre-allocate UVs array.
  • Use case-sensitive str comparison.
  • Only draw the caches' keyframes for the objects in the current scene.
  • Avoid calling BKE_object_material_slot_remove.
  • Simplify material creation and assignment.
  • Consolidate alembic export options into a struct
  • Merge branch 'master' into alembic_basic_io
  • Implement CacheFile in the legacy dependency graph.
  • Merge branch 'master' into alembic_basic_io
  • Merge branch 'master' into alembic_basic_io
  • Add a make local function to CacheFile id block.
  • Make mesh validation optional.
  • Make sure edges are computed.
  • Merge branch 'master' into alembic_basic_io
  • Merge branch 'master' into alembic_basic_io
  • Fix RNA property lookup failure.
  • Fix crash dereferencing nullptr.
  • Fix crash accessing array out of bounds.
  • Merge branch 'master' into alembic_basic_io
  • Merge branch 'master' into alembic_basic_io
  • Remove old comment.
  • Expose CacheFile.is_sequence to the UI, to avoid issues when checking
  • Expose reading flags to the UI.
  • De-duplicate point reading logic, add support to read custom data for
  • Cleanup.
  • DNA/RNA changes
  • Merge branch 'master' into alembic_basic_io
  • Remove cycles motion blur support for now.
  • Disable NURBS reading.
Bastien Montagne (mont29) requested changes to this revision.Aug 5 2016, 4:09 PM

Mostly skim-checked own 'area' (i.e. file read/write, and datablock handling), most points raised below should be easy to fix.

Am a bit wondering why you reload cache immediately on reading/copying datablock, can’t this be easily deferred to when you actually need the data?

Besides that, realized library_query.c do not 'case' all ID types, will fix that in master, so you should add (empty) case for new type here as well. ;)

source/blender/blenkernel/intern/cachefile.c
56–61

This should be split off into a BKE_cachefile_init() function (to be called from BKE_libblock_init_empty() too).

80–87

Why not use BKE_libblock_copy()? Would avoid you having to manually copy over basic data, you could just focus on updating pointers. And *very important*, you would not forget to set cache_file->id.newid to new_cache_file!

I know some of our current _copy functions do not use it, but I aim to change that - so please use common API as much as possible for new datablocks! ;)

89–91

Same as in readfile code, I would rather just set handle to NULL here, and do deferred reload.

source/blender/blenloader/intern/readfile.c
2709

I second Campbell here, couldn’t this make loading blend file really slow in some extreme cases? Like a lot of alembic caches, or some on network drives, etc.

Why not rather set handle to NULL here, and actually load cache when requested?

This revision now requires changes to proceed.Aug 5 2016, 4:09 PM
  • Merge branch 'master' into alembic_basic_io
  • Add an init function for cachefiles.
  • Use BKE_libblock_copy for copying the cachefiles.
  • Add BKE_cachefile_ensure_handle, avoid creating handles when reading/
  • Fix heap-use-after-free when closing a file containing a copied
  • Merge branch 'master' into alembic_basic_io
  • Add missing case (following master merge).
This revision was automatically updated to reflect the committed changes.