Contributing to Skia

You can contribute Skia project by reporting problems, submitting patches, etc. Here is how.

Reporting problems

If you find a problem in Skia, you can file a bug for it. The bugs are filed under [http://code.google.com/p/skia/issues/list Google Code issue tracker]. Click "New issue" link on the page, then follow the template. Reproduction code and screenshots of the problem is welcome.

Getting checkout, Building the library and tests

See http://code.google.com/p/skia/wiki/DocRoot

Adding a unit test

If you are willing to change Skia codebase, it's nice to add a test at the same time. Skia has a simple unittest framework so you can add a case to it.

Test code is located under tests directory. Assuming we are adding tests/FooTest.cpp, The test code will look like:

/*
 * ... Apache License here ...
 */
#include "Test.h"

static void TestFoo(skiatest::Reporter* reporter) {
   REPORTER_ASSERT(reporter, 1 + 1 == 2);
}

#include "TestClassDef.h"
DEFINE_TESTCLASS("Foo", FooTestClass, TestFoo)

And we need to add this new file to [http://code.google.com/p/skia/source/browse/trunk/tests/tests_files.mk tests/test_files.mk]. Note that file names are sorted alphabetically.

SOURCE := \
   BitmapCopyTest.cpp \
   ...
   FooTeset.cpp
   ...
   UtilsTest.cpp

Unit tests are best, but if your change touches rendering and you can't think of an automated way to verify the results, consider writing a GM test or a new page of SampleApp. Also, if your change is the GPU code, you may not be able to write it as part of the standard unit test suite, but there are GPU-specific testing paths you can extend.

Submitting a patch

For your code to be accept into the codebase, you must complete the Individual Contributor License Agreement. You can do this online, and it only takes a minute. If you are contributing on behalf of a corporation, you must fill out the Corporate Contributor License Agreement and send it to us as described on that page.

Now that you've made a change and written a test for it, it's ready for the code review! Submit a patch and getting it reviewed is fairly easy with depot tools.

Use either gcl (for svn) or git-cl (for git-svn). Both gcl and git-cl come with depot tools.

• Inside a subversion checkout, you need to use gcl. For help, run gcl help.
• Inside a git checkout, you need to use git-cl. For help, run git-cl help.

Configuring git-cl

Before using any git-cl commands you will need to configure it to point at the correct code review server. This is accomplished with the following command:

git cl config file://`readlink -f codereview.settings`

The readlink is generating the full path to the codereview.settings file. This command should be run from the top level directory of the project. This is the directory that gclient sync creates and it contains the codereview.settings file.

Preparing your change list with GCL

Run gcl help for some documentation.

Make a changelist with the gcl change command . The changelist name is only to help you refer to it on your local computer, so call it whatever you want:

C:\code\src\skia\trunk> gcl change mychange

This will open your text editor. Write the change description at the top of the file. The description should describe what your patch changes and why. This is important for people who are looking at commit logs in the future to track down an issue: they should be able to see why you changed it without going anywhere else. If there's an associated bug file, you should also add a BUG=bug_number line so they can find the associated bug. If you have manual test instructions that you want to pass to the test team so that they can validate the fix, add TEST=how_test_test at the end of the description. If there is no bug or test, leave out the lines. Multiple bugs can be listed, comma separated. Example:

Increase the goat teleporter timeout threshold to 100 because the old
value of 10 caused problems for extremely overweight goats. Tests show
that the largest goat in existence should be teleported in 50ms, so...

BUG=31337,2754
TEST=Try loading an overweight goat and confirm the teleporter works.

Cut and paste the filenames above or below the divider to add files to or remove files from the changelist. You can use the gcl opened command to see your changed files and changelists:

C:\code\src\skia\trunk> gcl opened
M       src\core\SkMatrix.cpp       ← this is a modified file not in any changelist

--- Changelist mychange:
M       src\core\SkCanvas.cpp

Preparing your change list with GIT-CL

Creating a change in git is really just creating a branch.

git checkout -t -b mychange master
echo "This describes the goat teleporter." > GOATS
git add GOATS
git commit

Find a reviewer

Ideally, the reviewer is someone who is familiar with the area of code you are touching. If you have doubts, look at the svn blame or git blame for the file to see who else has been editing it.

Upload your change to Rietveld

Rietveld is an open source code review tool.

GCL

Use the gcl upload command:

C:\code\src\skia\trunk> gcl upload mychange
Issue created. URL: http://codereview.appspot.com/123456
Uploading src\core\SkMatrix.cpp (1/1)

You may have to enter a Google Account username and password, for example your Gmail credentials but it's not required to be a Gmail account. The URL it gives you is the page where the review can be accessed. If you want to upload a new copy of your patch, optionally add or remove files using gcl change and upload it again using the same gcl upload command.

Once you've uploaded your change and know its patch URL, add that to the bug comments. That way someone coming along later can make comments or see how the bug was fixed.

GIT-CL

Use the git cl upload command:

git cl upload

Request review

Go to the supplied URL or go to the code review page and click Issues created by me. Select the change you want to submit for review and click Edit Issue. Enter at least one reviewer's email address and click Update Issue. Now click on Publish+Mail Comments, add any optional notes, and send your change off for review. Unless you publish your change, no one will know to look at it.

Note: If you don't see editing commands on the review page, click Log In in the upper right. Hint: You can add -r reviewer@example.com --send-mail to send the email directly when uploading a change in both gcl and git-cl.

The review process

If you submit a giant patch, or do a bunch of work without discussing it with the relevant people, you may have a hard time convincing anyone to review it!

Code reviews are an important part of the engineering process. The reviewer will almost always have suggestions or style fixes for you, and it's important not to take such suggestions personally or as a commentary on your abilities or ideas. This is a process where we work together to make sure that the highest quality code gets submitted!

You will likely get email back from the reviewer with comments. Fix these and update the patch set in the issue by uploading again. The upload will explain that it is updating the current CL and ask you for a message explaining the change.

GCL

Add any new files that need to be added to your checkout

gcl change changelist_name

gcl upload changelist_name

GIT-CL

If you need to update code the code on an already uploaded CL, simply edit the code, commit it again locally, and then run git cl upload again e.g.

echo "" > GOATS
git add GOATS
git commit -m 'add newline fix to GOATS'
git cl upload

Once you're ready for another review, use Publish+Mail Comments again to send another notification (it is helpful to tell the review what you did with respect to each of their comments). When the reviewer is happy with your patch, they will say "LGTM" ("Looks Good To Me").

Note: As you work through the review process, both you and your reviewers should converse using the code review interface, and send notes using Publish+Mail Comments.

Final Testing

Skia's principal downstream user is Chromium, and any change to Skia rendering output can break Chromium. If your change alters rendering in any way, you are expected to test for and alleviate this. (You may be able to find a Skia team member to help you, but the onus remains on each individual contributor to avoid breaking Chrome.

Evaluating Impact on Chromium

You will need to:

1. check out the Chromium source tree (chromium.org)
2. make BUILDTYPE=Release DumpRenderTree
3. run the layout tests (src/webkit/tools/layout_tests/run_webkit_tests.sh) and keep track of which tests are initially unexpectedly broken
4. apply your patch to src/third_party/skia
5. copy any gyp changes you've made to src/skia/skia.gyp
6. make BUILDTYPE=Release DumpRenderTree
7. run the layout tests (src/webkit/tools/layout_tests/run_webkit_tests.sh) and look for any new broken tests

Avoiding Impact on Chromium

If the second layout test run shows any new breaks, your patch is at fault. If your patch is correct, there are three approaches you can take to avoid immediate impact on Chromium and WebKit:

1. add suppressions for the broken tests to the WebKit test_expectations.txt (http://trac.webkit.org/wiki/TestExpectations) and wait until those changes roll into Chromium - typically only a few days
2. add suppressions for the broken tests to the chromium 'downstream' test_expectations.txt
3. protect your code in Skia with an #ifdef; Skia may default to your new code, but add an appropriate #define to Chromium's src/skia/skia.gyp to take the old code path

If you don't do any of these, and we are unable to roll an updated Skia into Chromium (which may happen daily, so one of these must be done at the same time you check in your changes to Skia), you will be expected to revert your changes to Skia until you can. Our policy is that if a critical security fix comes in we should be able to roll Skia into Chromium with absolutely minimal delay or dependence on any individual committer.

Doing any of these three is a temporary fix: over the longer term, you are responsible for arranging for whatever Chromium or WebKit changes are required to use your new code in Skia.

Check in your changes

GCL

gcl commit changelist_name

Resist the temptation to just run svn commit, since (a) this will by default submit everything modified and not just your changelist, and (b) gcl commit will automatically create a log message based on your issue description and update the code review with the commit revision.

GIT-CL

git-cl will squash all your commits into a single one with the description you used when you uploaded your change.

git cl dcommit

Submitting a patch without depot tools

If you don't want to use depot tools...

Skia is using [Rietveld Code Review Tool. And upload.py script will help you to upload the patch for review.

$svn add tests/FooTest.cpp
$wget http://codereview.appspot.com/static/upload.py
$python upload.py
... Follow the instruction ...

By uploading the patch, you will get the review page URL (like http://codereview.appspot.com/3443041/). To ping the project committers, you can edit the review page and add "CC" or "Reviewers" entry.

Once the review is approved, you can ask the reviewer to commit your patch.

See upload.py manual or [http://code.google.com/p/rietveld/wiki/CodeReviewHelp Rietveld help page] for more detail.

• TODO: Add build instruction for Mac/XCode
• TODO: Pointing out the coding standard.
• TODO: Add git-svn usage.