Curves have offset animations (alembic / USD) #112308

New Issue

Closed

opened 2023-09-13 00:32:22 +02:00 by bernarddupuis · 12 comments

bernarddupuis commented

2023-09-13 00:32:22 +02:00

System Information
Operating system: Windows-10-10.0.19045-SP0 64 Bits
Graphics card: NVIDIA GeForce RTX 4080/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 536.99

Blender Version
Broken: version: 3.6.2, branch: blender-v3.6-release, commit date: 2023-08-16 16:43, hash: e53e55951e7a
Worked: 2.79

Hi ! When i import animated curves ( like hair ) as alembic from houdini or maya, and i play the timeline, the alembic have 1 frame offset compare to other primitives alembics ( like body supposed to have the same animation as hair ).
When i play the timeline backwards, it does the same thing in reverse, so it gets an offset of 1 frame backward.
If i chose one specific frame by clicking on it, the problem disappear.
Sadly, it's rendering with the offset.
It works fine on maya and Houdini.
I tried 3.5 and 3.6.

Here is the file, the alembic named "Test" contains Curve Shape and Primitive Shape. If you click on play, you should see an offset between the two objects.

**System Information** Operating system: Windows-10-10.0.19045-SP0 64 Bits Graphics card: NVIDIA GeForce RTX 4080/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 536.99 **Blender Version** Broken: version: 3.6.2, branch: blender-v3.6-release, commit date: 2023-08-16 16:43, hash: `e53e55951e7a` Worked: 2.79 Hi ! When i import animated curves ( like hair ) as alembic from houdini or maya, and i play the timeline, the alembic have 1 frame offset compare to other primitives alembics ( like body supposed to have the same animation as hair ). When i play the timeline backwards, it does the same thing in reverse, so it gets an offset of 1 frame backward. If i chose one specific frame by clicking on it, the problem disappear. Sadly, it's rendering with the offset. It works fine on maya and Houdini. I tried 3.5 and 3.6. Here is the file, the alembic named "Test" contains Curve Shape and Primitive Shape. If you click on play, you should see an offset between the two objects.

946 KiB

3.8 MiB

bernarddupuis added the

labels 2023-09-13 00:32:23 +02:00

Germano Cavalcante commented

2023-09-13 02:54:49 +02:00

Member

Thanks for the report. I can confirm the problem.

It seems to be an old issue, as I also tested it in Blender 3.0 and the same is seen there.

Thanks for the report. I can confirm the problem. It seems to be an old issue, as I also tested it in Blender 3.0 and the same is seen there.

Germano Cavalcante added

Pipeline, Assets & IO

Animation & Rigging

and removed

labels 2023-09-13 02:56:03 +02:00

Philipp Oeser commented

2024-01-19 12:46:07 +01:00

Member

This actually worked in 2.79 it seems...

This actually worked in 2.79 it seems...

Philipp Oeser self-assigned this 2024-01-19 14:52:57 +01:00

Philipp Oeser commented

2024-01-22 15:26:50 +01:00

Member

OK, a bit of progress here:

the issue is the result of the alembic generating the mesh that replaces the one in the modifier stack is actually the result of the current DispList (see BKE_mesh_new_nomain_from_curve / BKE_mesh_new_nomain_from_curve_displist how this works)
if we call BKE_displist_free and recreate the DispList with curve_to_displist, we get rid of the offset
doing that we loose the stuff from the Geometry panel though (Bevel/Extrude etc.) so a bit more investigation is needed for a final fix

OK, a bit of progress here: - the issue is the result of the alembic generating the mesh that replaces the one in the modifier stack is actually the result of the current `DispList` (see `BKE_mesh_new_nomain_from_curve` / `BKE_mesh_new_nomain_from_curve_displist` how this works) - if we call `BKE_displist_free` and recreate the DispList with `curve_to_displist`, we get rid of the offset - doing that we loose the stuff from the Geometry panel though (Bevel/Extrude etc.) so a bit more investigation is needed for a final fix

Philipp Oeser commented

2024-01-24 11:48:06 +01:00

Member

So the way I understand it:

alembic holds a reference to the object and curve
the modifier is not a pretessellatePoint (so nothing happens in terms of BKE_curve_calc_modifiers_pre)
the DispLists are created from the curve (still the alembic-unmodified curve)
in its AbcCurveReader::read_mesh (called via the modifier modify_mesh callback) it completely replaces that curve under the hood
also it creates a mesh from that new curve (via BKE_mesh_new_nomain_from_curve) and that replaces the one in the modifier stack
the real problem here is that BKE_mesh_new_nomain_from_curve just uses the existing DispLists (these dont include anything from the new curve, the whole thing hasnt been recreated, thus it is a frame too "old")
btw. the same thing is true for USD as well

