Details

Reviewers: None
Commits: rB3e77f747c0d1: Alembic export: export UV maps on every frame

Summary

Currently when exporting a mesh as an alembic cache it only writes the UVs on the first frame the mesh is written. If the object has animated UVs or the geometry changes (i.e. a mask modifier removing portions of the mesh) then the UVs from the first frame are no longer valid and do not work properly.

This patch exports the UV maps on all frames. Vertex colors were once a part of this patch as well, but were a bit troublesome, for which T88074 was created.

Original description:

This patch adds an option to force the UVs to be written on every frame so that the UVs will be valid for the entire cache.

This is not the most optimal way to fix this issue, but is a good first step in making it possible to export changing UVs. With this option on all objects being exported will get UVs written on every frame even if they do not have changing UVs. This will result in larger cache files.

In the future it would be good to improve this with a way to detect if the UVs of an object change and need to be written on multiple frames.

Diff Detail

Repository: rB Blender

Event Timeline

Cody Winchester (CodyWinch) requested review of this revision.Jul 27 2020, 2:59 PM

Cody Winchester (CodyWinch) created this revision.

Jacques Lucke (JacquesLucke) added a subscriber: Jacques Lucke (JacquesLucke).Jul 28 2020, 1:59 PM

Jacques Lucke (JacquesLucke) added inline comments.

source/blender/io/alembic/exporter/abc_writer_mesh.cc
269	Haven't compiled it, but looks like this might result in a warning. Please add parenthesis, so that the order of evaluation is more obvious. Same below.

Thanks for the patch, it seems quite useful.

Please make sure that the patch description follows the ingredients of a patch. Not only does it make reviewing easier, it also helps when writing the release notes, update the manual, etc.

