On Mon, 12 Aug 2024, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
> On Mon, 12 Aug 2024, Jocelyn Falempe <jfalempe@xxxxxxxxxx> wrote:
>> On 12/08/2024 15:49, Jani Nikula wrote:
>>> On Mon, 12 Aug 2024, Jocelyn Falempe <jfalempe@xxxxxxxxxx> 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. :)
Yeah, my bad, sorry for the noise.
I think I was thrown off by the comparisons mixing r1 and r2 as the
first operand. Something like this might have been easier for *me* to
parse, but not sure if it's worth changing anything:
return (a->x1 < b->x2 && a->x2 > b->x1 &&
a->y1 < b->y2 && a->y2 > b->y1);
BR,
Jani.
>
> 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@xxxxxxxxxx>
>>>> ---
>>>> 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
