I was just reading the file and noticed these old leftovers
from before https://skia-review.googlesource.com/c/skia/+/255086.
Change-Id: I8711809e90ac49d0a7190030d8e4afe78459a1b8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313096
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Although cool, I don't know if this is worth the increased complexity
for an unused feature.
Change-Id: Id229beab176cebf5b7810fd1e44a5d1e25abcabc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312848
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Store packing in a separate byte; we can't use the upper
pointer bits like this these days.
Simple refactoring of how footers are installed and read,
adding installRaw(const T&) and using it to implement the
rest. Footer still exists as a layout reminder, and to
answer sizeof(Footer), but it's all memcpy() in and out.
Bug: skia:10663
Change-Id: I7e55a005e4f6ce11d48e827cca9edf9e517bc620
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313066
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
There is no more 'inout half4 color'. Effects return their output color.
If an effect wants the input color, it must use the (already existing)
approach of sampling a nullptr input shader.
The change is guarded for Chromium (so we can update their runtime color
filters in skia_renderer.cc).
For the GPU backend, FPs can now override usesExplicitReturn to indicate
that their emitCode will generate a return statement. If that's true,
then writeProcessorFunction doesn't inject the automatic return of the
output color, and emitFragProc will *always* wrap that FP in a helper
function, even as a top-level FP. GrSkSLFP opts in to this behavior, so
that the user-supplied return becomes the actual return in the FP's
emitCode.
Adapting the skvm code to this wasn't too bad: It looks fragile (what
happens if there are multiple returns?), but that's not really possible
today, without varying control flow.
Bug: skia:10613
Change-Id: I205b81fd87dd32bab30b6d6d5fc78853485da036
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310756
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Within a DRAW_BEGIN/DRAW_END pair, all other call sites use
`draw.paint()` as their paint. Certain types of ImageFilter won't work
properly unless we use the SkPaint from the AutoLayerForImageFilter.
New tests were added to the `imagefiltersbase` GM to demonstrate the
issue, at http://review.skia.org/312842
Change-Id: Ifa8bfa4b8a44cfa2c756d9633424027292a9319f
Bug: skia:10660
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312838
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
4f7edbe1f6..7bce5194d1
2020-08-25 syoussefi@chromium.org Vulkan: Simplify image read barrier necessity check
2020-08-25 m.maiya@samsung.com Vulkan: Add FastIntegerSet and FastIntegerMap class
2020-08-24 lehoangq@gmail.com Metal: autogen for 3D texture's mipmap generating shader.
2020-08-24 angle-autoroll@skia-public.iam.gserviceaccount.com Roll Vulkan-ValidationLayers from 0d6b4440549b to 76e8dee41452 (3 revisions)
2020-08-24 angle-autoroll@skia-public.iam.gserviceaccount.com Roll SPIRV-Tools from b79773a35d52 to 4dd122392f3a (1 revision)
2020-08-24 angle-autoroll@skia-public.iam.gserviceaccount.com Roll SwiftShader from 44e1791f100c to 622558b02e1a (3 revisions)
2020-08-24 angle-autoroll@skia-public.iam.gserviceaccount.com Roll glslang from f257e0ea6b9a to 983698bb34ec (2 revisions)
If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/angle-skia-autoroll
Please CC mtklein@google.com on the revert to ensure that a human
is aware of the problem.
To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug
Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/master/autoroll/README.md
Cq-Include-Trybots: skia/skia.primary:Build-Debian10-Clang-x86_64-Release-ANGLE;skia/skia.primary:Test-Win10-Clang-AlphaR2-GPU-RadeonR9M470X-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-Golo-GPU-QuadroP400-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-NUC5i7RYH-GPU-IntelIris6100-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-NUC6i5SYK-GPU-IntelIris540-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-NUC8i5BEK-GPU-IntelIris655-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-NUCD34010WYKH-GPU-IntelHD4400-x86_64-Debug-All-ANGLE
Bug: None
Tbr: mtklein@google.com
Test: Test: angle_unittests.exe --gtest_filter=FastInteger*
Change-Id: Ief36f9d5114cc694be918cbc81a753700b769ef9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312978
Reviewed-by: skia-autoroll <skia-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: skia-autoroll <skia-autoroll@skia-public.iam.gserviceaccount.com>
This reverts commit cace33fc31.
Reason for revert: Breaks nexus 7
Original change's description:
> change atlas uv encoding
>
> Move the page encoding to bit 14 of UV from bit 0. It would be nice
> to use bit 15, but older versions of the iphone don't handle the bit 15
> correctly.
>
> Change-Id: Ia8f1a742dfbc85514f8057fdace0e7954aecc593
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312640
> Reviewed-by: Jim Van Verth <jvanverth@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
TBR=jvanverth@google.com,herb@google.com
Change-Id: If3634d8798ee940e02aa5d53c53ca5186060cdc9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312889
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This is a reland of 0baaa23722
Original change's description:
> Add a patch and an atlas to the ImageFilter GM slide.
>
> This exposes a recently-discovered issue where image filters were not
> always applied to some types of canvas draws.
>
> Change-Id: I17ef000d067ad2a41f06ff6e2220ea9915db6f99
> Bug: skia:10660
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312842
> Commit-Queue: Mike Reed <reed@google.com>
> Reviewed-by: Mike Reed <reed@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
Bug: skia:10660
Change-Id: Ia6c9278be6e920c1217a89593e4007c2ed0e8af7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312887
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This reverts commit 0baaa23722.
Reason for revert: MSAN bot issue
Original change's description:
> Add a patch and an atlas to the ImageFilter GM slide.
>
> This exposes a recently-discovered issue where image filters were not
> always applied to some types of canvas draws.
>
> Change-Id: I17ef000d067ad2a41f06ff6e2220ea9915db6f99
> Bug: skia:10660
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312842
> Commit-Queue: Mike Reed <reed@google.com>
> Reviewed-by: Mike Reed <reed@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
TBR=mtklein@google.com,bsalomon@google.com,reed@google.com,johnstiles@google.com
Change-Id: I9c672de3e0d14a183b1f64475b56331754a9fe7e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10660
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312883
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
- Don't create an empty options object just to destroy it.
- Don't copy the shader source into the NSString.
- Allow UTF-8 shader source, not just ASCII.
Change-Id: Ic15ba5d315e42875e6db43af086c23a905c1dac4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312837
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This exposes a recently-discovered issue where image filters were not
always applied to some types of canvas draws.
Change-Id: I17ef000d067ad2a41f06ff6e2220ea9915db6f99
Bug: skia:10660
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312842
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Move the page encoding to bit 14 of UV from bit 0. It would be nice
to use bit 15, but older versions of the iphone don't handle the bit 15
correctly.
Change-Id: Ia8f1a742dfbc85514f8057fdace0e7954aecc593
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312640
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Herb Derby <herb@google.com>
I'm going to make this function even more complex (with a thread-safe
shared uniquely-keyed proxy cache) so remove it from the header.
Change-Id: I810ed095729ac30862a8713624e6440380b1128d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312862
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This reverts commit 6541013b53.
Reason for revert: TSAN issues with GrFence, and crash in GrMtlPipelineStateBuilder::CreatePipelineState.
Original change's description:
> Remove ARC from Metal backend
>
> Change-Id: I5ab28f6eda3b37d1b82c94c7cc6eaa2ce59157da
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311113
> Reviewed-by: Adlai Holler <adlai@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Jim Van Verth <jvanverth@google.com>
TBR=jvanverth@google.com,bsalomon@google.com,adlai@google.com,johnstiles@google.com
Change-Id: I031629b483fc46de8bd3751253e5391c2ce87853
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312843
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Log the SkImageInfo's width and height directly since those are the
values later used by the HeapAllocator.
Change-Id: I01322f55df711141e03fab60da65e22a5c835ca1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312839
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Change-Id: Icc731852d3b9efdec70c649cfcc4b5c9a47bc584
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312492
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>
The SkSL analyzer did not visit VarDeclarations program elements,
whereas the previous nodeCount() system did, leading to an unintentional
difference in effective inlining thresholds. The more relaxed inlining
threshold was leading to compiler hangs on Metal.
Change-Id: Ic4980f3238cb644e395525edca6737c996225b2e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312836
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
The font bounds may be empty, so label the tight bounds instead,
especially since the labels are of the glyph id that touches that edge
of the tight bounds. Also rotate the labels so they dont' run into each
other.
This also fixes SkMetaData::set so that changing an existing value
doesn't cause strange issues with iterators or attempt to use data from
the previous rec after it's been freed. (Found by running viewer in a
asan build.)
Change-Id: Id255beff5d05310f098bd14baf0935e5fd349e7e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312494
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Change-Id: I7362e16182d42c971a2cb5bdcbdc242ef900d6b5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312493
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Tunnels through SkImageGenerator as well.
The new SkCodec interface doesn't assume three 8 bit planes.
New SkYUVASpec more clearly defines chroma subsampling and siting of
the planes.
The intent is to use this for other YUVA APIs as well, in particular
SkImage factories in the future.
In this change we convert to the SkYUVASpec to SkYUVASizeInfo
and SkYUVAIndex[4] representation. But the intent is to use
the SkYUVASpec representation throughout the pipeline once
legacy APIs are removed.
orientation GM is replicated to test a variety of chroma
subsampling configs.
Bug: skia:10632
Change-Id: I3fad35752b87cac16c51b24824331f2ae7d458d3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309658
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Namespace comments don't assist readability for blocks that can easily
fit on a single screen.
Change-Id: I93cbebe8e51400dead794c9eb41cb1eaa86bf756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312639
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Most of this is just not mentioning things that need not be mentioned
anymore, but there's one slight style tweak which I think is better,
from
case Foo: {
...
break;
}
to
case Foo: {
...
} break;
which is a little clearer that all paths through case Foo break
when the contents of the {} block become complicated.
Change-Id: Id7fe5ab09437006d125313b07862613316dd403c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312576
Reviewed-by: Brian Salomon <bsalomon@google.com>
It is on skia_sksl_gpu_sources list already.
Bug: skia:10650
Change-Id: I1e4bb0b2346ad8a74ddbdb3d3ff44ea7de6c0cd3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312736
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
The nullptr/0 distinction should go without saying,
and I think worrying about any of the rest just burns brainpower.
Change-Id: I7d0aea300f114e512437c3820f4e80a1408575c1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312472
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
d3e800e9ad..4f7edbe1f6
2020-08-23 jmadill@chromium.org State: Add dirty object for active textures.
2020-08-23 dpranke@google.com Set use_xvfb where needed in test wrappers.
2020-08-23 lehoangq@gmail.com Metal: Implement GL_OES_texture_3D
2020-08-23 lehoangq@gmail.com D3D11: Disable OES_texture_3D.
2020-08-23 lehoangq@gmail.com Metal: Implement EXT_draw_buffers & ANGLE_framebuffer_blit
2020-08-22 cclao@google.com Vulkan: Use context staging buffer for copyImageDataToBuffer
2020-08-22 cnorthrop@google.com Capture/Replay: Fixes for PUBG:Mobile
2020-08-21 jmadill@chromium.org Feedback Loop Redesign 3/3: Remove feedback loop tracking.
2020-08-21 ianelliott@google.com Vulkan: Fix compilation error
2020-08-21 ianelliott@google.com Vulkan: do not end render pass when invalidating
2020-08-21 jmadill@chromium.org Remove feedback loop support from back-end.
2020-08-21 angle-autoroll@skia-public.iam.gserviceaccount.com Roll Vulkan-Loader from 2979391e5b0c to e1c7eaa74142 (1 revision)
2020-08-21 angle-autoroll@skia-public.iam.gserviceaccount.com Roll SPIRV-Tools from a711c594b8cc to b79773a35d52 (1 revision)
2020-08-21 angle-autoroll@skia-public.iam.gserviceaccount.com Roll SwiftShader from cbfd396756c6 to 44e1791f100c (1 revision)
2020-08-21 cclao@google.com Vulkan: Rename mStagingBufferStorage to mStagingBuffer
If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/angle-skia-autoroll
Please CC mtklein@google.com on the revert to ensure that a human
is aware of the problem.
To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug
Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/master/autoroll/README.md
Cq-Include-Trybots: skia/skia.primary:Build-Debian10-Clang-x86_64-Release-ANGLE;skia/skia.primary:Test-Win10-Clang-AlphaR2-GPU-RadeonR9M470X-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-Golo-GPU-QuadroP400-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-NUC5i7RYH-GPU-IntelIris6100-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-NUC6i5SYK-GPU-IntelIris540-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-NUC8i5BEK-GPU-IntelIris655-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-NUCD34010WYKH-GPU-IntelHD4400-x86_64-Debug-All-ANGLE
Bug: chromium:816629
Tbr: mtklein@google.com
Test: Test: Capture first 2000 frames of PUBG:MobileTest: Test: PUBG MOBILE on AndroidTest: Test: angle_deqp_gles3_tests --gtest_filter=dEQP.GLES3/functional_fbo_invalidate_* --use-angle=vulkanTest: Test: angle_white_box_tests --gtest_filter=VulkanPerformanceCounterTest.InvalidatingAndUsingDepthDoesNotBreakRenderPass/*
Change-Id: Ic115bb23eed6904e9292db4b52f670c2296fe38d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312702
Reviewed-by: skia-autoroll <skia-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: skia-autoroll <skia-autoroll@skia-public.iam.gserviceaccount.com>
Where not using the input attachments yet, but I want to land this first
to make sure there are no regressions from setting this flag.
Bug: skia:10409
Change-Id: I0d43a9a8feea7f1ce67eb661ada7963fd7602489
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312490
Auto-Submit: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Variable fonts don't vary the conservitive bounds so they get reported
as empty. Handle this by doing layout based on the join of the tight and
conservative bounds and draw both.
Change-Id: I6e980a2d20514d40a4b97ca55a647055c847c171
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312479
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This is a reland of 26766ad427
Original change's description:
> Add utilities to SkGeometry for quad and cubic rotation angles
>
> Adds methods to measure the rotation angles of quadratics and cubics,
> and to chop curves at locations that divide the rotation angle in half.
>
> Bug: skia:10419
> Change-Id: I840e12034fc66e1a459de875fefda07a27a78335
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308880
> Reviewed-by: Jim Van Verth <jvanverth@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
Bug: skia:10419
Change-Id: I62ea6847a91c054174f829962a901f21910c013b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312438
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Change-Id: Id2f3ed80c76c4c409afdd2fa86c9b8e7fd1266ab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312485
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>
Change-Id: Ic09346b6079e6f316c28e03ddb02f12b4af8a38d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312482
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
We want to ensure perf of ES3 is always >= ES2
Bug: skia:10644
Change-Id: Idf92157f6b0f1bcd705f8365c256f1ae4266616b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312400
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Fuzzer found one of these, and I spotted the other during visual
inspection and manual testing.
Bug: oss-fuzz:25109
Change-Id: I8273231d9f76fe55459d4b742225627dcc524e6f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312476
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:10563
Change-Id: I64f12f57de4482cb6676ad8dc9b96d150cc4b3de
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312337
Commit-Queue: Ravi Mistry <rmistry@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
We will still need to go through clients to remove uses of the main ctors
so that it can be deleted.
Change-Id: I7bdfa00ac56b2404cc7b2f183104ee97b4da1de7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311452
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>