Bug: skia:12701
Change-Id: Ibaeedcbf478546f2942df95d362bee8632ba0ded
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548419
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Just a minor cleanup to hide some cruft.
Bug: skia:12701
Change-Id: I70e9c8ba89a5b7d100a3d3f9b9dd084ad0a70715
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548418
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
We were returning Expressions from ConstructorArrayCast which didn't
match the passed-in Position. We now make sure to set the position of
the returned expression properly.
Change-Id: I2099d006e7dff2c94a9590c7159c4b0947c91257
Bug: oss-fuzz:47935
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548483
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
We are trying to enable ANGLE on Android. However with ANGLE, chrome
will use L16F instead of R16F textures, so we should not disable it
with ANGLE. BTW, if there is a driver bug for L16F, ANGLE can emulate
L16F with R16F.
Bug: chromium:983167
Change-Id: I6d0edfdc8d33c2b8a650f67ab36c7c84ef448372
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547836
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Symbol name lookup is generally simple, except for overload handling.
This factors out the complex, rarely-hit parts into a separate function.
Also, this replaces a vector with a span, which is probably a wash
since it's generally either empty or needs to be modified.
Change-Id: Ia4207fabc08e11b0214406de372cf429c4967ffd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548158
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Replace the empty label string with the label name for
different Skia components.
Bug: chromium:1164111
Change-Id: I10bf62f8febace584db624a33b07b272517df38e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548857
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Change-Id: I0925a52cc3a504c95ec4453e74c4580ce692275c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548896
Reviewed-by: Eric Boren <borenet@google.com>
Reviewed-by: Heather Miller <hcm@google.com>
Auto-Submit: Heather Miller <hcm@google.com>
Commit-Queue: Eric Boren <borenet@google.com>
In this CL:
1. Replace the empty label string with the label name for
different Skia components.
2. If the label string is empty, avoid sending it.
3. Append "_Skia_" at the beginning of the label string.
Bug: chromium:1164111
Change-Id: I8154f960591f0c001c6746f25e1939e0eb65d7fa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548516
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
SkMakeSpan can automatically deduce the length of our arrays; we don't
need to add an extra variable to tell it. (We could also have used
SK_ARRAY_COUNT to let the compiler automatically deduce the size.)
Change-Id: Ib0e86d18b8b4105824e97d6195cca7930ea1f1f3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548782
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:12701
Change-Id: I17ff95cd7f7fc518158e951d24c109fc5d4d003d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548417
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
The code was using C-style casts, accidentally stripping away const.
Change-Id: Ie34105a3974dcbf1879322cd15e2a9e392db95e8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548476
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This improves performance with sandboxing.
One can have Bazel output performance data [1][2], which can be
viewed via chrome://tracing or https://ui.perfetto.dev/.
With Perfetto, the following SQL queries [3][4] can summarize
the sandboxing metrics, as well as the actual compilation
time. http://screen/5TxbeZTso4gNDfD
Note that the dur column is in nanoseconds, so we convert
to seconds. These numbers could further be divided by
the number of processes (in my case 48) to get a scaled output.
SELECT SUM(dur) / 1000000000.0 FROM slice WHERE name = "sandbox.createFileSystem";
SELECT SUM(dur) / 1000000000.0 FROM slice WHERE name = "sandbox.delete";
SELECT SUM(dur) / 1000000000.0 FROM slice WHERE name = "subprocess.run";
I benchmarked the local compilation of //:skia_public using
--config=clang_linux (our custom Linux toolchain). I was
sure to clear the Bazel cache before each run and not count
the time to download/update a toolchain for the first time.
The control measurements (without this CL) are:
Wall Time = 272.2s
sandbox.createFileSystem = 5466.9s
subprocess.run = 2961.0s
sandbox.delete = 4112.3s
With this CL:
Wall Time = 53.9s (5.05x faster)
sandbox.createFileSystem = 403.4s
subprocess.run = 1610.3s
sandbox.delete = 348.8s
The control measurement is a touch misleading. Due to it being
so slow, I had recommended developers use a ramdisk when building
on machines with sufficient RAM via the Bazel flag
--sandbox_base=/dev/shm
Here is the control measurement when using a RAM disk:
Wall Time = 21.2s
sandbox.createFileSystem = 58.9s
subprocess.run = 705.1s
sandbox.delete = 46.6s
With this CL and a RAM disk:
Wall Time = 19.2s (10% faster)
sandbox.createFileSystem = 21.8s
subprocess.run = 722.9s
sandbox.delete = 16.2s
For devs who cannot or are not using a RAM disk, this is
a huge win. With a RAM disk, it's a modest improvement.
On an RBE build, this had minimal impact. I did my best
to bust the action cache with a fake define and the before
and after times were about the same.
This was inspired by [5] and [6].
[1] Add --profile=/tmp/profile.gz to any command
[2] https://bazel.build/rules/performance#performance-profiling
[3] https://perfetto.dev/docs/quickstart/trace-analysis#sample-queries
[4] https://perfetto.dev/docs/analysis/sql-tables#slice
[5] 93f21c9ef3
[6] 311acff345
Change-Id: Iceb2606e86111159141a286d01fc002d09fe3fe4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547822
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
This is just the lowest level of the combination system. The higher
level iteration to accumulate the keys will be in a following CL.
Bug: skia:12701
Change-Id: I4015790a7bca49f11f8eb1a4fda32f235845c049
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547819
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This removes an if-check from the top of get_transition, removes an if-
check from the top of Lexer::next(), simplifies a bounds check, and
removes bitfields from the index array. Disappointingly, on my machine,
I can't measure any change at all; `get_transition` and `next`
stubbornly remain at about 4-5% of total nanobench time for
`sksl_large`. However, it's still simpler and hopefully slightly smaller
code.
Change-Id: If4187c01f350fe642b7af7cb6bd2c8250ca3c00e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548396
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
codesize.skia.org was missing data and a misunderstanding of
file names was one cause.
To be consistent with Gold (and Perf?) we should use UTC as the
time zone for the folders and add hours. The server has been
switched to UTC in https://skia-review.googlesource.com/c/buildbot/+/548262/
This also fixes a race condition caused by upload the .tsv files
before the .json files. When the server sees a Pub/Sub event
for a .tsv file, it indexs the corresponding .json file.
Frequently, the .json file wouldn't have been uploaded yet,
so the indexing would fail.
Change-Id: Iabe64786db6e5c6020a3fc5dda244ccbe478c401
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548357
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
The goal of this CL is to upload SKPs produced by the Test-Debian10-Clang-GCE-CPU-AVX2-x86_64-Debug-All-OldestSupportedSkpVersion into a new "old-skp" Gold corpus. This will make triaging SKPs easier.
Other approaches I considered:
- Define a new --oldSkp flag. Redundant since we can recover this information by looking at the --config flag.
- Define a new "old-skp" value accepted by the --src flag. More complex; requires larger changes in DM.cpp, and changes to the Test-Debian10-Clang-GCE-CPU-AVX2-x86_64-Debug-All-OldestSupportedSkpVersion task definition.
Bug: skia:13398
Change-Id: Ie8311e715eee0daf335e277132b7484a46b94489
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548076
Commit-Queue: Leandro Lovisolo <lovisolo@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
In this CL, the GrSurfaceProxy's and GrDrawOpAtlas's label strings
are plumbed so that it can be stored in the label string of
GrGpuResource. onSetLabel method, which is called from setLabel
method of GrGpuResource, will pass labels to Skia OpenGL backend
using ANGLE's labeling API.
Bug: chromium:1164111
Change-Id: I516c06f0ebbf6bbe6d31ea5a4a64b2baeedd1560
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545717
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
and rename it BeginBlock.
The motivation for this CL is that it is simpler for the combination
system and the shaders to iterate over their children themselves rather
than shoehorning it into AddToKey.
Bug: skia:12701
Change-Id: I334fbcb7ce83af0681fb06d8449125fdbe8b05f3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547446
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
We decided to be ok with an empty version because caching
shader compilations is not that much overhead compared to
the rest of the backends that just have to redo them.
Change-Id: I995b3c75a7e723a1188184f8753450634fa0b629
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548356
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
We have a SkPaintParamsKeyBuilder at precompile time but not a
SkPipelineDataGatherer. We need the blend info at precompilation
time so that we can create a complete SkShaderCodeDictionary entry
for the precompiled combination(s).
Bug: skia:12701
Change-Id: I7686162ba93cb331f7c7dc2422e2d6937f367397
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547820
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
SKPBench tiles the SkPicture into multiple surfaces, with tile WH
differing between GPU and CPU backends. Graphite was being incorrectly
classified as a CPU backend and ended up using smaller tile sizes, so
the SkPicture would be played back many more times relative to Ganesh.
In addition, each surface's contents is a subset of the total picture,
so batching was artificially limited compared to Ganesh.
Added a call to Device::flushPendingWorkToRecorder() in
Surface_Graphite::onFlush(). This ensures DrawPass::Make() is called
when nanobench and viewer are measuring the bulk of the work
(viewer's "flush" time was always 0 for Graphite since the
DrawPass::Make was only being counted in the total time when it
was executed for swapBuffers()). Flushing in this manner also prevents
batching across loops in nanobench, or resetting/clearing prior loops
recorded draws when the benchmark starts with a fullscreen clear.
The SKPBench change should make all graphite benchmarks report lower
times compared to what's in perf.skia.org. The flush change should
increase their reported times for benchmarks that required multiple
loops to get an accurate time measurement (for expensive SKPs with
loops == 1, it shouldn't be affected).
Change-Id: I9256dbfc4c7c021377be8f5137b48036cc67e4a2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548157
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This unifies GrXferProcessor::BlendInfo and SkPipelineDataGatherer::BlendInfo.
The motivation for this is that I want to store a blend info on the
SkPaintParamsBuilder and having the BlendInfo in SkPipelineDataGatherer
made that difficult. Once moved to Blend.h it didn't make sense to have
the GrXferProcessor version.
Change-Id: I8c36feeef0bfe85dfc3f53b6bbb0193b29136bfd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547816
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Instead of copying a vector, now we use a span. Also, migrate out a
parameter-equality test to a helper function (allowing a "match"
temporary variable to be removed, and improving readability a bit).
Change-Id: Ic7f3274f244f8dd9444a6780c08465f9026b63f4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548156
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: Ifc4616c99bddd7d40d5a0bd2dd3c57bfb973500f
Bug: skia:12559
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547825
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
When an array type like `float[4]` is parsed, we now search for and
attempt to reuse an existing Type object instead of creating a new array
Type each time. These redundant array Types were stashed in the symbol
table, so this even rippled out to wasted space in our dehydrated data.
For builtin types, we stash their array types in the topmost non-builtin
symbol table of the program. For non-builtin types (structs), we play
it safe by stashing the array type in the current symbol table to avoid
mixing up types that share a name. Today's SkSL only allows structs at
the top-level and doesn't actually have a way to create two Types with
the same name, so this is perhaps an overabundance of caution.
Change-Id: I6feed44b7e15be57f0cac93b07b0d43dffb768b8
Bug: skia:12329
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547826
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This CL also removes a few from SkSLUtil but the majority of these will
be cleaned up in a followup. (Some of these are currently in active use
in SkSL.)
Change-Id: I7a018d3f6d8d21d69805f91d81a49c09636e4661
Bug: skia:12559
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547818
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Similar to other animatable text props, implement support for stroke
width.
Change-Id: Ia6ef65e8528edfec20def1b77da8df817e56cc7e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546244
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
We have a TODO to get rid of the accessor functions. We can use direct
member pointers instead.
Change-Id: I81470d8e03905be583677ef41a6de3547c8b318e
Bug: skia:12559
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547823
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This caps-bit was always set to true. The last reference was removed
here: http://review.skia.org/512936
Change-Id: Ifbad66f58847f7bd8579c837de3d3b5428625e34
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547821
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
