Development Notes
-Development Notes
+ -Adding Extentions
+ +Coding Style
-To add a new GL extension to Mesa you have to do at least the following. +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. +
+ ++Basic formatting guidelines +
-
-
-
- If glext.h doesn't define the extension, edit include/GL/gl.h and add
- code like this:
-
- #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 -
-
- - - In the src/mesa/glapi/ 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. - -
-
- Add a new entry to the
gl_extensions
struct in mtypes.h -
- -
- Update the
extensions.c
file. -
- - - 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. - -
- - If the new extension adds new GL state, the functions in get.c, enable.c - and attrib.c will most likely require new code. - +
- 3-space indentation, no tabs. +
- 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. +
- Opening braces go on the same line as the if/for/while statement.
+For example:
+
+ if (condition) { + foo; + } else { + bar; + } +
+ + - Put a space before/after operators. For example, a = b + c; +and not a=b+c; + +
- This GNU indent command generally does the right thing for formatting:
+
+ indent -br -i3 -npcs --no-tabs infile.c -o outfile.c +
+ + - Use comments wherever you think it would be helpful for other developers.
+Several specific cases and style examples follow. Note that we roughly
+follow Doxygen conventions.
+
+
+Single-line comments: ++ /* null-out pointer to prevent dangling reference below */ + bufferObj = NULL; +
+Or, ++ bufferObj = NULL; /* prevent dangling reference below */ +
+Multi-line comment: ++ /* If this is a new buffer object id, or one which was generated but + * never used before, allocate a buffer object now. + */ +
+We try to quote the OpenGL specification where prudent: ++ /* 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: + * + * *
+Function comment example: +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. + */ + + /** + * 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 */ + } +
+ + - 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
+
grep ^function_name dir/*
to find function definitions. Also, +the opening brace goes on the next line by itself (see above.) + + - Function names follow various conventions depending on the type of 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 +
+ + - Constants, macros and enumerant names are ALL_UPPERCASE, with _ between +words. +
- Mesa usually uses camel case for local variables (Ex: "localVarname") +while gallium typically uses underscores (Ex: "local_var_name"). +
- Global variables are almost never used because Mesa should be thread-safe. + +
- Booleans. Places that are not directly visible to the GL API +should prefer the use of bool, true, and +false over GLboolean, GL_TRUE, and +GL_FALSE. In C code, this may mean that +#include <stdbool.h> needs to be added. The +try_emit_* methods in src/mesa/program/ir_to_mesa.cpp and +src/mesa/state_tracker/st_glsl_to_tgsi.cpp can serve as examples. +
Submitting patches
+ ++The basic guidelines for submitting patches are: +
+ +-
+
- Patches should be sufficiently tested before submitting. +
- Code patches should follow Mesa coding conventions. +
- Whenever possible, patches should only effect individual Mesa/Gallium +components. +
- Patches should never introduce build breaks and should be bisectable (see
+
git bisect
.) + - Patches should be properly formatted (see below). +
- Patches should be submitted to mesa-dev for review using
+
git send-email
. + - Patches should not mix code changes with code formatting changes (except, +perhaps, in very trivial cases.) +
Coding Style
+Patch formatting
-Mesa's code style has changed over the years. Here's the latest. +The basic rules for patch formatting are: +
+ +-
+
- 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). +
- The first line should be a short, concise summary of the change prefixed
+with a module name. Examples:
+
+ 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. +
+ - Subsequent patch comments should describe the change in more detail,
+if needed. For example:
+
+ 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. +
+ - A "Signed-off-by:" line is not required, but not discouraged either. +
- If a patch address a bugzilla issue, that should be noted in the
+patch comment. For example:
+
+ Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89689 +
+ - If there have been several revisions to a patch during the review
+process, they should be noted such as in this example:
+
+ 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) +
+ - If someone tested your patch, document it with a line like this:
+
+ Tested-by: Joe Hacker <jhacker@foo.com> +
+ - If the patch was reviewed (usually the case) or acked by someone,
+that should be documented with:
+
+ Reviewed-by: Joe Hacker <jhacker@foo.com> + Acked-by: Joe Hacker <jhacker@foo.com> +
+
Testing Patches
+ ++It should go without saying that patches must be tested. In general, +do whatever testing is prudent.
-Comment your code! It's extremely important that open-source code be -well documented. Also, strive to write clean, easily understandable code. +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.
-3-space indentation +Whenever possible and applicable, test the patch with +Piglit to +check for regressions. +
+ + +Mailing Patches
+ ++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 +able to provide in-line review comments.
-If you use tabs, set them to 8 columns +When submitting follow-up patches you can use --in-reply-to to make v2, v3, +etc patches show up as replies to the originals. This usually works well +when you're sending out updates to individual patches (as opposed to +re-sending the whole series). Using --in-reply-to makes +it harder for reviewers to accidentally review old patches.
-Brace example: +When submitting follow-up patches you should also login to +patchwork and change the +state of your old patches to Superseded.
+ +Reviewing Patches
+ ++When you've reviewed a patch on the mailing list, please be unambiguous +about your review. That is, state either
- if (condition) { - foo; - } - else { - bar; - } + Reviewed-by: Joe Hacker <jhacker@foo.com>+or +
+ Acked-by: Joe Hacker <jhacker@foo.com> ++Rather than saying just "LGTM" or "Seems OK". +
-Here's the GNU indent command which will best approximate my preferred style: -
+If small changes are suggested, it's OK to say something like:- indent -br -i3 -npcs --no-tabs infile.c -o outfile.c + With the above fixes, Reviewed-by: Joe Hacker <jhacker@foo.com>+which tells the patch author that the patch can be committed, as long +as the issues are resolved first. + + + +
Marking a commit as a candidate for a stable branch
+ ++If you want a commit to be applied to a stable branch, +you should add an appropriate note to the commit message. +
++Here are some examples of such a note: +
+-
+
- CC: <mesa-stable@lists.freedesktop.org> +
- CC: "9.2 10.0" <mesa-stable@lists.freedesktop.org> +
- CC: "10.0" <mesa-stable@lists.freedesktop.org> +
Criteria for accepting patches to the stable branch
+ +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 +branches. Everyone else should simply nominate patches using the mechanism +described above. + +The stable-release manager will work with the list of nominated patches, and +for each patch that meets the crtieria below will cherry-pick the patch with: +git cherry-pick -x <commit>
. The -x
option is
+important so that the picked patch references the comit ID of the original
+patch.
+
+The stable-release manager may at times need to force-push changes to the
+stable branches, for example, to drop a previously-picked patch that was later
+identified as causing a regression). These force-pushes may cause changes to
+be lost from the stable branch if developers push things directly. Consider
+yourself warned.
+
+The stable-release manager is also given broad discretion in rejecting patches
+that have been nominated for the stable branch. The most basic rule is that
+the stable branch is for bug fixes only, (no new features, no
+regressions). Here is a non-exhaustive list of some reasons that a patch may
+be rejected:
+
+-
+
- Patch introduces a regression. Any reported build breakage or other + regression caused by a particular patch, (game no longer work, piglit test + changes from PASS to FAIL), is justification for rejecting a patch. + +
- Patch is too large, (say, larger than 100 lines) + +
- Patch is not a fix. For example, a commit that moves code around with no + functional change should be rejected. + +
- Patch fix is not clearly described. For example, a commit message + of only a single line, no description of the bug, no mention of bugzilla, + etc. + +
- Patch has not obviously been reviewed, For example, the commit message + has no Reviewed-by, Signed-off-by, nor Tested-by tags from anyone but the + author. + +
- Patch has not already been merged to the master branch. As a rule, bug + fixes should never be applied first to a stable branch. Patches should land + first on the master branch and then be cherry-picked to a stable + branch. (This is to avoid future releases causing regressions if the patch + is not also applied to master.) The only things that might look like + exceptions would be backports of patches from master that happen to look + significantly different. + +
- Patch depends on too many other patches. Ideally, all stable-branch + patches should be self-contained. It sometimes occurs that a single, logical + bug-fix occurs as two separate patches on master, (such as an original + patch, then a subsequent fix-up to that patch). In such a case, these two + patches should be squashed into a single, self-contained patch for the + stable branch. (Of course, if the squashing makes the patch too large, then + that could be a reason to reject the patch.) + +
- Patch includes new feature development, not bug fixes. New OpenGL + features, extensions, etc. should be applied to Mesa master and included in + the next major release. Stable releases are intended only for bug fixes. + + Note: As an exception to this rule, the stable-release manager may accept + hardware-enabling "features". For example, backports of new code to support + a newly-developed hardware product can be accepted if they can be reasonably + determined to not have effects on other hardware. + +
- Patch is a performance optimization. As a rule, performance patches are + not candidates for the stable branch. The only exception might be a case + where an application's performance was recently severely impacted so as to + become unusable. The fix for this performance regression could then be + considered for a stable branch. The optimization must also be + non-controversial and the patches still need to meet the other criteria of + being simple and self-contained + +
- Patch introduces a new failure mode (such as an assert). While the new + assert might technically be correct, for example to make Mesa more + conformant, this is not the kind of "bug fix" we want in a stable + release. The potential problem here is that an OpenGL program that was + previously working, (even if technically non-compliant with the + specification), could stop working after this patch. So that would be a + regression that is unaacceptable for the stable branch. +
Making a New Mesa Release
+ ++These are the instructions for making a new Mesa release. +
+Get latest source files
-Local variable name example: localVarName (no underscores) +Use git to get the latest Mesa files from the git repository, from whatever +branch is relevant. This document uses the convention X.Y.Z for the release +being created, which should be created from a branch named X.Y.
+Perform basic testing
-Constants and macros are ALL_UPPERCASE, with _ between words +The release manager should, at the very least, test the code by compiling it, +installing it, and running the latest piglit to ensure that no piglit tests +have regressed since the previous release.
-Global variables are not allowed. +The release manager should do this testing with at least one hardware driver, +(say, whatever is contained in the local development machine), as well as on +both Gallium and non-Gallium software drivers. The software testing can be +performed by running piglit with the following environment-variable set:
++LIBGL_ALWAYS_SOFTWARE=1 ++ +And Gallium vs. non-Gallium software drivers can be obtained by using the +following configure flags on separate builds: + +
+--with-dri-drivers=swrast +--with-gallium-drivers=swrast ++
-Function name examples: +Note: If both options are given in one build, both swrast_dri.so drivers will +be compiled, but only one will be installed. The following command can be used +to ensure the correct driver is being tested:
+- glFooBar() - a public GL entry point (in 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 +LIBGL_ALWAYS_SOFTWARE=1 glxinfo | grep "renderer string"+If any regressions are found in this testing with piglit, stop here, and do +not perform a release until regressions are fixed. -
Making a New Mesa Release
+Update version in file VERSION
-These are the instructions for making a new Mesa release. +Increment the version contained in the file VERSION at Mesa's top-level, then +commit this change.
-Get latest source files
+Create release notes for the new release
+-Use "cvs update -dAP " to get the latest Mesa files from CVS. +Create a new file docs/relnotes/X.Y.Z.html, (follow the style of the previous +release notes). Note that the sha256sums section of the release notes should +be empty at this point.
++Two scripts are available to help generate portions of the release notes: + +
+ ./bin/bugzilla_mesa.sh + ./bin/shortlog_mesa.sh ++ +
+The first script identifies commits that reference bugzilla bugs and obtains +the descriptions of those bugs from bugzilla. The second script generates a +log of all commits. In both cases, HTML-formatted lists are printed to stdout +to be included in the release notes. +
-Verify and update version info
-Create/edit the docs/RELNOTES-X.Y file to document what's new in the release. -Add the new RELNOTES-X.Y file to relnotes.html. -Update the docs/VERSIONS file too. +Commit these changes
+Make the release archives, signatures, and the release tag
++From inside the Mesa directory: +
+ ./autogen.sh + make -j1 tarballs ++
-Edit the MESA_MAJOR, MESA_MINOR and MESA_TINY version numbers in -configs/default. +After the tarballs are created, the sha256 checksums for the files will +be computed and printed. These will be used in a step below.
-Make sure the values in src/mesa/main/version.h are correct. +It's important at this point to also verify that the constructed tar file +actually builds:
++ tar xjf MesaLib-X.Y.Z.tar.bz2 + cd Mesa-X.Y.Z + ./configure --enable-gallium-llvm + make -j6 + make install ++
-Edit the top-level Makefile and verify that DIRECTORY, LIB_NAME and -DEMO_NAME are correct. +Some touch testing should also be performed at this point, (run glxgears or +more involved OpenGL programs against the installed Mesa).
-Update the docs/news.html file and docs/download.html files. +Create detached GPG signatures for each of the archive files created above:
++ gpg --sign --detach MesaLib-X.Y.Z.tar.gz + gpg --sign --detach MesaLib-X.Y.Z.tar.bz2 + gpg --sign --detach MesaLib-X.Y.Z.zip ++
-Check in all updates to CVS. +Tag the commit used for the build:
++ git tag -s mesa-X.Y.X -m "Mesa X.Y.Z release" ++
-Tag the CVS files with the release name (in the form mesa_X_Y). +Note: It would be nice to investigate and fix the issue that causes the +tarballs target to fail with multiple build process, such as with "-j4". It +would also be nice to incorporate all of the above commands into a single +makefile target. And instead of a custom "tarballs" target, we should +incorporate things into the standard "make dist" and "make distcheck" targets.
+Add the sha256sums to the release notes
-Make the tarballs
-Make a symbolic link from $(DIRECTORY) to 'Mesa'. For example, -ln -s Mesa Mesa-6.3 -This is needed in order to make a correct tar file in the next step. +Edit docs/relnotes/X.Y.Z.html to add the sha256sums printed as part of "make +tarballs" in the previous step. Commit this change.
+Push all commits and the tag created above
+-Make the distribution files. From inside the Mesa directory: +This is the first step that cannot easily be undone. The release is going +forward from this point: +
+- make tarballs + git push origin X.Y --tags+
Install the release files and signatures on the distribution server
+-After the tarballs are created, the md5 checksums for the files will -be computed. -Add them to the docs/news.html file. +The following commands can be used to copy the release archive files and +signatures to the freedesktop.org server:
++ scp MesaLib-X.Y.Z* people.freedesktop.org: + ssh people.freedesktop.org + cd /srv/ftp.freedesktop.org/pub/mesa + mkdir X.Y.Z + cd X.Y.Z + mv ~/MesaLib-X.Y.Z* . ++ +
Back on mesa master, add the new release notes into the tree
+-Copy the distribution files to a temporary directory, unpack them, -compile everything, and run some demos to be sure everything works. +Something like the following steps will do the trick:
-Update the website and announce the release
++ cp docs/relnotes/X.Y.Z.html /tmp + git checkout master + cp /tmp/X.Y.Z.html docs/relnotes + git add docs/relnotes/X.Y.Z.html ++
-Follow the directions on SourceForge for creating a new "release" and -uploading the tarballs. +Also, edit docs/relnotes.html to add a link to the new release notes, and edit +docs/index.html to add a news entry. Then commit and push: +
+ ++ git commit -a -m "docs: Import X.Y.Z release notes, add news item." + git push origin ++ +
Update the mesa3d.org website
+ ++NOTE: The recent release managers have not been performing this step +themselves, but leaving this to Brian Paul, (who has access to the +sourceforge.net hosting for mesa3d.org). Brian is more than willing to grant +the permission necessary to future release managers to do this step on their +own.
Update the web site by copying the docs/ directory's files to
-/home/users/b/br/brianp/mesa-www/htdocs/
+/home/users/b/br/brianp/mesa-www/htdocs/ with:
+
+
+sftp USERNAME,mesa3d@web.sourceforge.net
+
Announce the release
Make an announcement on the mailing lists: -mesa3d-dev@lists.sf.net, -mesa3d-users@lists.sf.net + +mesa-dev@lists.freedesktop.org, and -mesa3d-announce@lists.sf.net +mesa-announce@lists.freedesktop.org + +Follow the template of previously-sent release announcements. The following +command can be used to generate the log of changes to be included in the +release announcement: + +
+ git shortlog mesa-X.Y.Z-1..mesa-X.Y.Z ++
Adding Extensions
+ ++To add a new GL extension to Mesa you have to do at least the following. + +
-
+
-
+ If glext.h doesn't define the extension, edit include/GL/gl.h and add
+ code like this:
+
+ #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 +
+
+ - + 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. + +
-
+ Add a new entry to the
gl_extensions
struct in mtypes.h +
+ -
+ Update the
extensions.c
file. +
+ - + 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. + +
- + If the new extension adds new GL state, the functions in get.c, enable.c + and attrib.c will most likely require new code. + +
- + 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' + +