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. :) 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
