Message ID | 20240822073852.562286-3-jfalempe@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm/panic: Add a QR code panic screen | expand |
Am 22.08.24 um 09:33 schrieb Jocelyn Falempe: > 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. > > Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> There's an optional comment further below. > --- > > v7: > * rename r1/r2 to a/b in drm_rect_overlap() (Jani Nikula) > > 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..46f09cf68458c 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 > + * @a: first rectangle > + * @b: second rectangle > + * > + * RETURNS: > + * %true if the rectangles overlap, %false otherwise. > + */ > +static inline bool drm_rect_overlap(const struct drm_rect *a, > + const struct drm_rect *b) > +{ > + return (a->x2 > b->x1 && b->x2 > a->x1 && > + a->y2 > b->y1 && b->y2 > a->y1); I found this hard to understand. You may want to use the existing _intersect helper bool overlap(a, b) { struct drm_rect tmp = *a return intersect(tmp, b); } Up to you. Best regards Thomas > +} > + > 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);
On 22/08/2024 14:58, Thomas Zimmermann wrote: > > > Am 22.08.24 um 09:33 schrieb Jocelyn Falempe: >> 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. >> >> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> > > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> > > There's an optional comment further below. > >> --- >> >> v7: >> * rename r1/r2 to a/b in drm_rect_overlap() (Jani Nikula) >> >> 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..46f09cf68458c 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 >> + * @a: first rectangle >> + * @b: second rectangle >> + * >> + * RETURNS: >> + * %true if the rectangles overlap, %false otherwise. >> + */ >> +static inline bool drm_rect_overlap(const struct drm_rect *a, >> + const struct drm_rect *b) >> +{ >> + return (a->x2 > b->x1 && b->x2 > a->x1 && >> + a->y2 > b->y1 && b->y2 > a->y1); > > I found this hard to understand. You may want to use the existing > _intersect helper > > bool overlap(a, b) > { > struct drm_rect tmp = *a > > return intersect(tmp, b); > } I considered this, but it creates an unused rect, and compute the intersection rectangle, which we are not interested in. Even if the compiler may optimize and throw all this away, I prefer the more straightforward version. Thanks for the review,
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..46f09cf68458c 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 + * @a: first rectangle + * @b: second rectangle + * + * RETURNS: + * %true if the rectangles overlap, %false otherwise. + */ +static inline bool drm_rect_overlap(const struct drm_rect *a, + const struct drm_rect *b) +{ + return (a->x2 > b->x1 && b->x2 > a->x1 && + a->y2 > b->y1 && b->y2 > a->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);
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. Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> --- v7: * rename r1/r2 to a/b in drm_rect_overlap() (Jani Nikula) drivers/gpu/drm/drm_panic.c | 3 +-- include/drm/drm_rect.h | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-)