[cairo] [PATCH 09/72] core: fixed code duplication

[Bryce Harrington]

On Tue, Dec 29, 2015 at 10:16:47AM +0100, Enrico Weigelt, metux IT consult wrote:
> We have two places where copying from box set to clip is
> implemented in the same way. Just move that to one function.
> 
> Signed-off-by: Enrico Weigelt, metux IT consult <enrico.weigelt at gr13.net>
> ---
>  src/cairo-clip-boxes.c | 50 ++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 34 insertions(+), 16 deletions(-)
> 
> diff --git a/src/cairo-clip-boxes.c b/src/cairo-clip-boxes.c
> index b1ff705..cad431e 100644
> --- a/src/cairo-clip-boxes.c
> +++ b/src/cairo-clip-boxes.c
> @@ -264,6 +264,35 @@ _cairo_clip_intersect_box (cairo_clip_t *clip,
>      return _cairo_clip_intersect_rectangle_box (clip, &r, box);
>  }
>  
> +/**
> + * copy a box set to an clip
> + *
> + * @param	box	the box set to copy from
> + * @param	clip	the clip to copy to (return buffer)
> + * @result		zero if the allocation failed - the clip will be set to all-clipped
> + *			otherwise non-zero
> + */
> +static cairo_bool_t
> +_cairo_boxes_copy_to_clip(const cairo_boxes_t *boxes, cairo_clip_t *clip)
> +{
> +    /* XXX cow-boxes? */
> +    if (boxes->num_boxes == 1) {
> +	clip->boxes = &clip->embedded_box;
> +	clip->boxes[0] = boxes->chunks.base[0];
> +	clip->num_boxes = 1;
> +	return 1;
> +    }
> +
> +    clip->boxes = _cairo_boxes_to_array(boxes, &clip->num_boxes);
> +    if (unlikely(clip->boxes == NULL))
> +    {
> +	_cairo_clip_set_all_clipped (clip);
> +	return 0;
> +    }
> +
> +    return 1;
> +}

Since this new routine is defined to return a bool, should probably use
bool values rather than 0/1 here.

>  cairo_clip_t *
>  _cairo_clip_intersect_boxes (cairo_clip_t *clip,
>  			     const cairo_boxes_t *boxes)
> @@ -301,13 +330,10 @@ _cairo_clip_intersect_boxes (cairo_clip_t *clip,
>      if (boxes->num_boxes == 0) {
>  	clip = _cairo_clip_set_all_clipped (clip);
>  	goto out;
> -    } else if (boxes->num_boxes == 1) {
> -	clip->boxes = &clip->embedded_box;
> -	clip->boxes[0] = boxes->chunks.base[0];
> -	clip->num_boxes = 1;
> -    } else {
> -	clip->boxes = _cairo_boxes_to_array (boxes, &clip->num_boxes);
>      }
> +
> +    _cairo_boxes_copy_to_clip(boxes, clip);

Since we're bothering to have this routine propagate an error code,
probably ought to go ahead and handle it here.

If we don't care about the error code, then that maybe suggests a
different design should be used.

> +
>      _cairo_boxes_extents (boxes, &limits);
>  
>      _cairo_box_round_to_rectangle (&limits, &extents);
> @@ -574,16 +600,8 @@ _cairo_clip_from_boxes (const cairo_boxes_t *boxes)
>      if (clip == NULL)
>  	return _cairo_clip_set_all_clipped (clip);
>  
> -    /* XXX cow-boxes? */
> -    if(boxes->num_boxes == 1) {
> -	clip->boxes = &clip->embedded_box;
> -	clip->boxes[0] = boxes->chunks.base[0];
> -	clip->num_boxes = 1;
> -    } else {
> -	clip->boxes = _cairo_boxes_to_array (boxes, &clip->num_boxes);
> -	if (clip->boxes == NULL)
> -	    return _cairo_clip_set_all_clipped (clip);
> -    }
> +    if (unlikely(!_cairo_boxes_copy_to_clip(boxes, clip)))
> +	return clip;
>  
>      _cairo_boxes_extents (boxes, &extents);
>      _cairo_box_round_to_rectangle (&extents, &clip->extents);
> -- 
> 2.6.4.442.g545299f
> 
> -- 
> cairo mailing list
> cairo at cairographics.org
> http://lists.cairographics.org/mailman/listinfo/cairo