Bug 78339 - Certain rectilienar path+clip combinations crash the image backend at stroke
Summary: Certain rectilienar path+clip combinations crash the image backend at stroke
Status: RESOLVED MOVED
Alias: None
Product: cairo
Classification: Unclassified
Component: image backend (show other bugs)
Version: 1.12.14
Hardware: All All
: medium normal
Assignee: Chris Wilson
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: cairo-1.14
  Show dependency treegraph
 
Reported: 2014-05-06 13:21 UTC by Zoltan Turanyi
Modified: 2018-08-25 13:36 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
The provided test case in a form suitable for cairo-test-suite (1.33 KB, text/plain)
2014-05-11 19:01 UTC, Uli Schlachter
Details
A quick patch fixing the crash (5.16 KB, patch)
2015-03-23 12:26 UTC, Marek Kasik
Details | Splinter Review

Description Zoltan Turanyi 2014-05-06 13:21:04 UTC
If we have a complex, non-pixel aligned, rectilinear clip region and a rectilinear path, the image backend crashes at a stroke. (See offending code example below.) I saw this with 1.12.14, have not checked with 1.12.16, but none of the commits seem to address this.

The problem happens in composite_aligned_boxes() (file src\cairo-spans-compositor.c), where the local variable 'need_clip_mask' gets initialized to true via _clip_is_region() returning false. The latter returns false, because some of the clip boxes are not pixel aligned. (These are such boxes that do not intersect with the stroke (or the path of the stroke would not become pixel-aligned and composite_aligned_boxes() would not be called), but are kept because the fall within the stroke. See the third rectangle added to the clip in the below example. If you remove that third rectangle or make it pixel-aligned, the code runs fine.)

The code in composite_aligned_boxes() later assumes that if 'need_clip_mask' is true then the clip has a path (clip->path!=NULL) and calls get_clip_surface(), which asserts that clip->path is nonzero. Here we crash.

My proposal for a fix (without knowing all the complexity) would be to test for clip->path before setting need_clip_mask or calling get_clip_surface().

See below the code that causes the crash for me. I am not sure how to rate the severity of a bug causing a crash on rare situations, so I set it to normal. I use cairo on Windows, compiled with Visual Studio. 

Cairo is a great library folks, thanks a lot for the effort!


cairo_surface_t *surface = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, 100, 100);
cairo_t *cr = cairo_create(surface);
cairo_rectangle(cr, 10, 10, 80, 80);
cairo_new_sub_path(cr);
cairo_move_to(cr, 20, 20);
cairo_line_to(cr, 20, 50);
cairo_line_to(cr, 50, 50);
cairo_line_to(cr, 50, 20);
cairo_line_to(cr, 20, 20);
cairo_new_sub_path(cr);
cairo_move_to(cr, 31.5, 31.5);
cairo_line_to(cr, 32.5, 31.5);
cairo_line_to(cr, 32.5, 32.5);
cairo_line_to(cr, 31.5, 32.5);
cairo_line_to(cr, 31.5, 31.5);
cairo_clip(cr);
cairo_rectangle(cr, 5.5, 30.5, 90, 5);
cairo_set_line_width(cr, 1.);
cairo_stroke(cr);
cairo_destroy(cr);
cairo_surface_destroy (surface);
Comment 1 Uli Schlachter 2014-05-11 19:01:19 UTC
Created attachment 98868 [details]
The provided test case in a form suitable for cairo-test-suite

Thanks for that excellent test case, Zoltan. I hacked it into something suitable for cairo-test-suite and I can confirm that this still dies from the failed assertion on current git/master. And nope, sorry, no quick suggestions for a fix (but I am not sure about ignoring the clip for extents->clip->path == NULL is the right thing unless something somewhere already intersects the drawn boxes with the clip's boxes).

Are you ok with the following header being added to your test case?

/*
 * Copyright © 2014 Zoltan Turanyi
 *
 * 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.
 *
 * Author: Zoltan Turanyi <teknos@gmail.com>
 */
Comment 2 Zoltan Turanyi 2014-05-12 05:01:35 UTC
Thanks Uli, please go ahead adding the header.
Comment 3 Marek Kasik 2015-03-23 12:26:43 UTC
Created attachment 114548 [details] [review]
A quick patch fixing the crash

Hi,

I've observed the same crash when I was looking at this bug: https://bugzilla.redhat.com/show_bug.cgi?id=1203907.
Attached is a quick patch which fixes the crash for me although I'm not sure whether it is correct. It was inspired by the function "_cairo_clip_get_polygon()" which accepts 'clip->path == NULL' if there are some boxes in the clip.

Regards

Marek
Comment 4 GitLab Migration User 2018-08-25 13:36:05 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/cairo/cairo/issues/90.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.