Add a helper macro to initialize a rectangle from x, y, width and
height information.
Reviewed-by: Jani Nikula <[email protected]>
Acked-by: Thomas Zimmermann <[email protected]>
Signed-off-by: José Expósito <[email protected]>
---
include/drm/drm_rect.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
index 6f6e19bd4dac..e8d94fca2703 100644
--- a/include/drm/drm_rect.h
+++ b/include/drm/drm_rect.h
@@ -47,6 +47,22 @@ struct drm_rect {
int x1, y1, x2, y2;
};
+/**
+ * DRM_RECT_INIT - initialize a rectangle from x/y/w/h
+ * @x: x coordinate
+ * @y: y coordinate
+ * @w: width
+ * @h: height
+ *
+ * RETURNS:
+ * A new rectangle of the specified size.
+ */
+#define DRM_RECT_INIT(x, y, w, h) ((struct drm_rect){ \
+ .x1 = (x), \
+ .y1 = (y), \
+ .x2 = (x) + (w), \
+ .y2 = (y) + (h) })
+
/**
* DRM_RECT_FMT - printf string for &struct drm_rect
*/
--
2.25.1
On Tue, Jun 21, 2022 at 12:06 AM José Expósito
<[email protected]> wrote:
>
> Add a helper macro to initialize a rectangle from x, y, width and
> height information.
>
> Reviewed-by: Jani Nikula <[email protected]>
> Acked-by: Thomas Zimmermann <[email protected]>
> Signed-off-by: José Expósito <[email protected]>
> ---
This looks good to me, though I have one minor concern about the macro
name. (But if it's okay with the DRM folks, which it seems to be, I
won't object.)
Either way,
Reviewed-by: David Gow <[email protected]>
> include/drm/drm_rect.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
> index 6f6e19bd4dac..e8d94fca2703 100644
> --- a/include/drm/drm_rect.h
> +++ b/include/drm/drm_rect.h
> @@ -47,6 +47,22 @@ struct drm_rect {
> int x1, y1, x2, y2;
> };
>
> +/**
> + * DRM_RECT_INIT - initialize a rectangle from x/y/w/h
> + * @x: x coordinate
> + * @y: y coordinate
> + * @w: width
> + * @h: height
> + *
> + * RETURNS:
> + * A new rectangle of the specified size.
> + */
> +#define DRM_RECT_INIT(x, y, w, h) ((struct drm_rect){ \
> + .x1 = (x), \
> + .y1 = (y), \
> + .x2 = (x) + (w), \
> + .y2 = (y) + (h) })
> +
My only slight concern here is that it might be a little bit confusing
that a macro called DRM_RECT_INIT() accepts x/y/w/h, whereas the
actual struct drm_rect is x1/y1/x2/y2. If the macro were called
something like DRM_RECT_INIT_FROM_XYWH() or similar.
> /**
> * DRM_RECT_FMT - printf string for &struct drm_rect
> */
> --
> 2.25.1
>
Hi
Am 21.06.22 um 11:38 schrieb David Gow:
> On Tue, Jun 21, 2022 at 12:06 AM José Expósito
> <[email protected]> wrote:
>>
>> Add a helper macro to initialize a rectangle from x, y, width and
>> height information.
>>
>> Reviewed-by: Jani Nikula <[email protected]>
>> Acked-by: Thomas Zimmermann <[email protected]>
>> Signed-off-by: José Expósito <[email protected]>
>> ---
>
> This looks good to me, though I have one minor concern about the macro
> name. (But if it's okay with the DRM folks, which it seems to be, I
> won't object.)
>
> Either way,
> Reviewed-by: David Gow <[email protected]>
>
>> include/drm/drm_rect.h | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
>> index 6f6e19bd4dac..e8d94fca2703 100644
>> --- a/include/drm/drm_rect.h
>> +++ b/include/drm/drm_rect.h
>> @@ -47,6 +47,22 @@ struct drm_rect {
>> int x1, y1, x2, y2;
>> };
>>
>> +/**
>> + * DRM_RECT_INIT - initialize a rectangle from x/y/w/h
>> + * @x: x coordinate
>> + * @y: y coordinate
>> + * @w: width
>> + * @h: height
>> + *
>> + * RETURNS:
>> + * A new rectangle of the specified size.
>> + */
>> +#define DRM_RECT_INIT(x, y, w, h) ((struct drm_rect){ \
>> + .x1 = (x), \
>> + .y1 = (y), \
>> + .x2 = (x) + (w), \
>> + .y2 = (y) + (h) })
>> +
>
> My only slight concern here is that it might be a little bit confusing
> that a macro called DRM_RECT_INIT() accepts x/y/w/h, whereas the
> actual struct drm_rect is x1/y1/x2/y2. If the macro were called
> something like DRM_RECT_INIT_FROM_XYWH() or similar.
The existing drm_rect_init() function uses xywh arguments. So the
current name is consistent with existing practice. I don't think we
refer to x2,y2 much, if ever.
Best regards
Thomas
>
>
>> /**
>> * DRM_RECT_FMT - printf string for &struct drm_rect
>> */
>> --
>> 2.25.1
>>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
On Tue, 21 Jun 2022, Thomas Zimmermann <[email protected]> wrote:
> Hi
>
> Am 21.06.22 um 11:38 schrieb David Gow:
>> On Tue, Jun 21, 2022 at 12:06 AM José Expósito
>> <[email protected]> wrote:
>>>
>>> Add a helper macro to initialize a rectangle from x, y, width and
>>> height information.
>>>
>>> Reviewed-by: Jani Nikula <[email protected]>
>>> Acked-by: Thomas Zimmermann <[email protected]>
>>> Signed-off-by: José Expósito <[email protected]>
>>> ---
>>
>> This looks good to me, though I have one minor concern about the macro
>> name. (But if it's okay with the DRM folks, which it seems to be, I
>> won't object.)
>>
>> Either way,
>> Reviewed-by: David Gow <[email protected]>
>>
>>> include/drm/drm_rect.h | 16 ++++++++++++++++
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
>>> index 6f6e19bd4dac..e8d94fca2703 100644
>>> --- a/include/drm/drm_rect.h
>>> +++ b/include/drm/drm_rect.h
>>> @@ -47,6 +47,22 @@ struct drm_rect {
>>> int x1, y1, x2, y2;
>>> };
>>>
>>> +/**
>>> + * DRM_RECT_INIT - initialize a rectangle from x/y/w/h
>>> + * @x: x coordinate
>>> + * @y: y coordinate
>>> + * @w: width
>>> + * @h: height
>>> + *
>>> + * RETURNS:
>>> + * A new rectangle of the specified size.
>>> + */
>>> +#define DRM_RECT_INIT(x, y, w, h) ((struct drm_rect){ \
>>> + .x1 = (x), \
>>> + .y1 = (y), \
>>> + .x2 = (x) + (w), \
>>> + .y2 = (y) + (h) })
>>> +
>>
>> My only slight concern here is that it might be a little bit confusing
>> that a macro called DRM_RECT_INIT() accepts x/y/w/h, whereas the
>> actual struct drm_rect is x1/y1/x2/y2. If the macro were called
>> something like DRM_RECT_INIT_FROM_XYWH() or similar.
>
> The existing drm_rect_init() function uses xywh arguments. So the
> current name is consistent with existing practice. I don't think we
> refer to x2,y2 much, if ever.
Agreed, and if we initialized with x1,y1,x2,y2 we wouldn't need the
function/macro in the first place.
BR,
Jani.
>
> Best regards
> Thomas
>
>>
>>
>>> /**
>>> * DRM_RECT_FMT - printf string for &struct drm_rect
>>> */
>>> --
>>> 2.25.1
>>>
--
Jani Nikula, Intel Open Source Graphics Center
On Tue, Jun 21, 2022 at 6:02 PM Thomas Zimmermann <[email protected]> wrote:
>
> Hi
>
> Am 21.06.22 um 11:38 schrieb David Gow:
> > On Tue, Jun 21, 2022 at 12:06 AM José Expósito
[ ... ]
> >> +/**
> >> + * DRM_RECT_INIT - initialize a rectangle from x/y/w/h
> >> + * @x: x coordinate
> >> + * @y: y coordinate
> >> + * @w: width
> >> + * @h: height
> >> + *
> >> + * RETURNS:
> >> + * A new rectangle of the specified size.
> >> + */
> >> +#define DRM_RECT_INIT(x, y, w, h) ((struct drm_rect){ \
> >> + .x1 = (x), \
> >> + .y1 = (y), \
> >> + .x2 = (x) + (w), \
> >> + .y2 = (y) + (h) })
> >> +
> >
> > My only slight concern here is that it might be a little bit confusing
> > that a macro called DRM_RECT_INIT() accepts x/y/w/h, whereas the
> > actual struct drm_rect is x1/y1/x2/y2. If the macro were called
> > something like DRM_RECT_INIT_FROM_XYWH() or similar.
>
> The existing drm_rect_init() function uses xywh arguments. So the
> current name is consistent with existing practice. I don't think we
> refer to x2,y2 much, if ever.
>
Ah, fair enough. I wasn't aware of the existing drm_rect_init()
function, so this makes sense as-is.
Cheers,
-- David