As a solution, we could:

[1] use functionality from mesh_new_from_curve_type_object
-- this completely wipes previous derived_caches and re-creates DispLists
-- this calls BKE_displist_make_curveTypes "again" -- which does modifiers "again" -- shouldnt this get into a loop? wasted efforts? works in a local patch though... see below :)
-- use that mesh instead
[2] OR: move alembic to a pretessellatePoint (this seems a bit cleaner to me -- more tweaks needed though since atm. this is only meant for deform only modifiers, cant replace the whole curve from here [yet])

before I try the second solution as well, I'd like input from @Sergey and/or @HooglyBoogly regarding the plan of attack

diff --git a/source/blender/blenkernel/BKE_mesh.h b/source/blender/blenkernel/BKE_mesh.h
index fa6374c5d6f..5f7cf791b89 100644
--- a/source/blender/blenkernel/BKE_mesh.h
+++ b/source/blender/blenkernel/BKE_mesh.h
@@ -138,6 +138,7 @@ struct Mesh *BKE_mesh_copy_for_eval(const struct Mesh *source);
 struct Mesh *BKE_mesh_new_nomain_from_curve(const struct Object *ob);
 struct Mesh *BKE_mesh_new_nomain_from_curve_displist(const struct Object *ob,
                                                      const struct ListBase *dispbase);
+struct Mesh *mesh_new_from_curve_type_object(const struct Object *ob, const bool skip_evaluated);
 
 bool BKE_mesh_attribute_required(const char *name);
 
diff --git a/source/blender/blenkernel/intern/mesh_convert.cc b/source/blender/blenkernel/intern/mesh_convert.cc
index 0357d4fd82d..8cef54620b6 100644
--- a/source/blender/blenkernel/intern/mesh_convert.cc
+++ b/source/blender/blenkernel/intern/mesh_convert.cc
@@ -716,11 +716,11 @@ static Mesh *mesh_new_from_evaluated_curve_type_object(const Object *evaluated_o
   return nullptr;
 }
 
-static Mesh *mesh_new_from_curve_type_object(const Object *object)
+Mesh *mesh_new_from_curve_type_object(const Object *object, const bool skip_evaluated)
 {
   /* If the object is evaluated, it should either have an evaluated mesh or curve data already.
    * The mesh can be duplicated, or the curve converted to wire mesh edges. */
-  if (DEG_is_evaluated_object(object)) {
+  if (!skip_evaluated && DEG_is_evaluated_object(object)) {
     return mesh_new_from_evaluated_curve_type_object(object);
   }
 
@@ -850,7 +850,7 @@ Mesh *BKE_mesh_new_from_object(Depsgraph *depsgraph,
     case OB_FONT:
     case OB_CURVES_LEGACY:
     case OB_SURF:
-      new_mesh = mesh_new_from_curve_type_object(object);
+      new_mesh = mesh_new_from_curve_type_object(object, false);
       break;
     case OB_MBALL:
       new_mesh = mesh_new_from_mball_object(object);
diff --git a/source/blender/io/alembic/intern/abc_reader_curves.cc b/source/blender/io/alembic/intern/abc_reader_curves.cc
index d397fe73b57..218c24358c7 100644
--- a/source/blender/io/alembic/intern/abc_reader_curves.cc
+++ b/source/blender/io/alembic/intern/abc_reader_curves.cc
@@ -334,7 +334,7 @@ Mesh *AbcCurveReader::read_mesh(Mesh *existing_mesh,
     }
   }
 
-  return BKE_mesh_new_nomain_from_curve(m_object);
+  return mesh_new_from_curve_type_object(m_object, true);
 }
 
 }  // namespace blender::io::alembic

