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>
This analysis pass assumes we have a program with a valid structure--all
loops must be ES2-compliant, and all function-calls must reference real
functions that exist. If we detected an error during compilation, our
program might not meet these criteria.
Change-Id: I4c7aefb3221438643614f1e0cbc2bad40b94b161
Bug: skia:12396
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444982
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Calling `visitProgramElement` on a FunctionDefinition should always set
fFunctionSize on any path through the code. However, this is difficult
to confirm by inspection, since there are several exit conditions. It's
safest to initialize the variables at construction time, just in case.
Change-Id: I8bfda5fc5c4d71f5593ea2469586d668be762449
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445636
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>
Includes .eval() as a new alternative syntax for sample/shade.
Bug: skia:12302
Change-Id: Ia921c141b3eeeba7e5309d921967b941f9cd055e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444756
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This moves the swizzle domain test into the path used by the DSL so that
the error check benefits both sides. To make this possible we need to
be able to distinguish between equivalent swizzle components like x and
r, so they aren't collapsed down to the same component until the very
end.
Change-Id: I48f2582886391eabd7ce6eae949babdeead6051e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445280
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Previously, deeply-nested loops could lead to stack overflow during SkVM
dead-code elimination. The DCE algorithm in SkVM now stores its worklist
on the heap and does not rely on recursion. Since we now enforce an
upper bound on maximum program size within SkSL, this worklist's size
should always be reasonable and bounded.
Change-Id: I8b7955298b2540a9f600fc5d94d3bc804c68632c
Bug: skia:12396, oss-fuzz:37827, oss-fuzz:37837
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445597
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The previous values are about to be removed from Dawn.
Bug: chromium:1199057
Change-Id: Iec6c4e4d641a99bceae975e68ab4b53ccdc75ef0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445436
Auto-Submit: Brandon Jones <bajones@chromium.org>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Our program-size analysis pass needs to recurse into called functions;
depending on the exact order of functions in the program, this recursion
can hypothetically be as deep as the deepest function-call chain. Set an
upper bound on recursion here, so we don't overflow the stack while
trying to check the program size. In practice, 50 frames is far deeper
than a regular shader should ever go.
Change-Id: I733ee48dad6f8053facdfd9f6d8a2b9b2a4af188
Bug: skia:12396
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445279
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
AfterEffects and Bodymovin support an "AllCaps" text flag which forces
text capitalization.
* add toUpper() bindings to SkUnicode/SkICU
* add capitalization options to SkottieShaper
* plumb existing Lottie 'ca' (AllCaps) prop
* also fix a couple of unrelated whoopsies
Change-Id: I8e80921b66530e9830938004946082c6e450b04b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445104
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
The DSLParser struct nesting check didn't work, and on top of that
should have been part of the core DSL itself as opposed to being
relegated to the parser.
Change-Id: I680d0dc703309136970353204add14a6c9e03aa8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445113
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This allows us to remove the static-recursion analysis pass entirely,
while still providing the same results.
Change-Id: If1564cd4df55be86ca4e0bf53ecc094ba76007df
Bug: skia:12396
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445296
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This picks up the change to use Bot-Commit+1 for RecreateSKPs
Bug: skia:12124
Change-Id: I4a22536e216a05fef15686503dbbfbb02b5e4808
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445109
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Ravi Mistry <rmistry@google.com>
The previous version of the code would actually end up skipping over
recursive code silently. (In fact, CheckProgramUnrolledSize is always
called immediately after Analysis::DetectStaticRecursion regardless of
whether or not any cycles were found, and the "cycle detected" assert
was not firing.) The code now actually detects cycles correctly; this
can be verified by uncommenting the call to error().
In a followup CL, I will improve the error reporting so that the cycle
is tracked and can be reported. Right now, we can detect a cycle but
don't keep a copy of the stack.
Change-Id: I9eb7b6333522d334657af4aa8d09fd6ad371ad69
Bug: skia:12396
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445112
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>
Also fixes some minor warnings.
Bug: skia:12086
Change-Id: Ia476a7a196b490022978761e92f935a6d668136a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441797
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This work is being done by the skiastatus Gerrit plugin.
Bug: skia:12403
Change-Id: I42b81dcc9e2b4d5c99f9756eabed186f489a9adb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445105
Commit-Queue: Ravi Mistry <rmistry@google.com>
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Private / unsupported types are now restricted to module code.
Change-Id: I98cb2e0822560a274758d99ecf0ca09883d0c3a1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445097
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
A client of Flutter is seeing a crash when trying to release the
MTLTexture in GrMtlAttachment. The range of CLs involved includes the
one that disables retained references in the command buffer, so this
is a speculative change to see if that is the culprit.
Change-Id: Idd6e6b264086d671be52f456ef56f1fc5bafade8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444498
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
We were reporting immediate errors in this function at the correct
location, but by not setting the offset of the statement itself, later
errors discovered during analysis were being reported with no line
number.
Change-Id: I7f4c5a0db774bbcf3901b38e05a43cebbda0d1ed
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445102
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
RedeclareStructTypeWithName was reporting an error on the wrong line.
Change-Id: Ibe954ca5013dbedca8cf015660cf3d28fa97987d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445101
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Once we have created a DSLVar, we need to return it to avoid internal
errors.
Change-Id: I28dacf49e2f5219660ddbc6e21100e3fb0675742
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445099
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
The fuzzer is currently learning to make unboundedly-large programs by
nesting medium-size loops repeatedly. SkVM doesn't have a mechanism to
limit the ensuing explosion of code and ends up making unreasonably deep
stacks and/or unreasonably large programs.
SkSL now enforces an upper bound of approximately 100,000 IR nodes on a
fully-flattened, fully-inlined strict-ES2 program. The limit is picked
out of thin air, but this should be enough to prevent SkVM from going
haywire while still being large enough to handle any reasonable program.
We can definitely tune this value if we find that it is too large
(admitting dangerous code) or too small (rejecting good code).
Change-Id: I11735636175721fbc79460b4e194d8e4b42dc47d
Bug: skia:12396, oss-fuzz:37827, oss-fuzz:37837
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444358
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We were previously relying on the RTAdjust var's (essentially
meaningless in that case) value being non-null even when RTAdjust was
an interface block field instead of a var. This breaks runtime effects
built by the DSLParser, which does not set RTAdjust unless there's
actually an RTAdjust var.
Change-Id: I0938e1562ebf997af7b1f4dd9beb60fcd7909a20
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445096
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This allows processing to continue in the face of invalid array sizes,
matching the error reporting behavior of SkSLParser.
Change-Id: Ifc0572aecf829563e4a77f01962a6e05aa9e98e8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444761
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I06ba9dd9ed8af8555233ddfa10d3e0ec6babc2ea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444759
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
There was no good reason for this behavior in the first place, and this
change makes the error reporting behavior better match SkSLParser.
Change-Id: I7b69e7bcc64173c0ac6523e075c1f24e2be00ed0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444758
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Most of the logic in IRGenerator::finish has moved to
Compiler::finalize. The @if/@switch pass has been combined with the pass
that verifies no dangling FunctionReference/TypeReference expressions,
saving one walk through the IR tree. Most program-finalization logic now
exists in Compiler and Analysis.
This change reorders our error generation logic slightly, and manages to
squeeze a few extra (valid) errors out of one of our fuzzer-generated
tests, but is not really intended to affect results in any significant
way.
Change-Id: I461de7c31f3980dedf74424e7826c032b1f40fd2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444757
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>
As these checks were in the AST processing side of things, they were not
catching errors in DSL code. SkSLAnalysis is also a better home for this
function than SkSLIRGenerator.
Change-Id: I8149825047570300cc09425deba1339ac0edb7ab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444656
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This doesn't rely on anything Compiler-specific, and can just be a plain
Analysis pass.
Change-Id: I8564ae2a750c6daa6c449e6fa56355cc047f7010
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444496
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:11837
Change-Id: Ia79f76c18587741000367edba303c5f7c0c1087d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444178
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This matches the behavior of SkSLParser and fixes some spurious
followup errors.
Change-Id: Id1eae353eb6536a6385d04d0426e0de736db6b57
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444360
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This adds line numbers to a bunch of errors which were previously
missing them.
Change-Id: Id77c08d168b0a2d8a0131a53aa004bf37b8bec02
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444176
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
