Skip to content

Latest commit

 

History

History
495 lines (353 loc) · 17.5 KB

CONTRIBUTING.md

File metadata and controls

495 lines (353 loc) · 17.5 KB
Raw

Contributing to the GFXReconstruct Project

Guidelines for contributing to the GFXReconstruct project.

Creative Commons

Copyright © 2018-2024 LunarG, Inc.

Index

  1. Introduction
  2. License and Copyrights
  3. Work In Forks
  4. Creating A Branch For Work
    1. Updating Your Dev Branch
    2. Issue Fix
    3. New Feature or Improvement
    4. Shared Forks
  5. Coding Guidelines
    1. GFXR Do's
    2. GFXR Don'ts
    3. C++ Styling
    4. Python Styling
    5. Commit Message Format
  6. Testing Changes
  7. Before Submission
    1. Rebase on Dev
  8. Submitting Changes
    1. Contributor License Agreement
  9. Finishing the PR
  10. Platform-specific ClangFormat Installation

Introduction

Although the GFXReconstruct project is under active development, external contributions are always welcome.

Open issues and available tasks can can be found in the project issues list.

When working on changes that are not already in the issues list, please consider creating a new issue to avoid duplication of effort, and feel free to contact any of the project developers should you wish to coordinate further.

Repository Issue labels:

  • Bug: These issues refer to invalid or broken functionality and are the highest priority.
  • Enhancement: These issues refer to tasks for extending or improving the GFXReconstruct software.

If you would like to work on an issue that is already assigned, please coordinate with the current assignee.


License and Copyrights

All contributions made to the GFXReconstruct repository are LunarG branded and as such, any new files must have the MIT license and copyright included. Please see an existing file in this repository for an example.

You can include your individual copyright after any existing copyrights.


Work In Forks

For PR development, the GFXReconstruct project does not allow commits directly into the main repository.

Instead, all developers are expected to create a personal fork of the LunarG/gfxreconstruct repo, commit their changes to that repository during development, and only when the changes are done should a pull request be created to pull the source into the main repository.

After creating the fork, pull down the source and add the original LunarG/gfxreconstruct repo as a remote with the name upstream:

git remote add upstream git@github.com:LunarG/gfxreconstruct.git

Adding this upstream remote will allow updating your tree with newer source from the original repo with ease.


Creating A Branch For Work

GFXR has some guidelines with regards to branch names when performing work. These guidelines suggest that every branch be named with a clear descriptive name indicating the purpose of the branch.

Each branch should be based off of the dev branch which is where all active work is eventually submitted to.

Updating Your Dev Branch

Before creating your branch, it is recommended that you pull down all recent changes to your fork's dev branch from the main repository.

First, make sure you are in your dev branch on your local machine

git checkout dev

Next, fetch all the branch info from the server:

git fetch --all --prune

Then you will need to merge in the changes from the LunarG/gfxreconstruct repository (mine remote pointing to that repo is called upstream):

git merge upstream/dev

You may have to update submodules as part of this change if the branch has update Vulkan-Headers or some other important repository. So to be safe, just re-update all submodules:

git submodule update

Then push your changes to your Github fork:

git push origin dev

Now you can freely create your branch

Branch Naming

Naming your branches a clear name will also help to identify what changes are present in a branch just by looking at the availabe branches on your local machine or a remote repo. Because of this, we suggest naming in the following fashion:

Issue Fix

If fixing an issue, create a branch based on the issue number, like fix-1234 where 1234 is the issue number in Github.

New Feature or Improvement

If adding a new feature or cleaning up existing changes, provide a descriptive branch name.

If the change fixes a race condition in pipeline submit, it could be named fix-pipeline-submit-race-condition. Even if the name conflicts with another developer's branch, the branch resides on your fork and appears to the main Github repo as (fork):(branch).

Shared Forks

If sharing a fork with multiple people (for example all from the same company), it might be useful to prepend the branch with your user nam.

For example, instead of fix-1234 one might create a branch bob-fix-1234 to differentiate the work from other people working in the repo.


Coding Guidelines

GFXR Do's

  1. Use GFXRECON_ASSERT and not assert
  2. Explicitly compare pointer values against nullptr For example: if (pointer_variable == nullptr)
  3. Avoid editing code unrelated to the new functionality or bugfix.

