Re: [PATCH v2 3/4] drm/rect: Keep the clipped dst rectangle in place

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Nov 22, 2019 at 07:56:22PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Now that we've constrained the clipped source rectangle such
> that it can't have negative dimensions doing the same for the
> dst rectangle seems appropriate. Should at least result in
> the clipped src and dst rectangles being a bit more consistent
> with each other.
> 
> Cc: Benjamin Gaignard <benjamin.gaignard@st.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_rect.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
> index a9c7f90836f3..1e1e2101007a 100644
> --- a/drivers/gpu/drm/drm_rect.c
> +++ b/drivers/gpu/drm/drm_rect.c
> @@ -52,7 +52,7 @@ bool drm_rect_intersect(struct drm_rect *r1, const struct drm_rect *r2)
>  }
>  EXPORT_SYMBOL(drm_rect_intersect);
>  
> -static u32 clip_scaled(u32 src, u32 dst, u32 clip)
> +static u32 clip_scaled(int src, int dst, int *clip)

Mild scare about this one here, but I think we've clamped everything
sufficiently now that silly stuff can't happen anymore. And everything in
here seems to cast to sufficiently big types (and we shouldn't have
negative values here I hope). I'll see how realistic that assumption is
when looking at the selftests. For this:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  {
>  	u64 tmp;
>  
> @@ -60,9 +60,9 @@ static u32 clip_scaled(u32 src, u32 dst, u32 clip)
>  		return 0;
>  
>  	/* Only clip what we have. Keeps the result bounded. */
> -	clip = min(clip, dst);
> +	*clip = min(*clip, dst);
>  
> -	tmp = mul_u32_u32(src, dst - clip);
> +	tmp = mul_u32_u32(src, dst - *clip);
>  
>  	/*
>  	 * Round toward 1.0 when clipping so that we don't accidentally
> @@ -95,34 +95,34 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>  	diff = clip->x1 - dst->x1;
>  	if (diff > 0) {
>  		u32 new_src_w = clip_scaled(drm_rect_width(src),
> -					    drm_rect_width(dst), diff);
> +					    drm_rect_width(dst), &diff);
>  
>  		src->x1 = src->x2 - new_src_w;
> -		dst->x1 = clip->x1;
> +		dst->x1 += diff;
>  	}
>  	diff = clip->y1 - dst->y1;
>  	if (diff > 0) {
>  		u32 new_src_h = clip_scaled(drm_rect_height(src),
> -					    drm_rect_height(dst), diff);
> +					    drm_rect_height(dst), &diff);
>  
>  		src->y1 = src->y2 - new_src_h;
> -		dst->y1 = clip->y1;
> +		dst->y1 += diff;
>  	}
>  	diff = dst->x2 - clip->x2;
>  	if (diff > 0) {
>  		u32 new_src_w = clip_scaled(drm_rect_width(src),
> -					    drm_rect_width(dst), diff);
> +					    drm_rect_width(dst), &diff);
>  
>  		src->x2 = src->x1 + new_src_w;
> -		dst->x2 = clip->x2;
> +		dst->x2 -= diff;
>  	}
>  	diff = dst->y2 - clip->y2;
>  	if (diff > 0) {
>  		u32 new_src_h = clip_scaled(drm_rect_height(src),
> -					    drm_rect_height(dst), diff);
> +					    drm_rect_height(dst), &diff);
>  
>  		src->y2 = src->y1 + new_src_h;
> -		dst->y2 = clip->y2;
> +		dst->y2 -= diff;
>  	}
>  
>  	return drm_rect_visible(dst);
> -- 
> 2.23.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel