From 19dbe049db9198f7f018c92c42f69daa2d7faeeb Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Fri, 21 Apr 2023 14:01:21 +1000 Subject: [PATCH] Fix #106469: unstable tessellation with quad-flipping detection The result of detecting if a quad should flip the default 0-2 split when tessellated only used a pre-calculated normal when available, since the method of detecting the flip was different, the check for a concave face could change depending on the existence of polygon-normals. In practice this meant cycles render preview could use a different tessellation than the GPU display. While [0] exposed the bug, it's an inherent problem with having 2 methods of detecting concave quads. Remove is_quad_flip_v3_first_third_fast_with_normal(..) and always use is_quad_flip_v3_first_third_fast(..), because having to calculate the normal inline has significant overhead. Note that "bow-tie" quads may now render with a subdivision in a different direction although they must be very distorted with both triangles along the 0-2 split pointing away from each other. Thanks to @HooglyBoogly for investigating the issue. [0]: 16fbadde363c8074ec725fa1e1079add096bd741. --- .../blenkernel/intern/mesh_tessellate.cc | 18 ++++-------------- source/blender/blenlib/BLI_math_geom.h | 5 ----- source/blender/blenlib/intern/math_geom.c | 16 +++------------- .../blender/blenlib/intern/mesh_intersect.cc | 18 ++++++++++-------- .../bmesh/intern/bmesh_mesh_tessellate.c | 7 ++----- 5 files changed, 19 insertions(+), 45 deletions(-) diff --git a/source/blender/blenkernel/intern/mesh_tessellate.cc b/source/blender/blenkernel/intern/mesh_tessellate.cc index 7e01552482c..fca2de49c8e 100644 --- a/source/blender/blenkernel/intern/mesh_tessellate.cc +++ b/source/blender/blenkernel/intern/mesh_tessellate.cc @@ -63,20 +63,10 @@ BLI_INLINE void mesh_calc_tessellation_for_face_impl(const Span corner_vert MLoopTri *mlt_a = mlt++; create_tri(0, 2, 3); MLoopTri *mlt_b = mlt; - - if (UNLIKELY(face_normal ? is_quad_flip_v3_first_third_fast_with_normal( - /* Simpler calculation (using the normal). */ - positions[corner_verts[mlt_a->tri[0]]], - positions[corner_verts[mlt_a->tri[1]]], - positions[corner_verts[mlt_a->tri[2]]], - positions[corner_verts[mlt_b->tri[2]]], - normal_precalc) : - is_quad_flip_v3_first_third_fast( - /* Expensive calculation (no normal). */ - positions[corner_verts[mlt_a->tri[0]]], - positions[corner_verts[mlt_a->tri[1]]], - positions[corner_verts[mlt_a->tri[2]]], - positions[corner_verts[mlt_b->tri[2]]]))) { + if (UNLIKELY(is_quad_flip_v3_first_third_fast(positions[corner_verts[mlt_a->tri[0]]], + positions[corner_verts[mlt_a->tri[1]]], + positions[corner_verts[mlt_a->tri[2]]], + positions[corner_verts[mlt_b->tri[2]]]))) { /* Flip out of degenerate 0-2 state. */ mlt_a->tri[2] = mlt_b->tri[2]; mlt_b->tri[0] = mlt_a->tri[1]; diff --git a/source/blender/blenlib/BLI_math_geom.h b/source/blender/blenlib/BLI_math_geom.h index e24f3c3bde8..126bd158140 100644 --- a/source/blender/blenlib/BLI_math_geom.h +++ b/source/blender/blenlib/BLI_math_geom.h @@ -145,11 +145,6 @@ bool is_quad_flip_v3_first_third_fast(const float v1[3], const float v2[3], const float v3[3], const float v4[3]); -bool is_quad_flip_v3_first_third_fast_with_normal(const float v1[3], - const float v2[3], - const float v3[3], - const float v4[3], - const float normal[3]); /** \} */ diff --git a/source/blender/blenlib/intern/math_geom.c b/source/blender/blenlib/intern/math_geom.c index e55eb46235a..c21b07573e8 100644 --- a/source/blender/blenlib/intern/math_geom.c +++ b/source/blender/blenlib/intern/math_geom.c @@ -5889,6 +5889,9 @@ bool is_quad_flip_v3_first_third_fast(const float v1[3], const float v3[3], const float v4[3]) { + /* NOTE: if the faces normal has been calculated it's possible to simplify the following checks, + * however this means the solution may be different depending on the existence of normals + * causing tessellation to be "unstable" depending on the existence of normals, see #106469. */ float d_12[3], d_13[3], d_14[3]; float cross_a[3], cross_b[3]; sub_v3_v3v3(d_12, v2, v1); @@ -5899,19 +5902,6 @@ bool is_quad_flip_v3_first_third_fast(const float v1[3], return dot_v3v3(cross_a, cross_b) > 0.0f; } -bool is_quad_flip_v3_first_third_fast_with_normal(const float v1[3], - const float v2[3], - const float v3[3], - const float v4[3], - const float normal[3]) -{ - float dir_v3v1[3], tangent[3]; - sub_v3_v3v3(dir_v3v1, v3, v1); - cross_v3_v3v3(tangent, dir_v3v1, normal); - const float dot = dot_v3v3(v1, tangent); - return (dot_v3v3(v4, tangent) >= dot) || (dot_v3v3(v2, tangent) <= dot); -} - float cubic_tangent_factor_circle_v3(const float tan_l[3], const float tan_r[3]) { BLI_ASSERT_UNIT_V3(tan_l); diff --git a/source/blender/blenlib/intern/mesh_intersect.cc b/source/blender/blenlib/intern/mesh_intersect.cc index a066fe724a5..401ee6e1ce3 100644 --- a/source/blender/blenlib/intern/mesh_intersect.cc +++ b/source/blender/blenlib/intern/mesh_intersect.cc @@ -1962,17 +1962,19 @@ static Face *cdt_tri_as_imesh_face( return facep; } -/* Like BLI_math's is_quad_flip_v3_first_third_fast_with_normal, with const double3's. */ +/* Like BLI_math's is_quad_flip_v3_first_third_fast, with const double3's. */ static bool is_quad_flip_first_third(const double3 &v1, const double3 &v2, const double3 &v3, - const double3 &v4, - const double3 &normal) + const double3 &v4) { - double3 dir_v3v1 = v3 - v1; - double3 tangent = math::cross(dir_v3v1, normal); - double dot = math::dot(v1, tangent); - return (math::dot(v4, tangent) >= dot) || (math::dot(v2, tangent) <= dot); + const double3 d_12 = v2 - v1; + const double3 d_13 = v3 - v1; + const double3 d_14 = v4 - v1; + + const double3 cross_a = math::cross(d_12, d_13); + const double3 cross_b = math::cross(d_14, d_13); + return math::dot(cross_a, cross_b) > 0.0f; } /** @@ -2008,7 +2010,7 @@ static Array polyfill_triangulate_poly(Face *f, IMeshArena *arena) int eo_23 = f->edge_orig[2]; int eo_30 = f->edge_orig[3]; Face *f0, *f1; - if (UNLIKELY(is_quad_flip_first_third(v0->co, v1->co, v2->co, v3->co, f->plane->norm))) { + if (UNLIKELY(is_quad_flip_first_third(v0->co, v1->co, v2->co, v3->co))) { f0 = arena->add_face({v0, v1, v3}, f->orig, {eo_01, -1, eo_30}, {false, false, false}); f1 = arena->add_face({v1, v2, v3}, f->orig, {eo_12, eo_23, -1}, {false, false, false}); } diff --git a/source/blender/bmesh/intern/bmesh_mesh_tessellate.c b/source/blender/bmesh/intern/bmesh_mesh_tessellate.c index 69510263ec9..d18680e8839 100644 --- a/source/blender/bmesh/intern/bmesh_mesh_tessellate.c +++ b/source/blender/bmesh/intern/bmesh_mesh_tessellate.c @@ -81,11 +81,8 @@ BLI_INLINE void bmesh_calc_tessellation_for_face_impl(BMLoop *(*looptris)[3], efa->no, l_ptr_a[0]->v->co, l_ptr_a[1]->v->co, l_ptr_a[2]->v->co, l_ptr_b[2]->v->co); } - if (UNLIKELY(is_quad_flip_v3_first_third_fast_with_normal(l_ptr_a[0]->v->co, - l_ptr_a[1]->v->co, - l_ptr_a[2]->v->co, - l_ptr_b[2]->v->co, - efa->no))) { + if (UNLIKELY(is_quad_flip_v3_first_third_fast( + l_ptr_a[0]->v->co, l_ptr_a[1]->v->co, l_ptr_a[2]->v->co, l_ptr_b[2]->v->co))) { /* Flip out of degenerate 0-2 state. */ l_ptr_a[2] = l_ptr_b[2]; l_ptr_b[0] = l_ptr_a[1];