aboutsummaryrefslogtreecommitdiffhomepage
path: root/site/dev/contrib
diff options
context:
space:
mode:
authorGravatar Andrew Monshizadeh <amonshiz@fb.com>2018-01-10 09:55:05 -0500
committerGravatar Skia Commit-Bot <skia-commit-bot@chromium.org>2018-01-11 19:47:58 +0000
commit9d6681cc70095b5be6523873a9de54b02ec65086 (patch)
treebd92d441e53c164e9bfbfb94ee3ade8ba89902fd /site/dev/contrib
parentd75fdc64be5cbbc11660ea4e6e9be4b84e407c79 (diff)
Changes to site documentation
Mostly just formatting fixes with a few grammatical changes. Two real notable changes: - Removed references to SkGLCanvas from Tips & FAQ and replaced with references to `SkDevice` and `SkSurface`. - Deleted deprecated "Quick Start Guides" folder Docs-Preview: https://skia.org/?cl=92361 Bug: skia: Change-Id: Ief790b1c2bae8fe0e39aa8d66c79f80560d18c9e Reviewed-on: https://skia-review.googlesource.com/92361 Reviewed-by: Heather Miller <hcm@google.com> Reviewed-by: Joe Gregorio <jcgregorio@google.com> Commit-Queue: Joe Gregorio <jcgregorio@google.com>
Diffstat (limited to 'site/dev/contrib')
-rw-r--r--site/dev/contrib/flatten.md18
-rw-r--r--site/dev/contrib/patch.md25
-rw-r--r--site/dev/contrib/simd.md18
-rw-r--r--site/dev/contrib/style.md43
-rw-r--r--site/dev/contrib/submit.md20
5 files changed, 63 insertions, 61 deletions
diff --git a/site/dev/contrib/flatten.md b/site/dev/contrib/flatten.md
index 192571e1c7..0b3663f560 100644
--- a/site/dev/contrib/flatten.md
+++ b/site/dev/contrib/flatten.md
@@ -1,13 +1,13 @@
Flattenables
============
-Many objects in Skia, such as SkShaders and other effects on SkPaint, need to be
+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
+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):
+1: Override the method `flatten` (the default scope is protected):
<!--?prettify?-->
~~~~
@@ -18,7 +18,7 @@ virtual void flatten(SkFlattenableWriteBuffer& buffer) const override {
~~~~
2: Override the (protected) constructor that creates an object from an
-SkFlattenableReadBuffer:
+`SkFlattenableReadBuffer`:
<!--?prettify?-->
~~~~
@@ -39,10 +39,10 @@ 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
+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
+and shaders are. Then in the parent class header file (such as `SkGradientShader`) you
need to add:
<!--?prettify?-->
@@ -69,9 +69,9 @@ 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.
+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?-->
diff --git a/site/dev/contrib/patch.md b/site/dev/contrib/patch.md
index cf8b35bdeb..b7b29f06fd 100644
--- a/site/dev/contrib/patch.md
+++ b/site/dev/contrib/patch.md
@@ -7,30 +7,31 @@ 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
+See also [Contributing Code for The Chromium Projects]
+(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:
+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.
+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
+ * 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...
+ to that same revision.
2. Download the raw patch set.
@@ -50,7 +51,7 @@ If you use gcl, or the above doesn't work, the following should always work.
~~~~
* Otherwise, figure out some other way to download this file and save it as
- 'patch.txt'
+ `patch.txt`
3. Apply this patch to your local checkout.
@@ -61,12 +62,12 @@ If you use gcl, or the above doesn't work, the following should always work.
patch -p1 <patch.txt
~~~~
- * Then you can run diff and visually check the local changes.
+ * 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.)
+ * 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/simd.md b/site/dev/contrib/simd.md
index e2e63107d2..6ec29c525a 100644
--- a/site/dev/contrib/simd.md
+++ b/site/dev/contrib/simd.md
@@ -32,7 +32,7 @@ This temptation starts to break down when you notice:
- math written with either SSE or NEON instrinsics is still very hard to read; and
- sometimes we want to work with 4 floats, but sometimes 2, maybe even 8, etc.
-So we use a wrapper class `SkNf<N>`, parameterized on N, how many floats the vector contains, constrained at compile time to be a power of 2. `SkNf` provides all the methods you'd expect on vector of N floats: loading and storing from float arrays, all the usual arithmetic operators, min and max, low and high precision reciprocal and sqrt, all the usual comparison operators, and a `.thenElse()` method acting as a non-branching ternary `?:` operator. To support Skia's main graphic needs, `SkNf` can also load and store from a vector of N _bytes_, converting up to a float when loading and rounding down to [0,255] when storing.
+So we use a wrapper class `SkNf<N>`, parameterized on N, how many floats the vector contains, constrained at compile time to be a power of 2. `SkNf` provides all the methods you'd expect on a vector of N floats: loading and storing from float arrays, all the usual arithmetic operators, min and max, low and high precision reciprocal and sqrt, all the usual comparison operators, and a `.thenElse()` method acting as a non-branching ternary `?:` operator. To support Skia's main graphic needs, `SkNf` can also load and store from a vector of N _bytes_, converting up to a float when loading and rounding down to [0,255] when storing.
As a convenience, `SkNf<N>` has two default implementations: `SkNf<1>` performs all these operations on a single float, and the generic `SkNf<N>` simply recurses onto two `SkNf<N/2>`. This allows our different backends to inject specialiations where most natural: the portable backend does nothing, so all `SkNf<N>` recurse down to the default `SkNf<1>`; the NEON backend specializes `SkNf<2>` with `float32x2_t` and 64-bit SIMD methods, and `SkNf<4>` with `float32x4_t` and 128-bit SIMD methods; the SSE backend specializes both `SkNf<4>` and `SkNf<2>` to use the full or lower half of an `__m128` vector, respectively. A future AVX backend could simply drop in an `SkNf<8>` specialization.
@@ -41,16 +41,16 @@ Our most common float use cases are working with 2D coordinates and with 4-float
`SkNf` in practice
----------------
-To date we have implemented several parts of Skia using Sk4f:
+To date we have implemented several parts of Skia using `Sk4f`:
1. `SkColorMatrixFilter`
2. `SkRadialGradient`
3. `SkColorCubeFilter`
4. Three complicated `SkXfermode` subclasses: `ColorBurn`, `ColorDodge`, and `SoftLight`.
-In all these cases, we have been able to write a single implementation, producing the same results cross-platform. The first three of those sites using Sk4f are entirely newly vectorized, and run much faster than the previous portable implementations. The 3 Sk4f transfermodes replaced portable, SSE, and NEON implementations which all produced different results, and the Sk4f versions are all faster than their predecessors.
+In all these cases, we have been able to write a single implementation, producing the same results cross-platform. The first three of those sites using `Sk4f` are entirely newly vectorized, and run much faster than the previous portable implementations. The 3 `Sk4f` transfermodes replaced portable, SSE, and NEON implementations which all produced different results, and the `Sk4f` versions are all faster than their predecessors.
-`SkColorCubeFilter` stands out as a particularly good example of how and why to use Sk4f over custom platform-specific intrinsics. Starting from some portable code and a rather slow SSE-only sketch, a Google Chromium dev, an Intel contributor, and I worked together to write an Sk4f version that's more than twice as fast as the original, and runs fast on _both_ x86 and ARM.
+`SkColorCubeFilter` stands out as a particularly good example of how and why to use `Sk4f` over custom platform-specific intrinsics. Starting from some portable code and a rather slow SSE-only sketch, a Google Chromium dev, an Intel contributor, and I worked together to write an `Sk4f` version that's more than twice as fast as the original, and runs fast on _both_ x86 and ARM.
`SkPx` for 8- and 16-bit fixed point math
----------------------------------------
@@ -96,7 +96,7 @@ Building an abstraction layer over 8- and 16-bit fixed point math has proven to
// A faster approximation of (SkPx * Alpha).div255().
SkPx.approxMulDiv255(Alpha) -> SkPx
-We allow each `SkPx` backend to choose how it physically represents `SkPx`, `SkPx::Wide`, and `SkPx::Alpha` and to choose any power of two as its `SkPx::N` sweet spot. Code working with SkPx typically runs a loop like this:
+We allow each `SkPx` backend to choose how it physically represents `SkPx`, `SkPx::Wide`, and `SkPx::Alpha` and to choose any power of two as its `SkPx::N` sweet spot. Code working with `SkPx` typically runs a loop like this:
while (n >= SkPx::N) {
// Apply some_function() to SkPx::N pixels.
@@ -112,7 +112,7 @@ The portable code is of course the simplest place to start looking at implementa
The most important difference between SSE and NEON when working in fixed point is that SSE works most naturally with 4 interlaced pixels at a time (argbargbargbargb), while NEON works most naturally with 8 planar pixels at a time (aaaaaaaa, rrrrrrrr, gggggggg, bbbbbbbb). Trying to jam one of these instruction sets into the other's idiom ends up somewhere between not quite optimal (working with interlaced pixels in NEON) and ridiculously inefficient (trying to work with planar pixels in SSE).
-So `SkPx`'s SSE backend sets N to 4 pixels, stores them interlaced in an `__m128i`, representing `Wide` as two `__m128i` and `Alpha` as an `__m128i` with each pixel's alpha component replicated four times. SkPx's NEON backend works with 8 planar pixels, loading them with `vld4_u8` into an `uint8x8x4_t` struct of 4 8-component `uint8x8_t` planes. `Alpha` is just a single `uint8x8_t` 8-component plane, and `Wide` is NEON's natural choice, `uint16x8x4_t`.
+So `SkPx`'s SSE backend sets N to 4 pixels, stores them interlaced in an `__m128i`, representing `Wide` as two `__m128i` and `Alpha` as an `__m128i` with each pixel's alpha component replicated four times. `SkPx`'s NEON backend works with 8 planar pixels, loading them with `vld4_u8` into an `uint8x8x4_t` struct of 4 8-component `uint8x8_t` planes. `Alpha` is just a single `uint8x8_t` 8-component plane, and `Wide` is NEON's natural choice, `uint16x8x4_t`.
(It's fun to speculate what an AVX2 backend might look like. Do we make `SkPx` declare it wants to work with 8 pixels at a time, or leave it at 4? Does `SkPx` become `__m256i`, or maybe only `SkPx::Wide` does? What's the best way to represent `Alpha`? And of course, what about AVX-512?)
@@ -126,8 +126,8 @@ These details will inevitably change over time. The important takeaway here is,
I am in the process of rolling out `SkPx`. Some Skia code is already using its precursor, `Sk4px`, which is a bit like `SkPx` that forces `N=4` and restricts the layout to always use interlaced pixels: i.e. fine for SSE, not great for NEON.
1. All ~20 other `SkXfermode` subclasses that are not implemented with `SkNf`.
- 2. SkBlitRow::Color32
- 3. SkBlitMask::BlitColor
+ 2. `SkBlitRow::Color32`
+ 3. `SkBlitMask::BlitColor`
I can certainly say that the `Sk4px` and `SkPx` implementations of these methods are clearer, less buggy, and that all the `SkXfermode` implementations sped up at least 2x when porting from custom per-platform intrinsics. `Sk4px` has lead to some pretty bad performance regressions that `SkPx` is designed to avoid. This is an area of active experiementation and iteration.
@@ -138,4 +138,4 @@ I am confident that Skia developers soon will be able to write single, clear, ma
I'm also confident that if you're looking to use floats, `SkNf` is ready. Do not write NEON or SSE SIMD code if you're looking to use floats, and do not accept external contributions that do so. Use `SkNf` instead.
-`SkPx` is less proven, and while its design and early tests look promising, it's still at the stage where we should try it aware that we might need to fall back on hand-written SSE or NEON. \ No newline at end of file
+`SkPx` is less proven, and while its design and early tests look promising, it's still at the stage where we should try it aware that we might need to fall back on hand-written SSE or NEON.
diff --git a/site/dev/contrib/style.md b/site/dev/contrib/style.md
index 17989b3c82..6a8c604af5 100644
--- a/site/dev/contrib/style.md
+++ b/site/dev/contrib/style.md
@@ -81,7 +81,7 @@ int herdCats(const Array& cats) {
Enum values are prefixed with k. Unscoped enum values are post fixed with
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&lt;singular enum name&gt;Count and not be a member of the enum (see example):
+is `k<singular enum name>Count` and not be a member of the enum (see example):
<!--?prettify?-->
~~~~
@@ -167,8 +167,8 @@ Ganesh macros that are GL-specific should be prefixed GR_GL.
#define GR_GL_TEXTURE0 0xdeadbeef
~~~~
-Ganesh prefers that macros are always defined and the use of #if MACRO rather than
-#ifdef MACRO.
+Ganesh prefers that macros are always defined and the use of `#if MACRO` rather than
+`#ifdef MACRO`.
<!--?prettify?-->
~~~~
@@ -179,14 +179,14 @@ Ganesh prefers that macros are always defined and the use of #if MACRO rather th
#endif
~~~~
-Skia tends to use #ifdef SK_MACRO for boolean flags.
+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
+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.
+interferes. Braces are always used with `if`, `else`, `while`, `for`, and `do`.
<!--?prettify?-->
~~~~
@@ -296,8 +296,8 @@ 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
+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?-->
@@ -410,6 +410,7 @@ if (count > 0) {
~~~~
Comments
+--------
We use doxygen-style comments.
@@ -435,16 +436,16 @@ We follow the Google C++ guide for ints and are slowly making older code conform
(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
+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.
-nullptr, 0
+`nullptr`, 0
-------
-Use nullptr for pointers, 0 for ints. We prefer explicit nullptr comparisons when
-checking for nullptr pointers (as documentation):
+Use `nullptr` for pointers, 0 for ints. We prefer explicit `nullptr` comparisons when
+checking for `nullptr` pointers (as documentation):
<!--?prettify?-->
~~~~
@@ -453,7 +454,7 @@ if (nullptr == x) { // slightly preferred over if (!x)
}
~~~~
-When checking non-nullptr pointers explicit comparisons are not required because it
+When checking non-`nullptr` pointers explicit comparisons are not required because it
reads like a double negative:
<!--?prettify?-->
@@ -485,8 +486,8 @@ 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
+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?-->
@@ -504,7 +505,7 @@ bool modify_foo(SkFoo* foo) {
Function Parameters
-------------------
-Mandatory constant object parameters are passed to functions as const references
+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.
@@ -521,8 +522,8 @@ SkScalar SkPaint::getFontMetrics(FontMetric* metrics, SkScalar scale) const;
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
+If function arguments or parameters do not all fit on one line, the overflowing
+parameters may be lined up with the first parameter on the next line
<!--?prettify?-->
~~~~
@@ -533,7 +534,7 @@ void drawBitmapRect(const SkBitmap& bitmap, const SkRect& dst,
}
~~~~
-or placed on the next line indented eight spaces
+or all parameters placed on the next line and indented eight spaces
<!--?prettify?-->
~~~~
diff --git a/site/dev/contrib/submit.md b/site/dev/contrib/submit.md
index 4dd380d77a..1d36e1f9d7 100644
--- a/site/dev/contrib/submit.md
+++ b/site/dev/contrib/submit.md
@@ -65,9 +65,9 @@ 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
+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.
+For help, run `git cl help`.
### Find a reviewer
@@ -78,7 +78,7 @@ has been editing it.
### Uploading changes for review
Skia uses the Gerrit code review tool. Skia's instance is [skia-review](http://skia-review.googlesource.com).
-Use git cl to upload your change:
+Use `git cl` to upload your change:
<!--?prettify lang=sh?-->
@@ -99,11 +99,11 @@ Skia's trybots allow testing and verification of changes before they land in the
repo. You need to have permission to trigger try jobs; if you need permission,
ask a committer. After uploading your CL to [Gerrit](https://skia-review.googlesource.com/),
you may trigger a try job for any job listed in tasks.json, either via the
-Gerrit UI, using "git cl try", eg.
+Gerrit UI, using `git cl try`, eg.
git cl try -B skia.primary -b Some-Tryjob-Name
-or using bin/try, a small wrapper for "git cl try" which helps to choose try jobs.
+or using bin/try, a small wrapper for `git cl try` which helps to choose try jobs.
From a Skia checkout:
bin/try --list
@@ -125,7 +125,7 @@ at it.
_Note_: If you don't see editing commands on the review page, click **Sign in**
in the upper right. _Hint_: You can add -r reviewer@example.com --send-mail to
-send the email directly when uploading a change using git-cl.
+send the email directly when uploading a change using `git-cl`.
The review process
@@ -155,8 +155,8 @@ code, commit it again locally, and then run git cl upload again e.g.
git cl upload
Once you're ready for another review, use **Reply** 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
+notification (it is helpful to tell the reviewer what you did with respect to
+each of their comments). When the reviewer is happy with your patch, they will
approve your change by setting the Code-Review label to "+1".
_Note_: As you work through the review process, both you and your reviewers
@@ -176,7 +176,7 @@ 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
+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.
@@ -219,7 +219,7 @@ adjustments at his/her discretion). If so, you can mark your change as
* 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` will squash all your commits into a single one with the description you used when you uploaded your change.
~~~~
git cl land