Previously, this code assumed that IndexExpression::Convert had done
range checking and that it was safe to access the base expression at
the passed-in index. The inliner violates this assumption, because it
can replace unknowns (where out-of-range access is undefined but non-
fatal) with knowns (where out-of-range access is forbidden).
We now do range-checking inside IndexExpression::Make and report the
error cleanly, instead of asserting inside of Swizzle::Make due to an
invalid component index.
Change-Id: If0f31b1f694bcc2a875d124f70be311d6634c77b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469535
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We only can guarantee that the positions are strictly monotonic. The
assert was testing that pos n+1 minus pos n would produce a positive
value which may not be true if the subtraction underflows.
Bug: chromium:1267605
Change-Id: Iaeff8f83f901f9b5af990cc866bfc24fb4af921d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469522
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Previously, a dangling type or function reference would be eliminated
silently with optimizations on, or would assert when optimizations were
off.
Change-Id: Ib2e273b6f069724e8872c9cb97351b647b875a62
Bug: skia:12472
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469525
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The ExpressionStatement currently eliminates dangling references without
reporting them as an error. This happens due to optimization; these
expressions (being meaningless) have no side effects, and so the
optimizer replaces them with Nop. When the optimizer is off, these
programs trigger an assert:
https://osscs.corp.google.com/skia/skia/+/main:src/sksl/SkSLAnalysis.cpp;l=582;drc=e7a953524787e3bd0c437ec52de4e40986689825
A followup CL will fix ExpressionStatements so that they report
incomplete expressions as an error.
Change-Id: Ica49166032e670749fc1b4e7a869fbab03364d4f
Bug: skia:12472
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469524
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
In some contexts, we rely on constant expressions to fold in order for
SkSL to work properly. (e.g. an array size is allowed to be any
constant-expression in GLSL, but the compiler really needs to know the
actual size). Previously, turning off optimization would break several
tests. Now, constant-expression folding always occurs even when the
optimizer is disabled.
Note that disabling the optimizer isn't an end-user option, so this
only affects internal usage.
Change-Id: I106ecb7e5bff3f7a8235cdccf0a7a60b48a97e2e
Bug: skia:12472
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469520
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This cycles the primaries, just like SkColorSpace::makeColorSpin.
Helpful for debugging more exotic color space issues.
Change-Id: I3434c7a9f24642f13be0ac3513599a15247d1f6f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469360
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Fixes inconsistent results in other color types/spaces, where the output
can be negative.
Change-Id: I8a346cf7bb02cb298c11d658948c117d74b6ddaa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469359
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This CL focuses on removing the SkStrikeSpec from the GlyphVector.
Now the GlyphVector can use an SkStrike directly. This CL
is mostly a bunch of tricky plumbing.
Change-Id: I4eb8ccfc44d9e8b7f35578bf754c188a7fd55596
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469176
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This reverts commit 2ed7729180.
Reason for revert: Didn't help, introduced small regressions.
Original change's description:
> [MiddleOutPolygonTriangulator] Convert pushVertex(p0) to close()
>
> This is the pre-iterator behavior of MiddleOutPolygonTriangulator.
> It's slower, but may lead to better triangulations.
> We may or may not keep it, depending on Perf's opinion.
>
> Bug: skia:12620
> Change-Id: I46f39b551b32af4eebfe8221cbb48a4332db83a8
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/468096
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
Bug: skia:12620
Change-Id: I58e7cc066540c594144f1e4d0cc1646051441af4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469230
Auto-Submit: Chris Dalton <csmartdalton@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
We had never set up the ModifiersPool here, so if the inliner tries to
make a scratch variable, it ends up accessing a null ModifiersPool and
crashing. I discovered this error by disabling optimization during the
initial loadModule call (during an unrelated experiment).
Change-Id: Ifa3e0007dc1dea02745d55a544b6ddd7b5a7c6de
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469517
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: Ia5da00913d0b96e9edc47f8b2274bd26c35c251c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469457
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
This enables stepping over function calls automatically.
Change-Id: Ie15ed745377d851cb7752f651b573efa2cc8195f
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469077
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: NA
Though we set this variable, we never use its value. This causes a
warning in Android.
Change-Id: I84492fd97e8c6f23db7bf51c116d227551ae7a94
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469358
Commit-Queue: Leon Scroggins <scroggo@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Auto-Submit: Leon Scroggins <scroggo@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Previously, the for statement's "increment/test" expressions were
executed without moving the trace-line back up to the for statement.
When stepping through code, we will now explicitly step to the next/test
line on each loop iteration.
Change-Id: I5d9f005a42150670cec77218323cf932ee1cbdb0
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469180
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This writes an entry to the trace buffer every time a slot value is
changed.
Change-Id: Iac3912be71ad654f70a7158e306e0643086c6cb0
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469179
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:12466
Change-Id: I1e92dcf66c117ca351cebf20722524672c88e5cd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469178
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This is a reland of b61804e94c
Original change's description:
> Add ConvertPixels versions of PremulAlphaRoundTrip
>
> Prior to the force_highp trick, the GrConvertPixels version failed, just
> like the GPU would do if we disabled the canvas2D fast path. With the
> highp trick, all tests pass.
>
> Bug: skia:12592
> Change-Id: I63ad2fd3b67863b6a736316e7c7b3b9bd2ee8970
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467516
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
Bug: skia:12592
Change-Id: I4c83c8d20959ab13cc493748a22fff5133706a77
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/468458
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
These rules can be used to build our GMs on WASM+WebGL and
libskia.a with just the CPU backend (and most other features
turned off).
This can be done with the following commands:
- bazel build //modules/canvaskit:gm-bindings-wasm --gpu_backend=gl_backend --with_gl_standard=webgl_standard
- bazel build :skia-core --config clang
This pivots slightly from http://review.skia.org/463517
by using config_settings [1] instead of platforms for
the optional features that we control. This pivot was
suggested in [2]
We have BUILD.bazel files in many of the subdirectories
that specify filegroups for the appropriate files. In
an effort to make //BUILD.bazel more readable, it is
the responsibility of these subfolders to deal with
conditionally including certain .h or .cpp files.
This is done using select statements and config_settings
or platform constraints as necessary.
For example, src/gpu/BUILD.bazel will different private
filegroups for each of the supported gpu backends [3]
and a more-visible filegroup called "srcs" that has
the right selection of the private files to be used
for compilation.
An effort has been made to avoid using glob() in our
BUILD.bazel files. These file lists were made by using
`ls -1` and some regex to add in quotes. We might want
to make a helper script to assist with that, if necessary.
To specify which options we have, the settings in
//bazel/common_config_settings/BUILD.bazel have been
redesigned. They make use of a macro `string_flag_with_values`
that removes the boilerplate. Patchset 36 shows what the
file looks like w/o the macro.
The top level BUILD.bazel file will still need to use
some logic to handle defines, because local_defines is
a list of strings, not a list of labels [4].
Suggested Review Order:
- WORKSPACE.bazel to see the new dependencies on the
emsdk toolchain and bazel_skylib
- bazel/common_config_settings/* to see the few settings
defined (we have more to define, see BUILD.gn and
//gn/skia.gni for ideas)
- BUILD.bazel to see the "skia-core" cc_library rule.
See also "gms" and "tests"
- modules/canvaskit/BUILD.bazel to see the use of
the emscripten "wasm_cc_binary" rule, which depends
on the "skia-core", "gms", and "tests" rule. Note that
it only builds some of the gms as a proof of concept.
- The other BUILD.bazel files. Some of these are not
platform or feature dependent (e.g. pathops). Others
are (e.g. gpu).
- All other files.
[1] https://docs.bazel.build/versions/4.2.1/skylark/config.html#user-defined-build-settings
[2] https://github.com/emscripten-core/emsdk/pull/920
[3] In this CL, that's just the webgl one.
[4] https://docs.bazel.build/versions/main/be/c-cpp.html#cc_library.local_defines
Change-Id: Ieecf9c106d5e3a6ae97d13d66be06b4b3c207089
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458637
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Owners-Override: Kevin Lubick <kjlubick@google.com>
When paths closed themselves explicitly, we were double counting the
start point. This was technically fine, since any fan point in
(-Inf, +Inf) will work, but this is a common enough case that it's
worth it to try and place the fan point closer to center.
Bug: skia:12524
Change-Id: Id94be4f2f28e4c0d287439db4ed83f389b163d57
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469096
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This gives us a natural foothold to write variable-change trace ops.
Change-Id: I0616ed374a3cf63ad33b6f14696acefde0741384
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/468826
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
It is useful to know the type of data in a slot (float or int) if we
plan to show it in human-readable form.
Change-Id: I4befdfcca6826792cd09b6a06a71cfd639d55822
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469076
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Previously, the fSlots array only carried skvm::Val data. Using a struct
will allow fSlots to carry additional information. In particular, it
will be useful to know the type of data in a slot (float or int) if we
plan to show it in human-readable form.
Change-Id: I6270bfc587045736f647ae744cfa36a2e4b5b65f
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469059
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Resolves an issue found with mismatched-new-delete in newer GCC.
Change-Id: Ifa7f133a16699d7f6a84f63b07ef4d5fde55dd32
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/468822
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
It was added for Chromium, and Chromium has switched to using
fTypeString instead.
Change-Id: I8cd8ae00b0c3abf3691ce14837afbe3be939538e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316209
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
As long as we don't have DMSAA, convex paths should be drawn directly
as opposed to through an atlas.
Bug: skia:12524
Change-Id: Ib17ce390cbdb7109ecdee4b665d21a8345eb3773
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469036
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Change-Id: I0bd34550c762e5d0f88cf7e3e44cbf38d2a8a7ba
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469057
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I814a42a1367ce0ebe1d065192bd2c7d8596bf77d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469056
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Eric Boren <borenet@google.com>
With paint conversion changed, this CL cleans up several SkShader
classes, based on the fact that input alpha will always be opaque.
Bug: skia:11942
Change-Id: I492a87bf1702f1553d20f3d05dcaf023069ae905
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/468456
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
* Remove the SkDescriptor from SkScalerCache where it is never really
used.
* Have SkStrike hold the descriptor
* Add a method to the SkStrikeSpec to create SkScalerContexts
Flow all the parameter changes through the code.
Change-Id: I11f2eec590b509eef0396b9288a6297fbe967dff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/468457
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
We define this to be 1 or 0, so #ifdef is not what
was intended.
Change-Id: I0182718980c39dced98bf90255703a6f080f9cac
Bug: skia:12584
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/468956
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
