The earlier implementation generated a very very long expression on a
single line. I split out the logic across into multiple lines, and
also realized that the majority of the logic was repeated two times. Now
we use a vec2 to avoid spelling out the math twice.
Change-Id: I1a60e3663de3d774c5abdcd166734b07ea3dbaaa
Bug: skia:13109
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525841
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This lets most simple Porter-Duff based Compose operations share the
same shader text; the values in a uniform control which blend mode we
will use. This is only enabled in reduced-shader mode.
Best-case scenario I could find:
- Original: http://screen/EkXsnfNPC9CxiwE
- Uniforms: http://screen/9rJLe6JMrhteD24
Change-Id: I0edc7910a9a2ae7f4e5abbd57128d7b3b52971bf
Bug: skia:13109
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525317
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This doesn't actually change the memory allocation but sets up to do so.
90% of the CL is just renaming SkPipelineData to SkPipelineDataGatherer.
The main interesting changes are those to:
ExtractPaintData
in DrawPass.cpp
and SkPipelineData.h/.cpp
Bug: skia:12701
Change-Id: I3e18f9b3f16166649de1bf1f4399d5521d817eb3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524763
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Qcomm did some investigations of this workaround, and even though it was
giving some cpu savings, it was causing a big hit to GPU performance and
power. So we're removing this now to get back the various wins on the
GPU.
Change-Id: I01ba4e271dc02ae4ab6155fc794a5a1e3c796341
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525638
Auto-Submit: Greg Daniel <egdaniel@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Change-Id: Icc6d1345c59d6f6529db39bf4b5bd8097f1f6b5f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525837
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This moves the Build-Debian10-BazelClang-x86_64-Release-IWYU
job from experimental to on when a file in one of the
folders that we enforce IWYU is modified (currently
for svg, sksl, and now, debugger).
Change-Id: Ia6fe1e7b30fc486db3eb081b6a64bc4c250cbf0b
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525796
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
When `Make<BlendOp>(src, dst)` is used, the blend mode will always be
hard-coded into the shader, instead of using a uniform to apply a
generic Porter-Duff blend.
Change-Id: I59a05eea3417b1880cef63738c0116d19f01ee3e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525641
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This should fix many issues that Chrome is seeing. The problem is the
picture recorder had no idea about Slug, so the canvas super class
would try to use the device to draw. This hooks up the recorder
to capture drawSlug into the picture.
I tested this with a specialized gl sink in DM which I'm struggling
to check in. But, it was functional enough to show that this works.
Bug: chromium:1302036
Bug: chromium:1307279
Bug: chromium:1306329
Bug: chromium:1307446
Change-Id: I6a27bf43702400c80b2044433e7b00347f522763
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525636
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Local measurements show that using RBE (with a warm cache)
vs local can result in a 2x faster build.
No-Try: true
Change-Id: Ib900a90564f105de848c9aeb0b745e5fec77da53
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525637
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Because these tasks use RBE, the machines they run on do not
need to be as powerful. In practice, we are seeing a lot of
the build steps be a hit on the remote-cache, so we don't need
the number of Bazel jobs to be as high, so I've arbitrarily
set it to be 100. We can revisit this later if we notice
things are slow.
To facilitate this change, I had to add cloud-platform scope
to all our GCE VMs. There is a script in the infra repo [1]
that helped with this:
go run ./scripts/add_gce_scopes/add_gce_scopes.go \
--zone us-central1-c --project skia-swarming-bots \
--scope https://www.googleapis.com/auth/cloud-platform \
--instance skia-e-gce-100,skia-e-gce-101,...
[1] b103ea24f5/scripts/add_gce_scopes/add_gce_scopes.go
No-Try: true
Change-Id: I7f1e7b1e9e4a22f5383cf9ce1c8c0350e62b5283
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525577
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
So we don't have to drag these shifts through the entire process.
This is the first step for simplified layout optimization.
(Need letter spacing because it's common in iOS Flutter)
Change-Id: I28b066e4dd8bcc6e489752dafbda1b73b0444fbe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524223
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
There are a few driver issues on the Golo machines that we
are trying to work out.
This removes 2 Perf-ASAN jobs, as one is probably sufficient
given that we have Test-ASAN variants too.
Change-Id: Id2aa492a64706562286b820709f082f2374e2222
Bug: 1309590
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525191
Reviewed-by: Ravi Mistry <rmistry@google.com>
Change-Id: I811c8d2b9a113c7bd35ea5a480017ea4b794a745
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525318
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Bug: skia:12974
Change-Id: I3524b7a7d15e5a4c55d6af5a6a1a0e0113ea76e3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525319
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
The root disk on our GCE VMs typically only have 15GB
and have a much larger disk attached to them.
We want the Bazel cache to be on this larger disk so
we don't run out of disk space as often.
BuildTaskDrivers was not doing that, but the task
drivers which use Bazel are.
Change-Id: I0f797188576707341972a1db7418e8916633333c
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525456
Reviewed-by: Ravi Mistry <rmistry@google.com>
Bug: skia:12845
Change-Id: I8e9d7dcfaf4f356b65638f0f8490edd8b0dd2644
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525177
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
The hb_font will hold a reference to an hb_face created for the typeface
along with other attributes associated with the SkTypeface
(in particular, the variation design position)
See https://github.com/flutter/flutter/issues/100523
Change-Id: I5e211d670996f8f36e0d1027006c7bb67a9b8d2a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524801
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
In nanobench we want to try and simulate a GPUs swapbuffering and not
get too far ahead on the CPU. Thus we use finished callbacks to know if
we get more than 3 frames ahead of the GPU. This CL adds support for
Graphite to do this.
Bug: skia:12974
Change-Id: I8be505c5769399dcc0f5954f9f999f4448633647
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525186
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
We need to convert the final result of this subtraction to an SkScalar
anyway. Doing it before the subtraction avoids potential undefined
behavior.
Bug: oss-fuzz:46051
Change-Id: I7e0880238fd894ad836cd5e9e1ab24a17ec0d1dc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525183
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This simplifies the IR and emits cleaner Metal code; Metal does not
directly support matrix-construction from a list of scalars.
Change-Id: I0f2415e4c84d4f999aaaeaec3623f0eae41c24dc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525179
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
The Metal function-requirements checking logic assumed that a builtin
function would never have any requirements. This would cause us to
generate invalid Metal code if a builtin function included a reference
to sk_FragCoord.
Change-Id: I9992981b03306b254a3fc4b87b940ddb4c646bf1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525182
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Update our utils to fix a couple of nested animation issues:
1) Use RenderFlag::kSkipTopLevelIsolation to prevent unconditional
nested animation layers. This matches default AE semantics.
2) Use the main animation ResourceProvider when loading nested
animations (otherwise any nested resources are ignored).
Change-Id: Ib489549066c9e42a96e5113fc817278d9ed06d59
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524636
Auto-Submit: Florin Malita <fmalita@chromium.org>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
Steps to run:
make -C bazel generate
bazelisk build //example:hello_world_gl --config=linux-rbe \
--features skia_enforce_iwyu
# manual fixes of the .h and .cpp files
make -C bazel generate
This will be followed up by a CQ job that checks the generated
Bazel files.
Bug: skia:12541
Change-Id: I7651f885e182a60177839cd78a2d4047e73a676a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525181
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
* Removes unnecessary static function (it's been copied elsewhere).
* Uses Caps info for colorTypeToFormat.
Bug: skia:12845
Change-Id: I557da58ec4456db1d8b1bb9a3d419e3330200a47
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525178
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
It looks like the fuzzer specified many subruns, but all were failing.
There was a TODO to exit early after the first subrun failed.
I implemented the TODO.
Bug: oss-fuzz:45704
Change-Id: I719d5bf8fa3fe8d7eb6963dbd79854dae877e7d6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525176
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Originally, these radio buttons would allow the user to immediately
switch between different compile stages and view the change in output.
At some point, this broke, and clicking the radio buttons would clear
the shader list, so that the user would need to click View again to see
the shaders. This made it much harder to visualize the difference in
compilation stages at a glance.
Now the radio buttons work normally again.
Change-Id: I234c305817909c4345dd12318df3cbe4505121a8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524936
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This only needed a helper function to make error-reporting optional, but
NoOpErrorReporter (previously TrivialErrorReporter) can serve this
purpose equally well.
Change-Id: Iac249483f2013cbf8563c0ea44c680d3e03e2894
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524766
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The builtin variable scanner did not check builtin code for the presence
of sk_FragColor, etc. We currently get away with this because none of
the existing builtin code uses a builtin variable.
Now FindAndDeclareBuiltinVariables checks shared program elements too.
Change-Id: Ifb3ee3857ef73b18d9e4f406970f0f67681dd4be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525042
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:12845
Change-Id: If3ac2b6ba2c8e28328ee5805a29fc83353220364
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524756
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
No visible effect yet, but this will enable better error reporting in a
future CL.
Change-Id: I09e1c5d3bb423a7ce42701f15c4bb142b0a9473c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524638
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
The toolchain now uses extract_ar from
https://skia-review.googlesource.com/c/buildbot/+/524764
which is a static executable to extract the .deb files.
This was necessary because the llvm-ar that had previously
been used requires glibc 2.31+ to run, but our Debian10
machines on Swarming have an older version (2.28).
A longer-term fix is to have Bazel support .ar files,
which I plan to attempt to contribute this week.
The RBE task will be added as an experimental CQ job, to
see how it handles the load of running often. With the
remote execution cache, I hope it performs well, once
the toolchains are cached on both the Swarming
machines and in the RBE workers.
Note: We had to add several files to the CAS spec
(see compile_cas.go) which are required for Bazel to work.
Change-Id: Ie70c70d5f33768c957760f9eeb7835025109b487
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524759
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
A new RBE worker-pool called gce_linux was created in
conjunction with this CL. See
https://docs.google.com/document/d/14xMZCKews69SSTfULhE8HDUzT5XvPwZ4CvRufEvcZ74/edit#
for some details on that.
Note: everything under bazel/rbe/gce_linux was autogenerated
and can be ignored from manual review. It basically specifies
what files are on the RBE image that are necessary for running
Bazel.
Testing it out can be done by authenticating for RBE
gcloud auth application-default login --no-browser
Then, run make -C bazel rbe_known_good_builds
to test it out.
On my 4 core laptop with an empty local cache, but a
warm remote cache, the build took <2 min instead of the
10+ minutes it would have [1].
The folder structure in //bazel/rbe is meant to let us
have multiple remote configurations there, e.g.
//bazel/rbe/gce_windows.
Suggested Review Order:
- bazel/rbe/README.md
- bazel/rbe/gce_linux_container/Dockerfile to see the
bare-bones RBE image.
- bazel/rbe/BUILD.bazel to see a custom platform defined.
It is nearly identical to the autogenerated one
in bazel/rbe/gce_linux/config/BUILD, with one extra
field to force the gce_linux pool to be used.
- .bazelrc to see the settings needed to make
--config=linux-rbe work. The naming convention was
inspired by SkCMS's setup [2], and allows us to have
some common RBE settings (i.e. config:remote) and
some specialized ones for the given host machine
(e.g. config:linux-rbe) A very important, but subtle
configuration, is on line 86 of .bazelrc where we say
to use our hermetic toolchain and not whatever C++
compiler and headers are on the host machine (aka
the RBE container).
- toolchain/build_toolchain.bzl to see some additional
dependencies needed in the toolchain (to run IWYU) which
I had installed locally but didn't realize were important.
- third_party/BUILD.bazel to see an example of how failing
to specify all files can result in something that works
locally, but fails remotely.
--execution_log_json_file=/tmp/execlog.json helped debug
these issues.
- All other files.
[1] http://go/scrcast/NjM1ODE4MDI0NzM3MTc3Nnw3ODViZmFkMi1iOA
[2] https://skia.googlesource.com/skcms/+/30c8e303800c256febb03a09fdcda7f75d119b1b/.bazelrc#20
Change-Id: Ia0a9e6a06c1a13071949ab402dc5d897df6b12e1
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524359
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
The file generation logic that dawn [1] uses to make some
source files requires jinja2, which also requires MarkupSafe.
The GN build handles this by specifying those repos in
DEPS, checking them out at a certain git hash, and then
providing them via a command line arg [2].
We do not have to do it this way in Bazel to have reproducible
builds. This CL specifies an exact version (verified by sha256)
of those two deps and then uses a hermetic version of
Python 3.9 to run all py_binary commands.
Previously, we would rely on the system Python (and installed
libraries). That happened to work on my machine, but not on
other machines without jinja2 and MarkupSafe installed. After
this CL, it should work on machines that do not have python
even installed.
I chose the same jinja2 version used by Dawn [3], which was
2.11.3. Then I chose the newest version of MarkupSafe that
was compatible with jinja2 (2.0.1).
If we have other python scripts that need external deps, we
should be able to specify them in the py_binary that needs
them and in requirements.txt. Then, the pip_install() step
in WORKSPACE.bazel will download them and make them available.
[1] https://dawn.googlesource.com/dawn.git/+/refs/heads/main/docs/dawn/overview.md
[2] https://dawn.googlesource.com/dawn.git/+/e45ff6a4b3c2f06dade68ec0f01ddc3bfd70c282/generator/generator_lib.gni#77
[3] ee69aa00ee
Change-Id: I3d0074f3003de179400e239e00107c34f35f4901
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524217
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Most of these are pretty mechanical generated changes.
IWYU noticed one issue with DSLCore.h, which was fixed here.
Change-Id: I5629565ad3c2817daa71907c62f932d93f9d78ab
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524617
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This closes one of the last gaps in SkSL's constant-folding abilities.
Change-Id: I65c0f2e5fe11a7d47ab2069b2992403fca78b8a7
Bug: skia:12819
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524761
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
CTFontCopyVariationAxes appears to be extremely slow due to localizing
the name of the axis. WebKit moved to using the internal SPI
CTFontCopyVariationAxesInternal to avoid this cost. Since Skia would
like to avoid using internal SPI, just cache this information per
typeface to avoid the cost of calling it as often.
Bug: https://github.com/flutter/flutter/issues/100523
Bug: https://bugs.webkit.org/show_bug.cgi?id=232690
Change-Id: I175e34e9aa526d58e6b7a4ff54cb13d1ef8a9fd9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524760
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
