Karl Schultz | b0b3cac | 2016-03-09 12:48:04 -0700 | [diff] [blame] | 1 | ## How to Contribute to Vulkan Source Repositories |
| 2 | |
Mark Lobodzinski | 437e117 | 2018-01-04 14:55:26 -0700 | [diff] [blame] | 3 | ### **The Repository** |
Karl Schultz | b0b3cac | 2016-03-09 12:48:04 -0700 | [diff] [blame] | 4 | |
Mark Lobodzinski | 437e117 | 2018-01-04 14:55:26 -0700 | [diff] [blame] | 5 | The source code for The Vulkan-LoaderAndValidationLayer components is sponsored by Khronos and LunarG. |
Karl Schultz | b0b3cac | 2016-03-09 12:48:04 -0700 | [diff] [blame] | 6 | * [Khronos Vulkan-LoaderAndValidationLayers](https://github.com/KhronosGroup/Vulkan-LoaderAndValidationLayers) |
Karl Schultz | b0b3cac | 2016-03-09 12:48:04 -0700 | [diff] [blame] | 7 | |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 8 | |
Mark Lobodzinski | 5ce91da | 2017-05-11 13:54:52 -0600 | [diff] [blame] | 9 | ### **The Vulkan Ecosystem Needs Your Help** |
Karl Schultz | df03b95 | 2016-05-05 08:30:53 -0600 | [diff] [blame] | 10 | |
| 11 | The Vulkan validation layers are one of the larger and more important components in this repository. |
| 12 | While there are often active and organized development efforts underway to improve their coverage, |
| 13 | there are always opportunities for anyone to help by contributing additional validation layer checks |
| 14 | and tests for these validation checks. |
Mark Lobodzinski | 34cd2e6 | 2018-01-04 16:12:41 -0700 | [diff] [blame] | 15 | |
| 16 | There are a couple of methods to identify areas of need: |
| 17 | * Examine the [issues list](https://github.com/KhronosGroup/Vulkan-LoaderAndValidationLayers/issues) |
| 18 | in this repository and look for issues that are of interest |
| 19 | * Alternatively, examine the [vk_validation_error_database.txt](layers/vk_validation_error_database.txt) file -- unimplemented validation checks are marked |
| 20 | with an 'N' in the 'check_implemented' column and each of these needs coverage in the validation layers. |
| 21 | |
Karl Schultz | df03b95 | 2016-05-05 08:30:53 -0600 | [diff] [blame] | 22 | Of course, if you have your own work in mind, please open an issue to describe it and assign it to yourself. |
| 23 | Finally, please feel free to contact any of the developers that are actively contributing should you |
| 24 | wish to coordinate further. |
| 25 | Please see the [section about Validation Layers](#special-considerations-for-validation-layers) |
| 26 | later on this page. |
| 27 | |
Mark Lobodzinski | 5ce91da | 2017-05-11 13:54:52 -0600 | [diff] [blame] | 28 | Repository Issue labels: |
| 29 | |
Mark Lobodzinski | 5ce91da | 2017-05-11 13:54:52 -0600 | [diff] [blame] | 30 | * _Bug_: These issues refer to invalid or broken functionality and are the highest priority. |
Mark Lobodzinski | 437e117 | 2018-01-04 14:55:26 -0700 | [diff] [blame] | 31 | * _Incomplete_: These issues refer to missing validation checks that users have encountered during application |
| 32 | development that would have been directly useful, and are high priority. |
| 33 | * _Enhancement_: These issues refer to ideas for extending or improving the loader, demos, or validation layers. |
Mark Lobodzinski | 5ce91da | 2017-05-11 13:54:52 -0600 | [diff] [blame] | 34 | |
Mark Lobodzinski | 34cd2e6 | 2018-01-04 16:12:41 -0700 | [diff] [blame] | 35 | It is the maintainers goal for all issues to be assigned within one business day of their submission. If you choose |
Mark Lobodzinski | 437e117 | 2018-01-04 14:55:26 -0700 | [diff] [blame] | 36 | to work on an issue that is assigned, simply coordinate with the current assignee. |
Mark Lobodzinski | 5ce91da | 2017-05-11 13:54:52 -0600 | [diff] [blame] | 37 | |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 38 | ### **How to Submit Fixes** |
| 39 | |
Karl Schultz | b0b3cac | 2016-03-09 12:48:04 -0700 | [diff] [blame] | 40 | * **Ensure that the bug was not already reported or fixed** by searching on GitHub under Issues |
| 41 | and Pull Requests. |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 42 | * Use the existing GitHub forking and pull request process. |
| 43 | This will involve [forking the repository](https://help.github.com/articles/fork-a-repo/), |
| 44 | creating a branch with your commits, and then [submitting a pull request](https://help.github.com/articles/using-pull-requests/). |
Mark Lobodzinski | 34cd2e6 | 2018-01-04 16:12:41 -0700 | [diff] [blame] | 45 | * Please read and adhere to the style and process [guidelines ](#coding-conventions-and-formatting) enumerated below. |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 46 | * Please base your fixes on the master branch. SDK branches are generally not updated except for critical fixes needed to repair an SDK release. |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 47 | |
| 48 | |
| 49 | #### **Coding Conventions and Formatting** |
Mark Lobodzinski | 5ce91da | 2017-05-11 13:54:52 -0600 | [diff] [blame] | 50 | * Use the **[Google style guide](https://google.github.io/styleguide/cppguide.html)** for source code with the following exceptions: |
Jeremy Hayes | 46ec9f7 | 2017-01-19 14:10:18 -0700 | [diff] [blame] | 51 | * The column limit is 132 (as opposed to the default value 80). The clang-format tool will handle this. See below. |
Mark Lobodzinski | 9370489 | 2017-01-25 07:56:21 -0700 | [diff] [blame] | 52 | * The indent is 4 spaces instead of the default 2 spaces. Again, the clang-format tool will handle this. |
Mark Lobodzinski | 437e117 | 2018-01-04 14:55:26 -0700 | [diff] [blame] | 53 | * If you can justify a reason for violating a rule in the guidelines, then you are free to do so. Be prepared to defend your |
| 54 | decision during code review. This should be used responsibly. An example of a bad reason is "I don't like that rule." An example of |
| 55 | a good reason is "This violates the style guide, but it improves type safety." |
Jeremy Hayes | 46ec9f7 | 2017-01-19 14:10:18 -0700 | [diff] [blame] | 56 | |
Mark Lobodzinski | 437e117 | 2018-01-04 14:55:26 -0700 | [diff] [blame] | 57 | * Run **clang-format** on your changes to maintain consistent formatting |
| 58 | * There are `.clang-format files` present in the repository to define clang-format settings |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 59 | which are found and used automatically by clang-format. |
Dave Houlton | 068fdbb | 2018-02-06 17:49:16 -0700 | [diff] [blame] | 60 | * **clang-format** binaries are available from the LLVM orginization, here: https://clang.llvm.org/. Our CI system (Travis-CI) |
| 61 | currently uses clang-format version 5.0.0 to check that the lines of code you have changed are formatted properly. It is |
| 62 | recommended that you use the same version to format your code prior to submission. |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 63 | * A sample git workflow may look like: |
| 64 | |
| 65 | > # Make changes to the source. |
Karl Schultz | 41ee1a0 | 2016-04-28 14:20:13 -0600 | [diff] [blame] | 66 | > $ git add -u . |
| 67 | > $ git clang-format --style=file |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 68 | > # Check to see if clang-format made any changes and if they are OK. |
Karl Schultz | 41ee1a0 | 2016-04-28 14:20:13 -0600 | [diff] [blame] | 69 | > $ git add -u . |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 70 | > $ git commit |
| 71 | |
Mark Lobodzinski | 437e117 | 2018-01-04 14:55:26 -0700 | [diff] [blame] | 72 | * **Commit Messages** |
| 73 | * Limit the subject line to 50 characters -- this allows the information to display correctly in git/Github logs |
| 74 | * Begin subject line with a one-word component description followed by a colon (e.g. loader, layers, tests, etc.) |
| 75 | * Separate subject from body with a blank line |
| 76 | * Wrap the body at 72 characters |
| 77 | * Capitalize the subject line |
| 78 | * Do not end the subject line with a period |
| 79 | * Use the body to explain what and why vs. how |
| 80 | * Use the imperative mode in the subject line. This just means to write it as a command (e.g. Fix the sprocket) |
| 81 | |
| 82 | Strive for commits that implement a single or related set of functionality, using as many commits as is necessary (more is better). |
| 83 | That said, please ensure that the repository compiles and passes tests without error for each commit in your pull request. Note |
| 84 | that to be accepted into the repository, the pull request must [pass all tests](#testing your changes) on all supported platforms |
| 85 | -- the automatic Github Travis and AppVeyor continuous integration features will assist in enforcing this requirement. |
Jeremy Hayes | 46ec9f7 | 2017-01-19 14:10:18 -0700 | [diff] [blame] | 86 | |
Mark Lobodzinski | 5ce91da | 2017-05-11 13:54:52 -0600 | [diff] [blame] | 87 | #### **Testing Your Changes** |
| 88 | * Run the existing tests in the repository before and after each if your commits to check for any regressions. |
Karl Schultz | b0b3cac | 2016-03-09 12:48:04 -0700 | [diff] [blame] | 89 | There are some tests that appear in all repositories. |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 90 | These tests can be found in the following folders inside of your target build directory: |
Mark Lobodzinski | 5ce91da | 2017-05-11 13:54:52 -0600 | [diff] [blame] | 91 | |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 92 | (These instructions are for Linux) |
| 93 | * In the `demos` directory, run: |
| 94 | |
| 95 | > cube |
| 96 | > cube --validate |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 97 | > smoke |
| 98 | > smoke --validate |
| 99 | > vulkaninfo |
| 100 | |
| 101 | * In the `tests` directory, run: |
| 102 | |
| 103 | > run_all_tests.sh |
| 104 | |
Mark Lobodzinski | 437e117 | 2018-01-04 14:55:26 -0700 | [diff] [blame] | 105 | * On Windows, a quick sanity check can be run from inside Visual Studio -- just run the `vk_layer_validation_tests` project, |
| 106 | or you can run `run_all_tests.ps1` from a PowerShell window |
Mark Lobodzinski | 5ce91da | 2017-05-11 13:54:52 -0600 | [diff] [blame] | 107 | |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 108 | * Note that some tests may fail with known issues or driver-specific problems. |
Mark Lobodzinski | 34cd2e6 | 2018-01-04 16:12:41 -0700 | [diff] [blame] | 109 | The idea here is that your changes should not change the test results, unless that was the intent of your changes. |
Karl Schultz | b0b3cac | 2016-03-09 12:48:04 -0700 | [diff] [blame] | 110 | * Run tests that explicitly exercise your changes. |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 111 | * Feel free to subject your code changes to other tests as well! |
| 112 | |
Mark Lobodzinski | 437e117 | 2018-01-04 14:55:26 -0700 | [diff] [blame] | 113 | |
Karl Schultz | 43f4e6d | 2016-04-28 13:55:42 -0600 | [diff] [blame] | 114 | #### **Special Considerations for Validation Layers** |
Mark Lobodzinski | 437e117 | 2018-01-04 14:55:26 -0700 | [diff] [blame] | 115 | * **Validation Tests** If you are submitting a change that adds a new validation check, you should also construct a "negative" test function. |
Karl Schultz | 43f4e6d | 2016-04-28 13:55:42 -0600 | [diff] [blame] | 116 | The negative test function purposely violates the validation rule that the new validation check is looking for. |
| 117 | The test should cause your new validation check to identify the violation and issue a validation error report. |
| 118 | And finally, the test should check that the validation error report is generated and consider the test as "passing" |
| 119 | if the report is received. Otherwise, the test should indicate "failure". |
| 120 | This new test should be added to the validation layer test program in the `tests` directory and contributed |
Mark Lobodzinski | 5ce91da | 2017-05-11 13:54:52 -0600 | [diff] [blame] | 121 | at the same time as the new validation check itself, along with appropriate updates to `layers\vk_validation_error_database.txt`. |
Karl Schultz | 43f4e6d | 2016-04-28 13:55:42 -0600 | [diff] [blame] | 122 | There are many existing validation tests in this directory that can be used as a starting point. |
Mark Lobodzinski | 437e117 | 2018-01-04 14:55:26 -0700 | [diff] [blame] | 123 | * **Validation Checks** The majority of validation checks are carried out by the Core Validation layer. In general, this layer |
| 124 | contains checks that require some amount of application state to carry out. In contrast, the parameter validation layer contains |
| 125 | checks that require (mostly) no state at all. Please inquire if you are unsure of the location for your contribution. The other |
| 126 | layers (threading, object_tracker, unique_objects) are more special-purpose and are mostly code-generated from the specification. |
Karl Schultz | 43f4e6d | 2016-04-28 13:55:42 -0600 | [diff] [blame] | 127 | |
Karl Schultz | 2f21c0c | 2016-02-26 15:42:57 -0700 | [diff] [blame] | 128 | ### **Contributor License Agreement (CLA)** |
| 129 | |
Mark Lobodzinski | 34cd2e6 | 2018-01-04 16:12:41 -0700 | [diff] [blame] | 130 | You will be prompted with a one-time "click-through" CLA dialog as part of submitting your pull request |
Karl Schultz | df03b95 | 2016-05-05 08:30:53 -0600 | [diff] [blame] | 131 | or other contribution to GitHub. |
Karl Schultz | b0b3cac | 2016-03-09 12:48:04 -0700 | [diff] [blame] | 132 | |
| 133 | ### **License and Copyrights** |
| 134 | |
| 135 | All contributions made to the Vulkan-LoaderAndValidationLayers repository are Khronos branded and as such, |
Jon Ashburn | 43b53e8 | 2016-04-19 11:30:31 -0600 | [diff] [blame] | 136 | any new files need to have the Khronos license (Apache 2.0 style) and copyright included. |
Karl Schultz | b0b3cac | 2016-03-09 12:48:04 -0700 | [diff] [blame] | 137 | Please see an existing file in this repository for an example. |
| 138 | |
Jon Ashburn | 43b53e8 | 2016-04-19 11:30:31 -0600 | [diff] [blame] | 139 | All contributions made to the LunarG repositories are to be made under the Apache 2.0 license |
Karl Schultz | b0b3cac | 2016-03-09 12:48:04 -0700 | [diff] [blame] | 140 | and any new files need to include this license and any applicable copyrights. |
| 141 | |
| 142 | You can include your individual copyright after any existing copyrights. |