[Pixman] [PATCH 1/2] Add test for _intersect_rect(<region>, <empty-rectangle>)

[Bill McCloskey]

Thanks Siarhei. I would appreciate it if you could push these two patches
for me with the changes you suggested. I had trouble getting git send-email
to work with my SMTP server.

Sorry about the confusion with Reviewed-by. I'm more accustomed to working
with Bugzilla where things are a little more explicit.

-Bill

On Thu, Feb 4, 2016 at 4:10 PM, Siarhei Siamashka <
siarhei.siamashka at gmail.com> wrote:

> Hello Bill,
>
> Thanks for the patches. By the way, they are a little bit
> mangled, maybe because you did not use "git send-email" to
> send them? This is easily fixable for small patches like
> this, but may become a problem for larger patch sets.
>
>
> On Tue, 2 Feb 2016 17:35:28 -0800
> Bill McCloskey <wmccloskey at mozilla.com> wrote:
>
> > This test ensures that calling _intersect_rect on an empty
> > rectangle produces an empty region.
> >
> > Reviewed-by: Siarhei Siamashka <siarhei.siamashka at gmail.com>
>
> Your "Signed-off-by: Bill McCloskey <wmccloskey at mozilla.com>"
> should be also here.
>
> Also adding my "Reviewed-by" was a little bit premature because
> I had no idea how this patch would look in the end. I only
> promised that getting my "Reviewed-by" would be easy.
>
> > ---
> >  test/region-test.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/test/region-test.c b/test/region-test.c
> > index bfc219b..e5743e8 100644
> > --- a/test/region-test.c
> > +++ b/test/region-test.c
> > @@ -116,10 +116,19 @@ main ()
> >
> >         assert (pixman_region32_equal (&r1, &r2));
> >         pixman_region32_fini (&r1);
> >         pixman_region32_fini (&r2);
> >
> >      }
> >      pixman_image_unref (fill);
> >
> > +    /* This would produce a region containing an empty
> > +     * rectangle in it. Such regions are considered malformed,
> > +     * but using an empty rectangle for initialization should
> > +     * work.
> > +     */
>
> Do I understand it right that you are trying to describe the
> implementation details of the current bug in this comment? But
> after we fix the bug, this description would instantly become
> obsolete and will just confuse the people reading the code.
>
> I think that just using the commit message here would make a
> perfect comment:
>
> "This test ensures that calling _intersect_rect on an empty
> rectangle produces an empty region."
>
> > +    pixman_region32_init_rects (&r1, boxes, 3);
> > +    pixman_region32_intersect_rect (&r1, &r1, 11, 11, 0, 0);
> > +    assert (!pixman_region32_not_empty(&r1));
>
> This is not too critical, but having "pixman_region32_fini (&r1);"
> here would be nice.
>
> > +
> >      return 0;
> >  }
>
> You can send a v2 patch (trying to use "git send-email" this time).
>
> Or if you prefer, I can push this patch with the above mentioned
> adjustments.
>
> --
> Best regards,
> Siarhei Siamashka
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/pixman/attachments/20160204/d5ea598e/attachment.html>