Change-Id: I2619784eca0f7a4dd66f2db0104cb746d9266b4e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/244369
Commit-Queue: John Rosasco <rosasco@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Hopefully this makes it clearer that the GrProgramInfo isn't copying these
objects.
Bug: skia:9455
Change-Id: Id43b7978d63b6d53d531601f3446745f33afcf2d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/254970
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
It is, arguably, not a good metric but the max number of AA Quads that are merged or chained together across all Skia's GMs and SKPs is 276.
This should also reduce the memory consumed from 245,760 bytes to 30,720 bytes.
Bug: b/143572065 skia:9601
Change-Id: I86f3ae2830d8a64ebcc6f5b7da8c0a4dac8761d5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/254916
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This is required before we can lower the max AA quad count (again).
Bug: b/143572065 skia:9601
Change-Id: Ibd029a59f6589e1ae6c66d072e5039f0e9de06dc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/254581
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This is required before we can lower the max AA quad count (again).
Bug: b/143572065 skia:9601
Change-Id: Iea5884ffff9bd62f62527fcb597dac97d33ecd76
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/254438
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This fixes an issue where fragment processors which were not written
in pure SkSL did not interact properly with coordinate overrides.
Change-Id: I960173986fd8d6be7322f55ab0d81a533b6b89e5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252916
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This all comes together as
uminv tmp, condition
fmov gp, tmp
cbnz gp, all_true
brk 0
all_true:
...
The key idea is uminv(vec) will return 0 if any of the inputs are 0,
and non-zero if all of the inputs are non-zero, namely 0xffffffff.
fmov moves that minimum from a vector register to a general purpose
register where we can test it with cbnz, compare and branch if non-zero.
This jumps over the `brk 0` debug trap when all inputs are true.
Change-Id: If5deb77a77f52221d0649e537179743c45eb9cc5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/254479
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This reverts commit dd947ceb55.
Reason for revert: CL isn't getting any respect from Android
Original change's description:
> Respect the max indexBuffer limits in the bulk texture draw API
>
> This is required before we can lower the max AA quad count (again).
>
> Bug: b/143572065 skia:9601
> Change-Id: Id34123476ad49a57dc9ce7fe13f941c06f721b74
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252603
> Commit-Queue: Robert Phillips <robertphillips@google.com>
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
TBR=robertphillips@google.com,michaelludwig@google.com
Change-Id: Ibe396fe5a0351190c214ed54611ce72b7928cadd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b/143572065 skia:9601
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/254437
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This is required before we can lower the max AA quad count (again).
Bug: b/143572065 skia:9601
Change-Id: Id34123476ad49a57dc9ce7fe13f941c06f721b74
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252603
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Enance SkBulkGlyphMetrics, SkBulkGlyphMetricsAndPaths, and SkBulkGlyphMetricsAndImages
with single glyph calls. In addtion, add calls needed to have the rest of the system
work with these interfaces.
As a resulte move the glyph, prepareImage, and preparePath calls to private.
Change-Id: I8d383b649390e45f621dcb9d62fb8367a55cee02
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/254056
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Change-Id: I90f40467be145537cc08461fd3805606432f0cfd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/254240
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Rosasco <rosasco@google.com>
Auto-Submit: John Rosasco <rosasco@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Lots of x86 instructions can take their right hand side argument from
memory directly rather than a register. We can use this to avoid the
need to allocate a register for many constants.
The strategy in this CL is one of several I've been stewing over, the
simplest of those strategies I think. There are some trade offs
particularly on ARM; this naive ARM implementation means we'll load&op
every time, even though the load part of the operation can logically be
hoisted. From here on I'm going to just briefly enumerate a few other
approaches that allow the optimization on x86 and still allow the
immediate splats to hoist on ARM.
1) don't do it on ARM
A very simple approach is to simply not perform this optimization on
ARM. ARM has more vector registers than x86, and so register pressure
is lower there. We're going to end up with splatted constants in
registers anyway, so maybe just let that happen the normal way instead
of some roundabout complicated hack like I'll talk about in 2). The
only downside in my mind is that this approach would make high-level
program descriptions platform dependent, which isn't so bad, but it's
been nice to be able to compare and diff debug dumps.
2) split Op::splat up
The next less-simple approach to this problem could fix this by
splitting splats into two Ops internally, one inner Op::immediate that
guantees at least the constant is in memory and is compatible with
immediate-aware Ops like mul_f32_imm, and an outer Op::constant that
depends on that Op::immediate and further guarantees that constant has
been broadcast into a register to be compatible with non-immediate-aware
ops like div_f32. When building a program, immediate-aware ops would
peek for Op::constants as they do today for Op::splats, but instead of
embedding the immediate themselves, they'd replace their dependency with
the inner Op::immediate.
On x86 these new Ops would work just as advertised, with Op::immediate a
runtime no-op, Op::constant the usual vbroadcastss. On ARM
Op::immediate needs to go all the way and splat out a register to make
the constant compatible with immediate-aware ops, and the Op::constant
becomes a noop now instead. All this comes together to let the
Op::immediate splat hoist up out of the loop while still feeding
Op::mul_f32_imm and co. It's a rather complicated approach to solving
this issue, but I might want to explore it just to see how bad it is.
3) do it inside the x86 JIT
The conceptually best approach is to find a way to do this peepholing
only inside the JIT only on x86, avoiding the need for new
Op::mul_f32_imm and co. ARM and the interpreter don't benefit from this
peephole, so the x86 JIT is the logical owner of this optimization.
Finding a clean way to do this without too much disruption is the least
baked idea I've got here, though I think the most desirable long-term.
Cq-Include-Trybots: skia.primary:Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-SK_USE_SKVM_BLITTER,Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Release-All-SK_USE_SKVM_BLITTER
Change-Id: Ie9c6336ed08b6fbeb89acf920a48a319f74f3643
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/254217
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
The logic implemented here is roughly
assert_true(v):
if any ~v {
int3()
}
in assembly as
```
vptest v, constant 0xffffffff mask
jc ok
int3
ok:
```
jc branches if (~v & mask) are all zero, with mask set fully, that's
branch if ~v are all zero, which is to say, v are all ~0, true. So we
jump over the int3 breakpoint if v are all true.
Cq-Include-Trybots: skia.primary:Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-SK_USE_SKVM_BLITTER
Change-Id: Ie0fc1da15b1a0dba00c66af610ccde18f5985f8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253897
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Replaces numSamples with numRasterSamples, and adds isMixedSampled.
The sample count that vulkan and metal actually want to know is how
many samples the rasterizer will compute, which may not match the
number of samples in the render target when we have mixed samples.
They will also need to know whether a program is mixed sampled in
order to set up coverage modulation.
Change-Id: I133c11f74b7dc6a7580818ef73d6deec1d201b64
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253550
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Metal swapchain textures (equivalent to GL's FBO 0) can't be used with
MTLBlitCommandEncoder. For copySurface we need to detect this and fail.
Future changes will handle GrSurfaceProxy.
Change-Id: I9235d91cf31e96260c8d1da5938f5441d2e49d27
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252045
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Will use these to implement assert_true on x86.
Change-Id: I9d2595a35518b6971dd8e418b583febd3960c7f6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253896
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
This is an assert that is active in debug mode. For the moment it only
works in the interpreter, but I plan to follow up with JIT code too.
assert_true() is a data sink like a store() as far as lifetime goes,
though we take care to allow it to be hoisted if its inputs are. An
assert_true's existence will keep all its inputs alive, and in release
builds where we skip the instruction, those inputs will all drop away
automatically.
Tested locally by forcing the interpreter. It shouldn't be long before
I have at least x86 JIT asserts working too.
Change-Id: I7aba40d040436a57a6b930790f7b8962bafb1a8c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253756
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This plumbs through round but doesn't use it. I want that change to be
its own CL. It's nice to have assembler support and the name changes
even if I revert using round.
Change-Id: I6d67ec5c63546069eb7cc1c91599b599bafcda66
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253724
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
No diffs.
Change-Id: Ia0b35c2787e27d74763f21b81072affa6caf1e5a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253720
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
This is a reland of 12cea8d6c4
Now implementing float comparisons on ARM also.
Only vaguely tricky thing is that x!=y is ~(x==y).
Original change's description:
> hook up float comparisons to x86 JIT
>
> This gets the draws in gm/skvm.cpp all JITing again,
> and in one of the unit tests.
>
> (Everything draws the same of course.)
>
> Change-Id: Iada28690d9df78f9d444ee3765e21beb29239672
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253166
> Auto-Submit: Mike Klein <mtklein@google.com>
> Reviewed-by: Mike Klein <mtklein@google.com>
> Commit-Queue: Mike Klein <mtklein@google.com>
Cq-Include-Trybots: skia.primary:Test-Android-Clang-NVIDIA_Shield-CPU-TegraX1-arm64-Debug-All-Android
Change-Id: I771b8a327a958db8a0d509d55863ade935a00035
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253401
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
JIT code isn't MSAN-instrumented, so we won't see when it uses
uninitialized memory, and we'll not see the writes it makes as properly
initializing memory. Instead force the interpreter, which should let
MSAN see everything our programs do properly.
This refactors so that SkVM.cpp is the only code to look at whether
SKVM_JIT is defined, and undefines it when built with MSAN. Added
a simple regression test too.
Cq-Include-Trybots: skia.primary:Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-MSAN
Change-Id: Ic7cca2621f84dfba7174127738744d6c68f85f2e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253410
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This reverts commit 12cea8d6c4.
Reason for revert: unit tests failing on ARM... will try again once I have float comparisons implemented for ARM too.
Original change's description:
> hook up float comparisons to x86 JIT
>
> This gets the draws in gm/skvm.cpp all JITing again,
> and in one of the unit tests.
>
> (Everything draws the same of course.)
>
> Change-Id: Iada28690d9df78f9d444ee3765e21beb29239672
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253166
> Auto-Submit: Mike Klein <mtklein@google.com>
> Reviewed-by: Mike Klein <mtklein@google.com>
> Commit-Queue: Mike Klein <mtklein@google.com>
TBR=mtklein@google.com,reed@google.com
Change-Id: Ie07e580b4998199338217a27d4fad34c679ffc23
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253399
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This gets the draws in gm/skvm.cpp all JITing again,
and in one of the unit tests.
(Everything draws the same of course.)
Change-Id: Iada28690d9df78f9d444ee3765e21beb29239672
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253166
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This further consolidates the information required to compute the program key (esp. for Vulkan). This CL mainly comprises the plumbing portion - a follow up CL will actually use it.
Bug: skia:9455
Change-Id: Iaac716c289916981a1757a333bfa57b3051fd35b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252161
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
I tracked down all the places where things were assuming 2-bits
of sub-pixel positions. I generalized them all to take a variable
number of bits.
Change-Id: I304061635c838fedc3881aaa256bab943ff6a817
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253236
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
The previous version only translated the sub-position reliably
if the position < 2^10. This code extends the range to 2^21.
Change-Id: I7fcdfe2c3f7336ac0788e282c2bd0fc73067d8fb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253101
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This was accidentally loosened up
Change-Id: I7a95ed5b56aff06114ceaa9c2a3719bd85d4add4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252934
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This reverts commit 078cf3e3b2.
Reason for revert: Appears to be blocking the Chrome roll due to Fuchsia build failures
Original change's description:
> Fully delineate GL usage w/ skia_use_gl
>
> Change-Id: I17424d2235c7a54c4cda036f01a0b99dadc4b632
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252085
> Commit-Queue: Mike Klein <mtklein@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=mtklein@google.com,bsalomon@google.com,halcanary@google.com,rosasco@google.com
Change-Id: I17ba79bfe6e7f81c126eddbd50ca5384cfa4179c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253098
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Change-Id: I17424d2235c7a54c4cda036f01a0b99dadc4b632
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252085
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This reverts commit 1803f4ef6f.
And also fixes the primitive shaper.
Change-Id: Ieaeda5522c98d8a9e6f628b8a6cc30cf41278350
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252929
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This reverts commit 0f3a26dd18.
Reason for revert: Windows bots are broken
Original change's description:
> Fix empty run handling in trivial shaper iterators
>
> When the text run is of zero length the iterator starts at the end. The
> trivial itereators did not handle this case.
>
> Change-Id: Id41304500e33d821874f56ab20085cbc4b2d9b0b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252857
> Reviewed-by: Herb Derby <herb@google.com>
> Commit-Queue: Ben Wagner <bungeman@google.com>
TBR=bungeman@google.com,herb@google.com
Change-Id: Ia38e46ac4c04def5d374fbbce450538096d90d64
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252923
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
When the text run is of zero length the iterator starts at the end. The
trivial itereators did not handle this case.
Change-Id: Id41304500e33d821874f56ab20085cbc4b2d9b0b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252857
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This CL itself doesn't really change how anything is running today. However,
it is the first step into gracefully supporting a vulkan device lost return
error and being able to correctly clean everything up.
Bug: skia:9603
Change-Id: I9693c514a32ca437df61a7aee63651e2e40ee1c0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252819
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Rm unused function SkColorTypeIsGray.
Expect kGray_8 readback to work in tests.
Bug: skia:8962
Change-Id: Iad9d2be3e0a2e594e62dc681b79c59e0a833116a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/245162
Auto-Submit: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This lets shaders that use 'x' JIT on x86.
I started with paddd and {0,-1,-2,-3,...}, which worked fine but on
second thought seemed a bit odd. I've switched to psubd and
{0,1,2,3,...} but I've left in support for paddd with a memory arg.
gm/skvm.cpp now JITs fully again and continues to draw the same as
the interpreter did.
Simplify embedded data alignment a little... memory operands don't
need full register alignment in AVX like they used to in SSE. So
just align everything to the vector element size like we do on ARM,
and reorder [splats,bytes_masks,iota] to match the order we declare
and handle them in the code above.
Add unit tests for vpaddd + vpsubd.
Cq-Include-Trybots: skia.primary:Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-SK_USE_SKVM_BLITTER
Change-Id: I6b8d060450cca7f437a1d2a597a8a0e0e8d51b33
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252797
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
This initializes more formats correctly with solid colors and relies
less on GrPixelConfig.
It also includes the changes to get gray8 read pixels working
and almost everything to initialize GL_RGB8 textures with data. Minor
stuff to follow to get RGB8 fully working and update test expectations.
Bug: skia:8962
Bug: skia:6718
Bug: skia:9358
Change-Id: Ic044b4c4badc37f14fb46c898cd3b3c21a6fc7fd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/251199
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Propagating usage of these helpers will, hopefully, improve code reuse and centralize future changes.
This is pulled out of https://skia-review.googlesource.com/c/skia/+/252161/ (Add GrPrimitiveType to GrProgramInfo)
Bug: skia:9455
Change-Id: I9b46e42db8561d98fb724a41a1f7a3bec4bf97b0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252484
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
isValid read memory beyond the buffer that was uninitialized.
BUG=oss-fuzz:18555
Change-Id: I5576add2cd7bfbe146d495239b96210fe15fe562
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252538
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Change-Id: I8ef13b78a5f2f49ff9c59db285b3e0e7ee708c9b
Bug: chromium:960620
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252160
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Performance measurments showed that SkPackedGlyphID was too much
time.
SkDiffBench-lorem_ipsum 955µs -> 873µs
BUG=chromium:881505
Change-Id: I8d42f02d87817777cc7c95404c73c723f0bc100c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/251818
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Change-Id: Ic286357dc637edf0c8df003422a43252590250f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/251768
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
