CONTRIBUTING.md

   1 Initial design decisions
   2 ------------------------
   3
   4 Before I started working on Piglit, I asked around for OpenGL testing methods.
   5 There were basically two kinds of tests:
   6
   7 1. Glean, which is fully automatic and intends to test the letter of the
   8    OpenGL specification (at least partially).
   9
  10 2. Manual tests using Mesa samples or third-party software.
  11
  12 The weakness of Glean is that it is not flexible, not pragmatic enough for
  13 driver development. For example, it tests for precision requirements in
  14 blending, where a driver just cannot reasonably improve on what the hardware
  15 delivers. As a driver developer, one wants to consider a test successful
  16 when it reaches the optimal results that the hardware can give, even when
  17 these results may be non-compliant.
  18
  19 Manual tests are not well repeatable. They require a considerable amount of
  20 work on the part of the developer, so most of the time they aren't done at all.
  21 On the other hand, those manual tests have sometimes been created to test for
  22 a particular weakness in implementations, so they may be very suitable to
  23 detect future, similar weaknesses.
  24
  25 Due to these weaknesses, the test coverage of open source OpenGL drivers
  26 is suboptimal at best. My goal for Piglit is to create a useful test system
  27 that helps driver developers in improving driver quality.
  28
  29 With that in mind, my sub-goals are:
  30
  31 1. Combine the strengths of the two kinds of tests (Glean, manual tests)
  32    into a single framework.
  33
  34 2. Provide concise, human readable summaries of the test results, with the
  35    option to increase the detail of the report when desired.
  36
  37 3. Allow easy visualization of regressions.
  38
  39 4. Allow easy detection of performance regressions.
  40
  41 I briefly considered extending Glean, but then decided for creating an
  42 entirely new project. The most important reasons are:
  43
  44 1. I do not want to pollute the very clean philosophy behind Glean.
  45
  46 2. The model behind Glean is that one process runs all the tests.
  47    During driver development, one often gets bugs that cause tests to crash.
  48    This means that one failed test can disrupt the entire test batch.
  49    I want to use a more robust model, where each test runs in its own process.
  50    This model does not easily fit onto Glean.
  51
  52 3. The amount of code duplication and forking overhead is minimal because
  53  a) I can use Glean as a "subroutine", so no code is actually duplicated,
  54     there's just a tiny amount of additional Python glue code.
  55  b) It's unlikely that this project makes significant changes to Glean
  56     that need to be merged upstream.
  57
  58 4. While it remains reasonable to use C++ for the actual OpenGL tests,
  59    it is easier to use a higher level language like Python for the framework
  60    (summary processing etc.)
  61
  62
  63
  64 Coding style
  65 ------------
  66
  67 Basic formatting:
  68
  69 * Indent with 8-column tabs
  70 * Limit lines to 78 characters or less
  71 * Function return type and name go on successive lines
  72 * Opening function brace goes on line by itself
  73 * Opening statement braces go on same line as the 'for' or 'else'
  74 * Use /* C style comments */, not // C++ style
  75 * Don't write 'if (condition) statement;' on one line - put the statement on
  76   a separate line.  Braces around a single statement are optional.
  77
  78 The following indent command will generally format your code for piglit's
  79 style:
  80
  81   indent -br -i8 -npcs -ce input.c -o output.c
  82
  83 Though, that doesn't give perfect results.  It messes up the
  84 PIGLIT_GL_TEST_CONFIG_BEGIN/END section.  And array initializers sometimes
  85 come out funny.
  86
  87 When in doubt see other recently added piglit tests for coding style.
  88
  89
  90 Code conventions:
  91
  92 * Use "const" qualifiers whenever possible on array declarations, pointers
  93   and global variables.
  94 * Use "static const" for initialized arrays whenever possible.
  95 * Preprocessor macros should be UPPER_CASE
  96 * Enumeration tokens should be UPPER_CASE
  97 * Most other identifiers are lower_case_with_underscores
  98 * Library, executable, and source file names are '<base>_<api>.'
  99   e.g. libpiglitutil_gles2
 100 * Test names are '<lowercasegroupname>-<testname>.'  e.g. glsl-novertexdata
 101 * Use int, float, bool except when GL types (GLint, GLfloat) are really needed
 102 * Always declare GL entrypoint pointer type with APIENTRY, or use piglit
 103   dispatch typedef
 104
 105 Test conventions:
 106
 107 * The goal is to find bugs and demonstrate them as simply as possible, not
 108   to measure performance or demonstrate features.
 109 * Write tests that are easily read, understood and debugged.  Long, complicated
 110   functions are frowned upon.
 111 * Don't try to test too much in a single test program.  Most piglit programs
 112   are less than 300 lines long.
 113 * If a test doesn't render anything, it doesn't need to set the window size.
 114 * Avoid changing an existing testing such that it might fail where it
 115   previously passed.  Break it into subtests and add a new subtest, or add
 116   a new test which supersedes the old one.
 117 * Things that should be seen are drawn in green (or blue as a second color)
 118   while red is for things that shouldn't be seen.
 119 * Calculate drawing coordinates from piglit_width/piglit_height as needed,
 120   instead of hard coding.
 121 * If a test can safely run at the same time as other tests, add it as a
 122   concurrent test in 'all.tests' (or wherever you add it).
 123
 124
 125 Utility code:
 126
 127 Piglit has a rich set of utility functions for basic drawing, setting
 128 up shaders, probing pixels, error checking, etc.  Try to use them before
 129 rolling your own.
 130
 131 Python framework:
 132
 133 Piglit uses python's [PEP8](http://www.python.org/dev/peps/pep-0008/) standard for formatting of python code; using only
 134 spaces with no tabs for indenting.
 135
 136
 137
 138 Release Philosophy
 139 ------------------
 140
 141 Since Piglit is a test suite, it is "production software" at all times.
 142 Test case might be incorrect, but despite that it is not useful to speak of
 143 "stable" and "unstable" versions of a test suite, especially one that sees
 144 a relatively small rate of change like Piglit.
 145
 146 For this reason, developers of OpenGL drivers and related software, and even
 147 testers are encouraged to follow the [git
 148 repository](https://gitlab.freedesktop.org/mesa/piglit).
 149
 150
 151
 152 Nevertheless, for purposes of marking a specific point in time for packaging
 153 in an environment where non-developers do tests on a wide range of hardware,
 154 it has been pointed out that it would be useful to have official releases.
 155
 156 For this reason, we will occasionally bump the version number in the file
 157 RELEASE and create an appropriate tag in the git repository.
 158
 159 This tag is the official way of marking a release, so the tarballs provided
 160 automatically by the cgit frontend are official release tarballs.
 161
 162
 163 Contributing Patches
 164 --------------------
 165
 166 If you want to contribute patches, please submit an
 167 [MR](https://gitlab.freedesktop.org/mesa/piglit/-/merge_requests/new)
 168
 169 Patches should not mix code changes with code formatting changes (except,
 170 perhaps, in very trivial cases.)
 171
 172 Patches should never introduce build breaks and should be bisectable (see Git
 173 bisect.)
 174
 175 Reviewed MRs must be assigned to marge-bot for merging into the main repository,
 176 rather than directly merging them or using "Merge after pipeline succeeds".
 177 This ensures that the CI pipeline stays green, and each commit links to its MR.
 178
 179 Reviewers may use the "Approve" button to indicate that they think the whole MR
 180 is acceptable for merge, but want to let the submitter collect more review.
 181
 182 Note that Piglit reviewers use the terms "Reviewed-by", "Tested-by", and
 183 "Acked-by" in the same way as they are used for [Linux kernel
 184 patches](https://www.kernel.org/doc/Documentation/SubmittingPatches).  They are
 185 not required to be squashed into commit messages, given that marge-bot will
 186 include a link to the MR discussion in each commit message.  You are also
 187 welcome to add a "Signed-off-by" line to your patch, but it is not required.
 188
 189 Please be patient -- most of us develop graphics drivers (such as Mesa) as our
 190 primary job, so we have limited time to respond to your MRs.  If your MR hasn't
 191 received a reply in a week, you may try sending a ping to a developer of related
 192  tests, or who provided feedback on your MR.  If you have questions that are
 193 better discussed in real time, many piglit developers can also be found in the
 194 #dri-devel channel on OFTC IRC.