diff mbox series

[v7,2/4] drm/rect: Add drm_rect_overlap()

Message ID 20240822073852.562286-3-jfalempe@redhat.com (mailing list archive)
State New, archived
Headers show
Series drm/panic: Add a QR code panic screen | expand

Commit Message

Jocelyn Falempe Aug. 22, 2024, 7:33 a.m. UTC
Check if two rectangles overlap.
It's a bit similar to drm_rect_intersect() but this won't modify
the rectangle.
Simplifies a bit drm_panic.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---

v7:
 * rename r1/r2 to a/b in drm_rect_overlap() (Jani Nikula)

 drivers/gpu/drm/drm_panic.c |  3 +--
 include/drm/drm_rect.h      | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

Comments

Thomas Zimmermann Aug. 22, 2024, 12:58 p.m. UTC | #1
Am 22.08.24 um 09:33 schrieb Jocelyn Falempe:
> Check if two rectangles overlap.
> It's a bit similar to drm_rect_intersect() but this won't modify
> the rectangle.
> Simplifies a bit drm_panic.
>
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

There's an optional comment further below.

> ---
>
> v7:
>   * rename r1/r2 to a/b in drm_rect_overlap() (Jani Nikula)
>
>   drivers/gpu/drm/drm_panic.c |  3 +--
>   include/drm/drm_rect.h      | 15 +++++++++++++++
>   2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
> index 0a047152f88b8..59fba23e5fd7a 100644
> --- a/drivers/gpu/drm/drm_panic.c
> +++ b/drivers/gpu/drm/drm_panic.c
> @@ -529,8 +529,7 @@ static void draw_panic_static_user(struct drm_scanout_buffer *sb)
>   	/* Fill with the background color, and draw text on top */
>   	drm_panic_fill(sb, &r_screen, bg_color);
>   
> -	if ((r_msg.x1 >= logo_width || r_msg.y1 >= logo_height) &&
> -	    logo_width <= sb->width && logo_height <= sb->height) {
> +	if (!drm_rect_overlap(&r_logo, &r_msg)) {
>   		if (logo_mono)
>   			drm_panic_blit(sb, &r_logo, logo_mono->data, DIV_ROUND_UP(logo_width, 8),
>   				       fg_color);
> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
> index 73fcb899a01da..46f09cf68458c 100644
> --- a/include/drm/drm_rect.h
> +++ b/include/drm/drm_rect.h
> @@ -238,6 +238,21 @@ static inline void drm_rect_fp_to_int(struct drm_rect *dst,
>   		      drm_rect_height(src) >> 16);
>   }
>   
> +/**
> + * drm_rect_overlap - Check if two rectangles overlap
> + * @a: first rectangle
> + * @b: second rectangle
> + *
> + * RETURNS:
> + * %true if the rectangles overlap, %false otherwise.
> + */
> +static inline bool drm_rect_overlap(const struct drm_rect *a,
> +				    const struct drm_rect *b)
> +{
> +	return (a->x2 > b->x1 && b->x2 > a->x1 &&
> +		a->y2 > b->y1 && b->y2 > a->y1);

I found this hard to understand. You may want to use the existing 
_intersect helper

bool overlap(a, b)
{
   struct drm_rect tmp = *a

   return intersect(tmp, b);
}

Up to you.

Best regards
Thomas

> +}
> +
>   bool drm_rect_intersect(struct drm_rect *r, const struct drm_rect *clip);
>   bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
>   			  const struct drm_rect *clip);
Jocelyn Falempe Aug. 22, 2024, 1:12 p.m. UTC | #2
On 22/08/2024 14:58, Thomas Zimmermann wrote:
> 
> 
> Am 22.08.24 um 09:33 schrieb Jocelyn Falempe:
>> Check if two rectangles overlap.
>> It's a bit similar to drm_rect_intersect() but this won't modify
>> the rectangle.
>> Simplifies a bit drm_panic.
>>
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> 
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> There's an optional comment further below.
> 
>> ---
>>
>> v7:
>>   * rename r1/r2 to a/b in drm_rect_overlap() (Jani Nikula)
>>
>>   drivers/gpu/drm/drm_panic.c |  3 +--
>>   include/drm/drm_rect.h      | 15 +++++++++++++++
>>   2 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
>> index 0a047152f88b8..59fba23e5fd7a 100644
>> --- a/drivers/gpu/drm/drm_panic.c
>> +++ b/drivers/gpu/drm/drm_panic.c
>> @@ -529,8 +529,7 @@ static void draw_panic_static_user(struct 
>> drm_scanout_buffer *sb)
>>       /* Fill with the background color, and draw text on top */
>>       drm_panic_fill(sb, &r_screen, bg_color);
>> -    if ((r_msg.x1 >= logo_width || r_msg.y1 >= logo_height) &&
>> -        logo_width <= sb->width && logo_height <= sb->height) {
>> +    if (!drm_rect_overlap(&r_logo, &r_msg)) {
>>           if (logo_mono)
>>               drm_panic_blit(sb, &r_logo, logo_mono->data, 
>> DIV_ROUND_UP(logo_width, 8),
>>                          fg_color);
>> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
>> index 73fcb899a01da..46f09cf68458c 100644
>> --- a/include/drm/drm_rect.h
>> +++ b/include/drm/drm_rect.h
>> @@ -238,6 +238,21 @@ static inline void drm_rect_fp_to_int(struct 
>> drm_rect *dst,
>>                 drm_rect_height(src) >> 16);
>>   }
>> +/**
>> + * drm_rect_overlap - Check if two rectangles overlap
>> + * @a: first rectangle
>> + * @b: second rectangle
>> + *
>> + * RETURNS:
>> + * %true if the rectangles overlap, %false otherwise.
>> + */
>> +static inline bool drm_rect_overlap(const struct drm_rect *a,
>> +                    const struct drm_rect *b)
>> +{
>> +    return (a->x2 > b->x1 && b->x2 > a->x1 &&
>> +        a->y2 > b->y1 && b->y2 > a->y1);
> 
> I found this hard to understand. You may want to use the existing 
> _intersect helper
> 
> bool overlap(a, b)
> {
>    struct drm_rect tmp = *a
> 
>    return intersect(tmp, b);
> }

I considered this, but it creates an unused rect, and compute the 
intersection rectangle, which we are not interested in.
Even if the compiler may optimize and throw all this away, I prefer the 
more straightforward version.

Thanks for the review,
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 0a047152f88b8..59fba23e5fd7a 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -529,8 +529,7 @@  static void draw_panic_static_user(struct drm_scanout_buffer *sb)
 	/* Fill with the background color, and draw text on top */
 	drm_panic_fill(sb, &r_screen, bg_color);
 
-	if ((r_msg.x1 >= logo_width || r_msg.y1 >= logo_height) &&
-	    logo_width <= sb->width && logo_height <= sb->height) {
+	if (!drm_rect_overlap(&r_logo, &r_msg)) {
 		if (logo_mono)
 			drm_panic_blit(sb, &r_logo, logo_mono->data, DIV_ROUND_UP(logo_width, 8),
 				       fg_color);
diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
index 73fcb899a01da..46f09cf68458c 100644
--- a/include/drm/drm_rect.h
+++ b/include/drm/drm_rect.h
@@ -238,6 +238,21 @@  static inline void drm_rect_fp_to_int(struct drm_rect *dst,
 		      drm_rect_height(src) >> 16);
 }
 
+/**
+ * drm_rect_overlap - Check if two rectangles overlap
+ * @a: first rectangle
+ * @b: second rectangle
+ *
+ * RETURNS:
+ * %true if the rectangles overlap, %false otherwise.
+ */
+static inline bool drm_rect_overlap(const struct drm_rect *a,
+				    const struct drm_rect *b)
+{
+	return (a->x2 > b->x1 && b->x2 > a->x1 &&
+		a->y2 > b->y1 && b->y2 > a->y1);
+}
+
 bool drm_rect_intersect(struct drm_rect *r, const struct drm_rect *clip);
 bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
 			  const struct drm_rect *clip);