So the way I understand it: - alembic holds a reference to the object and curve - the modifier is not a `pretessellatePoint` (so nothing happens in terms of `BKE_curve_calc_modifiers_pre`) - the DispLists are created from the curve (still the alembic-unmodified curve) - in its `AbcCurveReader::read_mesh` (called via the modifier `modify_mesh` callback) it completely replaces that curve under the hood - also it creates a mesh from that new curve (via `BKE_mesh_new_nomain_from_curve`) and that replaces the one in the modifier stack - the real problem here is that `BKE_mesh_new_nomain_from_curve` just uses the existing DispLists (these dont include anything from the new curve, the whole thing hasnt been recreated, thus it is a frame too "old") - btw. the same thing is true for USD as well As a solution, we could: - [1] use functionality from `mesh_new_from_curve_type_object` -- this completely wipes previous derived_caches and re-creates DispLists -- this calls `BKE_displist_make_curveTypes` "again" -- which does modifiers "again" -- shouldnt this get into a loop? wasted efforts? works in a local patch though... see below :) -- use that mesh instead - [2] OR: move alembic to a `pretessellatePoint` (this seems a bit cleaner to me -- more tweaks needed though since atm. this is only meant for deform only modifiers, cant replace the whole curve from here [yet]) before I try the second solution as well, I'd like input from @Sergey and/or @HooglyBoogly regarding the plan of attack ```diff diff --git a/source/blender/blenkernel/BKE_mesh.h b/source/blender/blenkernel/BKE_mesh.h index fa6374c5d6f..5f7cf791b89 100644 --- a/source/blender/blenkernel/BKE_mesh.h +++ b/source/blender/blenkernel/BKE_mesh.h @@ -138,6 +138,7 @@ struct Mesh *BKE_mesh_copy_for_eval(const struct Mesh *source); struct Mesh *BKE_mesh_new_nomain_from_curve(const struct Object *ob); struct Mesh *BKE_mesh_new_nomain_from_curve_displist(const struct Object *ob, const struct ListBase *dispbase); +struct Mesh *mesh_new_from_curve_type_object(const struct Object *ob, const bool skip_evaluated); bool BKE_mesh_attribute_required(const char *name); diff --git a/source/blender/blenkernel/intern/mesh_convert.cc b/source/blender/blenkernel/intern/mesh_convert.cc index 0357d4fd82d..8cef54620b6 100644 --- a/source/blender/blenkernel/intern/mesh_convert.cc +++ b/source/blender/blenkernel/intern/mesh_convert.cc @@ -716,11 +716,11 @@ static Mesh *mesh_new_from_evaluated_curve_type_object(const Object *evaluated_o return nullptr; } -static Mesh *mesh_new_from_curve_type_object(const Object *object) +Mesh *mesh_new_from_curve_type_object(const Object *object, const bool skip_evaluated) { /* If the object is evaluated, it should either have an evaluated mesh or curve data already. * The mesh can be duplicated, or the curve converted to wire mesh edges. */ - if (DEG_is_evaluated_object(object)) { + if (!skip_evaluated && DEG_is_evaluated_object(object)) { return mesh_new_from_evaluated_curve_type_object(object); } @@ -850,7 +850,7 @@ Mesh *BKE_mesh_new_from_object(Depsgraph *depsgraph, case OB_FONT: case OB_CURVES_LEGACY: case OB_SURF: - new_mesh = mesh_new_from_curve_type_object(object); + new_mesh = mesh_new_from_curve_type_object(object, false); break; case OB_MBALL: new_mesh = mesh_new_from_mball_object(object); diff --git a/source/blender/io/alembic/intern/abc_reader_curves.cc b/source/blender/io/alembic/intern/abc_reader_curves.cc index d397fe73b57..218c24358c7 100644 --- a/source/blender/io/alembic/intern/abc_reader_curves.cc +++ b/source/blender/io/alembic/intern/abc_reader_curves.cc @@ -334,7 +334,7 @@ Mesh *AbcCurveReader::read_mesh(Mesh *existing_mesh, } } - return BKE_mesh_new_nomain_from_curve(m_object); + return mesh_new_from_curve_type_object(m_object, true); } } // namespace blender::io::alembic ```

Philipp Oeser changed title from ~~Curves or points alembics have offset animations~~ to Curves have offset animations (alembic / USD)

2024-01-24 11:49:24 +01:00

Philipp Oeser added the

label 2024-01-24 11:49:30 +01:00

Philipp Oeser commented

2024-01-24 11:49:55 +01:00

Member

CC @kevindietrich

CC @kevindietrich

Sergey Sharybin commented

2024-01-24 12:16:11 +01:00

Owner

To fix the issue properly the Alembic curve reader needs to be changed. Currently it violates design in a sense that it modifies object/mesh which is an input of the modifier. Instead, it should output new mesh with curves geometry set.

To fix the issue properly the Alembic curve reader needs to be changed. Currently it violates design in a sense that it modifies object/mesh which is an input of the modifier. Instead, it should output new mesh with curves geometry set.

