Fix T73348: Surface Deform modifier - distortion on bind (depending on scale)
AbandonedPublic
Actions

Authored by Cody Winchester (CodyWinch) on Feb 7 2020, 11:05 PM.

Details

Reviewers

Bastien Montagne (mont29)
Sybren A. Stüvel (sybren)
Campbell Barton (campbellbarton)
Howard Trickey (howardt)
Luca Rood (LucaRood)

Maniphest Tasks

T73348: Surface Deform modifier - distortion on bind (depending on scale)

Summary

Bug description: https://developer.blender.org/T73348

A solution to the surface deform bind artifacts when using small scale geometry.

When the weights of the target geometry are calculated, if the geometry is too small it returns incorrect weights resulting in deformations being wrong and jumping around post bind. When the same geometry is upscaled the binding works correctly.

This patch upscales the data before weights are calculated then downscales it immediately afterwards. I was able to get clean binds on geometry that could no longer even be seen in orthographic mode it was so small.

May not be the most sophisticated solution, but should prevent the errors from occuring on pretty much any model.

Diff Detail

Repository: rB Blender

Event Timeline

Cody Winchester (CodyWinch) created this revision.Feb 7 2020, 11:05 PM

Thanks for looking into this and defining that it is an issue with the weights computation, but I don't consider this an acceptable fix.

I would say the actual issue in interp_weights_poly_v2 and/or interp_weights_tri_v3 should be fixed, rather than working around it by scaling.
If the weights are incorrect with small geometry, it is most likely some epsilon value being too high in a comparison somewhere.

However, note that if it is indeed the case of a large epsilon, the fix might not be as simple as reducing it (see rBa3a438699181).

For a proper fix, I'd recommend first finding if the issue is in interp_weights_poly_v2 or interp_weights_tri_v3, or both. Then checking if a smaller epsilon fixes the SDef issue (without breaking the bind on large meshes).
If the smaller epsilon fixes the SDef issue without introducing new problems, it might be a case of adding an epsilon argument to the offending function(s), as not to break other code making use of those functions. And maybe turn the existing functions into wrappers calling the new functions with a default epsilon.

This is all I can contribute at the moment. Unfortunately I don't have time to test the code in detail myself right now.

As a side note, Blender code makes heavy use of absolute epsilon comparisons. In the future, wouldn't it make more sense to use relative epsilon or ulp comparisons for many of these cases? Just mentioning to get the thoughts from people in this thread.

This revision now requires changes to proceed.Feb 8 2020, 11:52 AM

Luca Rood (LucaRood) mentioned this in T73348: Surface Deform modifier - distortion on bind (depending on scale).Feb 8 2020, 11:58 AM

@Luca Rood (LucaRood) Thank you for your input. I will get to work on making the revision.

I figured that this patch would be too hacky to get accepted, but thought it was worth a try to find out.

@Luca Rood (LucaRood) It seems the point of failure for interp_weights_poly_v2 is in this function:

static float mean_value_half_tan_v2(const struct Float2_Len *d_curr,
                                    const struct Float2_Len *d_next)
{
  float area;
  /* different from the 3d version but still correct */
  area = cross_v2v2(d_curr->dir, d_next->dir);

  if (LIKELY(fabsf(area) > FLT_EPSILON)) {
    const float dot = dot_v2v2(d_curr->dir, d_next->dir);
    const float len = d_curr->len * d_next->len;
    return (len - dot) / area;
  }
  else {
    return 0.0f;
  }
}

The area for small geometry is too small to pass the FLT_EPSILON check and gets returned as 0.0f

and interp_weights_tri_v3 fails in this function:

static bool barycentric_weights(const float v1[3],
                                const float v2[3],
                                const float v3[3],
                                const float co[3],
                                const float n[3],
                                float w[3])
{
  float wtot;
  int i, j;

  axis_dominant_v3(&i, &j, n);

  w[0] = tri_signed_area(v2, v3, co, i, j);
  w[1] = tri_signed_area(v3, v1, co, i, j);
  w[2] = tri_signed_area(v1, v2, co, i, j);

  wtot = w[0] + w[1] + w[2];

  if (fabsf(wtot) > FLT_EPSILON) {
    mul_v3_fl(w, 1.0f / wtot);
    return false;
  }
  else {
    /* zero area triangle */
    copy_v3_fl(w, 1.0f / 3.0f);
    return true;
  }
}

same thing the area calculation is too small and returns the wrong weights.

Lowering the FLT_EPSILON value does fix the binding errors and doesn't effect larger scale bindings either.

FLT_EPSILON seems like a value that we can't/shouldn't change since it is so widely used.

Switches FLT_EPSILON for DBL_EPSILON in the offending functions.

Solves the bind errors on the given example files and when they are further scaled down somewhat. Also does not look like it will have an effect on larger scale bindings.

Cody Winchester (CodyWinch) added a task: T73348: Surface Deform modifier - distortion on bind (depending on scale).Feb 11 2020, 7:32 AM

Committed alternate fix rBc5d0a2320498: Fix T73348: Surface Deform distortion on bind with small faces

This allows small values but ensures the result is finite.