This reverts commit a9c187e5cc.
Change-Id: Icbfb8abdfc67fc2e6428d97a6cdede2726fb56e4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385596
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
The record time allocator's initial block size is 12,800
(100 * sizeof GrPipline) bytes.
This means that the fibonacci progression grows rapidly to large
block sizes. This is causing out of memory errors in Chrome and
Android as they try to allocate very large blocks.
Reduce the initial allocation to 1024 bytes to reduce growth
rate.
F *1,024 *12,800
=====================
0 1,024 12,800
1 1,024 12,800
2 2,048 25,600
3 3,072 38,400
4 5,120 64,000
5 8,192 102,400
6 13,312 166,400
7 21,504 268,800
8 34,816 435,200
9 56,320 704,000
10 91,136 1,139,200
11 147,456 1,843,200
Bug: b/182959903
Bug: chromium:1188071
Change-Id: I5ef1c736efb42b2bccd78549d129154c0857bbca
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385938
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This replaces a potentially expensive tree walk with a map lookup. It
should speed up the inliner (on sksl_large) by about 4%, and speed up
the benchmark overall by >1%.
Change-Id: I68ef492d2aba6303753fad01dcbef1eaffb7a6f9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386556
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
They aren't otherwise declared, but ProgramUsage::get() should be able
to find them, even if they are unread and unwritten.
Change-Id: I7200212945f9360e63b3108ab60420bcae8192c8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386558
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>
All of these events use "skia.shaders" now, so that clients can enable
them independent of any other skia events.
For new APIs, add new trace events to cover (potentially) long API
calls, and give better visibility into how pipeline creation breaks
down. GL still has a few extra events around caching, but the vagaries
of what those mean (depending on what's in the cache) probably make them
more confusing than useful, so I didn't try to replicate them.
Bug: skia:11360
Change-Id: I97c7e89ad1f2f05d7b90bd783a20a6b8c2036e8d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386157
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: 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>
This test causes the Adreno 330 driver to crash, and does not pass on
Quadro P400 in wasm. The CPU test confirms that we can get it right,
even if not all drivers do.
Change-Id: I5ffb72ac647a49dab7130ab2c6e94f587ded6cf9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386216
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>
Previously, finalizeFunction took the FunctionDefinition as its input.
This change makes it possible to calculate statistics in
finalizeFunction, such as "how many return statements exist in this
function?" and incorporate that data in the FunctionDefinition at
make_unique time.
Change-Id: I72bb67c802f5416588a5516dfcb61aa56ccf9684
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386057
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>
By pass the GrTextBlobCache, and create the op directly, with
the op owning the GrAtlasSubRun.
The new types of SubRuns have the GrAtlasTextOp's Geometry
embedded in them because there is a 1-to-1 correspondence
between the SubRun and the Geometry.
There is a lot of duplicate code. I wanted to keep the
implementation similar before I started top optimize
the direct code path.
Change-Id: I86162a527613e092c184932f4496f59c07793d55
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336445
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This reverts commit 99b2d04d31.
Reason for revert: no benefit
Original change's description:
> Add 'kOnlySingleReturn' complexity type to the Inliner.
>
> This can be used in followup CLs to test various forms of inliner
> simplification. (More info: http://go/optimization-in-sksl-inliner )
>
> Change-Id: Icd12a1ae1481c9aeacf3f11e85872fecfa972ec3
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384836
> 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>
# Not skipping CQ checks because original CL landed > 1 day ago.
Change-Id: I1be7e0b08cf1c908823784c836c1fd680dfbd6f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386098
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Due to ongoing simplifications, the code had two variables that always
referred to the same object (inlineStatements, inlinedBlockStmts) and
reserve_back was called on it twice. The amount of space being reserved
was also wrong, as it accounted for out-param writebacks, which are no
longer supported.
We now only have one name for it, and reserve all the space we need the
first time.
Change-Id: If7b52ff8cef5bc7d4610384b8362f8a6a420f2ac
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385937
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>
Bug: skia:11754
This image is invalid - SkCodec draws *something* but it's not
particularly meaningful. Remove it from our CodecSrc tests and add it to
BadImage tests so that we still verify we don't crash (etc) but we no
longer expect to be able to draw it using the platform generator.
Change-Id: I4781d645896d9f01afbd70fb0c5acfd262dd3169
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385880
Commit-Queue: Leon Scroggins <scroggo@google.com>
Auto-Submit: Leon Scroggins <scroggo@google.com>
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
This enables SVG to build in official builds.
Change-Id: I4f64109983216baf9663061e23cc3757292ff448
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386096
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
It is difficult to do this both efficiently and correctly while honoring
GLSL semantics (which require the lvalues to be kept distinct, even when
they point to the same variable). We could make it work by making copies
of every out parameter in each direction (going in for inouts, and
coming out for outs and inouts).
However, this could be self-defeating if it makes it harder for the
driver to track variable lifetimes. Simply opting out of inlining these
functions entirely seems like the best tradeoff; let the driver optimize
them if it can, and we can enjoy reduced complexity in the SkSL inliner.
Change-Id: I62f7b4550cc181cfe789e4f2ff4e408ba1baf9cb
Bug: skia:11326
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370257
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
If there was "newState" passed in by the client during a flush we were
setting that state but then also changing it for present right after.
We don't support having this newState and present at the same time so
this was clearly a bug.
Change-Id: Ia5101994c11ef9c40ee7b7d9bac8150ffc242567
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385881
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
It turns out it is not legal to pass the results of OpAccessChain as a
function argument, for... reasons. This CL switches us over to passing
the argument via a temp variable instead.
Bug: skia:11748
Change-Id: Ib5e86c1d000655ebd7bb62ceea6a27b823808645
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385936
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This allows us to remove 100 LOC from the inliner and is very unlikely
to affect any existing benchmark. We don't have any evidence to support
the idea that a one-iteration `for` loop with `continue`-based exits
will be any faster than a standard function call on any existing GPU.
Our fragment processors are generally written to avoid early returns,
in large part to avoid hitting this path.
This drastically impacts BlendEnum.sksl (which can no longer flatten out
a switch over every blend function in SkSL) but is otherwise a wash.
See: http://go/optimization-in-sksl-inliner suggestion 4(a)
Change-Id: I1f9c27bcd7a8de46cc4e8d0b9768d75957cf1c50
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385377
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@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>
The vision here is for Register to be a not-yet-allocated
surface, so that we can allocate Registers, then check
the memory consumption of that allocation before committing
to it.
Bug: skia:10877
Change-Id: I06cd3a66b9794b702bfd08ab30c644d0d2f2c945
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385496
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
They don't inspect Expressions at all, but by default, a ProgramVisitor
will completely traverse every Expression. This change improved local
runs of nanobench on sksl_large by ~7%.
Change-Id: I7604ffa220c6081c3cd39b5a22520fb41421fcba
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385878
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This put the coverage for do-while loops on par with for loops.
Change-Id: I53e0d733edd02a6a139792a8d74c68116453e5ff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385500
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This makes it easier and faster to check for void types, especially
when you don't have a Context.
`Type::isOpaque` would previously return true for void, and this CL
preserves that behavior to avoid changing the meaning of existing code.
Change-Id: I6e1699dd8daee4c5fa8cf4746bab84c1d3fc15d0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385499
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I6bf9cad2dd7095afbc153e90510dd0f7c398797d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382936
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
intelwiki has html tables to test
Cq-Include-Trybots: luci.skia.skia.primary:Housekeeper-Nightly-RecreateSKPs_DryRun
Bug: skia:11354
Change-Id: I84fb2b91ffd7b28b46bb5d303f546f193a9883d5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385498
Commit-Queue: Ravi Mistry <rmistry@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
The detachAll function was unused before this CL. Gone!
This also removes the late-check of canSkipResourceAllocator
on the premise that, these days it seems impossible for that
value to change between addInterval, where the first check is,
and assign, where the current check is.
All path renderers work on my machine, will also run this against
the Nexus7 and AndroidOne bots.
Bug: skia:10877
Change-Id: Ife63ca2fb8e7c94c2053dbc10c42df18c48c0c20
Cq-Include-Trybots: luci.skia.skia.primary:Test-Android-Clang-AndroidOne-GPU-Mali400MP2-arm-Debug-All-Android,Test-Android-Clang-AndroidOne-GPU-Mali400MP2-arm-Release-All-Android,Test-Android-Clang-Nexus7-GPU-Tegra3-arm-Debug-All-Android,Test-Android-Clang-Nexus7-GPU-Tegra3-arm-Release-All-Android
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385278
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This can eliminate const variables which have been completely folded
away, unnecessary synthetic variables created during codegen/inlining,
or code that simply didn't need to exist at all.
Change-Id: I37a65e455e6527a6a6c2f4dde918f48d84dc2638
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/383496
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
A global variable should be considered "dead" if it's never written and
never read. The previous code checked if it was never written OR never
read, which is not the same.
This would generate GLSL/Metal that didn't compile. In SPIR-V, it would
SkASSERT, then crash, during codegen. The fuzzer was able to detect the
SPIR-V issue, but it was wrong in all three cases.
Change-Id: Id59a2499eb5baa3839b93826bfbc24191bfd490b
Bug: oss-fuzz:32005
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385280
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>
Add ClearCache() {} to the non-caching case.
This is a reland of cfdae5a56c
Original change's description:
> remove the OpMemoryPool
>
> Change-Id: I987384f8009445ba8308e85b75daf4880a8d36be
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/383957
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
Change-Id: I03f80ba61f5643c40f812a9687b12a5d2aa663b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385276
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This reverts commit 30980650f7.
Reason for revert: iphone6 on ios12 fails to find vertexMain
Original change's description:
> Merge MSL vertex and fragment shaders into one string before compiling.
>
> Invoking the MSL compiler seems to have a significant startup cost.
> So rather than running it twice -- one each for the vertex and fragment
> shaders -- this CL merges them into a single string using namespaces,
> thus allowing a single compiler invocation.
>
>
> Bug: skia:11389, skia:11392
> Change-Id: I49facf995cc7f7529e61dc9d2af194a00ed1ccb6
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384700
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Jim Van Verth <jvanverth@google.com>
TBR=jvanverth@google.com,brianosman@google.com,johnstiles@google.com
Change-Id: Ifa96e7a370264f878fb0413c3acbb7c505c47905
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11389
Bug: skia:11392
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385238
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This reverts commit e7a8f85e4f.
Reason for revert: must revert dependent CL
Original change's description:
> Only include header once in combined MSL shader.
>
> Bug: skia:11389
> Change-Id: I3e24dcaa2cfeddc7efd7985f9f42a59bfc8175f2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385137
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Jim Van Verth <jvanverth@google.com>
TBR=jvanverth@google.com,johnstiles@google.com
Change-Id: I7a886b6c57a666e54e65365e41dcb57bd9ab4ba6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11389
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385237
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
SkTDynamicHash is semi-deprecated and this simplifies life a little bit.
Bug: skia:10877
Change-Id: Ie64f476fcab0a53d8cb8044fe4c68571bc0d2a66
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384857
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
Our benchmarks had drifted away from actual codegen in several ways, and
were no longer representative of actual output. In particular, every
function in the existing benchmarks used a scratch `_output` variable
which would make it ineligible for return-statement-only inlining.
Updated the tests to match modern codegen. `small` and `large` use the
same sources as before. The code from `medium` is older and had diverged
quite far from today's output; I could not find a representative code
snippet at all. It has been replaced with SkSL of roughly similar
complexity.
Change-Id: Iadd32cbb6725554355ddfe384ce538f3dc49aca3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385160
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
'in' variables without locations aren't allowed. Use uniforms instead.
Bug: skia:11738
Change-Id: Ic066106deb7409cff154b4be7cfb3e03a7025c7d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385000
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
