[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d5de80ed-adc2-4307-bb57-27fc4e611100@redhat.com>
Date: Thu, 22 Aug 2024 15:12:08 +0200
From: Jocelyn Falempe <jfalempe@...hat.com>
To: Thomas Zimmermann <tzimmermann@...e.de>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>, Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>,
Wedson Almeida Filho <wedsonaf@...il.com>, Boqun Feng
<boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Bjorn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
Andreas Hindborg <a.hindborg@...sung.com>, Alice Ryhl
<aliceryhl@...gle.com>, linux-kernel@...r.kernel.org,
dri-devel@...ts.freedesktop.org, rust-for-linux@...r.kernel.org,
Danilo Krummrich <dakr@...hat.com>
Subject: Re: [PATCH v7 2/4] drm/rect: Add drm_rect_overlap()
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@...hat.com>
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@...e.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,
--
Jocelyn
>
> 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);
>
Powered by blists - more mailing lists