source/blender/editors/io/io_alembic.c
402	There is no need to repeat the name of the property in the description. I think it's more important to explain here what happens when the option is turned off (UVs are only written on the first frame that the mesh is written). This is a bit of a pet-peeve of mine when it comes to boolean options. It has been since I've played the original DOOM, which had an option "8-bit textures". It didn't describe what would happen when it was turned off; it could have used 4-bit or 16-bit textures, so I didn't know whether to turn it on or off for the best image quality.
source/blender/io/alembic/exporter/abc_writer_mesh.cc
269	Actually `&&` binds stronger than `\|\|`, so in this case the expression is actually incorrect. It should be something like this: if ((!frame_has_been_written_ \|\| args_.export_params->uvs_every_frame) && args_.export_params->uvs) {

This revision now requires changes to proceed.Jul 28 2020, 3:47 PM

Cody Winchester (CodyWinch) updated this revision to Diff 27202.Jul 29 2020, 3:22 AM

Cody Winchester (CodyWinch) edited the summary of this revision. (Show Details)

Cody Winchester (CodyWinch) marked 3 inline comments as done.

Cody Winchester (CodyWinch) added inline comments.

source/blender/editors/io/io_alembic.c
402	That makes a lot of sense.

Can't this be a general option for all mesh attributes to ignore frame_has_been_written_, why make it specific to UVs?

And doesn't Alembic automatically deduplicate unchanged UVs? Have you compared the file sizes? As far as I know we don't do anything special for vertex coordinates, and it works there.

In D8397#204915, @Brecht Van Lommel (brecht) wrote:

Can't this be a general option for all mesh attributes to ignore frame_has_been_written_, why make it specific to UVs?

I think this is a good idea.

And doesn't Alembic automatically deduplicate unchanged UVs? Have you compared the file sizes? As far as I know we don't do anything special for vertex coordinates, and it works there.

Alembic does indeed deduplicate data, I just did a quick test with writing a static mesh once or on every frame, and the output size is the same.

We do try and avoid writing multiple samples when they aren't animated, though, for example in void ABCAbstractWriter::write(HierarchyContext &context). This was already in place when I started working on the Alembic code, but it's not documented why this is done. One of the reasons is likely export speed, which is considerable when the geometry is complex. It also may have an impact on the import side, but that's just a guess from my side as Alembic's ITypedGeomParam::isConstant() function (which we use to see if data is animated) is undocumented.

Right, export performance matters which is why this would be an option. But besides that there should be no downside to having this enabled if all the attributes are static.

Modified to include other data sets not just UVs.

I tried to add it for the write_face_sets but it raised an exception on line 370 OFaceSet face_set = schema.createFaceSet(it->first);

In D8397#205188, @Cody Winchester (CodyWinch) wrote:

I tried to add it for the write_face_sets but it raised an exception on line 370 OFaceSet face_set = schema.createFaceSet(it->first);

That probably has to do with the reference to the C++ object being lost. When OFaceSet face_set goes out of scope, the Alembic library will finalize that property. When later schema.createFaceSet(it->first) is called again, it assumes you want to add a new property with the same name (rather than append a sample to an existing property). The solution lies in keeping face_set in scope for the duration of the export. You can look at rBa1c9d425844c5c2299daf9a89438d164f605407c to see how I solved it there.

I think it's a good idea to wrap (!frame_has_been_written_ || args_.export_params->write_every_frame) into a function, to avoid repeating the same condition.

swann (slumber) awarded a token.Aug 5 2020, 11:42 AM

I just tested the patch, It seems that only animation on the active UV channel is working (the other channels have no animation displayed, just a static UV for each of them). Is it the expected behavior ?

Or should it enable the support for multiple animated UV channel export/import (Not only the active one) ?

In D8397#206773, @swann (slumber) wrote:

I just tested the patch, It seems that only animation on the active UV channel is working (the other channels have no animation displayed, just a static UV for each of them). Is it the expected behavior ?

Or should it enable the support for multiple animated UV channel export/import (Not only the active one) ?

I agree that it should include all UVs to be animated.

I don't think this is an issue with alembic specifically. I recently had an object with multiple animated UV channels and when I would apply the modifier stack it would only properly update the active UVs. This makes me think that the issue lies elsewhere in the code.

Thanks for the quick answer @Cody Winchester (CodyWinch) and thanks for this patch !
I really hope that we could push the abc animated UV import/export to support multi-channel animated UVs 🤞
@Sybren A. Stüvel (sybren) do you have any idea about where the problem could be lying ?

In D8397#206773, @swann (slumber) wrote:

I just tested the patch, It seems that only animation on the active UV channel is working (the other channels have no animation displayed, just a static UV for each of them). Is it the expected behavior ?

How did you test? Are you looking at the raw data in Alembic? Are you importing into Blender, or into some other software?

In hindsight, I don't think this should be an option at all we should just always export everything. Export performance matters, but correctness should be the default.

If there is a need to have an option, the name and description need to be more consistent and better explain what this does.

@Sybren A. Stüvel (sybren) sorry for the lack of information. I am importing into Blender, I tested the ABC export in these two ways:

Method	Use a mesh with keyframe animation in the UV layer	Use a mesh with two modifiers UV projects
Blend file	test_animated_uv_from_keyframes.blend828 KBDownload	test_animated_uv_from_projectors.blend809 KBDownload
abc file	test_animated_uv_from_keyframes.abc21 KBDownload	test_animated_uv_from_projectors.abc12 KBDownload
result (abc import on top)
abc echo command result	abc_echo_keyframes.txt2 KBDownload	abc_echo_projector.txt2 KBDownload

Steps to reproduce:

Open one of the test blend file
Export the main object with the following parameters

Import the exported abc in the same blendfile and hit play

@swann (slumber) Thanks for the test files. Fortunately, this seems to be an issue with the Alembic import, rather than the export -- the written Alembic files do contain both animated UV maps. As such, your findings don't stand in the way of accepting this patch.

In D8397#216923, @Brecht Van Lommel (brecht) wrote:

In hindsight, I don't think this should be an option at all we should just always export everything. Export performance matters, but correctness should be the default.

If there is a need to have an option, the name and description need to be more consistent and better explain what this does.

I agree. Let's keep things simple, and treat UVs the same way as mesh normals are treated: just always export on every frame.

Since the inability to export animated UVs turned out to be a practical problem for the Blender Animation Studio, I want to get this patch committed ASAP. @Cody Winchester (CodyWinch) wasn't available in the Blender Coders chat for a quick back & forth, so I'll commandeer, update, and commit it.

Sybren A. Stüvel (sybren) removed a reviewer: Cody Winchester (CodyWinch).May 6 2021, 11:58 AM

Sybren A. Stüvel (sybren) mentioned this in T88074: Alembic: export animated vertex colors.May 6 2021, 12:17 PM

Remove export option, and always export on all frames.
Limit the patch functionality to export UV maps. The export of vertex colors doesn't work yet, for which T88074 has been created.

Harbormaster completed remote builds in B14409: Diff 36826.May 6 2021, 12:18 PM

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)May 6 2021, 12:18 PM

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)

This revision was not accepted when it landed; it landed in state Needs Review.May 6 2021, 12:21 PM

Closed by commit rB3e77f747c0d1: Alembic export: export UV maps on every frame (authored by Sybren A. Stüvel (sybren)). · Explain Why

This revision was automatically updated to reflect the committed changes.

Sybren A. Stüvel (sybren) added a commit: rB3e77f747c0d1: Alembic export: export UV maps on every frame.