This reverts commit 16cbfb41df.
Reason for revert: using ES3 features, breaks on ANGLE ES2 bots
Original change's description:
> Implement statements and expressions in DSL C++ code generator.
>
> This CL removes the bulk of the existing C++ code generator, especially
> all the complex format-string assembly code. It has been replaced with
> actual DSL code generation. Simple IR can now be successfully translated
> to a working DSL fragment processor.
>
> This CL also adds a simple test harness which is patterned after the
> existing SkSLTest; it renders a pixel, reads it back, and fails the test
> if the result isn't solid green (RGBA=0101).
>
> This CL doesn't implement every feature. Some obvious gaps include:
> - Sampling from children
> - Uniforms/inputs of any kind
> - Function calls of any kind
>
> Change-Id: Ib80c23fe1ba4453f7c3cb43b65f93c5ea0deb709
> Bug: skia:11854
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396757
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I4f3e7667bf1e3a5539d0248b6c47d9ae2296aa88
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398739
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This CL removes the bulk of the existing C++ code generator, especially
all the complex format-string assembly code. It has been replaced with
actual DSL code generation. Simple IR can now be successfully translated
to a working DSL fragment processor.
This CL also adds a simple test harness which is patterned after the
existing SkSLTest; it renders a pixel, reads it back, and fails the test
if the result isn't solid green (RGBA=0101).
This CL doesn't implement every feature. Some obvious gaps include:
- Sampling from children
- Uniforms/inputs of any kind
- Function calls of any kind
Change-Id: Ib80c23fe1ba4453f7c3cb43b65f93c5ea0deb709
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396757
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This ensures we don't lose the original SkSurfaceProps when creating
image filters, etc.
Bug: skia:11396
Change-Id: I6b412361c1005138278a1396faa7f7e069ec7eb2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397291
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Most backends don't like init-stmts with multiple VarDeclarations inside
them, so we no longer emit IR that does this. This lets us avoid doing
backend-level fixups.
Change-Id: Ide839de18953a73e0f9c7a690df59a7bc3523f89
Bug: skia:11860
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398221
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This is another strange, experimental feature that clutters the
implementation and isn't used by anyone (to my knowledge).
Change-Id: I538b7eca0cd28aab32f4739b23459731ade9105e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398226
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This was an experimental feature. It worked (but only the GPU backend).
It was never adopted or used by anyone, to my knowledge. It's a large
amount of code, and a strange corner of SkSL for users to stumble into.
Bug: skia:10680
Change-Id: I0dda0364bce7dbffa58c32de4c7801ec2a6bc42e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398222
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Unused InterfaceBlocks were not added to the ProgramUsage map. The
ProgramUsageVisitor now makes sure to account for them during its
initial scan.
Change-Id: If3afac8e954c5b685ddc6b63b0f771d8c0b8f207
Bug: oss-fuzz:33405
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398016
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
As explained, this is *very* conservative. It only works when the child
is sampled from within main, and using a direct reference to the coords
parameter. If that parameter is ever modified (even after being used),
the optimization doesn't happen. For most cases, this is fine.
Bug: skia:11869
Change-Id: Ia06181730a6d07e2a4fe2de4cc8e8c3402f0dc52
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397320
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
It should now be rare to require a RGBA texture from a YUVA image (e.g.
in image filter)
Removes a thread-safety issue. A step towards allowing clients to pump
new data into the planes and be sure Skia wont draw the old contents.
Bug: skia:11873
Change-Id: I007cf28e477155f85ed9f1c6cc4547f9dbbb73fd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397319
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
SPIR-V would previously jump to the top of the do-while loop when
encountering a `continue`. This is not correct; it should jump to the
loop-exit test. We don't have any real shaders which test this case, and
it can't be checked from inside Runtime Effects since do-while is not
part of ES2.
Change-Id: Iac58e9e7ad0a94640e6418afea096b19ba50a2df
Bug: skia:11874
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397576
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:11867
Change-Id: I21e71c77518f0a942651f31061f43f80505090ea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397150
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Cleans up registerChild calls, which were all on one long line.
Change-Id: I2b665b033edb88cf1de5e4da433833c0be26c92b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397147
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This is used in the fallback code when reordering produces
a DAG that goes over our memory budget.
Bug: skia:10877
Change-Id: I13772b30e270eb546957b88586ded1cf42d2dbeb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397136
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This is a reland of 5a2de5e72f
This is part one of two CLs. In this CL, I put a
sk_sp<GrArenas> on GrRenderTargetProxy where GrArenas wraps
a SkArenaAlloc to add ref counting. Creating
a GrOpsTask shares the GrArenas with the ops task. When an
GrOpsTask is destroyed, it nulls out the fArenas sk_sp on
the GrRenderTargetProxy to limit the life span of the arenas.
New plumbing was added to GR_DRAW_OP_TEST_DEFINE to allow a
proper GrSurfaceDrawContext to be passed to GrAtlasTextOp's
GR_DRAW_OP_TEST_DEFINE so the arena will have a proper lifetime.
The second CL will work on replacing GrOpsTask's fAllocators
system with the shared arena.
Change-Id: Ieb568e4533c17e31b3b015e7781365d7d898c483
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396818
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Herb Derby <herb@google.com>
These enforce stricter rules about the signature of main, and each one
uses a separate pre-include module. That prevents color filters from
being able to reference sk_FragCoord (or coords passed to main) at all.
It also limits the versions of sample() that are exposed.
In the new world, an effect created for a specific stage of the Skia
pipeline can only be used to create instances of that stage (SkShader or
SkColorFilter). For now, SkRuntimeEffect::Make uses kRuntimeEffect,
which continues to be more lenient and allow creation of either shaders
or color filters from a single effect. After we migrate all clients, we
can deprecate and then delete that mode.
Bug: skia:11813
Change-Id: I0afd79a72beeec84da42c86146e8fcd8d0e4c09f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395716
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
When a Block is detected in the for loop's init-statement, we now
synthesize a scope and write the init-statement directly above the for
loop and leave the for loop's init-stmt section empty.
Change-Id: I3e89c6a4328a79e49b8a19faae7975629cd0e152
Bug: skia:11860
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396820
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The instructions used to implement it don't deliever
the same results across processors, even on the same
architecture.
Bug: skia:11861
Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Change-Id: I7831640fa6c0f01540619fce3f509c8969aaaaff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396722
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This is structured differently than the GLSL fix, due to the different
semantics of array-types in Metal.
Change-Id: I27ad11539bbbb96abb0686d5686b8fcd2f5dd6d1
Bug: skia:11860
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396916
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
We lacked test coverage for this case, and this was broken when compound
VarDeclarations were split from one Statement into several.
Change-Id: I561b4d8acc0bfa01161d873a0c022ec58e316903
Bug: skia:11860
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396817
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Modifies helpers on GrGLSLGeometryProcessor that insert, set, and make
keys for view/local matrix uniforms to not omit the uniform when
the matrix is identity or use a float4 when it is scale/trans. Always
uses a 3x3.
Bug: skia:11844
Change-Id: I0f25b60b46b8932d7e2cac4a50159d22b9cd84d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395656
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
The instructions used to implement it don't deliever
the same results across processors, even on the same
architecture.
Bug: skia:11861
Change-Id: Ifb87fc04a4ec866f84d7c2f2472c7f4b65661d28
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396721
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Change-Id: Ie36ea4d5186f6cca341efc6300b813ec8255bdbb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388457
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This reuses the work we're already doing to compute the one-at-a-time
program, and lets us share the result across color filter instances
created from the same effect.
When we added caching of the flag to filter creation, pinpoint found a
performance regression. I expect this to resolve that.
Bug: chromium:1187432
Change-Id: Ie4f8551fa432d298ce950cba8642b9de28f19b81
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396758
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This requires the GN processing to be slightly more complex, as GN has
no native support for more than one extension on a file.
Context: https://groups.google.com/a/chromium.org/g/gn-dev/c/RdEpjeYtb-4
Change-Id: I630511fca9eb291f0e414481ef439f18a8e1b72f
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396197
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The long term goal is for the DSLCPPCodeGenerator to replace the
CPPCodeGenerator entirely, but we will need both to coexist while DSL is
still under development.
Currently, the DSLCPPCodeGenerator is cloned from CPPCodeGenerator and
emits almost exactly the same code (it adds a comment at the top to
distinguish its output). Its output will change in followup CLs.
This CL also allows skslc to recognize the `_dsl.cpp` output suffix and
generate code using DSLCPPCodeGenerator instead of CPPCodeGenerator.
This allows test DSL FPs to be created for inspection.
Change-Id: If5136279c307ea53a9df3a292caa18344c1eb259
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396096
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
A subset of these signatures will be available to public SkSL, and
structured in a way that really pushes the coords as the primary
argument (and color as an optional one). In any case, I find this
ordering more natural.
Change-Id: I7b3bc962c5b305b9eeed1ae11ae1dc2ce7269364
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396021
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This is a reland of 45889d1269
Original change's description:
> Remove code to push pixmaps to backend textures from GrGpu classes
>
> Replace GrGpu::updateBackendTexture with narrower method that clears
> a backend texture.
>
> Creation of data for a solid color compressed texture is lifted up to
> GrDirectContext and goes through updateCompressedBackendTexture.
>
> Bug: skia:11786
> Change-Id: I1d617623df5e65686f30e57c361a64f78d77f7bd
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392836
> Reviewed-by: Jim Van Verth <jvanverth@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:11786
Change-Id: Ibf85794a33e68acb17f0fb704f6815cd6460b3f2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396098
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Also switches GrFillRRectOp to call usesMSAASurface.
Bug: skia:11396
Change-Id: I66d0ee2c6dc76819efc0f3283f6e1a0af11f23f7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395136
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This reverts commit 45889d1269.
Reason for revert: compressed texture assertion on D3D bot
Original change's description:
> Remove code to push pixmaps to backend textures from GrGpu classes
>
> Replace GrGpu::updateBackendTexture with narrower method that clears
> a backend texture.
>
> Creation of data for a solid color compressed texture is lifted up to
> GrDirectContext and goes through updateCompressedBackendTexture.
>
> Bug: skia:11786
> Change-Id: I1d617623df5e65686f30e57c361a64f78d77f7bd
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392836
> Reviewed-by: Jim Van Verth <jvanverth@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
TBR=egdaniel@google.com,jvanverth@google.com,bsalomon@google.com
Change-Id: Ie58f52245c44c77f09742b0cb590cc97b97e6e37
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11786
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396097
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
We are up to having seven distinct types of codegen, and will soon have
an 8th (DSL C++).
Change-Id: I6758328390c234ba1d5c30c118199dbc820af52a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395817
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I8ca4fbba551624c61a4ebe7a4716750fe0b48196
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395818
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Replace GrGpu::updateBackendTexture with narrower method that clears
a backend texture.
Creation of data for a solid color compressed texture is lifted up to
GrDirectContext and goes through updateCompressedBackendTexture.
Bug: skia:11786
Change-Id: I1d617623df5e65686f30e57c361a64f78d77f7bd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392836
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Also add unit test of all GLSL type aliases.
Bug: skia:10679
Change-Id: I93e21621c11adfe3f114d0c55fb8043518e62696
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395718
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
In http://review.skia.org/393397, I replaced a Constructor::Convert call
with a call directly to ConstructorCompoundCast::Make.
This worked fine if the input expression was actually a compound, but
if it was not, the code would assert/crash. The fuzzer detected this
error right away. (Enums are not considered to be a scalar, a vector or
a matrix in SkSL.)
Change-Id: Ie0df4c5771ff4f4d8f5251d4703e9c3516b6baad
Bug: oss-fuzz:33113
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395720
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Two of the tests are now (correctly) detected as invalid code, but were
previously checked into /shared/ because they compiled when the fuzzer
first reported them (that is, after the crash was fixed). These have
been moved into the /errors/ test folder.
One of the tests was only generating an error because its main function
was named `a` instead of `main`, so I renamed it to `main`.
Change-Id: I1a2346fb16e304b0c66ff377a3f9bf7e7ee89ba9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394899
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This allows the caller to decide whether a plan is feasible before
committing to it. In this CL the drawing manager doesn't actually
call it, but we test it. We'll call it in a follow-up CL
Bug: skia:10877
Change-Id: Ie3a6c14a0196f595c522a0c961aba7b10c980711
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394157
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
A follow-up CL allows the user to check the budget headroom
in between these two steps.
Bug: skia:10877
Change-Id: Ie0e6f23029065158b2eadb0dc26caf32f54433e1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394999
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
These builtins have been obsolete for a long time; they were only
implemented on the GLSL backend. These values are provided in
u_skRTHeight and u_skRTWidth instead. (See GrGLSLProgramBuilder's
addRTHeightUniform and addRTWidthUniform.)
Change-Id: I8cceca348cbf9071939618f913693c316d35dbc6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395001
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The code which assembled a uniform-scale matrix (aka
`ConstructorDiagonalMatrix`) accidentally looped over columns instead of
rows when assembling the matrix.
Change-Id: Iefb9ae645a7a85f61443e6e96b752ba729f40c9c
Bug: skia:11278
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394997
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This test fell into disrepair when constant propagation was removed; it
assumed that @if and @switch would work on all unchanging variables, not
just those with the `const` modifier.
Change-Id: Ie9c1816f9c0852fdea998e4e156047e4cca9ad5b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395000
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Adding new tests in an upcoming CL. This sets the stage for that.
I also outlined the Register ctor so that the diff in the next CL is
more readable.
Bug: skia:10877
Change-Id: I604f1ce811cddf1d1105716f114e27ce12bf8249
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394998
Commit-Queue: Adlai Holler <adlai@google.com>
Auto-Submit: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
DSLVar is normally implicitly converted to DSLExpression wherever it is
used, but the naively template-ized nature of DSLFunction::operator()
meant that it was accepting DSLVar parameters directly. Since DSLVar is
non-copyable, this meant DSLVar couldn't be passed directly to a DSL
function.
The smarter templates in this change are able to pass DSLVar by
reference.
Change-Id: Id04531f909cefd29b377c46e37fc4525bb29953c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394161
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
