On 05/27/2015 04:21 AM, Ilia Mirkin wrote: > 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.
I wrote a lot of the leading-operator cases. I think most or all of those are cases where the operator is in an if-condition. Those cases improve readability because of how the indentation lines up if (x + y) z; vs. if (x + y) z; It's more obvious that the second line is still part of the condition when you're skimming. Matt has made the argument that (and I'm paraphrasing) like Yoda conditions, it may technically be better, but it's weird... and weird is usually bad. *shrug* _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev