diff options
Diffstat (limited to 'docs/devinfo.html')
| -rw-r--r-- | docs/devinfo.html | 400 |
1 files changed, 282 insertions, 118 deletions
diff --git a/docs/devinfo.html b/docs/devinfo.html index 8d20eea3c56..8ebf80f40e8 100644 --- a/docs/devinfo.html +++ b/docs/devinfo.html @@ -17,159 +17,241 @@ <h1>Development Notes</h1> -<h2>Adding Extensions</h2> - -<p> -To add a new GL extension to Mesa you have to do at least the following. - <ul> -<li> - If glext.h doesn't define the extension, edit include/GL/gl.h and add - code like this: - <pre> - #ifndef GL_EXT_the_extension_name - #define GL_EXT_the_extension_name 1 - /* declare the new enum tokens */ - /* prototype the new functions */ - /* TYPEDEFS for the new functions */ - #endif - </pre> -</li> -<li> - In the src/mapi/glapi/gen/ directory, add the new extension functions and - enums to the gl_API.xml file. - Then, a bunch of source files must be regenerated by executing the - corresponding Python scripts. -</li> -<li> - Add a new entry to the <code>gl_extensions</code> struct in mtypes.h -</li> -<li> - Update the <code>extensions.c</code> file. -</li> -<li> - From this point, the best way to proceed is to find another extension, - similar to the new one, that's already implemented in Mesa and use it - as an example. -</li> -<li> - 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> - 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> +<li><a href="#style">Coding Style</a> +<li><a href="#submitting">Submitting Patches</a> +<li><a href="#release">Making a New Mesa Release</a> +<li><a href="#extensions">Adding Extensions</a> </ul> - -<h2>Coding Style</h2> +<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> - if (condition) { - foo; - } - else { - bar; - } - - switch (condition) { - case 0: - foo(); - break; - - case 1: { - ... - break; - } - - default: - ... - break; - } + /* 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> -<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. + +</ul> -<p> -Constants and macros are ALL_UPPERCASE, with _ between words -</p> + +<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> - 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 + 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> + 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 <[email protected]> +</pre> +<li>If the patch was reviewed (usually the case) or acked by someone, +that should be documented with: +<pre> + Reviewed-by: Joe Hacker <[email protected]> + Acked-by: Joe Hacker <[email protected]> +</pre> +</ul> + + + +<h3>Testing Patches</h3> <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. +It should go without saying that patches must be tested. In general, +do whatever testing is prudent. </p> -<h2>Submitting patches</h2> - <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 patches to emails. Sending patches as attachments prevents people from being @@ -184,7 +266,38 @@ re-sending the whole series). Using --in-reply-to makes it harder for reviewers to accidentally review old patches. </p> -<h2>Marking a commit as a candidate for a stable branch</h2> +<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 <[email protected]> +</pre> +or +<pre> + Acked-by: Joe Hacker <[email protected]> +</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 <[email protected]> +</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> If you want a commit to be applied to a stable branch, @@ -221,7 +334,7 @@ the upcoming stable release can always be seen on the <a href="http://cworth.org/~cworth/mesa-stable-queue/">Mesa Stable Queue</a> page. -<h2>Criteria for accepting patches to the stable branch</h2> +<h3>Criteria for accepting patches to the stable branch</h3> Mesa has a designated release manager for each stable branch, and the release manager is the only developer that should be pushing changes to these @@ -306,7 +419,8 @@ be rejected: regression that is unaacceptable for the stable branch.</li> </ul> -<h2>Making a New Mesa Release</h2> + +<h2 id="release">Making a New Mesa Release</h2> <p> These are the instructions for making a new Mesa release. @@ -456,7 +570,7 @@ Edit docs/relnotes/X.Y.Z.html to add the sha256sums printed as part of "make 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 @@ -483,7 +597,7 @@ signatures to the freedesktop.org server: 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: @@ -543,6 +657,56 @@ release announcement: </pre> </p> + +<h2 id="extensions">Adding Extensions</h2> + +<p> +To add a new GL extension to Mesa you have to do at least the following. + +<ul> +<li> + If glext.h doesn't define the extension, edit include/GL/gl.h and add + code like this: + <pre> + #ifndef GL_EXT_the_extension_name + #define GL_EXT_the_extension_name 1 + /* declare the new enum tokens */ + /* prototype the new functions */ + /* TYPEDEFS for the new functions */ + #endif + </pre> +</li> +<li> + In the src/mapi/glapi/gen/ directory, add the new extension functions and + enums to the gl_API.xml file. + Then, a bunch of source files must be regenerated by executing the + corresponding Python scripts. +</li> +<li> + Add a new entry to the <code>gl_extensions</code> struct in mtypes.h +</li> +<li> + Update the <code>extensions.c</code> file. +</li> +<li> + From this point, the best way to proceed is to find another extension, + similar to the new one, that's already implemented in Mesa and use it + as an example. +</li> +<li> + 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> + 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> + + + + </div> </body> </html> |