aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar hcm <hcm@google.com>2015-01-21 12:05:47 -0800
committerGravatar Commit bot <commit-bot@chromium.org>2015-01-21 12:05:47 -0800
commitfd1ad48d4d9073b90f58e60219637046a8446267 (patch)
tree4730c1339e461c7aca49190a9b48afb71ec4a7c6
parentec7e12f3cf240a85a2409205e135c24256233733 (diff)
Add Contributing to Skia section of docs
-rw-r--r--site/dev/contrib/cqkeywords.md96
-rw-r--r--site/dev/contrib/flatten.md88
-rw-r--r--site/dev/contrib/index.md48
-rw-r--r--site/dev/contrib/patch.md72
-rw-r--r--site/dev/contrib/revert.md18
-rw-r--r--site/dev/contrib/style.md559
-rw-r--r--site/dev/contrib/submit.md248
7 files changed, 1129 insertions, 0 deletions
diff --git a/site/dev/contrib/cqkeywords.md b/site/dev/contrib/cqkeywords.md
new file mode 100644
index 0000000000..6c834e235e
--- /dev/null
+++ b/site/dev/contrib/cqkeywords.md
@@ -0,0 +1,96 @@
+Commit Queue Keywords
+=====================
+
+COMMIT
+------
+
+If you want to test your CL through the commit queue but are not ready to commit
+the changes yet, you can add the following line to the CL description:
+
+ COMMIT=false
+
+The CQ will run through its list of verifiers (reviewer check, trybots, tree check,
+presubmit check), and will close the issue instead of committing it.
+
+ CQ_INCLUDE_TRYBOTS
+
+Allows you to add arbitrary trybots to the CQ's list of default trybots.
+The CQ will block till these tryjobs pass just like the default list of tryjobs.
+
+This is the format of the values of this keyword:
+
+ CQ_INCLUDE_TRYBOTS=master1:bot1,bot2;master2:bot3,bot4
+
+Here are some real world examples:
+
+ CQ_INCLUDE_TRYBOTS=tryserver.chromium:linux_layout_rel
+
+ CQ_INCLUDE_TRYBOTS=tryserver.skia:Build-Ubuntu13.10-GCC4.8-NaCl-Release-Trybot
+
+ CQ_EXCLUDE_TRYBOTS
+
+Allows you to remove trybots from the CQ's list of default trybots. Should only be
+used when particular builders are failing for reasons unrelated to your code changes.
+
+This is the format of the values of this keyword:
+
+ CQ_EXCLUDE_TRYBOTS=master1:bot1,bot2;master2:bot3,bot4
+
+Here are some real world examples:
+
+ CQ_EXCLUDE_TRYBOTS=tryserver.chromium:win_chromium_compile_dbg
+
+ CQ_EXCLUDE_TRYBOTS=tryserver.skia:Build-Win7-VS2010-x86-Debug-Trybot
+
+ CQ_TRYBOTS
+
+Allows you to list every trybot that you want to run for your CL.
+
+This is the format of the values of this keyword:
+
+ CQ_TRYBOTS=master1:bot1,bot2;master2:bot3,bot4
+
+Here are some real world examples:
+
+ CQ_TRYBOTS=tryserver.chromium:linux_chromium_gn_rel,linux_chromium_chromeos_rel,
+ android_dbg_triggered_tests,android_dbg,mac_chromium_rel,win_chromium_x64_rel
+
+ CQ_TRYBOTS=tryserver.skia:Build-Win7-VS2010-x86-Debug-Trybot,
+ Test-Ubuntu13.10-ShuttleA-NoGPU-x86_64-Debug-Trybot,
+ Build-Ubuntu13.10-GCC4.8-x86_64-Release-Trybot,
+ Build-Ubuntu13.10-Clang-x86_64-Debug-Trybot,Build-Mac10.8-Clang-x86_64-Release-Trybot
+
+TBR
+---
+
+If you are a Skia committer and cannot wait for a review,
+then you can include the TBR keyword in your CL's description.
+
+Example:
+
+ TBR=rmistry@google.com
+
+ NOTREECHECKS
+
+If you want to skip the tree status checks, to make the CQ commit a CL even if the tree is closed,
+you can add the following line to the CL description:
+
+ NOTREECHECKS=true
+
+This is discouraged, since the tree is closed for a reason. However, in rare cases this is acceptable,
+primarily to fix build breakages (i.e., your CL will help in reopening the tree).
+
+ NOPRESUBMIT
+
+If you want to skip the presubmit checks, add the following line to the CL description:
+
+ NOPRESUBMIT=true
+
+NOTRY
+-----
+
+If you cannot wait for the try job results, you can add the following line to the CL description:
+
+ NOTRY=true
+
+The CQ will then not run any try jobs for your change and will commit the CL as soon as the tree is open, assuming the presubmit check passes.
diff --git a/site/dev/contrib/flatten.md b/site/dev/contrib/flatten.md
new file mode 100644
index 0000000000..c06a14b812
--- /dev/null
+++ b/site/dev/contrib/flatten.md
@@ -0,0 +1,88 @@
+Flattenables
+============
+
+Many objects in Skia, such as SkShaders and other effects on SkPaint, need to be
+flattened into a data stream for either transport or as part of the key to the
+font cache. Classes for these objects should derive from SkFlattenable or one of
+its subclasses. If you create a new flattenable class, you need to make sure you
+do a few things so that it will work on all platforms:
+
+1: Override the method flatten (the default scope is protected):
+
+<!--?prettify?-->
+~~~~
+virtual void flatten(SkFlattenableWriteBuffer& buffer) const SK_OVERRIDE {
+ this->INHERITED::flatten(buffer);
+ // Write any private data that needs to be stored to recreate this object
+}
+~~~~
+
+2: Override the (protected) constructor that creates an object from an
+SkFlattenableReadBuffer:
+
+<!--?prettify?-->
+~~~~
+SkNewClass(SkFlattenableReadBuffer& buffer)
+: INHERITED(buffer) {
+ // Read the data from the buffer in the same order as it was written to the
+ // SkFlattenableWriteBuffer and construct the new object
+}
+~~~~
+
+3: Declare a set of deserialization procs for your object in the class declaration:
+We have a macro for this:
+
+<!--?prettify?-->
+~~~~
+public:
+
+SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkNewClass)
+~~~~
+
+4: If your class is declared in a .cpp file or in a private header file, create a
+function to register its group:
+This occurs in cases where the classes are hidden behind a factory, like many effects
+and shaders are. Then in the parent class header file (such as SkGradientShader) you
+need to add:
+
+<!--?prettify?-->
+~~~~
+public:
+
+SK_DECLARE_FLATTENABLE_REGISTRAR_GROUP()
+~~~~
+
+Then in the cpp file you define all the members of the group together:
+
+<!--?prettify?-->
+~~~~
+SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_START(SkGroupClass)
+
+ SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkMemberClass1)
+
+ SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkMemberClass2)
+
+ // etc
+
+SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_END
+~~~~
+
+
+5: Register your flattenable with the global registrar:
+You need to add one line to SkFlattenable::InitalizeFlattenables(). To register the
+flattenable in a Skia build, that function is defined in SkGlobalInitialization_default.cpp.
+For Chromium, it is in SkGlobalInitialization_chromium.cpp.
+For a single flattenable add
+
+<!--?prettify?-->
+~~~~
+SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkNewClass)
+~~~~
+
+For a group, add
+
+<!--?prettify?-->
+~~~~
+SkGroupClass::InitializeFlattenables();
+~~~~
+
diff --git a/site/dev/contrib/index.md b/site/dev/contrib/index.md
new file mode 100644
index 0000000000..f1c730584d
--- /dev/null
+++ b/site/dev/contrib/index.md
@@ -0,0 +1,48 @@
+Contributing to Skia
+====================
+
+Here some ways you can get involved and help us improve Skia.
+
+
+Report Bugs
+-----------
+
+Find bugs to fix or report new bugs in the [Skia issue tracker](http://code.google.com/p/skia/issues/list).
+You can also search the [Chromium issue tracker](http://code.google.com/p/chromium/issues/list) for bugs related to graphics or Skia.
+
+Test
+----
+
+Write an application or tool that will exercise the Skia code differently than our
+current set of tests and verify that Skia works as expected. Draw something
+interesting and profile it to find ways to speed up Skia's implementation.
+We cannot always fix issues or support every scenario, but we welcome any bugs
+found so we can assess and prioritize them. (If you find _and_ fix a bug, even better!)
+
+Contribute Code
+---------------
+
+Whether you develop a new feature or a fix for an existing bug in the Skia code base,
+you will need a committer to review and approve the change. There are some steps that
+can speed up the review process:
+Keep your code submissions small and targeted.
+When possible, have a fellow contributor review your change in advance of submission.
+Propose new features to the project leads by opening a feature bug or posting to
+skia-discuss ahead of development. For more information, see [How to submit a patch](./submit).
+
+For background on the project and an outline of the types of roles interested parties
+can take on, see [Project Roles](../../roles).
+
+Anyone contributing code to Skia must sign a Contributor License Agreement and ensure
+they are listed in the AUTHORS file:
+Individual contributors can complete the [Individual Contributor License Agreement](https://developers.google.com/open-source/cla/individual) online.
+If you are contributing on behalf of a corporation, fill out the [Corporate Contributor License Agreement](https://developers.google.com/open-source/cla/corporate)
+and send it in as described on that page.
+If it is your first time submitting code or you have not previously done so, add your
+(or your organization's) name and contact info to the [AUTHORS file](https://skia.googlesource.com/skia/+/master/AUTHORS) as a part
+of your CL.
+REVIEWERS: Before you LGTM a change, verify that the contributor is listed in the AUTHORS file.
+If they are not, a Googler must ensure that the individual or their corporation has signed the
+CLA by searching [go/cla-signers](https://goto.google.com/cla-signers).
+Then have an entry added to the AUTHORS file with the CL.
+
diff --git a/site/dev/contrib/patch.md b/site/dev/contrib/patch.md
new file mode 100644
index 0000000000..cf8b35bdeb
--- /dev/null
+++ b/site/dev/contrib/patch.md
@@ -0,0 +1,72 @@
+Applying patches
+================
+
+If you are a Skia committer and have been asked to commit an
+externally-submitted patch, this is how to do it. (This technique is useful in
+other situations too, like if you just want to try out somebody else's patch
+locally.)
+
+Notes:
+ * For the examples below, we will assume that this is the change you want
+ to patch into your local checkout: https://codereview.appspot.com/6201055/
+ * These instructions should work on Mac or Linux; Windows is trickier,
+ because there is no standard Windows "patch" tool.
+
+See also:
+http://dev.chromium.org/developers/contributing-code#TOC-Instructions-for-Reviewer:-Checking-in-the-patch-for-a-non-committer
+
+If you use git cl, then you should be able to use the shortcut:
+
+~~~~
+git cl patch 6201055
+~~~~
+
+If you use gcl, or the above doesn't work, the following should always work.
+
+1. Prepare your local workspace to accept the patch.
+
+ * cd into the root directory (usually trunk/) of the workspace where you
+ want to apply the patch.
+ * Make sure that the workspace is up-to-date and clean (or "updated and
+ clean enough" for your purposes). If the codereview patch was against
+ an old revision of the repo, you may need to sync your local workspace
+ to that same revision...
+
+2. Download the raw patch set.
+
+ * Open the codereview web page and look for the "Download raw patch set"
+ link near the upper right-hand corner. Right-click on that link and copy
+ it to the clipboard. (In my case, the link is
+ https://codereview.appspot.com/download/issue6201055_1.diff )
+ * If you are on Linux or Mac and have "curl" or "wget" installed, you can
+ download the patch from the command line:
+
+ ~~~~
+ curl https://codereview.appspot.com/download/issue6201055_1.diff
+ --output patch.txt
+ # or...
+ wget https://codereview.appspot.com/download/issue6201055_1.diff
+ --output-document=patch.txt
+ ~~~~
+
+ * Otherwise, figure out some other way to download this file and save it as
+ 'patch.txt'
+
+3. Apply this patch to your local checkout.
+
+ * You should still be in the root directory of the workspace where you want
+ to apply the patch.
+
+ ~~~~
+ patch -p1 <patch.txt
+ ~~~~
+
+ * Then you can run diff and visually check the local changes.
+
+4. Complications: If the patch fails to apply, the following may be happening:
+
+ Wrong revision. Maybe your local workspace is not up to date? Or maybe the
+ patch was made against an old revision of the repository, and cannot be applied
+ to the latest revision? (In that case, revert any changes and sync your
+ workspace to an older revision, then re-apply the patch.)
+
diff --git a/site/dev/contrib/revert.md b/site/dev/contrib/revert.md
new file mode 100644
index 0000000000..09665a7742
--- /dev/null
+++ b/site/dev/contrib/revert.md
@@ -0,0 +1,18 @@
+How to revert a CL
+==================
+
+Using one-click revert
+----------------------
+ * Find the codereview issue for the CL you want to revert.
+ * Click the "revert" button.
+
+Using Git
+---------
+ * git checkout master
+ * git svn fetch && git svn rebase && gclient sync
+ * git checkout -t -b <branch_name>
+ * git log
+ * <Find the SHA1 of the commit you want to revert>
+ * git revert <SHA1>
+ * git cl upload
+ * git cl land
diff --git a/site/dev/contrib/style.md b/site/dev/contrib/style.md
new file mode 100644
index 0000000000..dd39a734a8
--- /dev/null
+++ b/site/dev/contrib/style.md
@@ -0,0 +1,559 @@
+Coding Style Guidelines
+=======================
+
+These conventions have evolved over time. Some of the earlier code in both
+projects doesn’t strictly adhere to the guidelines. However, as the code evolves
+we hope to make the existing code conform to the guildelines.
+
+Files
+-----
+
+We use .cpp and .h as extensions for c++ source and header files. We use
+foo_impl.h for headers with inline definitions for class foo.
+
+Headers that aren’t meant for public consumption should be placed in src
+directories so that they aren’t in a client’s search path.
+
+We prefer to minimize includes. If forward declaring a name in a header is
+sufficient then that is preferred to an include.
+
+Forward declarations and file includes should be in alphabetical order (but we
+aren't very strict about it).
+
+Do not use #if/#ifdef before including "SkTypes.h" (directly or indirectly).
+
+We use spaces not tabs (4 of them).
+
+We use Unix style endlines (LF).
+
+We prefer no trailing whitespace but aren't very strict about it.
+
+We wrap lines at 100 columns unless it is excessively ugly (use your judgement).
+The soft line length limit was changed from 80 to 100 columns in June 2012. Thus,
+most files still adhere to the 80 column limit. It is not necessary or worth
+significant effort to promote 80 column wrapped files to 100 columns. Please
+don't willy-nilly insert longer lines in 80 column wrapped files. Either be
+consistent with the surrounding code or, if you really feel the need, promote
+the surrounding code to 100 column wrapping.
+
+Naming
+------
+
+Both projects use a prefix to designate that they are Skia prefix for classes,
+enums, structs, typedefs etc is Sk. Ganesh’s is Gr. Nested types should not be
+prefixed.
+
+<!--?prettify?-->
+~~~~
+class SkClass {
+public:
+ class HelperClass {
+ ...
+ };
+};
+~~~~
+
+Data fields in structs, classes, unions begin with lowercase f and are then
+camel capped.
+
+<!--?prettify?-->
+~~~~
+struct GrCar {
+ ...
+ float fMilesDriven;
+ ...
+};
+~~~~
+
+Globals variables are similar but prefixed with g and camel-capped
+
+<!--?prettify?-->
+~~~~
+bool gLoggingEnabled
+Local variables begin lowercases and are camel-capped.
+
+int herdCats(const Array& cats) {
+ int numCats = cats.count();
+}
+~~~~
+
+Enum values are prefixed with k and have post fix that consists of an underscore
+and singular name of the enum name. The enum itself should be singular for
+exclusive values or plural for a bitfield. If a count is needed it is
+k<singular enum name>Count and not be a member of the enum (see example):
+
+<!--?prettify?-->
+~~~~
+enum SkPancakeType {
+ kBlueberry_PancakeType,
+ kPlain_PancakeType,
+ kChocolateChip_PancakeType,
+
+ kLast_PancakeType = kChocolateChip_PancakeType
+};
+
+static const SkPancakeType kPancakeTypeCount = kLast_PancakeType + 1;
+~~~~
+
+A bitfield:
+
+<!--?prettify?-->
+~~~~
+enum SkSausageIngredientBits {
+ kFennel_SuasageIngredientBit = 0x1,
+ kBeef_SausageIngredientBit = 0x2
+};
+~~~~
+
+or:
+
+<!--?prettify?-->
+~~~~
+enum SkMatrixFlags {
+ kTranslate_MatrixFlag = 0x1,
+ kRotate_MatrixFlag = 0x2
+};
+~~~~
+
+Exception: anonymous enums can be used to declare integral constants, e.g.:
+
+<!--?prettify?-->
+~~~~
+enum { kFavoriteNumber = 7 };
+~~~~
+
+Macros are all caps with underscores between words. Macros that have greater
+than file scope should be prefixed SK or GR.
+
+Static non-class functions in implementation files are lower case with
+underscores separating words:
+
+<!--?prettify?-->
+~~~~
+static inline bool tastes_like_chicken(Food food) {
+ return kIceCream_Food != food;
+}
+~~~~
+
+Externed functions or static class functions are camel-capped with an initial cap:
+
+<!--?prettify?-->
+~~~~
+bool SkIsOdd(int n);
+
+class SkFoo {
+public:
+ static int FooInstanceCount();
+};
+~~~~
+
+Macros
+------
+
+Ganesh macros that are GL-specific should be prefixed GR_GL.
+
+<!--?prettify?-->
+~~~~
+#define GR_GL_TEXTURE0 0xdeadbeef
+~~~~
+
+Ganesh prefers that macros are always defined and the use of #if MACRO rather than
+#ifdef MACRO.
+
+<!--?prettify?-->
+~~~~
+#define GR_GO_SLOWER 0
+...
+#if GR_GO_SLOWER
+ Sleep(1000);
+#endif
+~~~~
+
+Skia tends to use #ifdef SK_MACRO for boolean flags.
+
+Braces
+------
+
+Open braces don’t get a newline. “else” and “else if” appear on same line as
+opening and closing braces unless preprocessor conditional compilation
+interferes. Braces are always used with if, else, while, for, and do.
+
+<!--?prettify?-->
+~~~~
+if (...) {
+ oneOrManyLines;
+}
+
+if (...) {
+ oneOrManyLines;
+} else if (...) {
+ oneOrManyLines;
+} else {
+ oneOrManyLines;
+}
+
+for (...) {
+ oneOrManyLines;
+}
+
+while (...) {
+ oneOrManyLines;
+}
+
+void function(...) {
+ oneOrManyLines;
+}
+
+if (!error) {
+ proceed_as_usual();
+}
+#if HANDLE_ERROR
+else {
+ freak_out();
+}
+#endif
+~~~~
+
+Flow Control
+------------
+
+There is a space between flow control words and parentheses and between
+parentheses and braces:
+
+<!--?prettify?-->
+~~~~
+while (...) {
+}
+
+do {
+} while(...);
+
+switch (...) {
+...
+}
+~~~~
+
+Cases and default in switch statements are indented from the switch.
+
+<!--?prettify?-->
+~~~~
+switch (color) {
+ case kBlue:
+ ...
+ break;
+ case kGreen:
+ ...
+ break;
+ ...
+ default:
+ ...
+ break;
+}
+~~~~
+
+Fallthrough from one case to the next is commented unless it is trivial:
+
+<!--?prettify?-->
+~~~~
+switch (recipe) {
+ ...
+ case kCheeseOmelette_Recipe:
+ ingredients |= kCheese_Ingredient;
+ // fallthrough
+ case kPlainOmelette_Recipe:
+ ingredients |= (kEgg_Ingredient | kMilk_Ingredient);
+ break;
+ ...
+}
+~~~~
+
+When a block is needed to declare variables within a case follow this pattern:
+
+<!--?prettify?-->
+~~~~
+switch (filter) {
+ ...
+ case kGaussian_Filter: {
+ Bitmap srcCopy = src->makeCopy();
+ ...
+ break;
+ }
+ ...
+};
+~~~~
+
+Classes
+-------
+
+Unless there is a need for forward declaring something, class declarations
+should be ordered public, protected, private. Each should be preceded by a
+newline. Within each visibility section (public, private), fields should not be
+intermixed with methods.
+
+<!--?prettify?-->
+~~~~
+class SkFoo {
+
+public:
+ ...
+
+protected:
+ ...
+
+private:
+ SkBar fBar;
+ ...
+
+ void barHelper(...);
+ ...
+};
+~~~~
+
+Subclasses should have a private typedef of their super class called INHERITED:
+
+<!--?prettify?-->
+~~~~
+class GrDillPickle : public GrPickle {
+ ...
+private:
+ typedef GrPickle INHERITED;
+};
+~~~~
+
+Virtual functions that are overridden in derived classes should use SK_OVERRIDE
+(and not the override keyword). The virtual keyword can be omitted.
+
+<!--?prettify?-->
+~~~~
+void myVirtual() SK_OVERRIDE {
+}
+~~~~
+
+This should be the last element of their private section, and all references to
+base-class implementations of a virtual function should be explicitly qualified:
+
+<!--?prettify?-->
+~~~~
+void myVirtual() SK_OVERRIDE {
+ ...
+ this->INHERITED::myVirtual();
+ ...
+}
+~~~~
+
+As in the above example, derived classes that redefine virtual functions should
+use SK_OVERRIDE to note that explicitly.
+
+Constructor initializers should be one per line, indented, with punctuation
+placed before the initializer. This is a fairly new rule so much of the existing
+code is non-conforming. Please fix as you go!
+
+<!--?prettify?-->
+~~~~
+GrDillPickle::GrDillPickle()
+ : GrPickle()
+ , fSize(kDefaultPickleSize) {
+ ...
+}
+~~~~
+
+Constructors that take one argument should almost always be explicit, with
+exceptions made only for the (rare) automatic compatibility class.
+
+<!--?prettify?-->
+~~~~
+class Foo {
+ explicit Foo(int x); // Good.
+ Foo(float y); // Spooky implicit conversion from float to Foo. No no no!
+ ...
+};
+~~~~
+
+Method calls within method calls should be prefixed with dereference of the
+'this' pointer. For example:
+
+<!--?prettify?-->
+~~~~
+this->method();
+Memory Managemt
+~~~~
+
+All memory allocation should be routed through SkNEW and its variants. These are
+#defined in SkPostConfig.h, but the correct way to get access to the config
+system is to #include SkTypes.h, which will allow external users of the library
+to provide a custom memory manager or other adaptations.
+
+<!--?prettify?-->
+~~~~
+SkNEW(type_name)
+SkNEW_ARGS(type_name, args)
+SkNEW_ARRAY(type_name, count)
+SkNEW_PLACEMENT(buf, type_name)
+SkNEW_PLACEMENT_ARGS(buf, type_name, args)
+SkDELETE(obj)
+SkDELETE_ARRAY(array)
+~~~~
+
+Comparisons
+-----------
+
+We prefer that equality operators between lvalues and rvalues place the lvalue
+on the right:
+
+<!--?prettify?-->
+~~~~
+if (7 == luckyNumber) {
+ ...
+}
+~~~~
+
+However, inequality operators need not follow this rule:
+
+<!--?prettify?-->
+~~~~
+if (count > 0) {
+ ...
+}
+~~~~
+
+Comments
+
+We use doxygen-style comments.
+
+For grouping or separators in an implementation file we use 80 slashes
+
+<!--?prettify?-->
+~~~~
+void SkClassA::foo() {
+ ...
+}
+
+////////////////////////////////////////////////////////////////
+
+void SkClassB::bar() {
+ ...
+}
+~~~~
+
+Integer Types
+-------------
+
+We follow the Google C++ guide for ints and are slowly making older code conform to this
+
+(http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Integer_Types)
+
+Summary: Use int unless you have need a guarantee on the bit count, then use
+stdint.h types (int32_t, etc). Assert that counts, etc are not negative instead
+of using unsigned. Bitfields use uint32_t unless they have to be made shorter
+for packing or performance reasons.
+
+NULL, 0
+-------
+
+Use NULL for pointers, 0 for ints. We prefer explicit NULL comparisons when
+checking for NULL pointers (as documentation):
+
+<!--?prettify?-->
+~~~~
+if (NULL == x) { // slightly preferred over if (x)
+ ...
+}
+~~~~
+
+When checking non-NULL pointers explicit comparisons are not required because it
+reads like a double negative:
+
+<!--?prettify?-->
+~~~~
+if (x) { // slightly preferred over if (NULL != x)
+ ...
+}
+~~~~
+
+Returning structs
+-----------------
+
+If the desired behavior is for a function to return a struct, we prefer using a
+struct as an output parameter
+
+<!--?prettify?-->
+~~~~
+void modify_foo(SkFoo* foo) {
+ // Modify foo
+}
+~~~~
+
+Then the function can be called as followed:
+
+<!--?prettify?-->
+~~~~
+SkFoo foo;
+modify_foo(&foo);
+~~~~
+
+This way, if return value optimization cannot be used there is no performance
+hit. It also means that modify_foo can actually return a boolean for whether the
+call was successful. In this case, initialization of foo can potentially be
+skipped on failure (assuming the constructor for SkFoo does no initialization).
+
+<!--?prettify?-->
+~~~~
+bool modify_foo(SkFoo* foo) {
+ if (some_condition) {
+ // Modify foo
+ return true;
+ }
+ // Leave foo unmodified
+ return false;
+}
+~~~~
+
+Function Parameters
+-------------------
+
+Mandatory constant object parameters are passed to functions as const references
+if they are not retained by the receiving function. Optional constant object
+parameters are passed to functions as const pointers. Objects that the called
+function will retain, either directly or indirectly, are passed as pointers.
+Variable (i.e. mutable) object parameters are passed to functions as pointers.
+
+<!--?prettify?-->
+~~~~
+// src and paint are optional
+void SkCanvas::drawBitmapRect(const SkBitmap& bitmap, const SkIRect* src,
+ const SkRect& dst, const SkPaint* paint = NULL);
+// metrics is mutable (it is changed by the method)
+SkScalar SkPaint::getFontMetrics(FontMetric* metrics, SkScalar scale) const;
+// A reference to foo is retained by SkContainer
+void SkContainer::insert(const SkFoo* foo);
+~~~~
+
+If function arguments or parameters do not all fit on one line, they may be
+lined up with the first parameter on the same line
+
+<!--?prettify?-->
+~~~~
+void drawBitmapRect(const SkBitmap& bitmap, const SkRect& dst,
+ const SkPaint* paint = NULL) {
+ this->drawBitmapRectToRect(bitmap, NULL, dst, paint,
+ kNone_DrawBitmapRectFlag);
+}
+~~~~
+
+or placed on the next line indented eight spaces
+
+<!--?prettify?-->
+~~~~
+void drawBitmapRect(
+ const SkBitmap& bitmap, const SkRect& dst,
+ const SkPaint* paint = NULL) {
+ this->drawBitmapRectToRect(
+ bitmap, NULL, dst, paint, kNone_DrawBitmapRectFlag);
+}
+~~~~
+
+Python
+------
+
+Python code follows the [Google Python Style Guide](http://google-styleguide.googlecode.com/svn/trunk/pyguide.html).
+
diff --git a/site/dev/contrib/submit.md b/site/dev/contrib/submit.md
new file mode 100644
index 0000000000..fcbcf2077d
--- /dev/null
+++ b/site/dev/contrib/submit.md
@@ -0,0 +1,248 @@
+How to submit a patch
+=====================
+
+
+Making changes
+--------------
+
+First create a branch for your changes:
+
+~~~~
+$ git checkout --track origin/master -b my_feature master
+~~~~
+
+After making your changes, create a commit
+
+~~~~
+$ git add [file1] [file2] ...
+$ git commit
+~~~~
+
+If your branch gets out of date, you will need to update it:
+
+~~~~
+$ git pull --rebase
+$ gclient sync
+~~~~
+
+
+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 the 'tests' directory. Assuming we are adding
+tests/FooTest.cpp, The test code will look like:
+
+<!--?prettify?-->
+~~~~
+/*
+ * Copyright ........
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "Test.h"
+
+DEF_TEST(TestFoo, reporter) {
+ int x = 2 * 3;
+ if (x != 6) {
+ ERRORF(reporter, "x should be 6, but is %d", x);
+ return;
+ }
+ REPORTER_ASSERT(reporter, 1 + 1 == 2);
+}
+~~~~
+
+And we need to add this new file to gyp/tests.gyp. Note that file names are
+sorted alphabetically.
+
+<!--?prettify?-->
+~~~~
+'sources': [
+ '../tests/AAClipTest.cpp'
+ '../tests/FooTest.cpp',
+ '../tests/XfermodeTest.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 accepted into the codebase, you must complete the
+[Individual Contributor License
+Agreement](http://code.google.com/legal/individual-cla-v1.0.html). 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](http://code.google.com/legal/corporate-cla-v1.0.html)
+and send it to us as described on that page. Add your (or your organization's)
+name and contact info to the AUTHORS file as a part of your CL.
+
+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 git-cl, which comes with [depot tools](http://sites.google.com/a/chromium.org/dev/developers/how-tos/install-depot-tools).
+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 https://skia.googlesource.com/skia/+/master/codereview.settings
+~~~~
+
+### 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 git blame for the file to see who else
+has been editing it.
+
+### Uploading changes for review
+
+Skia uses Chromium's code review [site](http://codereview.chromium.org) and the
+Rietveld open source code review tool.
+Use git cl to upload your change:
+~~~~
+$ git cl upload
+~~~~
+
+You may have to enter a Google Account username and password to authenticate
+yourself to codereview.chromium.org. A free gmail account will do fine, or any
+other type of Google account. It does not have to match the email address you
+configured using git config --global user.email above, but it can.
+
+The command output should include a URL, similar to
+(https://codereview.chromium.org/111893004/), indicating where your changelist
+can be reviewed.
+
+### 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!
+
+Please follow the guidelines on how to conduct a code review detailed here:
+https://code.google.com/p/rietveld/wiki/CodeReviewHelp
+
+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. Be sure to respond to all comments before you request review of an
+update.
+
+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**.
+
+Once your commit has gone in, you should delete the branch containing your change:
+
+~~~~
+$ git checkout master
+$ git branch -D my_feature
+~~~~
+
+
+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
+
+Keep in mind that Skia is rolled daily into Blink and Chromium. Run local tests
+and watch canary bots for results to ensure no impact. If you are submitting
+changes that will impact layout tests, follow the guides below and/or work with
+your friendly Skia-Blink engineer to evaluate, rebaseline, and land your
+changes.
+
+Resources:
+
+[How to land Skia changes that change Blink layout test results](../chrome/layouttest)
+
+If you're changing the Skia API, you may need to make an associated change in Chromium.
+If you do, please follow these instructions: [Landing Skia changes which require Chrome changes](../chrome/changes)
+
+
+Check in your changes
+---------------------
+
+### Non-Skia-committers
+
+If you already have committer rights, you can follow the directions below to
+commit your change directly to Skia's repository.
+
+If you don't have committer rights in https://skia.googlesource.com/skia.git ...
+first of all, thanks for submitting your patch! We really appreciate these
+submissions. Unfortunately, we don't yet have a way for Skia committers to mark
+a patch as "approved" and thus allow non-committers to commit them. So instead,
+please ask a Skia committer to land your patch for you or land using the commit
+queue.
+
+As part of this process, the Skia committer may create a new codereview
+containing your patch (perhaps with some small adjustments at her discretion).
+If so, you can mark your codereview as "Closed", and update it with a link to
+the new codereview.
+
+### Skia committers:
+ * tips on how to apply the externally provided patch are [here](./patch)
+ * when landing externally contributed patches, please note the original
+ contributor's identity (and provide a link to the original codereview) in the commit message
+
+git-cl will squash all your commits into a single one with the description you used when you uploaded your change.
+
+~~~~
+git cl land
+~~~~
+or
+~~~~
+git cl land -c 'Contributor Name <email@example.com>'
+~~~~