Merge branch 'round-join' into 'master'

Fix for round joins

See merge request cairo/cairo!372

.gitlab-ci/ignore-quartz-argb32.txt

@@ -39,6 +39,8 @@ record-replay-extend-reflect
 record-replay-extend-repeat
 record-select-font-face
 record-text-transform
+round-join-bug-520-round
+round-join-bug-520-bevel
 simple-edge
 subsurface
 subsurface-outside-target

.gitlab-ci/ignore-quartz-rgb24.txt

@@ -67,6 +67,8 @@ record-replay-extend-repeat
 record-select-font-face
 record-text-transform
 rel-path
+round-join-bug-520-round
+round-join-bug-520-bevel
 scale-source-surface-paint
 set-source
 shifted-operator

src/cairo-path-stroke-polygon.c

@@ -399,16 +399,16 @@ outer_close (struct stroker *stroker,
 
     switch (stroker->style.line_join) {
     case CAIRO_LINE_JOIN_ROUND:
-	/* construct a fan around the common midpoint */
 	if ((in->dev_slope.x * out->dev_slope.x +
 	     in->dev_slope.y * out->dev_slope.y) < stroker->spline_cusp_tolerance)
 	{
+	    /* construct a fan around the common midpoint */
 	    add_fan (stroker,
 		     &in->dev_vector, &out->dev_vector, &in->point,
 		     clockwise, outer);
-	    break;
-	}
-	/* else fall through */
+	} /* else: bevel join */
+	break;
+
     case CAIRO_LINE_JOIN_MITER:
     default: {
 	/* dot product of incoming slope vector with outgoing slope vector */
@@ -587,10 +587,14 @@ outer_join (struct stroker *stroker,
 
     switch (stroker->style.line_join) {
     case CAIRO_LINE_JOIN_ROUND:
-	/* construct a fan around the common midpoint */
-	add_fan (stroker,
-		 &in->dev_vector, &out->dev_vector, &in->point,
-		 clockwise, outer);
+	if ((in->dev_slope.x * out->dev_slope.x +
+	     in->dev_slope.y * out->dev_slope.y) < stroker->spline_cusp_tolerance)
+	{
+	    /* construct a fan around the common midpoint */
+	    add_fan (stroker,
+		     &in->dev_vector, &out->dev_vector, &in->point,
+		     clockwise, outer);
+	} /* else: bevel join */
 	break;
 
     case CAIRO_LINE_JOIN_MITER:
@@ -1291,17 +1295,72 @@ _cairo_path_fixed_stroke_to_polygon (const cairo_path_fixed_t	*path,
     stroker.ctm_inverse = ctm_inverse;
     stroker.tolerance = tolerance;
     stroker.half_line_width = style->line_width / 2.;
-    /* To test whether we need to join two segments of a spline using
-     * a round-join or a bevel-join, we can inspect the angle between the
-     * two segments. If the difference between the chord distance
-     * (half-line-width times the cosine of the bisection angle) and the
-     * half-line-width itself is greater than tolerance then we need to
-     * inject a point.
+
+    /* If `CAIRO_LINE_JOIN_ROUND` is selected and a joint's `arc height`
+     * is greater than `tolerance` then two segments are joined with
+     * round-join, otherwise bevel-join is used.
+     *
+     * (See https://gitlab.freedesktop.org/cairo/cairo/-/merge_requests/372#note_1698225
+     *  for an illustration.)
+     *
+     * `Arc height` is the distance from the center of arc's chord to
+     * the center of the arc. It is also the difference of arc's radius
+     * and the "distance from a point where segments are joined to the
+     * chord" (distance to the chord). Arc's radius is the half of a line
+     * width and the "distance to the chord" is equal to "half of a line width"
+     * times `cos(half the angle between segment vectors)`. So
+     *
+     *     arc_height = w/2 - w/2 * cos(phi/2),
+     *
+     * where `w/2` is the "half of a line width".
+     *
+     * Using the double angle cosine formula we can express the `cos(phi/2)`
+     * with just `cos(phi)` which is also the dot product of segments'
+     * unit vectors.
+     *
+     *     cos(phi/2) = sqrt ( (1 + cos(phi)) / 2 );
+     *     cos(phi/2) is in [0; 1] range, cannot be negative;
+     *
+     *     cos(phi) = a . b  = (ax * bx + ay * by),
+     *
+     * where `a` and `b` are unit vectors of the segments to be joined.
+     *
+     * Since `arc height` should be greater than the `tolerance` to produce
+     * a round-join we can write
+     *
+     *     w/2 * (1 - cos(phi/2))  >  tolerance;
+     *     1 - tolerance / (w/2)  >  cos(phi/2);    [!]
+     *
+     * which can be rewritten with the above double angle formula to
+     *
+     *     cos(phi)  <  2 * ( 1 - tolerance / (w/2) )^2 - 1,
+     *
+     * [!] Note that `w/2` is in [tolerance; +inf] range, since `cos(phi/2)`
+     * cannot be negative. The left part of the above inequality is the
+     * dot product and the right part is the `spline_cusp_tolerance`:
+     *
+     *     (ax * bx + ay * by) < spline_cusp_tolerance.
+     *
+     * In the code below only the `spline_cusp_tolerance` is calculated.
+     * The dot product is calculated later, in the condition expression
+     * itself. "Half of a line width" must be scaled with CTM for tolerance
+     * condition to be properly met. Also, since `arch height` cannot exceed
+     * the "half of a line width" and since `cos(phi/2)` cannot be negative,
+     * when `tolerance` is greater than the "half of a line width" the
+     * bevel-join should be produced.
      */
-    stroker.spline_cusp_tolerance = 1 - tolerance / stroker.half_line_width;
-    stroker.spline_cusp_tolerance *= stroker.spline_cusp_tolerance;
-    stroker.spline_cusp_tolerance *= 2;
-    stroker.spline_cusp_tolerance -= 1;
+    double scaled_hlw = hypot(stroker.half_line_width * ctm->xx,
+			      stroker.half_line_width * ctm->yx);
+
+    if (scaled_hlw <= tolerance) {
+	stroker.spline_cusp_tolerance = -1.0;
+    } else {
+	stroker.spline_cusp_tolerance = 1 - tolerance / scaled_hlw;
+	stroker.spline_cusp_tolerance *= stroker.spline_cusp_tolerance;
+	stroker.spline_cusp_tolerance *= 2;
+	stroker.spline_cusp_tolerance -= 1;
+    }
+
     stroker.ctm_det_positive =
 	_cairo_matrix_compute_determinant (ctm) >= 0.0;
 

src/cairo-path-stroke-traps.c

@@ -102,17 +102,29 @@ stroker_init (struct stroker		*stroker,
     stroker->tolerance = tolerance;
     stroker->traps = traps;
 
-    /* To test whether we need to join two segments of a spline using
-     * a round-join or a bevel-join, we can inspect the angle between the
-     * two segments. If the difference between the chord distance
-     * (half-line-width times the cosine of the bisection angle) and the
-     * half-line-width itself is greater than tolerance then we need to
-     * inject a point.
+    /* If `CAIRO_LINE_JOIN_ROUND` is selected and a joint's `arc height`
+     * is greater than `tolerance` then two segments are joined with
+     * round-join, otherwise bevel-join is used.
+     *
+     * `Arc height` is the difference of the "half of a line width" and
+     * the "half of a line width" times `cos(half the angle between segment vectors)`.
+     *
+     * See detailed description in the `_cairo_path_fixed_stroke_to_polygon()`
+     * function in the `cairo-path-stroke-polygon.c` file or follow the
+     * https://gitlab.freedesktop.org/cairo/cairo/-/merge_requests/372#note_1698225
+     * link to see the detailed description with an illustration.
      */
-    stroker->spline_cusp_tolerance = 1 - tolerance / stroker->half_line_width;
-    stroker->spline_cusp_tolerance *= stroker->spline_cusp_tolerance;
-    stroker->spline_cusp_tolerance *= 2;
-    stroker->spline_cusp_tolerance -= 1;
+    double scaled_hlw = hypot(stroker->half_line_width * ctm->xx,
+			      stroker->half_line_width * ctm->yx);
+
+    if (scaled_hlw <= tolerance) {
+	stroker->spline_cusp_tolerance = -1.0;
+    } else {
+	stroker->spline_cusp_tolerance = 1 - tolerance / scaled_hlw;
+	stroker->spline_cusp_tolerance *= stroker->spline_cusp_tolerance;
+	stroker->spline_cusp_tolerance *= 2;
+	stroker->spline_cusp_tolerance -= 1;
+    }
 
     stroker->ctm_determinant = _cairo_matrix_compute_determinant (stroker->ctm);
     stroker->ctm_det_positive = stroker->ctm_determinant >= 0.0;

test/Makefile.sources

@@ -305,6 +305,7 @@ test_sources = \
 	rotated-clip.c					\
 	rounded-rectangle-fill.c			\
 	rounded-rectangle-stroke.c			\
+	round-join-bug-520.c                \
 	sample.c					\
 	scale-down-source-surface-paint.c		\
 	scale-offset-image.c				\

test/meson.build

@@ -305,6 +305,7 @@ test_sources = [
   'rotated-clip.c',
   'rounded-rectangle-fill.c',
   'rounded-rectangle-stroke.c',
+  'round-join-bug-520.c',
   'sample.c',
   'scale-down-source-surface-paint.c',
   'scale-offset-image.c',

test/round-join-bug-520.c

@@ -0,0 +1,109 @@
+/*
+ * Author: Christian Rohlfs, 2022
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy,
+ * modify, merge, publish, distribute, sublicense, and/or sell copies
+ * of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "cairo-test.h"
+
+#define WIDTH   100
+#define HEIGHT  WIDTH
+
+static cairo_test_status_t
+draw_round(cairo_t *cr, int width, int height)
+{
+    /* Fail condition:
+            0  >=  2 * ( 1 - tolerance / (line_width / 2) )^2 - 1,
+       with the default tolerance of 0.1, `failing_line_width` is in
+            [0.117157287525381; 0.682842712474619]
+       range.
+    */
+
+    double failing_line_width = 0.3;
+
+    cairo_set_tolerance(cr, 0.1);
+    cairo_scale(cr, width, height);
+
+    cairo_set_source_rgb(cr, 1, 1, 1);
+    cairo_paint(cr);
+
+    cairo_move_to(cr, 0.2, 0.2);
+    cairo_line_to(cr, 0.8, 0.2);
+    cairo_line_to(cr, 0.8, 0.8);
+    cairo_line_to(cr, 0.2, 0.8);
+    cairo_close_path(cr);
+
+    cairo_set_line_join(cr, CAIRO_LINE_JOIN_ROUND);
+    cairo_set_source_rgb(cr, 0, 0, 0);
+    cairo_set_line_width(cr, failing_line_width);
+    cairo_stroke(cr);
+
+    return CAIRO_TEST_SUCCESS;
+}
+
+static cairo_test_status_t
+draw_bevel(cairo_t *cr, int width, int height)
+{
+    /* Fail condition:
+            arc_height < tolerance
+       For 90° join `arc_height` is equal to
+            line_width / 2 * (1 - sqrt(1/2))
+       which is 0.1464466094067262 of a `line_width`.
+    */
+
+    double line_width = 0.3;
+    double failing_tolerance = 0.3 * line_width;
+
+    cairo_set_tolerance(cr, width * failing_tolerance);
+    cairo_scale(cr, width, height);
+
+    cairo_set_source_rgb(cr, 1, 1, 1);
+    cairo_paint(cr);
+
+    cairo_move_to(cr, 0.2, 0.2);
+    cairo_line_to(cr, 0.8, 0.2);
+    cairo_line_to(cr, 0.8, 0.8);
+    cairo_line_to(cr, 0.2, 0.8);
+    cairo_close_path(cr);
+
+    cairo_set_line_join(cr, CAIRO_LINE_JOIN_ROUND);
+    cairo_set_source_rgb(cr, 0, 0, 0);
+    cairo_set_line_width(cr, line_width);
+    cairo_stroke(cr);
+
+    return CAIRO_TEST_SUCCESS;
+}
+
+
+CAIRO_TEST (round_join_bug_520_round,
+    "Tests round joins within tolerance",
+    "stroke, cap, join", /* keywords */
+    NULL, /* requirements */
+    WIDTH, HEIGHT,
+    NULL, draw_round)
+
+CAIRO_TEST (round_join_bug_520_bevel,
+    "Tests round joins omission when `arc_height < tolerance`",
+    "stroke, cap, join", /* keywords */
+    NULL, /* requirements */
+    WIDTH, HEIGHT,
+    NULL, draw_bevel)
+