[Bf-blender-cvs] [71d6c7e9858] temp-lineart-contained: LineArt: Clean up occlusion code comments

YimingWu noreply at git.blender.org
Mon Nov 15 12:53:41 CET 2021


Commit: 71d6c7e9858f826e9d24959de7d35fc29f632206
Author: YimingWu
Date:   Mon Nov 15 19:52:58 2021 +0800
Branches: temp-lineart-contained
https://developer.blender.org/rB71d6c7e9858f826e9d24959de7d35fc29f632206

LineArt: Clean up occlusion code comments

===================================================================

M	source/blender/gpencil_modifiers/intern/lineart/MOD_lineart.h
M	source/blender/gpencil_modifiers/intern/lineart/lineart_cpu.c

===================================================================

diff --git a/source/blender/gpencil_modifiers/intern/lineart/MOD_lineart.h b/source/blender/gpencil_modifiers/intern/lineart/MOD_lineart.h
index f8ecb4bdff5..61273d16ffc 100644
--- a/source/blender/gpencil_modifiers/intern/lineart/MOD_lineart.h
+++ b/source/blender/gpencil_modifiers/intern/lineart/MOD_lineart.h
@@ -498,12 +498,24 @@ typedef struct LineartBoundingArea {
 #define LRT_DOUBLE_CLOSE_ENOUGH_TRI(a, b) \
   (((a) + DBL_TRIANGLE_LIM) >= (b) && ((a)-DBL_TRIANGLE_LIM) <= (b))
 
-BLI_INLINE int lineart_LineIntersectTest2d(const double *a1,
-                                           const double *a2,
-                                           const double *b1,
-                                           const double *b2,
-                                           double *aRatio,
-                                           bool *aAligned)
+/* Notes on this function:
+
+ * r_ratio: The ratio on segment a1-a2. When r_ratio is very close to zero or one, it
+ * fixes the value to zero or one, this makes it easier to identify "on the tip" situations.
+ *
+ * r_aligned: True when 1) a and b is exactly on the same straight line and 2) a and b share a
+ * common end-point.
+ *
+ * Important: if r_aligned is true, r_ratio will be either 0 or 1 depending on which point from
+ * segment a is shared with segment b. If it's a1 then r_ratio is 0, else then r_ratio is 1. This
+ * extra information is needed for line art occlusion stage to work correctly in such cases.
+ */
+BLI_INLINE int lineart_intersect_seg_seg(const double *a1,
+                                         const double *a2,
+                                         const double *b1,
+                                         const double *b2,
+                                         double *r_ratio,
+                                         bool *r_aligned)
 {
 /* Legacy intersection math aligns better with occlusion function quirks. */
 /* #define USE_VECTOR_LINE_INTERSECTION */
@@ -527,27 +539,27 @@ BLI_INLINE int lineart_LineIntersectTest2d(const double *a1,
     double rr;
 
     if (fabs(a2[0] - a1[0]) > fabs(a2[1] - a1[1])) {
-      *aRatio = ratiod(a1[0], a2[0], rx);
+      *r_ratio = ratiod(a1[0], a2[0], rx);
       if (fabs(b2[0] - b1[0]) > fabs(b2[1] - b1[1])) {
         rr = ratiod(b1[0], b2[0], rx);
       }
       else {
         rr = ratiod(b1[1], b2[1], ry);
       }
-      if ((*aRatio) > 0 && (*aRatio) < 1 && rr > 0 && rr < 1) {
+      if ((*r_ratio) > 0 && (*r_ratio) < 1 && rr > 0 && rr < 1) {
         return 1;
       }
       return 0;
     }
 
-    *aRatio = ratiod(a1[1], a2[1], ry);
+    *r_ratio = ratiod(a1[1], a2[1], ry);
     if (fabs(b2[0] - b1[0]) > fabs(b2[1] - b1[1])) {
       rr = ratiod(b1[0], b2[0], rx);
     }
     else {
       rr = ratiod(b1[1], b2[1], ry);
     }
-    if ((*aRatio) > 0 && (*aRatio) < 1 && rr > 0 && rr < 1) {
+    if ((*r_ratio) > 0 && (*r_ratio) < 1 && rr > 0 && rr < 1) {
       return 1;
     }
     return 0;
@@ -562,49 +574,52 @@ BLI_INLINE int lineart_LineIntersectTest2d(const double *a1,
   double x_diff = (a2[0] - a1[0]);
   double x_diff2 = (b2[0] - b1[0]);
 
-  *aAligned = false;
+  *r_aligned = false;
 
   if (LRT_DOUBLE_CLOSE_ENOUGH(x_diff, 0)) {
     if (LRT_DOUBLE_CLOSE_ENOUGH(x_diff2, 0)) {
+      /* This means two segments are both vertical. */
       if ((LRT_DOUBLE_CLOSE_ENOUGH(a2[0], b1[0]) && LRT_DOUBLE_CLOSE_ENOUGH(a2[1], b1[1])) ||
           (LRT_DOUBLE_CLOSE_ENOUGH(a2[0], b2[0]) && LRT_DOUBLE_CLOSE_ENOUGH(a2[1], b2[1]))) {
-        *aAligned = true;
-        *aRatio = 1;
+        *r_aligned = true;
+        *r_ratio = 1;
       }
       else if ((LRT_DOUBLE_CLOSE_ENOUGH(a1[0], b1[0]) && LRT_DOUBLE_CLOSE_ENOUGH(a1[1], b1[1])) ||
                (LRT_DOUBLE_CLOSE_ENOUGH(a1[0], b2[0]) && LRT_DOUBLE_CLOSE_ENOUGH(a1[1], b2[1]))) {
-        *aAligned = true;
-        *aRatio = 0;
+        *r_aligned = true;
+        *r_ratio = 0;
       }
       return 0;
     }
     double r2 = ratiod(b1[0], b2[0], a1[0]);
     x = interpd(b2[0], b1[0], r2);
     y = interpd(b2[1], b1[1], r2);
-    *aRatio = ratio = ratiod(a1[1], a2[1], y);
+    *r_ratio = ratio = ratiod(a1[1], a2[1], y);
   }
   else {
     if (LRT_DOUBLE_CLOSE_ENOUGH(x_diff2, 0)) {
       ratio = ratiod(a1[0], a2[0], b1[0]);
       x = interpd(a2[0], a1[0], ratio);
-      *aRatio = ratio;
+      *r_ratio = ratio;
     }
     else {
       k1 = (a2[1] - a1[1]) / x_diff;
       k2 = (b2[1] - b1[1]) / x_diff2;
 
       if (LRT_DOUBLE_CLOSE_ENOUGH(k2, k1)) {
+        /* This means two segments are parallel. This also handles k==0 (both completely
+         * horizontal) cases. */
         if ((LRT_DOUBLE_CLOSE_ENOUGH(a2[0], b1[0]) && LRT_DOUBLE_CLOSE_ENOUGH(a2[1], b1[1])) ||
             (LRT_DOUBLE_CLOSE_ENOUGH(a2[0], b2[0]) && LRT_DOUBLE_CLOSE_ENOUGH(a2[1], b2[1]))) {
-          *aAligned = true;
-          *aRatio = 1;
+          *r_aligned = true;
+          *r_ratio = 1;
         }
         else if ((LRT_DOUBLE_CLOSE_ENOUGH(a1[0], b1[0]) &&
                   LRT_DOUBLE_CLOSE_ENOUGH(a1[1], b1[1])) ||
                  (LRT_DOUBLE_CLOSE_ENOUGH(a1[0], b2[0]) &&
                   LRT_DOUBLE_CLOSE_ENOUGH(a1[1], b2[1]))) {
-          *aAligned = true;
-          *aRatio = 0;
+          *r_aligned = true;
+          *r_ratio = 0;
         }
         return 0;
       }
@@ -613,7 +628,7 @@ BLI_INLINE int lineart_LineIntersectTest2d(const double *a1,
 
       ratio = (x - a1[0]) / x_diff;
 
-      *aRatio = ratio;
+      *r_ratio = ratio;
     }
   }
 
@@ -627,11 +642,11 @@ BLI_INLINE int lineart_LineIntersectTest2d(const double *a1,
            (b2[0] < b1[0] && x < b2[0]))
     return 0;
 
-  if (LRT_DOUBLE_CLOSE_ENOUGH_TRI(*aRatio, 1)) {
-    *aRatio = 1;
+  if (LRT_DOUBLE_CLOSE_ENOUGH_TRI(*r_ratio, 1)) {
+    *r_ratio = 1;
   }
-  else if (LRT_DOUBLE_CLOSE_ENOUGH_TRI(*aRatio, 0)) {
-    *aRatio = 0;
+  else if (LRT_DOUBLE_CLOSE_ENOUGH_TRI(*r_ratio, 0)) {
+    *r_ratio = 0;
   }
 
   return 1;
diff --git a/source/blender/gpencil_modifiers/intern/lineart/lineart_cpu.c b/source/blender/gpencil_modifiers/intern/lineart/lineart_cpu.c
index e08df5cb5a0..9b635ffb8dc 100644
--- a/source/blender/gpencil_modifiers/intern/lineart/lineart_cpu.c
+++ b/source/blender/gpencil_modifiers/intern/lineart/lineart_cpu.c
@@ -541,8 +541,8 @@ static void lineart_main_occlusion_begin(LineartRenderBuffer *rb)
   rb->floating.last = rb->floating.first;
   rb->light_contour.last = rb->light_contour.first;
 
-  /* This is needed because the occlusion function needs camera vector to be in the direction of
-   * point to camera. */
+  /* This is needed because the occlusion function expects the camera vector to point towards the
+   * camera. */
   negate_v3_db(rb->view_vector);
 
   TaskPool *tp = BLI_task_pool_create(NULL, TASK_PRIORITY_HIGH);
@@ -2443,6 +2443,22 @@ static bool lineart_edge_from_triangle(const LineartTriangle *tri,
  * the occlusion status between 1(one) triangle and 1(one) line.
  * if returns true, then from/to will carry the occluded segments
  * in ratio from `e->v1` to `e->v2`. The line is later cut with these two values.
+ *
+ * TODO: (Yiming) This function uses a convoluted method that needs to be redesigned.
+ *
+ * 1) The lineart_intersect_seg_seg() and lineart_point_triangle_relation() are separate calls,
+ * which would potentially return results that doesn't agree, especially when it's an edge
+ * extruding from one of the triangle's point. To get the information using one math process can
+ * solve this problem.
+ *
+ * 2) Currently using discrete a/b/c/pa/pb/pc/is[3] values for storing
+ * intersection/edge_aligned/intersection_order info, which isn't optimal, needs a better
+ * representation (likely a struct) for redability and clarity of code path.
+ *
+ * I keep this function as-is because it's still fast, and more importantly the output value
+ * threshold is already in tune with the cutting function in the next stage.
+ * While current "edge aligned" fix isn't ideal, it does solve most of the precision issue
+ * expecially in ortho camera mode.
  */
 static bool lineart_triangle_edge_image_space_occlusion(SpinLock *UNUSED(spl),
                                                         const LineartTriangle *tri,
@@ -2491,9 +2507,9 @@ static bool lineart_triangle_edge_image_space_occlusion(SpinLock *UNUSED(spl),
   }
 
   /* Check if the line visually crosses one of the edge in the triangle. */
-  a = lineart_LineIntersectTest2d(LFBC, RFBC, FBC0, FBC1, &is[0], &pa);
-  b = lineart_LineIntersectTest2d(LFBC, RFBC, FBC1, FBC2, &is[1], &pb);
-  c = lineart_LineIntersectTest2d(LFBC, RFBC, FBC2, FBC0, &is[2], &pc);
+  a = lineart_intersect_seg_seg(LFBC, RFBC, FBC0, FBC1, &is[0], &pa);
+  b = lineart_intersect_seg_seg(LFBC, RFBC, FBC1, FBC2, &is[1], &pb);
+  c = lineart_intersect_seg_seg(LFBC, RFBC, FBC2, FBC0, &is[2], &pc);
 
   /* Sort the intersection distance. */
   INTERSECT_SORT_MIN_TO_MAX_3(is[0], is[1], is[2], order);
@@ -2525,13 +2541,16 @@ static bool lineart_triangle_edge_image_space_occlusion(SpinLock *UNUSED(spl),
     return false;
   }
 
+  /* If the edge doesn't visually cross any edge of the triangle... */
   if (!a && !b && !c) {
+    /* And if both end point from the edge is outside of the triangle... */
     if (!(st_l = lineart_point_triangle_relation(LFBC, FBC0, FBC1, FBC2)) &&
         !(st_r = lineart_point_triangle_relation(RFBC, FBC0, FBC1, FBC2))) {
-      return 0; /* Intersection point is not inside triangle. */
+      return 0; /* We don't have any occlusion. */
     }
   }
 
+  /* Whether two end points are inside/on_the_edge/outside of the triangle. */
   st_l = lineart_point_triangle_relation(LFBC, FBC0, FBC1, FBC2);
   st_r = lineart_point_triangle_relation(RFBC, FBC0, FBC1, FBC2);
 
@@ -2583,32 +2602,42 @@ static bool lineart_triangle_edge_image_space_occlusion(SpinLock *UNUSED(spl),
     return false; \
   }
 
-  /* Determine the pair of edges that the line has crossed. */
+  /* Determine the pair of edges that the line has crossed. The "|" symbol in the comment indicates
+   * triangle boundary. DBL_TRIANGLE_LIM is needed to for floating point precision tolerance. */
 
   if (st_l == 2) {
+    /* Left side is in the triangle. */
     

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list