More trivia: On Wed, May 27, 2015 at 5:49 AM, Kevin Rogovin <kevin.rogo...@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.rogo...@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev