From: Jocelyn Falempe <jfalempe@redhat.com>
To: Jani Nikula <jani.nikula@linux.intel.com>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Maxime Ripard <mripard@kernel.org>,
Thomas Zimmermann <tzimmermann@suse.de>,
David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
Miguel Ojeda <ojeda@kernel.org>,
Alex Gaynor <alex.gaynor@gmail.com>,
Wedson Almeida Filho <wedsonaf@gmail.com>,
Boqun Feng <boqun.feng@gmail.com>, Gary Guo <gary@garyguo.net>,
Bjorn Roy Baron <bjorn3_gh@protonmail.com>,
Benno Lossin <benno.lossin@proton.me>,
Andreas Hindborg <a.hindborg@samsung.com>,
Alice Ryhl <aliceryhl@google.com>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
rust-for-linux@vger.kernel.org,
Danilo Krummrich <dakr@redhat.com>
Subject: Re: [PATCH v6 2/4] drm/rect: Add drm_rect_overlap()
Date: Mon, 12 Aug 2024 16:19:01 +0200 [thread overview]
Message-ID: <60e55a9d-70bb-45d1-ac97-e4f6f6ffa9a9@redhat.com> (raw)
In-Reply-To: <87sev926na.fsf@intel.com>
On 12/08/2024 15:49, Jani Nikula wrote:
> On Mon, 12 Aug 2024, Jocelyn Falempe <jfalempe@redhat.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.
>
> 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@redhat.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);
>
next prev parent reply other threads:[~2024-08-12 14:19 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-12 12:24 [PATCH v6 0/4] drm/panic: Add a QR code panic screen Jocelyn Falempe
2024-08-12 12:24 ` [PATCH v6 1/4] drm/panic: Add integer scaling to blit() Jocelyn Falempe
2024-08-12 12:24 ` [PATCH v6 2/4] drm/rect: Add drm_rect_overlap() Jocelyn Falempe
2024-08-12 13:49 ` Jani Nikula
2024-08-12 14:06 ` Hamza Mahfooz
2024-08-12 14:19 ` Jocelyn Falempe [this message]
2024-08-12 14:32 ` Jani Nikula
2024-08-13 14:11 ` Jani Nikula
2024-08-13 14:34 ` Jocelyn Falempe
2024-08-12 12:24 ` [PATCH v6 3/4] drm/panic: Simplify logo handling Jocelyn Falempe
2024-08-12 12:24 ` [PATCH v6 4/4] drm/panic: Add a QR code panic screen Jocelyn Falempe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=60e55a9d-70bb-45d1-ac97-e4f6f6ffa9a9@redhat.com \
--to=jfalempe@redhat.com \
--cc=a.hindborg@samsung.com \
--cc=airlied@gmail.com \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=benno.lossin@proton.me \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=dakr@redhat.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=gary@garyguo.net \
--cc=jani.nikula@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tzimmermann@suse.de \
--cc=wedsonaf@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).