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

[Siarhei Siamashka]

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