Philipp Oeser commented

2024-01-24 13:05:45 +01:00

Member

Instead, it should output new mesh with curves geometry set.

It does output a new mesh, no?
It does modify the "original" curve though (dont even think this is necessary, could work on a tmp curve as well to get a mesh from I believe)

When you say "with curves geometry set", you mean the new Curves type?
Wouldnt we loose all the DispList features then?

>Instead, it should output new mesh with curves geometry set. It does output a new mesh, no? It does modify the "original" curve though (dont even think this is necessary, could work on a tmp curve as well to get a mesh from I believe) When you say "with curves geometry set", you mean the new `Curves` type? Wouldnt we loose all the DispList features then?

Sergey Sharybin commented

2024-01-24 13:58:28 +01:00

Owner

It does output a new mesh, no?

It does output new mesh, but first it modifies the input object.

It does modify the "original" curve though (dont even think this is necessary, could work on a tmp curve as well to get a mesh from I believe)

I don't think it modifies original curve, but the curve which is an input to the modifier. If it modifies the actual original curve then the issue is even worse, as it will be causing race conditions (as multiple dependency graphs can be evaluated in parallel).

When you say "with curves geometry set", you mean the new Curves type?

It is a new geometry type, yes.

Wouldnt we loose all the DispList features then?

We need to phase out old curves and DispList, which will also mean making a decision on the modifiers support.
It will be more robust and clean solution than adding more permutations in the conversion code.

> It does output a new mesh, no? It does output new mesh, but first it modifies the input object. > It does modify the "original" curve though (dont even think this is necessary, could work on a tmp curve as well to get a mesh from I believe) I don't think it modifies original curve, but the curve which is an input to the modifier. If it modifies the actual original curve then the issue is even worse, as it will be causing race conditions (as multiple dependency graphs can be evaluated in parallel). > When you say "with curves geometry set", you mean the new Curves type? It is a new geometry type, yes. > Wouldnt we loose all the DispList features then? We need to phase out old curves and `DispList`, which will also mean making a decision on the modifiers support. It will be more robust and clean solution than adding more permutations in the conversion code.

Philipp Oeser commented

2024-01-24 14:21:27 +01:00

Member

OK, thx @Sergey for the input.

I don't think it modifies original curve, but the curve which is an input to the modifier.

Sorry, this is what I meant to say (used "original" in the wrong way here)

We need to phase out old curves and DispList, which will also mean making a decision on the modifiers support.
It will be more robust and clean solution than adding more permutations in the conversion code.

Will step down from this issue then, to block others doing the port to the new Curves type.

OK, thx @Sergey for the input. >I don't think it modifies original curve, but the curve which is an input to the modifier. Sorry, this is what I meant to say (used "original" in the wrong way here) >We need to phase out old curves and DispList, which will also mean making a decision on the modifiers support. It will be more robust and clean solution than adding more permutations in the conversion code. Will step down from this issue then, to block others doing the port to the new Curves type.

Philipp Oeser removed their assignment 2024-01-24 14:21:45 +01:00

Philipp Oeser referenced this issue

2024-01-24 15:10:16 +01:00

Alembic/USD: use geometry sets to import data #115623

Philipp Oeser commented

2024-01-24 15:12:50 +01:00

Member

OK, so solution is already in the works (close to being landed it seems) : see !115623

OK, so solution is already in the works (close to being landed it seems) : see !115623

Kévin Dietrich commented

2024-01-28 13:23:46 +01:00

Member

This report is essentially a duplicate of #58704 (I don't know if we can merge, or what to do in this situation).

This report is essentially a duplicate of #58704 (I don't know if we can merge, or what to do in this situation).

Philipp Oeser referenced this issue

2024-02-19 09:40:38 +01:00

wrong motion blur from usd in cycles and reverting to the first frame when disabeling motion blur #118261

Jesse Yurkovich referenced this issue from a commit

2024-02-28 03:02:48 +01:00

Alembic/USD: Use GeometrySets to import data

Jesse Yurkovich added

and removed

labels 2024-02-28 03:11:50 +01:00

Jesse Yurkovich commented

2024-02-28 03:12:12 +01:00

Member

Resolved with ea256346a8

Resolved with ea256346a89808eca6aba27128699ed9037bd451

Jesse Yurkovich closed this issue

2024-02-28 03:12:17 +01:00

Sign in to join this conversation.

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

6 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#112308

No description provided.