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

[Rogovin, Kevin]

Hi,

> 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.

Considering there is a formatting issue below, I can change it to trailing. I don’t care.

>> +      if (ymax < bbox[3]) {
>> +        bbox[3] = ymax;
>indent looks messed up here.

Sighs, because it is. I missed that line, and I have missed that one since v2. I really wish I had a script to check the indenting. Sighs.

> 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.

I can kill the extern, I don't care; however, all the previous existing functions in the header had extern on them. Is it better for consistency within the file to keep the extern?

>>
>> +
> What's with the extra newline?

Double sighs. Apparently, my git skills are lacking and I let that added line sneak in when I had killed it from Patch 4. 


>  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