diff --git a/.gitlab-ci/ignore-quartz-argb32.txt b/.gitlab-ci/ignore-quartz-argb32.txt index e00077831..29face153 100644 --- a/.gitlab-ci/ignore-quartz-argb32.txt +++ b/.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 diff --git a/.gitlab-ci/ignore-quartz-rgb24.txt b/.gitlab-ci/ignore-quartz-rgb24.txt index 0941d59cd..7bceae075 100644 --- a/.gitlab-ci/ignore-quartz-rgb24.txt +++ b/.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 diff --git a/src/cairo-path-stroke-polygon.c b/src/cairo-path-stroke-polygon.c index 3f7c49802..44b6675e8 100644 --- a/src/cairo-path-stroke-polygon.c +++ b/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; diff --git a/src/cairo-path-stroke-traps.c b/src/cairo-path-stroke-traps.c index eab978235..4eabf6583 100644 --- a/src/cairo-path-stroke-traps.c +++ b/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; diff --git a/test/Makefile.sources b/test/Makefile.sources index 2b0464ec9..c01d3ac2a 100644 --- a/test/Makefile.sources +++ b/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 \ diff --git a/test/meson.build b/test/meson.build index e7e0a566d..f1612e670 100644 --- a/test/meson.build +++ b/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', diff --git a/test/reference/round-join-bug-520-bevel.pdf.ref.png b/test/reference/round-join-bug-520-bevel.pdf.ref.png new file mode 100644 index 000000000..b1a38e12f Binary files /dev/null and b/test/reference/round-join-bug-520-bevel.pdf.ref.png differ diff --git a/test/reference/round-join-bug-520-bevel.ref.png b/test/reference/round-join-bug-520-bevel.ref.png new file mode 100644 index 000000000..b888f9ba9 Binary files /dev/null and b/test/reference/round-join-bug-520-bevel.ref.png differ diff --git a/test/reference/round-join-bug-520-bevel.svg.ref.png b/test/reference/round-join-bug-520-bevel.svg.ref.png new file mode 100644 index 000000000..b1a38e12f Binary files /dev/null and b/test/reference/round-join-bug-520-bevel.svg.ref.png differ diff --git a/test/reference/round-join-bug-520-round.ref.png b/test/reference/round-join-bug-520-round.ref.png new file mode 100644 index 000000000..b1a38e12f Binary files /dev/null and b/test/reference/round-join-bug-520-round.ref.png differ diff --git a/test/reference/round-join-bug-520-round.xlib-window.ref.png b/test/reference/round-join-bug-520-round.xlib-window.ref.png new file mode 100644 index 000000000..14ed9cbfe Binary files /dev/null and b/test/reference/round-join-bug-520-round.xlib-window.ref.png differ diff --git a/test/reference/round-join-bug-520-round.xlib.ref.png b/test/reference/round-join-bug-520-round.xlib.ref.png new file mode 100644 index 000000000..14ed9cbfe Binary files /dev/null and b/test/reference/round-join-bug-520-round.xlib.ref.png differ diff --git a/test/round-join-bug-520.c b/test/round-join-bug-520.c new file mode 100644 index 000000000..be2b5aa0d --- /dev/null +++ b/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) +