Hi Ville,
Thanks for the patch.
On Wednesday 27 March 2013 17:46:22 ville.syrjala at linux.intel.com wrote:
> From: Ville Syrj?l? <ville.syrjala at linux.intel.com>
>
> struct drm_rect represents a simple rectangle. The utility
> functions are there to help driver writers.
>
> v2: Moved the region stuff into its own file, made the smaller funcs
> static inline, used 64bit maths in the scaled clipping function to
> avoid overflows (instead it will saturate to INT_MIN or INT_MAX).
> v3: Renamed drm_region to drm_rect, drm_region_clip to
> drm_rect_intersect, and drm_region_subsample to drm_rect_downscale.
>
> Signed-off-by: Ville Syrj?l? <ville.syrjala at linux.intel.com>
> ---
> drivers/gpu/drm/Makefile | 3 +-
> drivers/gpu/drm/drm_rect.c | 96 +++++++++++++++++++++++++++++++++
> include/drm/drm_rect.h | 132 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 230 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/drm_rect.c
> create mode 100644 include/drm/drm_rect.h
>
[snip]
> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
> new file mode 100644
> index 0000000..1ad4f5e
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_rect.c
> @@ -0,0 +1,96 @@
[snip]
> +#include <linux/errno.h>
> +#include <linux/export.h>
> +#include <linux/kernel.h>
> +#include <drm/drm_rect.h>
> +
> +/**
> + * drm_rect_intersect - intersect two rectangles
> + * @r1: first rectangle
> + * @r2: second rectangle
> + *
> + * Calculate the intersection of rectangles @r1 and @r2.
> + * @r1 will be overwritten with the intersection.
> + *
> + * RETURNS:
> + * @true if rectangle @r1 is still visible after the operation,
> + * @false otherwise.
Isn't @ used for function parameters only ?
> + */
> +bool drm_rect_intersect(struct drm_rect *r1, const struct drm_rect *r2)
> +{
> + r1->x1 = max(r1->x1, r2->x1);
> + r1->y1 = max(r1->y1, r2->y1);
> + r1->x2 = min(r1->x2, r2->x2);
> + r1->y2 = min(r1->y2, r2->y2);
> +
> + return drm_rect_visible(r1);
Do callers always need that information, or should they instead call
drm_rect_visible() explicitly when they need it ?
> +}
> +EXPORT_SYMBOL(drm_rect_intersect);
> +
> +/**
> + * drm_rect_clip_scaled - perform a scaled clip operation
> + * @src: source window rectangle
> + * @dst: destination window rectangle
> + * @clip: clip rectangle
> + * @hscale: horizontal scaling factor
> + * @vscale: vertical scaling factor
> + *
> + * Clip rectangle @dst by rectangle @clip. Clip rectangle @src by the
> + * same amounts multiplied by @hscale and @vscale.
> + *
> + * RETUTRNS:
> + * @true if rectangle @dst is still visible after being clipped,
> + * @false otherwise
> + */
> +bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
> + const struct drm_rect *clip,
> + int hscale, int vscale)
> +{
> + int diff;
> +
> + diff = clip->x1 - dst->x1;
> + if (diff > 0) {
> + int64_t tmp = src->x1 + (int64_t) diff * hscale;
> + src->x1 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX);
> + }
> + diff = clip->y1 - dst->y1;
> + if (diff > 0) {
> + int64_t tmp = src->y1 + (int64_t) diff * vscale;
> + src->y1 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX);
> + }
> + diff = dst->x2 - clip->x2;
> + if (diff > 0) {
> + int64_t tmp = src->x2 - (int64_t) diff * hscale;
> + src->x2 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX);
> + }
> + diff = dst->y2 - clip->y2;
> + if (diff > 0) {
> + int64_t tmp = src->y2 - (int64_t) diff * vscale;
> + src->y2 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX);
> + }
> +
> + return drm_rect_intersect(dst, clip);
> +}
> +EXPORT_SYMBOL(drm_rect_clip_scaled);
> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
> new file mode 100644
> index 0000000..40b09a4
> --- /dev/null
> +++ b/include/drm/drm_rect.h
> @@ -0,0 +1,132 @@
[snip]
> +/**
> + * drm_rect - two dimensional rect
> + * @x1: horizontal starting coordinate (inclusive)
> + * @x2: horizontal ending coordinate (exclusive)
> + * @y1: vertical starting coordinate (inclusive)
> + * @y2: vertical ending coordinate (exclusive)
What's the rationale for making x2 and y2 exclusive ?
> + */
> +struct drm_rect {
> + int x1, y1, x2, y2;
> +};
> +
> +/**
> + * drm_rect_adjust_size - adjust the size of the rect
> + * @r: rect to be adjusted
> + * @x: horizontal adjustment
> + * @y: vertical adjustment
What about renaming x and y to dx and dy ? It would make it more explicit that
the adjustements are incremental and not absolute values.
> + * Change the size of rect @r by @x in the horizontal direction,
> + * and by @y in the vertical direction, while keeping the center
> + * of @r stationary.
> + *
> + * Positive @x and @y increase the size, negative values decrease it.
> + */
> +static inline void drm_rect_adjust_size(struct drm_rect *r, int x, int y)
> +{
> + r->x1 -= x >> 1;
> + r->y1 -= y >> 1;
> + r->x2 += (x + 1) >> 1;
> + r->y2 += (y + 1) >> 1;
> +}
> +
> +/**
> + * drm_rect_translate - translate the rect
> + * @r: rect to be tranlated
> + * @x: horizontal translation
> + * @y: vertical translation
dx and dy here too ?
> + *
> + * Move rect @r by @x in the horizontal direction,
> + * and by @y in the vertical direction.
> + */
> +static inline void drm_rect_translate(struct drm_rect *r, int x, int y)
> +{
> + r->x1 += x;
> + r->y1 += y;
> + r->x2 += x;
> + r->y2 += y;
> +}
> +
> +/**
> + * drm_rect_downscale - downscale a rect
> + * @r: rect to be downscaled
> + * @horz: horizontal downscale factor
> + * @vert: vertical downscale factor
> + *
> + * Divide the coordinates of rect @r by @horz and @vert.
> + */
> +static inline void drm_rect_downscale(struct drm_rect *r, int horz, int
> vert)
Shouldn't horz and vert be unsigned ?
> +{
> + r->x1 /= horz;
> + r->y1 /= vert;
> + r->x2 /= horz;
> + r->y2 /= vert;
> +}
> +
> +/**
> + * drm_rect_width - determine the rect width
> + * @r: rect whose width is returned
> + *
> + * RETURNS:
> + * The width of the rect.
> + */
> +static inline int drm_rect_width(const struct drm_rect *r)
> +{
> + return r->x2 - r->x1;
> +}
> +
> +/**
> + * drm_rect_height - determine the rect height
> + * @r: rect whose height is returned
> + *
> + * RETURNS:
> + * The height of the rect.
> + */
> +static inline int drm_rect_height(const struct drm_rect *r)
> +{
> + return r->y2 - r->y1;
> +}
> +
> +/**
> + * drm_rect_visible - determine if the the rect is visible
> + * @r: rect whose visibility is returned
> + *
> + * RETURNS:
> + * @true if the rect is visible, @false otherwise.
> + */
> +static inline bool drm_rect_visible(const struct drm_rect *r)
> +{
> + return drm_rect_width(r) > 0 && drm_rect_height(r) > 0;
> +}
> +
> +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,
> + int hscale, int vscale);
> +
> +#endif
--
Regards,
Laurent Pinchart