GFXR Don'ts

  1. Don’t hand-edit C++ headers or implementation files in framework/generated. To change those files, edit the Python generator scripts and run the generator as noted in Rebase on Dev

  2. Don't perform unnecessary work in your change (like performing additional cleanup beyond your change).

  3. Do not alter existing capture file block structs or IDs. Instead, create new IDs and structs, and deprecate the old blocks so they continue to be decoded and consumed.

    • Add a new metacommand to format.h, (e.g. kCreateHardwareBufferCommand2)
    • Change capture to write the new metacommand
    • Rename the old metacommand to ensure developers writing the command are notified and add documentation to the file at the deprecated name change.
    • Add code to handle the new metacommand on replay
    • Add a note to replay of the deprecated metacommand if it indicates limited or incorrect operation

C++ Styling

Changes to the GFXReconstruct project's C++ code should conform to the coding style defined by the Google C++ Style Guide.

C++ Code formatting is managed with a custom ClangFormat configuration file. This is the .clang-format file found at the base of the repo tree. It is intended for use with ClangFormat version 9.0.1 (see Platform-specific ClangFormat Installation for instructions on installing this version for your particular platform)

C++ code formatting can be applied to pending changes with the clang-format or git clang-format commands.

Here's an example of the workflow using clang-format to clean up formating issues in code changes:

# Make changes to the source.

# Add the changes to git's staging area
$ git add -u .

# Run clang-format on the files in the staging area
# any changes will appear in the unstaged portion of git
$ git clang-format

# Check for changes applied by clang-format, and if so:
$ git add -u .

$ git commit

Verifying Changes with the Build Script

For desktop, the Python 3 scripts\build.py script can be used to verify changes before they are committed. By default, the script performs a pre-build step to verify that the formatting of uncommitted changes adheres to the project's ClangFormat style.

The build script also has an option to apply clang-format to project files before build. Run the script with the -h option for additional usage information.

Python Styling

Changes to the GFXReconstruct project's Python code should conform to the coding style defined by PEP 8

Python code formatting may be automatically applied with yapf, based on the rules specified in the repository's .style.yapf file, using the following command:

# Apply formatting to files in place.
$ yapf -i <files>

# Apply formatting to all python files in this path.
$ yapf -i -r <path>

Commit Message Format

Keep commit message summary (the first line) to 50 characters and format the remainder of the message to 72-characters.

  • This allows the information to display correctly in git/Github logs

  • Because a 50-character commit summary works well with git log --oneline --graph on an 80-column window, e.g.:

    Example Commit

Separate subject from body with a blank line

  • Wrap the body at 72 characters
  • Capitalize the subject line
  • Do not end the subject line with a period
  • Use the body to explain what and why vs. how
  • Use the imperative mode in the subject line; this just means to write it as a command (e.g. Fix the sprocket)
  • Don't mention any proprietary application titles in the commits

Strive for commits that implement a single or related set of functionality, using as many commits as is necessary (more is better). That said, please ensure that the repository compiles and passes tests without error for each commit in your pull request.

Here's an example of a good commit message:

Fix `OverrideAllocateMemory` for trimmed traces
    
