On Wed, 13 Jan 2016 13:43:39 -0800 Bill McCloskey <wmcclos...@mozilla.com> wrote:
> When you call _intersect_rect and pass it an empty rectangle, > it creates an invalid region from that rectangle. The only valid > empty region is the one where the data pointer points to > pixman_region_empty_data. The region created by _intersect_rect has > a null data pointer, and so _not_empty returns true for it even if > the extents are empty. > > This patch uses code similar to what's in _union_rect to handle empty > rectangles. > --- > pixman/pixman-region.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/pixman/pixman-region.c b/pixman/pixman-region.c > index 59bc9c7..95911db 100644 > --- a/pixman/pixman-region.c > +++ b/pixman/pixman-region.c > @@ -1329,16 +1329,25 @@ PREFIX(_intersect_rect) (region_type_t *dest, > region_type_t region; > > region.data = NULL; > region.extents.x1 = x; > region.extents.y1 = y; > region.extents.x2 = x + width; > region.extents.y2 = y + height; > > + if (!GOOD_RECT (®ion.extents)) > + { > + if (BAD_RECT (®ion.extents)) > + _pixman_log_error (FUNC, "Invalid rectangle passed"); > + FREE_DATA (dest); > + PREFIX (_init) (dest); > + return TRUE; > + } > + > return PREFIX(_intersect) (dest, source, ®ion); > } > > /* Convenience function for performing union of region with a > * single rectangle > */ > */ > PIXMAN_EXPORT pixman_bool_t > PREFIX (_union_rect) (region_type_t *dest, Thanks for the patch. This looks like a reasonable fix to me. Just a minor nitpick. Currently the intersection of an empty region and an invalid region is expected to be invalid [1]. But with your patch, the intersection of an invalid region and an empty rectangle is going to be empty. Which is a little bit inconsistent. Also would it be too much to ask you to extend the test suite with a test for this particular usage of this function? You can find some information in the README file [2]. Basically, as the README file instructs, please create two patches. The first one should extend the test suite to demonstrate the bug. The second should fix the bug and make the tests pass again. Please have a look at the existing region* tests in the "test" directory. And finally, since some time ago, pixman contributors are using a "Signed-off-by:" tag in patches. Please add it to your commit messages. With all of the above addressed, consider your patch Reviewed-by: Siarhei Siamashka <siarhei.siamas...@gmail.com> [1] http://cgit.freedesktop.org/pixman/tree/pixman/pixman-region.c?id=pixman-0.33.6#n1173 [2] http://cgit.freedesktop.org/pixman/tree/README?id=pixman-0.33.6#n72 -- Best regards, Siarhei Siamashka _______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman