[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87frr924nj.fsf@intel.com>
Date: Mon, 12 Aug 2024 17:32:16 +0300
From: Jani Nikula <jani.nikula@...ux.intel.com>
To: Jocelyn Falempe <jfalempe@...hat.com>, Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, 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 v6 2/4] drm/rect: Add drm_rect_overlap()
On Mon, 12 Aug 2024, Jocelyn Falempe <jfalempe@...hat.com> wrote:
> On 12/08/2024 15:49, Jani Nikula wrote:
>> On Mon, 12 Aug 2024, Jocelyn Falempe <jfalempe@...hat.com> wrote:
>>> 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.
>>
>> Based on the name, I'd expect drm_rect_overlap() to return true for
>> *any* overlap, while this one seems to mean if one rectangle is
>> completely within another, with no adjacent borders.
>
> It's what I intended, but I may have messed up the formula.
Hmm, then I may have messed up the review. :)
Gotta run now, but I'll get back.
BR,
Jani.
>>
>> I'd expect a drm_rect_overlap() to return true for this:
>>
>> +-------+
>> | +---+---+
>> | | |
>> +---+ |
>> | |
>> +-------+
>
> if r1 is the top left rectangle, you've got:
>
> r1->x2 > r2->x1 => true
> r2->x2 > r1->x1 => true
> r1->y2 > r2->y1 => true
> r2->y2 > r1->y1 => true
>
> So they count as overlap.
>
> Checking in stackoverflow, they use the same formula:
> https://stackoverflow.com/questions/306316/determine-if-two-rectangles-overlap-each-other
>
>>
>> While this seems to be required instead:
>>
>> +-------+
>> | +---+ |
>> | | | |
>> | +---+ |
>> +-------+
>>
>>
>> IOW, I find the name misleading.
>>
>> BR,
>> Jani.
>>
>>
>>>
>>> Signed-off-by: Jocelyn Falempe <jfalempe@...hat.com>
>>> ---
>>> 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..7bafde747d560 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
>>> + * @r1: first rectangle
>>> + * @r2: second rectangle
>>> + *
>>> + * RETURNS:
>>> + * %true if the rectangles overlap, %false otherwise.
>>> + */
>>> +static inline bool drm_rect_overlap(const struct drm_rect *r1,
>>> + const struct drm_rect *r2)
>>> +{
>>> + return (r1->x2 > r2->x1 && r2->x2 > r1->x1 &&
>>> + r1->y2 > r2->y1 && r2->y2 > r1->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);
>>
>
--
Jani Nikula, Intel
Powered by blists - more mailing lists