<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>
+<li>This GNU indent command generally does the right thing for formatting:
+<pre>
+ indent -br -i3 -npcs --no-tabs infile.c -o outfile.c
+</pre>
-<p>
-Brace example:
-</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>
+ /* 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>
- if (condition) {
- foo;
- }
- else {
- bar;
- }
-
- switch (condition) {
- case 0:
- foo();
- break;
-
- case 1: {
- ...
- break;
- }
-
- default:
- ...
- break;
- }
+ /* 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>
-<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>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.)
+
+<li>Function names follow various conventions depending on the type of function:
<pre>
- indent -br -i3 -npcs --no-tabs infile.c -o outfile.c
+ 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>
+<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.
-<p>
-Local variable name example: localVarName (no underscores)
-</p>
+<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>
-Constants and macros are ALL_UPPERCASE, with _ between words
-</p>
+</ul>
+
+
+<h2 id="submitting">Submitting patches</h2>
<p>
-Global variables are not allowed.
+The basic guidelines for submitting patches are:
</p>
+<ul>
+<li>Patches should be sufficiently tested before submitting.
+<li>Code patches should follow Mesa coding conventions.
+<li>Whenever possible, patches should only effect individual Mesa/Gallium
+components.
+<li>Patches should never introduce build breaks and should be bisectable (see
+<code>git bisect</code>.)
+<li>Patches should be properly formatted (see below).
+<li>Patches should be submitted to mesa-dev for review using
+<code>git send-email</code>.
+<li>Patches should not mix code changes with code formatting changes (except,
+perhaps, in very trivial cases.)
+</ul>
+
+<h3>Patch formatting</h3>
+
<p>
-Function name examples:
+The basic rules for patch formatting are:
</p>
+
+<ul>
+<li>Lines should be limited to 75 characters or less so that git logs
+displayed in 80-column terminals avoid line wrapping. Note that git
+log uses 4 spaces of indentation (4 + 75 < 80).
+<li>The first line should be a short, concise summary of the change prefixed
+with a module name. Examples:
+<pre>
+ mesa: Add support for querying GL_VERTEX_ATTRIB_ARRAY_LONG
+
+ gallium: add PIPE_CAP_DEVICE_RESET_STATUS_QUERY
+
+ i965: Fix missing type in local variable declaration.
+</pre>
+<li>Subsequent patch comments should describe the change in more detail,
+if needed. For example:
+<pre>
+ i965: Remove end-of-thread SEND alignment code.
+
+ This was present in Eric's initial implementation of the compaction code
+ for Sandybridge (commit 077d01b6). There is no documentation saying this
+ is necessary, and removing it causes no regressions in piglit on any
+ platform.
+</pre>
+<li>A "Signed-off-by:" line is not required, but not discouraged either.
+<li>If a patch address a bugzilla issue, that should be noted in the
+patch comment. For example:
+<pre>
+ Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89689
+</pre>
+<li>If there have been several revisions to a patch during the review
+process, they should be noted such as in this example:
<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
+ st/mesa: add ARB_texture_stencil8 support (v4)
+
+ if we support stencil texturing, enable texture_stencil8
+ there is no requirement to support native S8 for this,
+ the texture can be converted to x24s8 fine.
+
+ v2: fold fixes from Marek in:
+ a) put S8 last in the list
+ b) fix renderable to always test for d/s renderable
+ fixup the texture case to use a stencil only format
+ for picking the format for the texture view.
+ v3: hit fallback for getteximage
+ v4: put s8 back in front, it shouldn't get picked now (Ilia)
</pre>
+<li>If someone tested your patch, document it with a line like this:
+<pre>
+ Tested-by: Joe Hacker <jhacker@foo.com>
+</pre>
+<li>If the patch was reviewed (usually the case) or acked by someone,
+that should be documented with:
+<pre>
+ Reviewed-by: Joe Hacker <jhacker@foo.com>
+ Acked-by: Joe Hacker <jhacker@foo.com>
+</pre>
+</ul>
-<p>
-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>
-<h2 id="submitting">Submitting patches</h2>
+<h3>Testing Patches</h3>
+
+<p>
+It should go without saying that patches must be tested. In general,
+do whatever testing is prudent.
+</p>
<p>
-You should always run the Mesa Testsuite before submitting patches.
-The Testsuite can be run using the 'make check' command. All tests
+You should always run the Mesa test suite before submitting patches.
+The test suite can be run using the 'make check' command. All tests
must pass before patches will be accepted, this may mean you have
to update the tests themselves.
</p>
+<p>
+Whenever possible and applicable, test the patch with
+<a href="http://piglit.freedesktop.org">Piglit</a> to
+check for regressions.
+</p>
+
+
+<h3>Mailing Patches</h3>
+
<p>
Patches should be sent to the Mesa mailing list for review.
When submitting a patch make sure to use git send-email rather than attaching
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:
</li>
<li>
Add a new entry to the <code>gl_extensions</code> struct in mtypes.h
+ if the extension requires driver capabilities not already exposed by
+ another extension.
</li>
<li>
- Update the <code>extensions.c</code> file.
+ Add a new entry to the src/mesa/main/extensions_table.h file.
</li>
<li>
From this point, the best way to proceed is to find another extension,
If the new extension adds new GL state, the functions in get.c, enable.c
and attrib.c will most likely require new code.
</li>
+<li>
+ To determine if the new extension is active in the current context,
+ use the auto-generated _mesa_has_##name_str() function defined in
+ src/mesa/main/extensions.h.
+</li>
<li>
The dispatch tests check_table.cpp and dispatch_sanity.cpp
should be updated with details about the new extensions functions. These
tests are run using 'make check'
</li>
</ul>
+</p>
projects / mesa.git / blobdiff