Page MenuHome

Fix T84623: Curve/Surface force not working in normal direction
ClosedPublic

Authored by Philipp Oeser (lichtwerk) on Jan 12 2021, 4:30 PM.

Details

Summary

Tweaking e.g. a field strength would then not use the curve/surface
normal anymore [but the object center instead].

If a curve has a forcefield with effector shape Curve (in code its shape
is PFIELD_SHAPE_SURFACE then), it wil get a SurfaceModifier.

Changing properties will free the SurfaceModifierData's bvhtree and mesh
And these dont get copied along when doing the CoW copy, these are
explicitly set to NULL. So this was also failing for meshes, not just curves.

Without the mesh & bvhtree though, get_effector_data() will not set the
EffectorData's normal correctly (it is closest_point_on_surface() which
does this). And without the right EffectorData's normal, the effector
will of course work unexpected.

Going in and out of editmode made this work because that goes down this
route:

  • BKE_object_handle_data_update
  • BKE_displist_make_curveTypes
  • do_makeDispListCurveTypes
  • curve_calc_modifiers_post
    • BKE_mesh_new_nomain_from_curve_displist
    • we then have our desired updated mesh from the curve
    • this will also call the SurfaceModifiers deformVerts [which - given we

have a valid mesh - will update the bvhtree properly]

Also note that _animating_ the effector actually works, (have not done
the deep dive why this works, assume the curve geometry is updated in
this case)

So, now just carefully tag the curve ID_RECALC_GEOMETRY in
rna_FieldSettings_update for this specific case.

Diff Detail

Repository
rB Blender

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Jan 12 2021, 4:30 PM
Sergey Sharybin (sergey) requested changes to this revision.Feb 3 2021, 4:32 PM

What you call Solution [1] is more correct one. What is confusing about it is the non-trivial condition prior to the tag. It mentions the need to rebuild mesh, but the check only includes curve-based object types. And on top of that some shape and forcefield combination.
It might be fine from code side, but having more comprehensive comment would help here.

The Solution [2] is not good because it either modifiers someone's else data, or requests data creation while dependencies might not be ready yet.

This revision now requires changes to proceed.Feb 3 2021, 4:32 PM
  • dont restrict this to curves (meshes were also failing)
  • make comment more informative

What you call Solution [1] is more correct one.

Sorry this has been lying around for so long.

What is confusing about it is the non-trivial condition prior to the tag. It mentions the need to rebuild mesh, but the check only includes curve-based object types.

Fully true, was failing for meshes as well, not sure how I missed that, updated patch.

And on top of that some shape and forcefield combination.

These are the forces eventually using the EffectorData's normal (the others dont need it and thus dont need the bvh rebuild)

It might be fine from code side, but having more comprehensive comment would help here.

Updated that in the the patch now as well.

Sorry, made a mistake on previous updated...

To me this seems acceptable solution, so I marking as accepted.

One thing which would be interesting to investigate is whether this is something the DEG can handle. In theory it should be aware of what depends on what. But this is likely beyond of this specific change.

Don't really have time to do anything more in-depth. I would say go ahead and keep an eye on this.
@Sybren A. Stüvel (sybren) Could be something to keep in mind for the depsgraph module.

This revision is now accepted and ready to land.Apr 6 2021, 10:53 AM