Use SkVMBlitter::DebugName for the string. This is easier to read, and
has some actually useful information about the blitter.
Added magenta highlight of the hovered element, similar to the GPU.
Change-Id: Ic9b5a0f61e092c8aa555f375d5d2f2de22cc45fe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467977
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Caveats:
- You want to run viewer with `--skvm` to see much/anything
- The cache is LRU, and doesn't get flushed automatically. Hitting
'Clear' will flush it, so it will pick up only the blitters used
on the current slide. (Otherwise they accumulate as you navigate).
- No way to determine which blitter is for which primitive, yet.
I'd like to do the GPU-style magenta highlight, but that may be
tricky, because we need to preserve the semantics of the original
blitter (including destination color type, most importantly).
Change-Id: I2df763fdb697d87471ca0816a3b7087ffb4fc4e4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467783
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Makes the path sniffing code from BisectSlide reusable.
Change-Id: I21c1e752590359c557c35804e5d5044d8041a308
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/460637
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This consolidates tools/flags/CommonFlagsFontMgr.h into
tools/flags/CommonFlags.h and adds all those common
flags into the CommonFlags namespace.
I also cleaned up a few unused includes in DM.cpp
Change-Id: I1d1bf6598dfb87619b6abbb9faa1e24631f76fb4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459476
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Also noticed while trying to build GMs in WASM with
Bazel.
Change-Id: I33d467a0da0893c1a5e376f4fd1a6096dad48af3
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459198
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Adds a type enum to WindowContext to determine which kind of
GPU context (GrDirectContext or skgpu::Context) we're using.
Bug: skia:12466
Change-Id: I288878740392a43cd9e82c925fbe2c372d140dc5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/454699
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Also update RELEASE_NOTES to describe new syntax.
Change-Id: I2666551b98f80b61ae3a48c92a9e306cdc7242b0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444735
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Bug: skia:12302
Change-Id: I8cf958acf9214d0de903a4097647afd74f2a659e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441541
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This CL just moves the files and renames them. It doesn't move them into the skgpu::v1 namespace.
Bug: skia:11837
Change-Id: Iab322d0dc5b5d1cfd32436785081539dc85c18d3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440776
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Having this enum on GrTessellationPathRenderer forced it to be over-#included and was blocking making GrTessellationPathRenderer.h v1-only.
Bug: skia:11837
Change-Id: I80660ed659946d7aa555057c9f4fd1136b44cca0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440536
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
If we manage to fix all the existing cases of variable shadowing, we
could enable -Wshadow.
(Turtle.cpp is #included from a cpp in the tools directory.)
Change-Id: I1685086ec0ceae1d51efa7daa0f46137b535ce77
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438476
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
I noticed Viewer lagging significantly when interacting with certain
sliders (such as transform state). It turns out that the UI tracking
would trigger calling Window::setRequestedDisplayParams and that in
turn can trigger a full context recreation, depending on backend.
I'd recently changed GPUs in my machine and apparently its context
creation is substantially slower than my previous one. Historically
I noticed minor jank when interacting, but it was never a deal
breaker.
This splits the parameter tracking into two categories so that lighter
weight widgets can still trigger window invalidation / re-rendering,
without triggering the context creation.
Change-Id: I3eb4c15b802f8b8ea8d8eca386de5dcee22ba9ec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437685
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Since:
https://skia-review.googlesource.com/c/skia/+/431539 (Feed all top-level GPU accessors through skgpu::BaseDevice (take 2))
The OveridePaintFilterCanvas now blocks access to the true SurfaceDrawContext that backs the top device of a GPU-backed SkCanvas. This is because the SkPaintFilterCanvas doesn't pass on SkCanvas::topDevice calls to the canvas it is wrapping so it always returns a SkNoPixelsDevice.
Given that accessing the top SDC is an incredibly specialized testing-only feature this CL keeps the feature working short-term w/o gumming up the public API.
Change-Id: I99012ba34c2800e0149251667156b412c4e8aa63
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/433362
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Creates a new path renderer, GrAtlasPathRenderer, that handles all the
atlasing. Managing the atlas in its own path renderer gives us more
control over when atlasing happens in the chain, will allow us to more
easily use the atlas in kCoverage mode, and makes the clipping code
cleaner.
Bug: skia:12258
Change-Id: Ie0b669974936c23895c8ab794e2d97206ed140f8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/431896
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
In a perfect world, this should always render the same, just perhaps a
bit more slowly.
Change-Id: I750ad43142d4d192be4db7396989d978025179a8
Bug: skia:12080
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/429101
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 is a reland of e58831cd95
Original change's description:
> Add format-specifier warnings to SkDebugf.
>
> This CL fixes up many existing format-specifier violations in Skia.
> Note that GCC has a warning for formatting nothing, so existing calls to
> `SkDebugf("")` have been removed, or replaced with `SkDebugf("%s", "")`.
> These were apparently meant to be used as a place to set a breakpoint.
>
> Some of our clients also use SkDebug with bad format specifiers, so this
> check is currently only enabled when SKIA_IMPLEMENTATION is true.
>
> Change-Id: I8177a1298a624c6936adc24e0d8f481362a356d0
> Bug: skia:12143
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/420902
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:12143
Change-Id: Id3c0c21436ebd13899908d5ed5d44c42a0e23921
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/421918
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit e58831cd95.
Reason for revert: looks like breaking a few build bots
Original change's description:
> Add format-specifier warnings to SkDebugf.
>
> This CL fixes up many existing format-specifier violations in Skia.
> Note that GCC has a warning for formatting nothing, so existing calls to
> `SkDebugf("")` have been removed, or replaced with `SkDebugf("%s", "")`.
> These were apparently meant to be used as a place to set a breakpoint.
>
> Some of our clients also use SkDebug with bad format specifiers, so this
> check is currently only enabled when SKIA_IMPLEMENTATION is true.
>
> Change-Id: I8177a1298a624c6936adc24e0d8f481362a356d0
> Bug: skia:12143
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/420902
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I07848c1bf8992925c9498e916744d0840355a077
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:12143
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/421917
Reviewed-by: Tyler Denniston <tdenniston@google.com>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
This CL fixes up many existing format-specifier violations in Skia.
Note that GCC has a warning for formatting nothing, so existing calls to
`SkDebugf("")` have been removed, or replaced with `SkDebugf("%s", "")`.
These were apparently meant to be used as a place to set a breakpoint.
Some of our clients also use SkDebug with bad format specifiers, so this
check is currently only enabled when SKIA_IMPLEMENTATION is true.
Change-Id: I8177a1298a624c6936adc24e0d8f481362a356d0
Bug: skia:12143
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/420902
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is a reland of 7bf6bc0d06
Original change's description:
> Purge ccpr
>
> Now that the clip atlas has been successfully migrated to
> tessellation, we don't need this code anymore!
>
> Change-Id: Ic97f50cff7c4ee59f4476f8410f0b30a32df4e90
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/419857
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
Change-Id: If0be86902e7cc4755eba91a89be1ec1a6a4b54b2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/419720
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This reverts commit 7bf6bc0d06.
Reason for revert: Android build references kCoverageCounting
Original change's description:
> Purge ccpr
>
> Now that the clip atlas has been successfully migrated to
> tessellation, we don't need this code anymore!
>
> Change-Id: Ic97f50cff7c4ee59f4476f8410f0b30a32df4e90
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/419857
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
TBR=robertphillips@google.com,brianosman@google.com,csmartdalton@google.com,michaelludwig@google.com
Change-Id: I01d99287978f848eb8bf900c07cba90ceb3b6edc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/419898
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Now that the clip atlas has been successfully migrated to
tessellation, we don't need this code anymore!
Change-Id: Ic97f50cff7c4ee59f4476f8410f0b30a32df4e90
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/419857
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This CL preserves the "StringFragment" name as an alias for
string_view to reduce the impact. The StringFragment alias
will be removed in a followup CL.
Change-Id: I89209bc626b0be0d0190823b6217f4c83cafe1bc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/416736
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Skia has an atlas clipping mode (historically called CCPR clipping) that
doesn't quite work. It will fail on blend modes that require a dst read
and is also drawing the complexclip_* tests wrong on several devices.
We plan to rewrite the clip atlas, but in the meantime we need to
disable the "CCPR" clipping.
Bug: b/188794626
Change-Id: I126030a2a81057ffca85fd8b8e7db97b716451ab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408557
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Also three fixes for drawing to offscreen layers in MSKPPlayer:
*Only play from last full redraw to next cmd
*Clear before full redraw
*Actually track current cmd in layer state.
Bug: skia:11900
Change-Id: I988afb61f96c8acb7e7554d65bfa6cd6020196c7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/407460
Reviewed-by: Nathaniel Nifong <nifong@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This is a reland of adadb95a9f
... adds a temporary workaround for some Android framework code.
Original change's description:
> Better first-class shader & color filter support in runtime effects
>
> This does a few things, because they're all intertwined:
>
> 1) SkRuntimeEffect's API now includes details about children (which Skia
> stage was declared, not just the name). The factories verify that the
> declared types in the SkSL match up with the C++ types being passed.
> Today, we still only support adding children of the same type, so the
> checks are simple. Once we allow mixing types, we'll be testing the
> declared type against the actual C++ type supplied for each slot.
> 2) Adds sample variants that supply the input color to the child. This
> is now the only way to invoke a colorFilter child. Internally, we
> support passing a color when invoking a child shader, but I'm not
> exposing that. It's not clearly part of the semantics of the Skia
> pipeline, and is almost never useful. It also exposes users to
> several inconsistencies (skbug.com/11942).
> 3) Because of #2, it's possible that we can't compute a reusable program
> to filter individual colors. In that case, we don't set the constant
> output for constant input optimization, and filterColor4f falls back
> to the slower base-class implementation.
>
> Bug: skia:11813 skia:11942
> Change-Id: I06c41e1b35056e486f3163a72acf6b9535d7fed4
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401917
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Mike Klein <mtklein@google.com>
Bug: skia:11813 skia:11942
Change-Id: I2c31b147ed86fa8c4dddefb7066bc1d07fe0d285
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/404637
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This reverts commit adadb95a9f.
Reason for revert: breaking android
Original change's description:
> Better first-class shader & color filter support in runtime effects
>
> This does a few things, because they're all intertwined:
>
> 1) SkRuntimeEffect's API now includes details about children (which Skia
> stage was declared, not just the name). The factories verify that the
> declared types in the SkSL match up with the C++ types being passed.
> Today, we still only support adding children of the same type, so the
> checks are simple. Once we allow mixing types, we'll be testing the
> declared type against the actual C++ type supplied for each slot.
> 2) Adds sample variants that supply the input color to the child. This
> is now the only way to invoke a colorFilter child. Internally, we
> support passing a color when invoking a child shader, but I'm not
> exposing that. It's not clearly part of the semantics of the Skia
> pipeline, and is almost never useful. It also exposes users to
> several inconsistencies (skbug.com/11942).
> 3) Because of #2, it's possible that we can't compute a reusable program
> to filter individual colors. In that case, we don't set the constant
> output for constant input optimization, and filterColor4f falls back
> to the slower base-class implementation.
>
> Bug: skia:11813 skia:11942
> Change-Id: I06c41e1b35056e486f3163a72acf6b9535d7fed4
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401917
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Mike Klein <mtklein@google.com>
TBR=mtklein@google.com,bsalomon@google.com,brianosman@google.com
Change-Id: I94ba57e73305b2302f86fd0c1d76f667d4e45b92
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11813 skia:11942
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/404117
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This does a few things, because they're all intertwined:
1) SkRuntimeEffect's API now includes details about children (which Skia
stage was declared, not just the name). The factories verify that the
declared types in the SkSL match up with the C++ types being passed.
Today, we still only support adding children of the same type, so the
checks are simple. Once we allow mixing types, we'll be testing the
declared type against the actual C++ type supplied for each slot.
2) Adds sample variants that supply the input color to the child. This
is now the only way to invoke a colorFilter child. Internally, we
support passing a color when invoking a child shader, but I'm not
exposing that. It's not clearly part of the semantics of the Skia
pipeline, and is almost never useful. It also exposes users to
several inconsistencies (skbug.com/11942).
3) Because of #2, it's possible that we can't compute a reusable program
to filter individual colors. In that case, we don't set the constant
output for constant input optimization, and filterColor4f falls back
to the slower base-class implementation.
Bug: skia:11813 skia:11942
Change-Id: I06c41e1b35056e486f3163a72acf6b9535d7fed4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401917
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Default the background color to transparent black, which is correct for
mskps captures from Android framework.
Bug: skia:11900
Change-Id: I97cab04993c0f85259e831f3d0c44f9b962365af
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/403077
Reviewed-by: Nathaniel Nifong <nifong@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This is a reland of 3b7587814d
Use SkTPin instead of std::clamp (C++17 library feature).
Original change's description:
> FPS and frame control for MSKP slide.
>
> Bug: skia:11900
> Change-Id: Ib4d8da6a86da7966e613de2d7cfd61ff545b296a
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/400676
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Jim Van Verth <jvanverth@google.com>
Bug: skia:11900
Change-Id: I29d3f6a717bbdd4e1fee3322e98d33e51a28f264
Cq-Include-Trybots: luci.skia.skia.primary:Housekeeper-PerCommit-CreateDockerImage_Skia_Release
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402917
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:11900
Change-Id: Ib4d8da6a86da7966e613de2d7cfd61ff545b296a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/400676
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
This reverts commit 9230fc59b7.
Reason for revert: broke something in housekeeping docker build.
Original change's description:
> Add mskp player, use in viewer slide
>
> viewer now takes --mskps <dir> and will have a slide per mskp and
> overview slide (just like --skps).
>
> Player uses offscreen surfaces to draw offscreen layers, allows
> random access to mskp frames.
>
> slide just plays mskp at fixed frame rate (for now).
>
> Bug: skia:11900
> Change-Id: I66104ffe88f5df721a1a835570acc3e4c23c3f07
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/400537
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Jim Van Verth <jvanverth@google.com>
TBR=jvanverth@google.com,bsalomon@google.com,nifong@google.com
Change-Id: I97fb7a64d5ef2ca14dba1cf9e2ba91ab0e9d0018
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11900
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402639
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
viewer now takes --mskps <dir> and will have a slide per mskp and
overview slide (just like --skps).
Player uses offscreen surfaces to draw offscreen layers, allows
random access to mskp frames.
slide just plays mskp at fixed frame rate (for now).
Bug: skia:11900
Change-Id: I66104ffe88f5df721a1a835570acc3e4c23c3f07
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/400537
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Mixed samples is no longer relevant for Ganesh. DMSAA and the new
Ganesh architecture both rely on full MSAA, and any platform where
mixed samples is supported will ultimately not use the old
architecture.
Change-Id: I5acc745010e090ef26310d92ec6240be2cd494cf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399837
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Change-Id: Ic5192b072c98617dee85f28f8f4214c7a805a1fd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397318
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Jim Van Verth <jvanverth@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>
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>
This code is not performance-sensitive, and there doesn't seem to be any
benefit to making it header-only.
Change-Id: Ic83be5cbb96f08c18a144954be7453aaabaa5a72
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394900
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Currently doesn't do anything other than add new configs that set
a GrContextOption that becomes available in GrCaps. Runs new configs
on Perf/Test bots (MTL/iPhone11 and GL/Ubuntu).
Bug: skia:11844
Change-Id: I58586cae0980e52701abd1633dbc79b381f6015b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394996
Reviewed-by: Eric Boren <borenet@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:11803
Change-Id: I925f14be282b96355721986de6049090b35adf3d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/391856
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Previously, the Compiler optimization overrides only supported force-
disabling a particular behavior; we assumed that the default state of
a Compiler was to allow all optimizations. This assumption is about to
be invalidated, as the Inliner will soon be off by default unless you
are using GLES.
The override flags are now a tri-state; optimization and inlining can
be set to "default", "on" or "off".
Change-Id: I5637693222ca1de74ca1073c24d86c8e7c5026f6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/390136
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>
Reland 188443be8d without the DMSAA bots
to ensure nothing else changes.
Original change's description:
> Add tooling support for dmsaa
>
> Adds a "fAlwaysAntiAlias" flag to GrContextOptions that can be set from
> tooling code. When dmsaa is set, SkGpuDevice draws everything
> antialiased. Adds new "gldmsaa" and "glesdmsaa" configs and creates bots
> to run them.
>
> Bug: skia:11396
> Change-Id: I165e89434b733f7b02312cea0e6649812528083b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384936
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com
Bug: skia:11396
Change-Id: Icb45097e0a34543dc577fa32f19a692e90643a35
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386338
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This reverts commit 188443be8d.
Reason for revert: It looks like non-dmsaa bots might have been
affected. Let's land first without the new bots to make sure nothing
else changes.
Original change's description:
> Add tooling support for dmsaa
>
> Adds a "fAlwaysAntiAlias" flag to GrContextOptions that can be set from
> tooling code. When dmsaa is set, SkGpuDevice draws everything
> antialiased. Adds new "gldmsaa" and "glesdmsaa" configs and creates bots
> to run them.
>
> Bug: skia:11396
> Change-Id: I165e89434b733f7b02312cea0e6649812528083b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384936
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,csmartdalton@google.com,michaelludwig@google.com
Change-Id: Ib805b417ebd34d1fad79e0e1fe625765ee487f65
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11396
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386336
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Auto-Submit: Chris Dalton <csmartdalton@google.com>
Adds a "fAlwaysAntiAlias" flag to GrContextOptions that can be set from
tooling code. When dmsaa is set, SkGpuDevice draws everything
antialiased. Adds new "gldmsaa" and "glesdmsaa" configs and creates bots
to run them.
Bug: skia:11396
Change-Id: I165e89434b733f7b02312cea0e6649812528083b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384936
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
We no longer derive a performance benefit from this pass in practice,
and it is a very expensive compilation step. It is also prone to fuzz-
related errors.
Doc: http://go/optimization-in-sksl
Change-Id: Ief08ffac659a8fe7fe92c92b9a5da14c9f713bc2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381261
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This lets us see at a glance what each optimization pass contributes to
our final compiled SkSL output.
Change-Id: I52c56c92c408eee34045c5e6f60298cf9548ff5d
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381257
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>
This gives Viewer a way to dynamically toggle CFA off and on, so we can
immediately see the impact on shader optimization.
Change-Id: I912df70de126a74a1f0d11c511b2acc64f1e9f28
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380456
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
- Added "Dump Shaders" button; this writes the shaders to disk in the
`/resources/sksl` directory. This will allow us to more easily check
real-world shader code generation, instead of synthetic examples.
- Renamed "Load" and "Save" to "View" and "Apply Changes," to clarify
that they are not about files on disk. (Otherwise they seemed
confusing when placed next to a Dump Shaders button.)
Also, fixed a bug which would prevent a new deferred action from being
registered while running deferred actions.
(This CL implements the "test real-world shaders" portion of
http://go/optimization-in-sksl)
Change-Id: I7626a9e9c4f3ecb31b51f29b8106e4ca55de4dd4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380317
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
In addition to the unsurprising changes to eliminate references to
src/, we also had to tighten up some C++17-isms as they are not
permitted in public headers.
Change-Id: Ie5005a33d7a135e69fb66beca5e7a5f960dbd453
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/378496
Reviewed-by: Brian Osman <brianosman@google.com>
This is a reland of bbbf1a7f50
Original change's description:
> Redesign program key construction
>
> This does two things:
> 1) Moves responsibility for bit-packing portions of the key into the key
> itself. A new GrKeyBuilder type manages adding bits, with asserts to
> ensure a value always fits in the requested number. In theory this
> will let us generate smaller keys overall, at the expense of slightly
> more complex code during construction.
> 2) Adds a string label parameter for key methods that fold in data. For
> new methods, the label is required. To ease migration, the old add32
> does not require a label (yet). This will let us generate detailed,
> human readable keys, either based on SK_DEBUG, or a runtime option
> (if we're comfortable paying the cost).
>
> Bug: skia:11372
> Change-Id: Ib0f941551e0dbadabbd2a7de912b00e9e766b166
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377876
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
Bug: skia:11372
Cq-Include-Trybots: luci.skia.skia.primary:Test-Win10-MSVC-Golo-GPU-QuadroP400-x86_64-Debug-All-Vulkan
Change-Id: I179ed581bc9ba772191e727274ac0ac6979ebdf3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/378778
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This reverts commit bbbf1a7f50.
Reason for revert: D3D Failures
Original change's description:
> Redesign program key construction
>
> This does two things:
> 1) Moves responsibility for bit-packing portions of the key into the key
> itself. A new GrKeyBuilder type manages adding bits, with asserts to
> ensure a value always fits in the requested number. In theory this
> will let us generate smaller keys overall, at the expense of slightly
> more complex code during construction.
> 2) Adds a string label parameter for key methods that fold in data. For
> new methods, the label is required. To ease migration, the old add32
> does not require a label (yet). This will let us generate detailed,
> human readable keys, either based on SK_DEBUG, or a runtime option
> (if we're comfortable paying the cost).
>
> Bug: skia:11372
> Change-Id: Ib0f941551e0dbadabbd2a7de912b00e9e766b166
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377876
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,robertphillips@google.com,brianosman@google.com
Change-Id: I7bfb20905c87083e84a1ea21bc53d63e882e2c68
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11372
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/378777
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This does two things:
1) Moves responsibility for bit-packing portions of the key into the key
itself. A new GrKeyBuilder type manages adding bits, with asserts to
ensure a value always fits in the requested number. In theory this
will let us generate smaller keys overall, at the expense of slightly
more complex code during construction.
2) Adds a string label parameter for key methods that fold in data. For
new methods, the label is required. To ease migration, the old add32
does not require a label (yet). This will let us generate detailed,
human readable keys, either based on SK_DEBUG, or a runtime option
(if we're comfortable paying the cost).
Bug: skia:11372
Change-Id: Ib0f941551e0dbadabbd2a7de912b00e9e766b166
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377876
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This will allow users to create PDFs with the actual text embedded. This
will allow for correct search and copy operations on the generated PDF.
Since these are now public, SkTextBlobBuilderPriv is no longer needed
and is removed. For consistency, the allocRunRSXform overload is renamed
to allocRunTextRSXform.
Change-Id: I44be82d9038a433e1221d5cbfd8ed113ecb6d4fa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375017
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
The 'lang' was never stored or used, so this makes no current practical
difference. The original intent was to be able to specify a language so
that it could be emitted as a 'Lang' override in the 'ActualText' when
generating a PDF. However, due to the way 'ActualText' is generally used
this would be impractical. If there is ever a desire to mark up sections
of the PDF with a specific language it would be better handled in a
different way.
Change-Id: Id63596190235fc45ce17249b9b578b6f9b838b2b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375060
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
The Viewer status box 's' is quite small on high resolution displays.
The 'u' key doubles its size and has been used as a work-around, but
let the status box scale take the backing scale into account.
Change-Id: I98db37a4ed86698ba7c02666478dff248ff6daae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/374377
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Create an actual hairline path instead of a filled path. Allow metric
and image generation for glyphs to handle hairlines in the event a color
glyph has an outline and a hairline is needed. This also fixes glyph
bounds issues with path based glyphs drawn with hairlines.
Stroke+Fill is not handled very well, but is currently being deprecated
and so given less weight. It works, but is somewhat arbitrary.
Also add more paint overrides to Viewer.
Note that this only adds hairline handling for backup purposes. The code
in SkStrikeSpec::ShouldDrawAsPath which causes most harline glyphs to be
drawn directly from paths is not changed.
Change-Id: Icfadcd818d20b2557e4703c8e99d6d7dd0b4af70
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368156
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Prior to this change Viewer would crash in debug or act quite strangely
in release when attempting to filter any RSXForm blobs. This adds the
ability to allocate an RSXForm run and directly query the number of
positioning scalers per glyph it contains.
Change-Id: I90e118102a0f4d535e97026eecad9ac2f70e448f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367957
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Change-Id: Ibb48d30b9c0d80db3bf7b92d8c4a60bd9ce6839d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365696
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This change treats all slides as being in units of points (not pixels).
Window dimensions are communicated to slides in terms of points. This
allows slides which use the window dimensions for scaling to remain in
the same unit space as the gms. Allowing thh slides to also know the
actual pixel dimensions is left for later (this may be wanted when
testing proper selection of bitmap resources).
This also splits the backing scale from the zoom factor. The backing
scale is allowed to be toggled by the user. This allows for easy
reproduction at nominal size. When the backing scale is turned off the
points and the pixels are the same size. Slides which use the window
dimensions for scaling have the backing scale moved from the Viewer
transform to the window dimensions which are reported in pixels
(which are equal to points) when the backing scale is turned off.
Change-Id: Id288c4d8664a0a0972f1171a6159101c2b4ae90f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/364018
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Ben Wagner <bungeman@google.com>
The macOS view system is in "big pixel" units, so translate these
into "physical pixel" units.
To compensate, add sk_app::Window::scaleFactor() which returns the
scale factor. The viewer app is modified to apply the inverse of
this scale factor to the current zoom level.
Change-Id: I4fac066a230c87793fc5a0e5a582d60d25e46e7b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/361558
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Minor tweak to the UI on the Particles slide
Change-Id: I4d60be7fa843ea2717f27406a18e133e30662010
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/362096
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This was the last remaining user of ByteCode. The skvm solution
is faster, and lets us delete the ByteCode system.
Testing on 15 instances of sinusoidal_emitter (90k particles):
- ByteCode ~9 ms
- ByteCode (older, optimized): ~5.5 ms
- skvm ~2.1 ms
Change-Id: Ia2e5c9ab2d36c97e59af28a6f989bf212889e439
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356919
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Is not used by a major client and can be implemented using runtime
effects for users who want this noise.
Bug: skia:10536
Change-Id: Iaa06e6e1406b808c7f8dc0f76621fecf2becabf5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352057
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
uint (and bitwise operations) aren't supported by our minimum spec, and
they're going to be removed from public SkSL. For now, convert the
random generator to a good-enough chaotic sequence of high-frequency
sine waves.
If/when the interpreter (and particles) are converted to the newer skvm
backend, it will be straightforward to support custom intrinsics that
emit skvm instructions directly into the builder, and re-introduce a
better integer-based PRNG, without requiring SkSL language support.
Bug: skia:11093
Change-Id: I885b15a51a9e5c12b4274b5938d8deb77219d41b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347036
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
No longer used by any example content, and it requires unsigned integers
and bitwise ops (which are both going to be hidden from public SkSL).
Bug: skia:11093
Change-Id: I1941f7a1bed6c8512a5117ef256d18e420cbabee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346779
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Before compiling, write the shader text to 'sksl.bak', then remove it if
we succeed. This avoids loss-of-work if you manage to uncover an assert.
Change-Id: Idec7740210ce56cc9522c31e79f6aae1d4c3da1d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343425
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: I9fe7e637028ca5345f4ef19e4aedd035ba71070c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343430
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
No reason to limit the user's painting to the upper left corner.
Change-Id: Ia71f57cda7a26ea9be874eca753e0929cfc10873
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343426
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I1eaffcce43921632df483e40aa365c645663093a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343418
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Also includes the tessellation path renderer in the list of path
renderers when tessellation via indirect draws is available.
Change-Id: Ieac9e00d204dd8f9c6401b39179c65e1d8f442aa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341491
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Adds an example effect that spawns particles along an SkPath.
Change-Id: I53f3c02fefec814bd9e16f3ac593eac4cf6a297c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341418
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Still needs more testing before we can enable this all the time.
Bug: skia:10804
Change-Id: I6d8416fa072054d2f44e337641ed5d8379e81559
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337216
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
We no longer support MoltenVK, so we can't render using Vulkan on Mac,
but this change allows Macs with the SDK installed to compile Vulkan
code and use SPIRV-Tools.
Change-Id: I5baaf80de259e406495002a5fbfec89dbd1357b8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336678
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
I keep getting blocked when I try to zoom in on strokes. I don't think
we need to limit zooming in our test app; the "tap to reset" feature
works very well if we get lost.
Change-Id: Icf2b364b94128512f7117299b607131c5065cc36
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333656
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
- We were using the wrong keycodes for letters, so we never triggered
undo/redo/copy/paste in response to Ctrl+Letter combos.
- ImGui comes with system clipboard support on Windows and Mac, but not
UNIX/Linux. I added a bare bones X11 implementation, enough to support
copying and pasting text in and out of the ImGui text windows. There
is a bit of a delay when pasting to another program, probably because
we're not running a dedicated thread to handle those events. It looks
like we get a flurry of events before the real SelectionRequest.
- Note that this leaves the virtual clipboard functions on Window only
implemented for UNIX. We could supply our own copies for Win/Mac and
other platforms, if anyone really cares.
Change-Id: I2701e077ff2b6dff5ae3c5127c24b0937b893c89
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334049
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Introduce a Builder helper and plumb the client-provided SkFontMgr for
font resolution.
Also clean up some of the legacy SkSVGDom factories.
Bug: skia:10840
Change-Id: I6e1eabe7c257cb75dfdb5bf67054f93f25769027
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333577
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Tyler Denniston <tdenniston@google.com>
The previous behavior leaked Skia-internal concepts into public SkSL.
Users coming from GLSL will expect that bindable/sampleable objects are
uniform (just like texture2D). This keeps the old support around (and
tested), but updates all of our examples to use 'uniform'.
Bug: skia:10679
Change-Id: I0c98162f5e21dad7014d9778ceb26143d2f6030e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332376
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: If4f614510b3d2a5802e87350b8f09797d9c52c96
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332036
Commit-Queue: Greg Daniel <egdaniel@google.com>
Auto-Submit: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: Ia1b57dc4af65e7625659c147077b5e47b09b6b62
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/329178
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
