2015-05-25 18:20 GMT+02:00 Brian Paul <[email protected]>: > This hasn't been updated in a long time and from recent discussion on > the mailing list, it's not always clear what's expected. Hopefully, > this will help a bit. > --- > docs/devinfo.html | 155 > ++++++++++++++++++++++++++++++------------------------ > 1 file changed, 86 insertions(+), 69 deletions(-) > > diff --git a/docs/devinfo.html b/docs/devinfo.html > index a6fb76b..4ab8e4b 100644 > --- a/docs/devinfo.html > +++ b/docs/devinfo.html > @@ -28,97 +28,114 @@ > <h2 id="style">Coding Style</h2> > > <p> > -Mesa's code style has changed over the years. Here's the latest. > +Mesa is over 20 years old and the coding style has evolved over time. > +Some old parts use a style that's a bit out of date. > +If the guidelines below don't cover something, try following the format of > +existing, neighboring code. > </p> > > <p> > -Comment your code! It's extremely important that open-source code be > -well documented. Also, strive to write clean, easily understandable code. > +Basic formatting guidelines > </p> > > -<p> > -3-space indentation > -</p> > - > -<p> > -If you use tabs, set them to 8 columns > -</p> > +<ul> > +<li>3-space indentation, no tabs. > +<li>Limit lines to 78 or fewer characters. The idea is to prevent line > +wrapping in 80-column editors and terminals. There are exceptions, such > +as if you're defining a large, static table of information. > +<li>Opening braces go on the same line as the if/for/while statement. > +For example: > +<pre> > + if (condition) { > + foo; > + } else { > + bar; > + } > +</pre> > > -<p> > -Line width: the preferred width to fill comments and code in Mesa is 78 > -columns. Exceptions are sometimes made for clarity (e.g. tabular data is > -sometimes filled to a much larger width so that extraneous carriage returns > -don't obscure the table). > -</p> > +<li>Put a space before/after operators. For example, <tt>a = b + c;</tt> > +and not <tt>a=b+c;</tt> > > -<p> > -Brace example: > -</p> > +<li>This GNU indent command generally does the right thing for formatting: > <pre> > - if (condition) { > - foo; > - } > - else { > - bar; > - } > - > - switch (condition) { > - case 0: > - foo(); > - break; > - > - case 1: { > - ... > - break; > - } > - > - default: > - ... > - break; > - } > + indent -br -i3 -npcs --no-tabs infile.c -o outfile.c > </pre> > > -<p> > -Here's the GNU indent command which will best approximate my preferred style: > -(Note that it won't format switch statements in the preferred way) > -</p> > +<li>Use comments wherever you think it would be helpful for other developers. > +Several specific cases and style examples follow. Note that we roughly > +follow <a href="http://www.stack.nl/~dimitri/doxygen/">Doxygen</a> > conventions. > +<br> > +<br> > +Single-line comments: > <pre> > - indent -br -i3 -npcs --no-tabs infile.c -o outfile.c > + /* null-out pointer to prevent dangling reference below */ > + bufferObj = NULL; > +</pre> > +Or, > +<pre> > + bufferObj = NULL; /* prevent dangling reference below */ > +</pre> > +Multi-line comment: > +<pre> > + /* If this is a new buffer object id, or one which was generated but > + * never used before, allocate a buffer object now. > + */ > +</pre> > +We try to quote the OpenGL specification where prudent: > +<pre> > + /* Page 38 of the PDF of the OpenGL ES 3.0 spec says: > + * > + * "An INVALID_OPERATION error is generated for any of the following > + * conditions: > + * > + * * <length> is zero." > + * > + * Additionally, page 94 of the PDF of the OpenGL 4.5 core spec > + * (30.10.2014) also says this, so it's no longer allowed for desktop GL, > + * either. > + */ > +</pre> > +Function comment example: > +<pre> > + /** > + * Create and initialize a new buffer object. Called via the > + * ctx->Driver.CreateObject() driver callback function. > + * \param name integer name of the object > + * \param type one of GL_FOO, GL_BAR, etc. > + * \return pointer to new object or NULL if error > + */ > + struct gl_object * > + _mesa_create_object(GLuint name, GLenum type) > </pre> > > +<li>Put the function return type and qualifiers on one line and the function > +name and parameters on the next, as seen above. This makes it easy to use > +<code>grep ^function_name dir/*</code> to find function definitions. >
Maybe add that we put the opening brace on a new line for functions? (and include that in the function definition example above) This kinda confused me at the beginning; why put the brace on the same line as if's/loops, but on its own line for functions? I got some nits due to that when I first started mesa-hacking. Having it stated here may just help others avoid that. It's your call, I don't have a strong opinion. I'm no expert on how to add extensions, but patch 2 and 3, and the html of patch one LGTM. (Apart from what Ilia pointed out) --Thomas > -<p> > -Local variable name example: localVarName (no underscores) > -</p> > - > -<p> > -Constants and macros are ALL_UPPERCASE, with _ between words > -</p> > - > -<p> > -Global variables are not allowed. > -</p> > - > -<p> > -Function name examples: > -</p> > +<li>Function names follow various conventions depending on the type of > function: > <pre> > - glFooBar() - a public GL entry point (in glapi_dispatch.c) > - _mesa_FooBar() - the internal immediate mode function > - save_FooBar() - retained mode (display list) function in dlist.c > - foo_bar() - a static (private) function > - _mesa_foo_bar() - an internal non-static Mesa function > + glFooBar() - a public GL entry point (in glapi_dispatch.c) > + _mesa_FooBar() - the internal immediate mode function > + save_FooBar() - retained mode (display list) function in dlist.c > + foo_bar() - a static (private) function > + _mesa_foo_bar() - an internal non-static Mesa function > </pre> > > -<p> > -Places that are not directly visible to the GL API should prefer the use > -of <tt>bool</tt>, <tt>true</tt>, and > +<li>Constants, macros and enumerant names are ALL_UPPERCASE, with _ between > +words. > +<li>Mesa usually uses camel case for local variables (Ex: "localVarname") > +while gallium typically uses underscores (Ex: "local_var_name"). > +<li>Global variables are almost never used because Mesa should be > thread-safe. > + > +<li>Booleans. Places that are not directly visible to the GL API > +should prefer the use of <tt>bool</tt>, <tt>true</tt>, and > <tt>false</tt> over <tt>GLboolean</tt>, <tt>GL_TRUE</tt>, and > <tt>GL_FALSE</tt>. In C code, this may mean that > <tt>#include <stdbool.h></tt> needs to be added. The > <tt>try_emit_</tt>* methods in src/mesa/program/ir_to_mesa.cpp and > src/mesa/state_tracker/st_glsl_to_tgsi.cpp can serve as examples. > -</p> > + > +</ul> > > > <h2 id="submitting">Submitting patches</h2> > -- > 1.9.1 > > _______________________________________________ > mesa-dev mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
