This is a reland of 202ce887ec
Original change's description:
> Prefer the NV_framebuffer_blit extension over ANGLE
>
> The ANGLE/CHROMIUM glBlitFramebuffer requires full size blits, which
> is really bad for DMSAA. The NV extension does allow blits of
> sub-rectangles, so we prefer that one if it's available.
>
> Bug: skia:12176
> Bug: skia:12177
> Bug: skia:12234
> Bug: skia:12235
> Change-Id: I463ac631fff64ad564f294cf2f26be7410e8c356
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/430576
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
Bug: skia:12176
Bug: skia:12177
Bug: skia:12234
Bug: skia:12235
Change-Id: I41d062f9894f5a81c78a37de27a288bc17812643
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/431817
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This reverts commit 202ce887ec.
Reason for revert: Need to update Chrome first to give us the NV one.
Original change's description:
> Prefer the NV_framebuffer_blit extension over ANGLE
>
> The ANGLE/CHROMIUM glBlitFramebuffer requires full size blits, which
> is really bad for DMSAA. The NV extension does allow blits of
> sub-rectangles, so we prefer that one if it's available.
>
> Bug: skia:12176
> Bug: skia:12177
> Bug: skia:12234
> Bug: skia:12235
> Change-Id: I463ac631fff64ad564f294cf2f26be7410e8c356
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/430576
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: skia:12176
Bug: skia:12177
Bug: skia:12234
Bug: skia:12235
Change-Id: Iaa9f82e1c0c935a67dd8f446d1513b2b0a98f725
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/430819
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
The ANGLE/CHROMIUM glBlitFramebuffer requires full size blits, which
is really bad for DMSAA. The NV extension does allow blits of
sub-rectangles, so we prefer that one if it's available.
Bug: skia:12176
Bug: skia:12177
Bug: skia:12234
Bug: skia:12235
Change-Id: I463ac631fff64ad564f294cf2f26be7410e8c356
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/430576
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This is a reland of 76b80eca64
Original change's description:
> Use GL_ANGLE_instanced_arrays
>
> It turns out the es2 command buffer has supported instanced rendering
> all along. We just weren't using it! This CL starts using
> GL_ANGLE_instanced_arrays, after which point chrome es2 will start
> taking many of our instanced codepaths. It can even use tessellation if
> we update the shaders to not rely on gl_VertexID.
>
> Bug: chromium:1220246
> Change-Id: I173f4c07771691143d2540ecb9c3f8222ed8e4b3
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/422379
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
Bug: chromium:1220246
Change-Id: I4d6f74adff4e2eda4b55a459faf16b5f5a230d06
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/422616
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This reverts commit 76b80eca64.
Reason for revert: Chrome layout test diffs
Original change's description:
> Use GL_ANGLE_instanced_arrays
>
> It turns out the es2 command buffer has supported instanced rendering
> all along. We just weren't using it! This CL starts using
> GL_ANGLE_instanced_arrays, after which point chrome es2 will start
> taking many of our instanced codepaths. It can even use tessellation if
> we update the shaders to not rely on gl_VertexID.
>
> Bug: chromium:1220246
> Change-Id: I173f4c07771691143d2540ecb9c3f8222ed8e4b3
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/422379
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
TBR=bsalomon@google.com,csmartdalton@google.com
Change-Id: I62c25be4f6cf6937c0b5ab6e765a7113aad76916
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1220246
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/422441
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
It turns out the es2 command buffer has supported instanced rendering
all along. We just weren't using it! This CL starts using
GL_ANGLE_instanced_arrays, after which point chrome es2 will start
taking many of our instanced codepaths. It can even use tessellation if
we update the shaders to not rely on gl_VertexID.
Bug: chromium:1220246
Change-Id: I173f4c07771691143d2540ecb9c3f8222ed8e4b3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/422379
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This logic looks to have been dropped when we moved to the auto
generated interfaces
Bug: skia:11083
Change-Id: I5069d81c872ed2c084b8ddf773f63e5202b123eb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348190
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Since we are now referencing the exact methods instead of calling
GetProcAddress, there is no need to cast the function pointers. This
also catches a bug where the emscripten signatures for glWaitSync and
glClientWaitSync did not match the OpenGL API.
Change-Id: If365a8f6090961cb4b44146d87e102430bc5b30f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320659
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
This saves about 15kb of wasm code size (4kb gzipped), 11kb
of JS code size (2kb gzipped) and about 10ms
when creating a new surface (~35ms -> 25ms in a local test).
This also gates the webgl code more strongly (off unless you
really request it), since the headers won't be available
except in an emscripten environment.
Change-Id: I303f6c342c72e7cfe29be241f55ae8f5631a3f75
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311916
Reviewed-by: Brian Salomon <bsalomon@google.com>
This also brings indirect draw support to CanvasKit because Emscripten
will expose WEBGL_multi_draw through these extensions.
Bug: skia:10419
Change-Id: Icea3fedb846bb932e7d19f8f4a1e5646e25d4c2d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303263
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
This is how we will implement indirect draws in ANGLE and WebGL.
Emscripten will expose WEBGL_multi_draw through this extension.
Bug: skia:10419
Change-Id: I3fc56b130b06cdcc05a25be69878b398b8820f50
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/301736
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This reverts commit 7d7cd2b178.
Reason for revert: breaking chrome roll
Original change's description:
> Make glPatchParameteri required when tessellation is supported
>
> Chrome started wrangling this entrypoint in the following CL:
>
> https://chromium-review.googlesource.com/c/chromium/src/+/2253344
>
> Change-Id: Ief4dfb1ddca8d86e0c09dfdc443e01830cb4de20
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/298405
> Reviewed-by: Kevin Lubick <kjlubick@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
TBR=kjlubick@google.com,csmartdalton@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Change-Id: I569f2a16de156c5b2e819cb2a45ea942ca2cdec7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/298819
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Surfaces to client whether GrContext has seen a GL_OUT_MEMORY,
VK_ERROR_OUT_OF_HOST_MEMORY, or VK_ERROR_OUT_OF_DEVICE_MEMORY error.
Bug: chromium:1093997
Change-Id: I8e9799a0f7d8a74df056629d7d1d07c0d0a0fe30
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/298216
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This is a reland of d84b691950
Original change's description:
> Reland "Use glDraw.*BaseInstance calls to avoid deferred buffer binding"
>
> This is a reland of e8c963d474
>
> Original change's description:
> > Use glDraw.*BaseInstance calls to avoid deferred buffer binding
> >
> > Change-Id: I968dab317673051acc65f87ea76a0d657d89b3d2
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/279538
> > Commit-Queue: Chris Dalton <csmartdalton@google.com>
> > Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
>
> Change-Id: I79b2d23e5e66d47214898a9068079b6fe2269599
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/280806
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
Change-Id: I9240c5cc6e58ce196be393c649bbdd86560602f0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/281077
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This is a reland of e8c963d474
Original change's description:
> Use glDraw.*BaseInstance calls to avoid deferred buffer binding
>
> Change-Id: I968dab317673051acc65f87ea76a0d657d89b3d2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/279538
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
Change-Id: I79b2d23e5e66d47214898a9068079b6fe2269599
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/280806
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This reverts commit e8c963d474.
Reason for revert: assert failures: https://logs.chromium.org/logs/skia/4b4b7fcb2ff3fe11/+/steps/dm/0/stdout
Original change's description:
> Use glDraw.*BaseInstance calls to avoid deferred buffer binding
>
> Change-Id: I968dab317673051acc65f87ea76a0d657d89b3d2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/279538
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,csmartdalton@google.com,michaelludwig@google.com
Change-Id: Ic760a56ca9d112e924baf7e833adb09b371928b0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/280817
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Change-Id: I968dab317673051acc65f87ea76a0d657d89b3d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/279538
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Adds the ANGLE extension to our list of options for
glDraw*BaseInstance.
Removes glDrawElementsInstancedBaseInstance since this method is not
supported by the ANGLE extension.
Change-Id: Ifd25324feeaad55a4d141b5064bcd8e059948ef7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/280388
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This CL has a complicated back story, but it's concrete change is
simple, just turning the warning on and converting a bunch of
return foo;
to
return std::move(foo);
These changes are exclusively in places where RVO and NRVO do not apply,
so it should not conflict with warnings like -Wpessimizing-move.
Since C++11, when you return a named local and its type doesn't match
the declared return type exactly, there's an implicit std::move()
wrapped around the value (what I'm making explicit here) so the move
constructor gets an opportunity to take precedence over the copy
constructor. You can read about this implicit move here under the
section "automatic move from local variables and parameters":
https://en.cppreference.com/w/cpp/language/return#Notes.
This situation comes up for us with smart pointers: a function declares
its return type as std::unique_ptr<Base> or sk_sp<Base>, and we return a
std::unique_ptr<Impl> or sk_sp<Impl>. Those types don't match exactly,
so RVO and NRVO don't come into play. They've always been going through
move constructors, and that's not changed here, just made explicit.
There was apparently once a bug in the C++11 standard and compilers
implementing that which made these copy instead of move, and then this
sort of code would do a little unnecessary ref/unref dance for sk_sp,
and would entirely fail to compile for uncopyable std::unique_ptr.
These explicit moves ostensibly will make our code more compatible with
those older compilers.
That compatibility alone is, I think, a terrible reason to land this CL.
Like, actively bad. But... to balance that out, I think the explicit
std::move()s here actually help remind us that RVO/NRVO are not in play,
and remind us we're going to call the move constructor. So that C++11
standard bug becomes kind of useful for us, in that Clang added this
warning to catch it, and its fix improves readability.
So really read this all as, "warn about implicit std::move() on return".
In the end I think it's just about readability. I don't really hold any
hope out that we'll become compatible with those older compilers.
Bug: skia:9909
Change-Id: Id596e9261188b6f10e759906af6c95fe303f6ffe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/271601
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Will be used for YUV420 readback on ANGLE ES 2 contexts in Chrome.
Marks the functions as required as Chrome now adds inits these on
GrGLInterface.
Bug: 1040643
Change-Id: I5504d4c9209874991592c9f86aaf7987c316aa40
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/265602
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Also add macro for ALL_COMPLETED
We plan to use this with ANGLE ES2 contexts.
Change-Id: I6c967fa9b9aefc41c31cd806aece8533f846f940
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/265756
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
There are devices that advertise this extension but don't actually
provide the GL entrypoints. This is causing live crashes.
Bug: flutter:47164
Bug: flutter:47804
Change-Id: Idded47e8dbd4462463ad91a62b06b0df3245f0c1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/263809
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Implements tessellation support at the Ganesh level, and adds back
door methods for supplying raw GLSL strings directly to the OpenGL
driver. Adds a new gm to verify tessellation is works in GL.
Change-Id: Idfc285b955cbe5e8e6bf0475be8b518b0cc6ed2c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/261196
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This is a reland of 7e6d3b00ad
Original change's description:
> Use GL_QCOM_TILED_RENDERING to explicitly discard stencil
>
> Bug: skia:
> Change-Id: I7a6234f6a9955dbda83ab2d8fc6930fe680e102b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/245529
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
Bug: skia:
Change-Id: Ifb2ac1a595be0ca55d75d53d85bf76f7d98e1c15
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/246282
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Change-Id: I127c979670c3dc7dac2e35908a795afbdefca8f6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/234902
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Change-Id: I6f9ee51f7c063ca03bf48fccd413dae244edd191
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/221778
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This reverts commit 753836fcad.
Reason for revert: fixed
Original change's description:
> Revert "Get EGLimage functions out of GrGLInterface."
>
> This reverts commit bc233135e4.
>
> Reason for revert:
> EGLImageTest and TextureBindingTest broken on Windows ANGLE
> Build failures on Debian9, unable to link
> ld.lld: error: undefined symbol: eglCreateImageKHR
>
> Original change's description:
> > Get EGLimage functions out of GrGLInterface.
> >
> > Only used in test code which can just directly call these.
> >
> > Also removed related code in interface assemble/validate generator.
> >
> > Change-Id: I7e821ebb9328bdd2a54d8ab3af929399ca0517d5
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/213480
> > Commit-Queue: Robert Phillips <robertphillips@google.com>
> > Reviewed-by: Robert Phillips <robertphillips@google.com>
>
> TBR=bsalomon@google.com,robertphillips@google.com
>
> Change-Id: I622fadf22c5d79f5b1d335a8a81455978d49a86a
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/213960
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
TBR=bsalomon@google.com,robertphillips@google.com,michaelludwig@google.com
EXTRA_TRYBOTS=Test-Win10-Clang-AlphaR2-GPU-RadeonR9M470X-x86_64-Debug-All-ANGLE
Change-Id: I71397072fa79ae3c72f3835c5e991b9ef2465a9a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/214040
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This reverts commit bc233135e4.
Reason for revert:
EGLImageTest and TextureBindingTest broken on Windows ANGLE
Build failures on Debian9, unable to link
ld.lld: error: undefined symbol: eglCreateImageKHR
Original change's description:
> Get EGLimage functions out of GrGLInterface.
>
> Only used in test code which can just directly call these.
>
> Also removed related code in interface assemble/validate generator.
>
> Change-Id: I7e821ebb9328bdd2a54d8ab3af929399ca0517d5
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/213480
> Commit-Queue: Robert Phillips <robertphillips@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
TBR=bsalomon@google.com,robertphillips@google.com
Change-Id: I622fadf22c5d79f5b1d335a8a81455978d49a86a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/213960
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Only used in test code which can just directly call these.
Also removed related code in interface assemble/validate generator.
Change-Id: I7e821ebb9328bdd2a54d8ab3af929399ca0517d5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/213480
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This reverts commit 77af4ecd35.
Reason for revert: iOS errors and bad images on Gold
Original change's description:
> Support GL_EXT_draw_buffers
>
> Change-Id: I078aa7c42de4368602b0ef43bd7e18efbfd1e049
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/212182
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
TBR=bsalomon@google.com,robertphillips@google.com,brianosman@google.com
Change-Id: Ib8759d4a1009f9a8774365a699e211834d634cff
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/212264
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I078aa7c42de4368602b0ef43bd7e18efbfd1e049
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/212182
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Tested by:
cd tools/gpu/gl/interface/
go run *.go --in_table "./interface.json5" -out_dir "../../../../src/gpu/gl/"
Cq-Include-Trybots: skia.primary:Housekeeper-PerCommit-CheckGeneratedFiles
Change-Id: I1fee1263e321580c0e8c4e41d345fc856ef2a4e7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/210128
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
There is some logic in here for 2.0 as well, just as a
"as long as I was looking at the specs", but only 1.0
is really supported.
This seems to resolve the bug where some GPUs weren't
advertising correctly that they had vertex array object
support, by checking for both extension names (with and
without "GL_" prefix)
Of note, this saves about 18 Kb (5.5 Kb gzipped) of code size
by compiling out the unneeded GLES checks/functionality.
Bug: skia:8378
Change-Id: I773bf4dbf231b991051d2a9f640b8047a9010e7d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/203461
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
