[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