Re: [PATCH v6 2/4] drm/rect: Add drm_rect_overlap()

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 12 Aug 2024, Jocelyn Falempe <jfalempe@redhat.com> wrote:
> 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.

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@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);
>> 
>

-- 
Jani Nikula, Intel