This looks like the GLSL driver in iOS generates wrong results when
returning a value from inside a switch.
Change-Id: I478a045c64c3dae9824f86f52e0c7f8f9685c9af
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/494476
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Since the SkShaders, etc will need access to SkPaintParamKeys and the SkShaderCodeDictionary they can't be buried in the skgpu namespace.
The currently planned signature for addToKey is:
void addToKey(SkShaderCodeDictionary*, SkBackend, SkPaintParamsKey*) const;
This CL doesn't modify the classes beyond what is needed to move them.
Bug: skia:12701
Change-Id: I18bbf6d6c3a768427112a3d19a9ccf2d46a23ad2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/494237
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Change-Id: I3dabd77890a73ea054bb57d466a6ed8273eae3e8
Bug: skia:12811
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/494196
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
SkPaintParamsKey can be variable length and is thus the path forward for encapsulating/representing PaintParams.
Note: SkPaintParamsKey isn't in the skgpu namespace bc, ultimately, the individual SkShaders, etc will be responsible for adding their own information to the key.
Bug: skia:12701
Change-Id: I7472dc00706a4548bc3ffb4e287a941a2b8ffcae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/492405
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Change-Id: Iecf1313af5f2938cb899f2a3e750ffc04554bae0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/493977
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:11862
Change-Id: Ia21fad6acb82a03c1ba8663be548bf58f5067b85
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/493636
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Track the scaling hints as the minimum of the maximum dimension and
the maximum of the maximum dimension. Having the range will allow
for better scaling choices in the drawing of last resort.
The next CL will start using this range to calculate scaling factors.
Bug: chromium:1280180
Change-Id: I2c07303d3303c71ede2a0b1133ee2e218f7bd84e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/493456
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Bug: skia:12700
Change-Id: Icbd1ff6f98f5de8830b40b8a714e8092e40b1c97
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/485217
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This CL creates a new build target with a customized set of GN
flags that enable the native components of SkQP to be built using
NDK APIs.
Bug: skia:12777
Change-Id: I8341eafa7fc794cfb759045b7c7238e69c29b0a3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491447
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
Bug: skia:12701
Change-Id: I8138633dee9eaf65077da8534600393196c7942e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/492404
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
The core of this CL is replacing the Combination in GraphicsPipelineDesc with a uniqueID. All the other changes are just in support of that.
Bug: skia:12701
Change-Id: Id5fe9b39d1e497adbc879e7c933a2d9bdb788a58
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491048
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Encapsulate the direct/SDFT decisions in the bool functions
isDirect() and isSDFT().
Encapsulate the notion of glyph approximations in:
approximateTransformedTextSize().
Use these to replace drawingType through the code.
Bug: chromium:1280180
Change-Id: I8de7ee8bc4a24e576e79eeb1ed8670179a860aa9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491448
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
As @johnstiles suggested I add the test first and the fix after.
bug: skia:12712
Change-Id: I9316cf40f71e756fc1730ee630bc0d0377f200d6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491936
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
We used to reject ES3-style array declarations in strict-ES2 mode, so
this test originally expected two errors.
Change-Id: I17f71630076cda4b37b7723225dcff951eba9dcc
Bug: skia:12410
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491997
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This reverts commit 6e686b8b8b.
Reason for revert: After internal discussion, we established that nobody was actually sure why this had needed to be an error in the old parser in the first place, so there does not appear to be a reason to carry the behavior forward.
Original change's description:
> Fixed SkSL error reporting on array types
>
> The DSLParser was not reporting errors when the array type appeared
> before the variable name (float[2] x) as opposed to after (float x[2])
> in strict ES2 mode.
>
> Bug: skia:12410
>
> Change-Id: Ia388aa150f65916dc3ccc58f7680dbde0a636c5f
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491819
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:12410
Change-Id: I355fd1ad89e2e64b0377be7672b7f3f824eebac8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491996
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
The DSLParser was not reporting errors when the array type appeared
before the variable name (float[2] x) as opposed to after (float x[2])
in strict ES2 mode.
Bug: skia:12410
Change-Id: Ia388aa150f65916dc3ccc58f7680dbde0a636c5f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491819
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Previously, type aliases ('vec2') were just an additional name which
could be used to refer to a type ('float2'). This was simple and worked,
except that error messages would be wrong - any type-related error
message would refer to the type as 'float2' rather than the 'vec2' that
the user actually typed.
This CL adds an AliasType class so that we can track which name was
used to refer to an aliased type and report messages using the correct
type name.
Bug: skia:12737
Change-Id: I40e234239ab47557033e0695e4fbbd5f01da354e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/490256
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This will make it possible for drawVertices and drawCustomMesh to share
implementation in Ganesh.
Bug: skia:12720
Change-Id: I8b84119a29b47071ef389879bb5287873629ed3e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/487978
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: oss-fuzz:43062
Change-Id: I10d8fa40c81c5b1595d30221d89c84f5cc3478fd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/490857
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
SkRuntimeEffect::MakeForXxxxx compiles its shaders before MakeTraced()
is invoked, with optimizations enabled. When MakeTraced() is called,
we recompile the shader with optimizations disabled; this allows us to
present the user with a shader that is as close to the original as
possible. (This obviously costs time, but debug trace generation isn't
focused on speed.)
Note that in some rare instances, disabling optimizations can cause
compilation to fail (e.g. optimization can simplify control flow and
eliminate "unreachable" paths). In those cases, we fall back to the
existing behavior of debugging an optimized shader.
Change-Id: Iba0f31bc42374c7048694fde5415b122177a9a68
Bug: skia:12666
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/489716
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, the VariableData returned from a DebugTracePlayer contained
values in an int32_t, regardless of the slot's NumberKind. We had a
helper function which could stringize the bits, but otherwise the caller
was responsible for bit-casting the value manually.
Now, the DebugTracePlayer will automatically manage bit-casting for the
caller. The value returned in the VariableData is now a double (so it
is able to store an int32, uint32, or float at full precision).
This change was inspired by the recent Typescript port. (The value in
the Typescript VariableData uses a compound `number | boolean` type, so
it is able to fully represent any slot value natively.)
Change-Id: I5eec414236f76ad0ff51b0b19974e4a0025c4d62
Bug: skia:12666
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/489896
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Interface blocks now guard against naming their member variables with
built-in type names like "float" or "bool".
Change-Id: Ia767542ace76fb8fbc2d50c81772b7f54b1bf973
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/489616
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Structs already handled this appropriately, but interface blocks did not
guard against naming their member variables built-in type names like
"float" or "bool".
Change-Id: I12ec054b3f158b83e35031449cf2a088ff8d0dc2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/489596
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Anonymous function parameters are now automatically assigned a name,
"_skAnonymousParamN", where N is the parameter index.
Change-Id: I87adcd51ed025c76ae2b333317f21b523a4632b4
Bug: skia:12769
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/489538
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
SkSL will reject ES2-compatible code because function parameters always
require a name in SkSL. (A followup CL relaxes this restriction and
allows anonymous parameters in SkSL.)
Change-Id: Ifdcf0fcbe0f52d16007c018b545631ca4033a8c4
Bug: skia:12769
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/489537
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Structs and interface blocks allow a trailing identifier which is added
to the symbol table. This identifier is now prohibited from
overlapping built-in types.
Change-Id: I33b9d6156a27ce017e6744a05979748c04a04767
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/489516
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Structs and interface blocks allow a trailing identifier which is added
to the symbol table. This identifier should be prohibited from
overlapping built-in types; at present, this is not checked. Add a test
demonstrating the issue.
Change-Id: I99aa915c1715c468cc369c97b7f12e031b86ea4a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/489496
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
When the SkVM interpreter is operating in "stride=1" mode, we should
only look at the first lane of the trace/execution masks. The other
lanes are not being actively used, but the "index" opcode still fills
all the lanes, so it's entirely possible for the trace coordinate to
match in an unused lane.
Change-Id: I7f04f5c59431b3c50b4b072bc3ea4f52f8aa1a1b
Bug: skia:12752
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/488796
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
We'll want to reuse this key class for Graphite resources as well. There
is nothing special about these keys that is Ganesh specific.
Bug: skia:12754
Change-Id: I39fe5a9968156985865bccbfc4978583120ac2e8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/487384
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Since we know the entire flow of execution ahead of time, at any point
during trace playback, we can know if a line will be reached again or
not. We no longer highlight lines as reachable (or allow setting
breakpoints) if the line will not be reached again during trace
playback.
Change-Id: Iff563b13e2f6efb5d4f2ff37215f2ff4fb5945ed
Bug: skia:12666
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/486496
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
At this point SkPromiseImageTexture is not much more than a wrapper
around a GrBackendTexture. The next steps would be to switch to just
directly using GrBackendTexture in the fulfill procs and deleting
SkPromiseImageTexture.
Bug: skia:12758
Change-Id: Ic0526b869a0730c25b41b46fd6523604dedaba40
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/487382
Auto-Submit: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Renderer::StencilAndFill() chooses between two instances based on fill
type (more to come when we add stencil pass). The inverse fill uses
different stencil settings and different geometry (hence why it must be
a distinct renderer, since stencil is part of the pipeline).
Also updates the command buffer asserts and types to support float3
attributes and has the fill bounds render step pre-transform vertices.
This matches the intended plan of device-space control points to
avoid matrix transform uniforms when no other coords are needed.
Makes DepthStencilSettings constexpr so they can be declared constants.
Cq-Include-Trybots: luci.skia.skia.primary:Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-Graphite,Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Debug-All-ASAN_Graphite,Build-Mac-Clang-arm64-Release-iOS_Graphite,Build-Mac-Clang-arm64-Release-Graphite,Build-Mac-Clang-arm64-Debug-iOS_Graphite,Build-Mac-Clang-arm64-Debug-Graphite_NoGpu,Build-Mac-Clang-arm64-Debug-Graphite,Build-Mac-Clang-arm64-Debug-ASAN_Graphite
Bug: skia:12703
Change-Id: I5be4151f533e4cc5c560baf96c59193162b48dab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/484559
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Change-Id: Ibff6f08e569b5365732404a56fc3b9bf7c2fa1f9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/487377
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This is a reland of e1e1c03229
Original change's description:
> Start on custom mesh API
>
> Bug: skia:12720
> Change-Id: I8ed0555f1c5b8b0e97e73a657635dac2c0df0ccf
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482457
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:12720
Change-Id: I5c54f37949d8bd45ea606264c439912b65158b79
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/487220
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12720
Change-Id: I8ed0555f1c5b8b0e97e73a657635dac2c0df0ccf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482457
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Simplified - "no static instructions"
Added to skslc with .cpp -> .html
Change-Id: I0b56ea0480f868ca4182acb7a23c03acf7f57519
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/473659
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
This also allows the KeyBuilder class to be used in Graphite. In Graphite
I plan to use it for other keys besides building up shaders/processors
so I dropped that from the name. There was nothing special abouts its
implementation that made it specific to Processors anyways.
Bug: skia:12754
Change-Id: I1b25730b061f9342b61e3b2e4ac43328b8074d59
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/486897
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This is a reland of 6927ab9311
Original change's description:
> Add support for explicit attribute offsets and strides.
>
> Previously attribute offsets were always computed based on their
> position in an attribute array and the stride was determined
> by the offset and size of the last attribute.
>
> Now a GP has the option to create attributes with explicit offsets
> and specify an explicit vertex stride. All attributes must either
> be implicit or explicit (enforced by assert).
>
> GrGeometryProcessor::AttributeSet is now responsible for handling
> implicitly determined attribute offsets and strides. The backends
> no longer compute them.
>
> Bug: skia:12720
> Change-Id: I0211673dc70d4797c2d66b2555d8f5fb430be056
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/484736
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:12720
Change-Id: Ic4975b5b2e52f2d8213154da0e585eca6dfdd78d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/486098
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: b/195936927
On Android, we may need to be able to seek the stream, but the input may
not be seekable. Copy in SkWuffsCodec as needed.
Change-Id: I7d5742e478532943a90400267175da80411cacd9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/485636
Commit-Queue: Leon Scroggins <scroggo@google.com>
Auto-Submit: Leon Scroggins <scroggo@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
This reverts commit 6927ab9311.
Reason for revert: breaking roll?
Original change's description:
> Add support for explicit attribute offsets and strides.
>
> Previously attribute offsets were always computed based on their
> position in an attribute array and the stride was determined
> by the offset and size of the last attribute.
>
> Now a GP has the option to create attributes with explicit offsets
> and specify an explicit vertex stride. All attributes must either
> be implicit or explicit (enforced by assert).
>
> GrGeometryProcessor::AttributeSet is now responsible for handling
> implicitly determined attribute offsets and strides. The backends
> no longer compute them.
>
> Bug: skia:12720
> Change-Id: I0211673dc70d4797c2d66b2555d8f5fb430be056
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/484736
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:12720
Change-Id: I938bc3776d598f0906465eb2677208c79676642f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/486016
Auto-Submit: Brian Salomon <bsalomon@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Users expect that breakpoints will be hit even when stepping over or
stepping out. Rather than have one step call which honors breakpoints,
now the trace holds an unordered_set of breakpoints which all step calls
will honor.
Change-Id: I4055fc008e8cce4c413f357f375ae4038a7bd2a9
Bug: skia:12747
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/485918
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This allows the user to run up to a specified line (or until execution
is complete).
Change-Id: Ied34798d2df1b4a9e1d723c8d2eff3d579ca9b2d
Bug: skia:12747
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/485860
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously attribute offsets were always computed based on their
position in an attribute array and the stride was determined
by the offset and size of the last attribute.
Now a GP has the option to create attributes with explicit offsets
and specify an explicit vertex stride. All attributes must either
be implicit or explicit (enforced by assert).
GrGeometryProcessor::AttributeSet is now responsible for handling
implicitly determined attribute offsets and strides. The backends
no longer compute them.
Bug: skia:12720
Change-Id: I0211673dc70d4797c2d66b2555d8f5fb430be056
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/484736
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
