Index: site/dev/contrib/style.md |
diff --git a/site/dev/contrib/style.md b/site/dev/contrib/style.md |
new file mode 100644 |
index 0000000000000000000000000000000000000000..dd39a734a80f9195d5867c002cbd3b3bf171b396 |
--- /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). |
+ |