Re: [Rapp-dev] Please also look at 7b42, fixing binary morph padding

[Johan Almbladh]

It looks to me that the original code for rectangular SEs actually
allowed different paddings in horizontal and vertial directions. Maybe
we should consider allowing it?

I also see that the internal driver for what is called isotropic SEs
(rapp_morph_isotropic) also checks its arguments, because it is called
directly from disc, octagon and diamond functions. However, it is also
called from the rectangular function for 1xm and nx1 SEs after
validation. If the argument validation becomes heavier we might want
to do that only once.

/Johan


On Mon, Dec 13, 2010 at 5:23 AM, Hans-Peter Nilsson
<address@hidden> wrote:
>
> As discussed, the overlap checks as well as the recent
> documentation adjustment required one more pixel padding than
> necessary.  Also, the overlap-check for rectangular SE's didn't
> check what the documentation mandated.  Usually
> misunderstandings like this is a sign that the documentation
> needed clarification, so done.  Please have a look.
>
> commit 7b42332c43bbab521408770faffe04b500e7094b
> Author: Hans-Peter Nilsson <address@hidden>
> Date:   Mon Dec 13 05:02:58 2010 +0100
>
>    Fix rectangular binary morphology overlap padding check and revert 
> previous change. Clarify docs.
>
>        * driver/rapp_morph_bin.c (rapp_morph_erode_rect_bin)
>        (rapp_morph_dilate_rect_bin): Adjust to documentation; when
>        checking for buffer overlap, actually check that all padding is
>        according to half the *max* side of the SE, and rounding
>        downwards, not upwards.
>        (rapp_morph_isotropic): Similarly, check overlap for padding to
>        radius minus one, not radius.
>        * include/rapp_morph_bin.h (Padding): Revert previous change.
>        Clarify bounds of padding for both cases, isotropic and
>        rectangular elements.
>        * test/rapp_test_morph_bin.c (rapp_test_rectangle_driver): Adjust
>        accordingly.  Tighten up, use only the necessary padding.
>        (rapp_test_isotropic_driver): Ditto.
>
>    Mention version and refer to project URL in doc and dev-doc.
>    Read URL from README.  Rerun autogen.sh.
>
> diff --git a/driver/rapp_morph_bin.c b/driver/rapp_morph_bin.c
> index 0f7db43..ea90f87 100644
> --- a/driver/rapp_morph_bin.c
> +++ b/driver/rapp_morph_bin.c
> @@ -428,8 +428,9 @@ RAPP_API(int, rapp_morph_erode_rect_bin,
>           int width, int height, int wrect, int hrect,
>           void *restrict work))
>  {
> -    int minxpadding = rc_align((MIN((wrect + 1) / 2, 16) + 7) / 8);
> -    int minypadding = MIN((hrect + 1) / 2, 16);
> +    int maxside     = MAX(wrect, hrect);
> +    int minxpadding = rc_align((MIN(maxside / 2, 16) + 7) / 8);
> +    int minypadding = MIN(maxside / 2, 16);
>
>     if (!RAPP_INITIALIZED()) {
>         return RAPP_ERR_UNINITIALIZED;
> @@ -526,8 +527,9 @@ RAPP_API(int, rapp_morph_dilate_rect_bin,
>           int width, int height, int wrect, int hrect,
>           void *restrict work))
>  {
> -    int minxpadding = rc_align((MIN((wrect + 1) / 2, 16) + 7) / 8);
> -    int minypadding = MIN((hrect + 1) / 2, 16);
> +    int maxside     = MAX(wrect, hrect);
> +    int minxpadding = rc_align((MIN(maxside / 2, 16) + 7) / 8);
> +    int minypadding = MIN(maxside / 2, 16);
>
>     if (!RAPP_INITIALIZED()) {
>         return RAPP_ERR_UNINITIALIZED;
> @@ -746,8 +748,8 @@ rapp_morph_isotropic(uint8_t *dst, int dst_dim,
>     uint8_t *tmp1;
>     uint8_t *tmp2;
>     int      dim;
> -    int      minxpadding = rc_align((MIN(radius, 16) + 7) / 8);
> -    int      minypadding = MIN(radius, 16);
> +    int      minxpadding = rc_align((MIN(radius - 1, 16) + 7) / 8);
> +    int      minypadding = MIN(radius - 1, 16);
>
>     if (!RAPP_INITIALIZED()) {
>         return RAPP_ERR_UNINITIALIZED;
> diff --git a/include/rapp_morph_bin.h b/include/rapp_morph_bin.h
> index 8263ffb..3fa3962 100644
> --- a/include/rapp_morph_bin.h
> +++ b/include/rapp_morph_bin.h
> @@ -70,10 +70,14 @@
>  *
>  * address@hidden Padding
>  *  The user is responsible for @ref padding "padding" the source buffer.
> - *  The padding needed is bounded by min((size + 1) / 2, 16), where size
> - *  is the maximum size of the structuring element in the horizontal
> - *  and vertical directions. If the source buffer is padded with
> - *  values other than all-zeros or all-ones the behaviour is undefined.
> + *  The padding needed is the same both in horizontal and vertical
> + *  directions. For rectangular structuring elements, it is bounded by
> + *  min(maxside / 2, 16), where maxside is the maximum size of the
> + *  structuring element in the horizontal and vertical directions, and
> + *  maxside / 2 is rounded <em>down</em> to the nearest integer. For
> + *  other objects, the padding is bounded by min(radius - 1, 16). If the
> + *  source buffer is padded with values other than all-zeros or all-ones
> + *  the behaviour is undefined.
>  *
>  *  <p>@ref padding "Next section: Border Padding"</p>
>  *
> diff --git a/test/rapp_test_morph_bin.c b/test/rapp_test_morph_bin.c
> index 241032a..da6f7f8 100644
> --- a/test/rapp_test_morph_bin.c
> +++ b/test/rapp_test_morph_bin.c
> @@ -386,16 +386,18 @@ rapp_test_rectangle_driver(int (*morph)(), int width,
>     int      dst_dim;         /* Destination buffer dimension  */
>     int      pad_len;         /* Padded source buffer in bytes */
>     int      size    = MIN(width, height);
> +    int      minpixpad = MIN(MAX(width, height) / 2, 16);
> +    int      minxpad = rapp_align((minpixpad + 7) / 8);
>     int      pos;
>     bool     ok = false;
>
>     /* Allocate the buffers */
>     dst_dim = rapp_align((width + 7) / 8);
> -    src_dim = dst_dim + 2*rapp_alignment;
> +    src_dim = dst_dim + 2*minxpad;
>     pat_buf = rapp_malloc(height*dst_dim, 0);
> -    pad_len = (height + 2*16)*src_dim;
> +    pad_len = (height + 2*minpixpad)*src_dim;
>     pad_buf = rapp_malloc(pad_len, 0);
> -    src_buf = &pad_buf[16*src_dim + rapp_alignment];
> +    src_buf = &pad_buf[minpixpad*src_dim + minxpad];
>     dst_buf = rapp_malloc(height*dst_dim, 0);
>     ref_buf = rapp_malloc(height*dst_dim, 0);
>     work    = rapp_malloc(rapp_morph_worksize_bin(width, height), 0);
> @@ -410,13 +412,11 @@ rapp_test_rectangle_driver(int (*morph)(), int width,
>                  width, height, work) != RAPP_ERR_OVERLAP
>         /* src = far end of dst */
>         || (*morph)(dst_buf, dst_dim,
> -                    dst_buf + dst_dim*(height - 1) +
> -                    rapp_align((width + 7) / 8) - rapp_alignment, src_dim,
> +                    dst_buf + dst_dim*height + minxpad - rapp_alignment, 
> src_dim,
>                     width, height, width, height, work) != RAPP_ERR_OVERLAP
>         /* src = before dst, but not long enough */
>         || (*morph)(dst_buf, dst_dim,
> -                    dst_buf - (dst_dim*(height - 1) +
> -                               rapp_align((width + 7) / 8) - rapp_alignment),
> +                    dst_buf - (dst_dim*height + minxpad - rapp_alignment),
>                     src_dim,
>                     width, height, width, height, work) != RAPP_ERR_OVERLAP
>         /* dst = work */
> @@ -519,6 +519,8 @@ rapp_test_isotropic_driver(int (*morph)(), int radius, 
> int area, bool dilate)
>     int      pad_len;         /* Padded source buffer in bytes */
>     int      pos;
>     int      size    = 2*radius - 1;
> +    int      minpixpad = MIN(radius - 1, 16);
> +    int      minxpad = rapp_align((minpixpad + 7) / 8);
>     bool     ok      = false;
>
>     /* Get the SE pattern image */
> @@ -531,10 +533,10 @@ rapp_test_isotropic_driver(int (*morph)(), int radius, 
> int area, bool dilate)
>     }
>
>     /* Allocate the buffers */
> -    src_dim = dst_dim + 2*rapp_alignment;
> -    pad_len = (size + 2*16)*src_dim;
> +    src_dim = dst_dim + 2*minxpad;
> +    pad_len = (size + 2*minpixpad)*src_dim;
>     pad_buf = rapp_malloc(pad_len, 0);
> -    src_buf = &pad_buf[16*src_dim + rapp_alignment];
> +    src_buf = &pad_buf[minpixpad*src_dim + minxpad];
>     dst_buf = rapp_malloc(size*dst_dim, 0);
>     ref_buf = rapp_malloc(size*dst_dim, 0);
>     work    = rapp_malloc(rapp_morph_worksize_bin(size, size), 0);
> @@ -545,13 +547,11 @@ rapp_test_isotropic_driver(int (*morph)(), int radius, 
> int area, bool dilate)
>                  work) != RAPP_ERR_OVERLAP
>         /* src = far end of dst */
>         || (*morph)(dst_buf, dst_dim,
> -                    dst_buf + dst_dim*(size - 1) +
> -                    rapp_align((size + 7) / 8) - rapp_alignment, src_dim,
> +                    dst_buf + dst_dim*size + minxpad - rapp_alignment, 
> src_dim,
>                     size, size, radius, work) != RAPP_ERR_OVERLAP
>         /* src = before dst, but not long enough */
>         || (*morph)(dst_buf, dst_dim,
> -                    dst_buf - (dst_dim*(size - 1) +
> -                               rapp_align((size + 7) / 8) - rapp_alignment),
> +                    dst_buf - (dst_dim*size + minxpad - rapp_alignment),
>                     src_dim,
>                     size, size, radius, work) != RAPP_ERR_OVERLAP
>         /* dst = work */
>
> brgds, H-P
>