| 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).
|
| +
|
|
|