When capturing a replay of a capture and `vkAllocateMemory` is called
with `VkMemoryOpaqueCaptureAddressAllocateInfo` already in the `pNext`
chain, the precedent behavior was to "add" another
VkMemoryOpaqueCaptureAddressAllocateInfo` in front of it.

Of course there are always exceptions. Prefer clarity in the history over pedantically keeping under limits.


Testing Changes

To best test any changes, a variety of applications should be captured and replayed via GFXReconstruct. If a new feature (or a broken feature) is known to be resolved by the changes being submitted, attempt to find or create a sample application that utilizes it appropriately.

It is recommended to capture and replay one or more Vulkan applications out of the following repositories:

If you modified one of the other tools, such as gfxrecon-info or gfxrecon-convert, make sure to generate the output and compare before and after snapshots.

Verify your changes on whatever OS/platforms you can.

NOTE The automated internal testing will verify on all platforms, but it would be best for you to do your own verification prior to submitting the changes to reduce the chance of failed automated testing during PR submission.


Before Submission

Do not submit without testing!

  • Run git clang-format and commit the changes and push before making the PR. (There’s no real danger to the build if you forget; GitHub Actions CI will fail on the PR until clang-format is applied anyway.)
  • Squash unnecessary commits
    • Squash commits that are simple things (i.e. commits like “apply clang-format” or “clean up”).
    • Squash pairs of commits that introduce a change and revert the change.
    • Multiple commits within a single PR are encouraged if the PR contains multiple sub-functions that make sense to be represented in the Git log.

Rebase on Dev

Since the dev branch may have changed since you started your branch, prior to submission you should update your fork's dev branch (as mentioned in Updating Your Dev Branch above).

Once this is done, rebase your working branch on the updated dev branch and also update submodules just in case:

git rebase dev
git submodule update

If this results in a submodule update and pulls in a new version of the Vulkan Headers at external/Vulkan-Headers or your branch has touched Python files related to Vulkan code generation, you may need to run the Python 3 code generator to regenerate some Vulkan component sources.

To regenerate generated source for Vulkan, cd to framework/generated and run:

python3 generate_vulkan.py

If you are attempting to update support for the DirectX headers or your branch has touched Python files related to DirectX code generation, you may need to run the Python 3 code generator to regenerate some Vulkan component sources.

To regenerate generated source for DirectX 12, cd to framework/generated and run:

python3 generate_dx12.py

NOTE The minimum supported Python version is 3.10.

NOTE On some systems, e.g. Windows, the Python 3 executable may be named just python.


Submitting Changes

Changes to the GFXReconstruct project should be made in a branch created off of thedev branch (as mentioned in Creating a Branch For Work).

When creating a Pull Request:

  • Make sure that base repository is set to the LunarG/gfxreconstruct repository
  • Make sure the base branch is set to dev
  • Update the head repository to point to your fork
  • Update compare to point to your branch.

It should look something like this:

Example PR Creation

Also make sure the PR title clearly states the purpose of the issue, like Fix crash in vulkaninfo or Add tracking for all types derived from Wrapper.

  • If a PR is submitted in order to share and receive comments and run CI before the PR is submitted for final approvals:
    • Add WIP - in front of the title of the PR
    • Mark it as Draft in Github

Make sure the description is also clear.

  • If it is targeting a specific platform, say which one (Linux, Android, etc)
  • In the case of an issue fix, put the issue number being fixed in the final commit message or at least in the PR description so the Github issue and PR are linked, and the issue is updated when the PR is merged. It has to be a particular phrasing to match the pattern to link the PR with the issue, like fixes #341 or Fixes #341; see the Github page on this.

If your change modified something such as gfxrecon-convert or gfxrecon-info:

  • Include a cut-and-paste of output before and after for review.
  • Verify that the JSON output validate correctly e.g. through the JSON tool jq

Contributor License Agreement (CLA)

The first time you submit to the GFXReconstruct repository, you will be prompted with a one-time "click-through" CLA dialog as part of submitting your pull request or other contribution to GitHub.


Finishing the PR

  • Did relevant test cases get run by hand against the PR?
  • Did Github Workflows CI pass?
    • This is the "checks passed" or "checks failed" section at the bottom of the PR
    • It represents whether the automated build in a container succeeded.
    • This set of “CI builds” includes no replay of capture files. Github Workflows CI Screenshot
  • Did the extended LunarG CI pass?
    • LunarG runs additional tests on our own machine and posts the results to the PR.
    • This will appear in the Conversation part of the PR and appear as comments made. LunarG CI Screenshot
  • Have you responded and/or resolved all code-review feedback?

Platform-specific ClangFormat Installation

The following is a collection of notes for obtaining ClangFormat version 9.0.1 on various platforms.

Visual Studio

  • Visual Studio 2019 has built-in support for ClangFormat version 9.
  • Visual Studio 2017 has built-in support for an older version of ClangFormat, but can be changed to use a separately installed ClangFormat version 9 by following the instructions described here:
    • Under Tools->Options..., expand Text Editor > C/C++ > Formatting. At the bottom is a checkbox for Use custom clang-format.exe file. Select this, and browse to the location of the 9.0.1 version of clang-format.exe that was installed separately.

Ubuntu

For Ubuntu 20, clang-format-9 is the default provided by the package manager.

For earlier versions of Ubuntu, the required version of clang-format can be obtained through the LLVM toolchain repository by following the instructions described here:

On any version of Ubuntu, continue with:

sudo apt update
sudo apt-get install clang-format-9 clang-tidy-9

Configure clang-format and clang-tidy so that the new version is used by default:

sudo update-alternatives --install /usr/bin/clang-format clang-format /usr/bin/clang-format-9 900
sudo update-alternatives --install /usr/bin/clang-tidy clang-tidy /usr/bin/clang-tidy-9 900