[Mesa-dev] [v4 PATCH 05/10] mesa: helper function for scissor box of gl_framebuffer

[Ilia Mirkin]

More trivia:

On Wed, May 27, 2015 at 5:49 AM, Kevin Rogovin <kevin.rogovin at intel.com> wrote:
> Add helper convenience function that intersects the scissor values
> against a passed bounding box. In addition, to avoid replicated code,
> make the function _mesa_scissor_bounding_box() use this new function.
>
> v2:
>  Split from patch "mesa:helper-conveniance functions for drivers to implement ARB_framebuffer_no_attachment".
>  White space and long line fixes.
>
> v3:
>  No changes.
>
> v4:
>  No changes.
>
>
> Signed-off-by: Kevin Rogovin <kevin.rogovin at intel.com>
> ---
>  src/mesa/main/framebuffer.c | 63 +++++++++++++++++++++++++++++++--------------
>  src/mesa/main/framebuffer.h |  5 ++++
>  2 files changed, 48 insertions(+), 20 deletions(-)
>
> diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
> index c2cfb92..dd9e4bc 100644
> --- a/src/mesa/main/framebuffer.c
> +++ b/src/mesa/main/framebuffer.c
> @@ -357,42 +357,38 @@ update_framebuffer_size(struct gl_context *ctx, struct gl_framebuffer *fb)
>  }
>
>
> +
>  /**
> - * Calculate the inclusive bounding box for the scissor of a specific viewport
> + * Given a bounding box, intersect the bounding box with the scissor of
> + * a specified vieport.
>   *
>   * \param ctx     GL context.
> - * \param buffer  Framebuffer to be checked against
>   * \param idx     Index of the desired viewport
>   * \param bbox    Bounding box for the scissored viewport.  Stored as xmin,
>   *                xmax, ymin, ymax.
> - *
> - * \warning This function assumes that the framebuffer dimensions are up to
> - * date (e.g., update_framebuffer_size has been recently called on \c buffer).
> - *
> - * \sa _mesa_clip_to_region
>   */
> -void
> -_mesa_scissor_bounding_box(const struct gl_context *ctx,
> -                           const struct gl_framebuffer *buffer,
> -                           unsigned idx, int *bbox)
> +extern void
> +_mesa_intersect_scissor_bounding_box(const struct gl_context *ctx,
> +                                     unsigned idx, int *bbox)
>  {
> -   bbox[0] = 0;
> -   bbox[2] = 0;
> -   bbox[1] = buffer->Width;
> -   bbox[3] = buffer->Height;
> -
>     if (ctx->Scissor.EnableFlags & (1u << idx)) {
> +      int xmax, ymax;
> +
> +      xmax = ctx->Scissor.ScissorArray[idx].X
> +        + ctx->Scissor.ScissorArray[idx].Width;

44 instances of a leading + in mesa/main compared to 78 trailing ones.
Huh, I was going to say that it's really uncommon to do this in mesa,
but that may not be supported by fact.

> +      ymax = ctx->Scissor.ScissorArray[idx].Y
> +        + ctx->Scissor.ScissorArray[idx].Height;
>        if (ctx->Scissor.ScissorArray[idx].X > bbox[0]) {
>           bbox[0] = ctx->Scissor.ScissorArray[idx].X;
>        }
>        if (ctx->Scissor.ScissorArray[idx].Y > bbox[2]) {
>           bbox[2] = ctx->Scissor.ScissorArray[idx].Y;
>        }
> -      if (ctx->Scissor.ScissorArray[idx].X + ctx->Scissor.ScissorArray[idx].Width < bbox[1]) {
> -         bbox[1] = ctx->Scissor.ScissorArray[idx].X + ctx->Scissor.ScissorArray[idx].Width;
> +      if (xmax < bbox[1]) {
> +         bbox[1] = xmax;
>        }
> -      if (ctx->Scissor.ScissorArray[idx].Y + ctx->Scissor.ScissorArray[idx].Height < bbox[3]) {
> -         bbox[3] = ctx->Scissor.ScissorArray[idx].Y + ctx->Scissor.ScissorArray[idx].Height;
> +      if (ymax < bbox[3]) {
> +        bbox[3] = ymax;

indent looks messed up here.

>        }
>        /* finally, check for empty region */
>        if (bbox[0] > bbox[1]) {
> @@ -402,6 +398,33 @@ _mesa_scissor_bounding_box(const struct gl_context *ctx,
>           bbox[2] = bbox[3];
>        }
>     }
> +}
> +
> +/**
> + * Calculate the inclusive bounding box for the scissor of a specific viewport
> + *
> + * \param ctx     GL context.
> + * \param buffer  Framebuffer to be checked against
> + * \param idx     Index of the desired viewport
> + * \param bbox    Bounding box for the scissored viewport.  Stored as xmin,
> + *                xmax, ymin, ymax.
> + *
> + * \warning This function assumes that the framebuffer dimensions are up to
> + * date (e.g., update_framebuffer_size has been recently called on \c buffer).
> + *
> + * \sa _mesa_clip_to_region
> + */
> +void
> +_mesa_scissor_bounding_box(const struct gl_context *ctx,
> +                           const struct gl_framebuffer *buffer,
> +                           unsigned idx, int *bbox)
> +{
> +   bbox[0] = 0;
> +   bbox[2] = 0;
> +   bbox[1] = buffer->Width;
> +   bbox[3] = buffer->Height;
> +
> +   _mesa_intersect_scissor_bounding_box(ctx, idx, bbox);
>
>     assert(bbox[0] <= bbox[1]);
>     assert(bbox[2] <= bbox[3]);
> diff --git a/src/mesa/main/framebuffer.h b/src/mesa/main/framebuffer.h
> index 8b2aa34..bb1f2ea 100644
> --- a/src/mesa/main/framebuffer.h
> +++ b/src/mesa/main/framebuffer.h
> @@ -76,6 +76,10 @@ _mesa_scissor_bounding_box(const struct gl_context *ctx,
>                             const struct gl_framebuffer *buffer,
>                             unsigned idx, int *bbox);
>
> +extern void

I don't think the "extern" isn't really necessary here. A lot of older
code still has it, but you don't have to stick it in on newer
additions.

> +_mesa_intersect_scissor_bounding_box(const struct gl_context *ctx,
> +                                     unsigned idx, int *bbox);
> +
>  static inline  GLuint
>  _mesa_geometric_width(const struct gl_framebuffer *buffer)
>  {
> @@ -83,6 +87,7 @@ _mesa_geometric_width(const struct gl_framebuffer *buffer)
>        buffer->Width : buffer->DefaultGeometry.Width;
>  }
>
> +

What's with the extra newline?

>  static inline  GLuint
>  _mesa_geometric_height(const struct gl_framebuffer *buffer)
>  {
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev