[Mesa-dev] [v4 PATCH 04/10] mesa: add helper functions for geometry of gl_framebuffer

Ilia Mirkin imirkin at alum.mit.edu
Wed May 27 06:31:08 PDT 2015


On Wed, May 27, 2015 at 9:21 AM, Rogovin, Kevin <kevin.rogovin at intel.com> wrote:
> Hi,
>>> +static inline  GLuint
>>Here and below, why 2 spaces between "inline" and "GLuint"?
>
> I have no clue.  I suspect it is a scar from  some search/replace fiasco over 3 weeks ago. You are the first person who spotted that nit.
>
>>> --- a/src/mesa/main/mtypes.h
>>> +++ b/src/mesa/main/mtypes.h
>>> @@ -3187,7 +3187,13 @@ struct gl_framebuffer
>>>      * GL_ARB_framebuffer_no_attachments must check for the flag _HasAttachments
>>>      * and if GL_FALSE, must then use the values in DefaultGeometry to initialize
>> >     * its viewport, scissor and so on (in particular _Xmin, _Xmax, _Ymin and
>> >-    * _Ymax do NOT take into account _HasAttachments being false)
>> >+    * _Ymax do NOT take into account _HasAttachments being false). To get the
>> >+    * geometry of the framebuffer, the  helper functions
>
>> Why 2 spaces between "the" and "helper"?
>
> No clue. You are again the first person to spot that space.
>
> It would be nice to get a "fix these nits I found and get a reviewed by" badge.

Unfortunately I'm not in a great position to do that -- I'm good at
noticing simple issues, but I'm not always familiar with the larger GL
implications. Framebuffers are still a bit in the great unknown for me
(but I think I've finally mastered the concept of a texture). But I
glanced at your patches and spotted a couple of things that seemed
odd, so figured I'd point them out. You should wait until you get a
"real" review from someone before reposting.

Cheers,

  -ilia


More information about the mesa-dev mailing list