aboutsummaryrefslogtreecommitdiffhomepage
path: root/site/dev/contrib/style.md
blob: 455eed6b38052b031d7885d8095e8cd384c89305 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
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).

<span id="no-define-before-sktypes"></span>
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).