<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>
+<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>
-If you use tabs, set them to 8 columns
-</p>
+<li>Put a space before/after operators. For example, <tt>a = b + c;</tt>
+and not <tt>a=b+c;</tt>
-<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>
-
-<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)
+ {
+ /* function body */
+ }
</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. Also,
+the opening brace goes on the next line by itself (see above.)
-<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>
<p>
Whenever possible and applicable, test the patch with
-<a href="http://people.freedesktop.org/~nh/piglit/">Piglit</a> to
+<a href="http://piglit.freedesktop.org">Piglit</a> to
check for regressions.
</p>
it harder for reviewers to accidentally review old patches.
</p>
+<p>
+When submitting follow-up patches you should also login to
+<a href="https://patchwork.freedesktop.org">patchwork</a> and change the
+state of your old patches to Superseded.
+</p>
+
+<h3>Reviewing Patches</h3>
+
+<p>
+When you've reviewed a patch on the mailing list, please be unambiguous
+about your review. That is, state either
+<pre>
+ Reviewed-by: Joe Hacker <jhacker@foo.com>
+</pre>
+or
+<pre>
+ Acked-by: Joe Hacker <jhacker@foo.com>
+</pre>
+Rather than saying just "LGTM" or "Seems OK".
+</p>
+
+<p>
+If small changes are suggested, it's OK to say something like:
+<pre>
+ With the above fixes, Reviewed-by: Joe Hacker <jhacker@foo.com>
+</pre>
+which tells the patch author that the patch can be committed, as long
+as the issues are resolved first.
+</p>
+
+
<h3>Marking a commit as a candidate for a stable branch</h3>
<p>
tarballs" in the previous step. Commit this change.
</p>
-<h3>Push all commits and the tag creates above</h3>
+<h3>Push all commits and the tag created above</h3>
<p>
This is the first step that cannot easily be undone. The release is going
mv ~/MesaLib-X.Y.Z* .
</pre>
-<h3>Back on mesa master, andd the new release notes into the tree</h3>
+<h3>Back on mesa master, add the new release notes into the tree</h3>
<p>
Something like the following steps will do the trick:
tests are run using 'make check'
</li>
</ul>
+</p>
projects / mesa.git / blobdiff