Commit Graph

3181 Commits

Author SHA1 Message Date
Robert Phillips
de60d7a64d Move more classes to skgpu::v1 namespace
Bug: skia:11837
Change-Id: If0bfb3009693b203b2080a1d43cc3b1865c3ab9e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448274
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-09-15 15:20:24 +00:00
Ethan Nicholas
fd1332f5a7 Reland "Fixed DSL assertion error on source files containing nulls"
This is a reland of db38ad7b14

Original change's description:
> Fixed DSL assertion error on source files containing nulls
>
> The assertion was there to make sure we weren't running off the end of
> the source, but naturally fails in the presence of legitimate embedded
> nulls.
>
> Change-Id: I3b80499e9b182c9ea046c479f35d7a965d548401
> Bug: oss-fuzz:38107
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447182
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

Bug: oss-fuzz:38107
Change-Id: Idb1a6b7c64d2bb954edadae828d6de808158fd3f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448660
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-09-15 15:00:23 +00:00
John Stiles
857c7f9dc5 Add program-too-large test using for loops without blocks.
The fuzzer has discovered a bug in our program size-checking logic; for
loops that immediately contain another for loop (with no block) were not
counting the inner loop's iterations. This allowed it to exceed our
maximum program-size threshold (and time out during SkVM compilation).

This test demonstrates the issue. A followup will fix it.

Change-Id: I3b7d4c8a4f0ed04cf0aba3f1a32fdad7d6d784e7
Bug: oss-fuzz:37837
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449096
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>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-09-15 14:19:00 +00:00
John Stiles
b701fa0ac0 Add non-square MatrixCompMult support to public SkSL in ES3 mode.
We already had a test case here, but it wasn't actually in operation.
The test has been split into ES2 (square) and ES3 (non-square) halves,
returns the color like a proper runtime effect, and it's now running in
dm.

Also, Metal doesn't natively support matrixCompMult, so it injects a
helper function; I tweaked the helper so it no longer requires an extra
result variable.

Change-Id: Ie79242768966fcbe879ad73461d17b4fb8e55670
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448117
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-14 18:15:23 +00:00
Greg Daniel
2bc96d6771 Merge GrVkAttachment into GrVkImage.
Bug: skia:12402
Change-Id: I743724f66db8d7666d4d627d6945ce6bc3dc6bc3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448261
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
2021-09-14 13:34:17 +00:00
John Stiles
7d19065eef Add test of off-kilter matrix constructors.
This exposes a bug in the Metal code generator which will be resolved
in a followup CL.

Change-Id: If073835dbee474ea9a805eb92b42dc1fca2afbd0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448378
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-09-14 13:01:26 +00:00
Brian Salomon
ae59426ea6 Revert "New approach to GrProcessor uniforms."
This reverts commit 10c9f36bdd.

Reason for revert:bad gms, maybe blocking chrome roll

Original change's description:
> New approach to GrProcessor uniforms.
>
> The important aspect is that it allows knowing the uniforms that will
> be used by a set of processors without having to create ProgramImpls.
>
> GrProcessor subclasses specify uniforms at creation time in a similar
> style to how GrGeometryProcessors already specify attributes. That is,
> they initialize a span of structs describing the uniform which may
> contain void uniforms that are skipped. Unlike attributes, the struct
> contains an offset into the processor where the data is stored.
>
> GrUniformAggregator is used to collect the uniforms from all processors
> that compose a draw and mangle their names. The ProgramImpl subclasses
> query the aggregator for their uniform names when emitting code.
>
> The old system for uniforms is left intact and only three processors,
> one GP, one FP, and one XP, are updated to use the new system.
>
> Some pieces that are missing before everything can be moved over:
> -support for uniforms not owned by GrProcessor (e.g. rt-adjust)
> -support for samplers
> -helpers for common patterns
>  (e.g. GrGeometryProcessor::ProgramImpl::setupUniformColor(),
>   and the various matrix helpers on ProgramImpl)
>
> Bug: skia:12182
>
> Change-Id: I21c1b7a8940eb9b8aad003f5a2569e43977a33d2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440841
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>

Bug: skia:12182
Change-Id: I6cc508900a599d27124f8ba48597593192d5d807
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448418
Auto-Submit: Brian Salomon <bsalomon@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
2021-09-13 21:22:16 +00:00
Brian Salomon
10c9f36bdd New approach to GrProcessor uniforms.
The important aspect is that it allows knowing the uniforms that will
be used by a set of processors without having to create ProgramImpls.

GrProcessor subclasses specify uniforms at creation time in a similar
style to how GrGeometryProcessors already specify attributes. That is,
they initialize a span of structs describing the uniform which may
contain void uniforms that are skipped. Unlike attributes, the struct
contains an offset into the processor where the data is stored.

GrUniformAggregator is used to collect the uniforms from all processors
that compose a draw and mangle their names. The ProgramImpl subclasses
query the aggregator for their uniform names when emitting code.

The old system for uniforms is left intact and only three processors,
one GP, one FP, and one XP, are updated to use the new system.

Some pieces that are missing before everything can be moved over:
-support for uniforms not owned by GrProcessor (e.g. rt-adjust)
-support for samplers
-helpers for common patterns
 (e.g. GrGeometryProcessor::ProgramImpl::setupUniformColor(),
  and the various matrix helpers on ProgramImpl)

Bug: skia:12182

Change-Id: I21c1b7a8940eb9b8aad003f5a2569e43977a33d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440841
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
2021-09-13 19:55:06 +00:00
John Stiles
7591d4b5ef Replace trio of XxxxLiteral types with a single Literal type.
Throughout SkSL we've begun using doubles as a convenient way to store
any SkSL value (int, float, bool) in a single type. This idea has now
been extended to literals. Rather than having three expression kinds for
integers, floats and boolean literals, we can have just one. These can
be accessed in a type-specific way (`floatValue`, `intValue`, and
`boolValue` return the expected type, or assert if it's not the
matching type), or in a type-agnostic way (`value` will return a double
and works on any type of Literal).

This allows us to remove a complex template trick (Literal<T> is gone),
removes two redundant Expression types, and and lets us reduce our code
size in ConstantFolder, FunctionCall, etc.

Most of the conversion process was pretty straightforward:
* `IntLiteral::Make` becomes `Literal::MakeInt`
* `x.is<IntLiteral>()` becomes `x.isIntLiteral()`
* `x.as<IntLiteral>.value()` becomes `x.as<Literal>.intValue()`

Change-Id: Ic328533611e4551669c7fc9d7f9c03e34699f3f6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447836
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-13 17:32:34 +00:00
John Stiles
dc652564a7 Fix for-statement assertion detected by the fuzzer.
The fuzzer found that the `DetectVarDeclarationWithoutScope` check was
placed too late in the function, and could be skipped over by for-loops
containing multiple variables. This was caught in ForStatement::Make,
which mirrors the Convert postconditions with matching assertions.

Change-Id: I6e9d97c7c9ca969aba65e601bbcd9fe676105838
Bug: oss-fuzz:38560
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448116
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>
2021-09-13 13:34:48 +00:00
Michael Ludwig
47d824415e Revert "Fixed DSL assertion error on source files containing nulls"
This reverts commit db38ad7b14.

Reason for revert: breaking g3 roll since it thinks the test case is "binary" not flagged as binary

Original change's description:
> Fixed DSL assertion error on source files containing nulls
>
> The assertion was there to make sure we weren't running off the end of
> the source, but naturally fails in the presence of legitimate embedded
> nulls.
>
> Change-Id: I3b80499e9b182c9ea046c479f35d7a965d548401
> Bug: oss-fuzz:38107
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447182
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

Bug: oss-fuzz:38107
Change-Id: I650d12d728b5d932bda79e81205b873d8b44771f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447936
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
2021-09-11 18:13:13 +00:00
Ethan Nicholas
db38ad7b14 Fixed DSL assertion error on source files containing nulls
The assertion was there to make sure we weren't running off the end of
the source, but naturally fails in the presence of legitimate embedded
nulls.

Change-Id: I3b80499e9b182c9ea046c479f35d7a965d548401
Bug: oss-fuzz:38107
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447182
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-10 18:44:22 +00:00
Ethan Nicholas
c9d65f0b8a Fixed duplicate function definition error discovered by fuzzer
Bug: oss-fuzz:38140
Change-Id: I76a1b3ef8289b3089192d043d173677c00741a54
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445836
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-10 17:35:52 +00:00
Ethan Nicholas
bf4a7d5235 Fixed DSLParser crash with invalid swizzle
In response to a non-identifier token after a dot, DSLParser would
attempt to swizzle a zero-length field and fail an assertion.

The same basic code path exists in the old compiler, but the resulting
parse error causes the process to abort before it attempts to process
the zero-length swizzle.

Bug: oss-fuzz:38106
Change-Id: Ifd997ce1d564b5f6ef0a9a785d8d9e254785e600
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446185
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-09-10 14:29:22 +00:00
John Stiles
8d0dd0d1c1 Add support for pack/unpackUnorm2x16 to public SkSL.
This includes compile-time optimization and tests.

The unit test is disabled in a followup CL
(http://review.skia.org/447057) because it exposes a Radeon 5300M bug
in OpenGL.

Change-Id: I8b2f0411358aeb68c4edfeb0bd7a2814c4be1f40
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447056
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-09-09 18:59:15 +00:00
Robert Phillips
769b4884a9 Move some more Ops to skgpu::v1 namespace
Bug: skia:11837
Change-Id: I522636c6dbf977929b0d368e18beb9c3a01fdeb1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446184
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-09-08 15:41:04 +00:00
Ethan Nicholas
a2c76c77c4 Fixed illegal interface block reporting
DSL was improperly allowing interface blocks in runtime shaders, which
caused PipelineStageGenerator to get upset.

Bug: oss-fuzz:38131
Change-Id: I593e68f2cab3db9151d606e65e2826ffa9c494e2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446324
Commit-Queue: Ravi Mistry <rmistry@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-09-07 20:18:00 +00:00
John Stiles
3ff77f4862 Fix inliner bug discovered by fuzzer.
Intrinsic-call optimization can be triggered during inlining. In this
case, inlining turned `normalize(x)` into `normalize(constant-value)`.

DSL is used to implement optimizations for a handful of intrinsic calls,
including `normalize`, which internally relies on `length`.

The DSL expects that it can use the IRGenerator to handle function
calls. This was not working because we were finished with the initial
compilation pass, and the IRGenerator's symbol table is removed when
finish() was called.

We now temporarily give a symbol table back to the IRGenerator while
the inliner runs. We remove it again as soon as inlining is complete.

Change-Id: I6da98788d93749ffeb008c1f4c3f72b436e8ceeb
Bug: oss-fuzz:37994
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445956
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-09-07 13:29:37 +00:00
Brian Osman
3099f798e7 SkSL: Add support for method calls on effect objects
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>
2021-09-03 14:53:33 +00:00
John Stiles
2af4b13925 Reject programs which exceed 50 stack frames during size analysis.
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>
2021-09-03 02:41:34 +00:00
John Stiles
61e5e202a5 Enforce an upper bound on Runtime Effect program size.
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>
2021-09-02 14:44:34 +00:00
Robert Phillips
fbf021492e Move more Ops to skgpu::v1 namespace
GrFillRRectOp
GrFillRectOp

Bug: skia:11837
Change-Id: Icdecc2ccd9def659b0d9402910b2072e02577a66
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444817
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-09-02 13:25:29 +00:00
Robert Phillips
46d7bd56f6 Move more Ops to skgpu::v1 namespace (take 2)
GrClearOp
GrDashOp

Bug: skia:11837
Change-Id: I9ada76c061154e769e7f486e7c57a93fd1d74907
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444636
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-09-01 18:57:02 +00:00
Robert Phillips
d2f53f9e3e Move more Ops to skgpu::v1 namespace
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>
2021-09-01 16:50:16 +00:00
Robert Phillips
5fe828accb Revert "Move more Ops to skgpu::v1 namespace"
This reverts commit f3158abda3.

Reason for revert: Broke Flutter build

Original change's description:
> Move more Ops to skgpu::v1 namespace
>
> GrClearOp
> GrDashOp
>
> Bug: skia:11837
> Change-Id: Ib65418952e7d4fb1e419e4942223823f22302b34
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443518
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Robert Phillips <robertphillips@google.com>

TBR=robertphillips@google.com,michaelludwig@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com

Change-Id: Ic181ced7f5f2a8ff834fdcfb4e5265b99eef1666
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11837
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444365
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-09-01 14:44:46 +00:00
Robert Phillips
f3158abda3 Move more Ops to skgpu::v1 namespace
GrClearOp
GrDashOp

Bug: skia:11837
Change-Id: Ib65418952e7d4fb1e419e4942223823f22302b34
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443518
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-09-01 14:03:55 +00:00
John Stiles
5cec187b36 Fix array timeout discovered by the fuzzer.
The fuzzer discovered that, when we attempt to verify that an array
doesn't contain any literal values that are out-of-range for its base
type, we pay a linear-time cost based on the size of the array. This
happens even when the array value isn't known at compile time; we still
iterate over its slot count and diligently discover that every single
constant-subexpression slot in the expression is "null".

We now have a helper function on Expression,
`allowsConstantSubexpressions`, which only returns true for expression
kinds that can contain constant subexpressions. We use this helper to
skip over this linear-per-subexpression check when the expression
cannot possibly contain a constant subexpression. In particular,
`AnyConstructor::compareConstant` and `Type::checkForOutOfRangeLiteral`
will now early-out for expressions that can't possibly contain a
constant subexpression.

Change-Id: Ia34e422afa67b478a8616acb0a0e9cd211b29698
Bug: oss-fuzz:37900
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444136
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>
2021-08-31 19:55:45 +00:00
Greg Daniel
0e32aa8a3a Rename some GrD3DTypes* header files.
I want to free up GrD3DTypesPriv to actually be private types that
include real d3d objects.

Change-Id: Id38d6baae4fa68c19301b27d4f9d51eb1d9c5db0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443676
Commit-Queue: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Auto-Submit: Greg Daniel <egdaniel@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
2021-08-31 18:49:46 +00:00
Robert Phillips
cadd5dbded Move three Op-derived classes to the skgpu::v1 namespace
GrRegionOp
GrShadowRRectOp
GrStrokeRectOp

Bug: skia:11837
Change-Id: I0b573031acb13138e1549712c555337247e0a08e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442836
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-08-30 21:16:11 +00:00
John Stiles
f89a8122a4 Fix flaws in minus-prefix optimization.
We had a logic bug when attempting to optimize the following code:
    const vecN x = vecN(a, b, c);
    -x;

The goal was to replace `-x` with `vecN(-a, -b, -c)` but we accidentally
tried to cast the `x` VariableReference to a Constructor. We
unfortunately didn't cover this in any of our test cases, but the fuzzer
managed to synthesize it by mixing and matching elements from its new
corpus.

This affected several different constructor types: splat, diagonal-
matrix, compound and array.

Change-Id: I10dd2460ab26ba3e820b0cff5db091368fb7e648
Bug: oss-fuzz:37764, oss-fuzz:37861
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443407
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>
2021-08-30 20:40:17 +00:00
Robert Phillips
31f763190f Move GrAtlasTextOp to skgpu::v1 namespace
Bug: skia:11837
Change-Id: I3dde13940e57763d5a8224cb1a4b555e904351d7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442716
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-08-30 14:56:55 +00:00
Robert Phillips
2145390fbe Move GrTextureOp to skgpu::v1 namespace
Bug: skia:11837
Change-Id: Ie99b1c512885404351d6917bbea751d99a2ca23e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442797
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-08-30 14:14:14 +00:00
Brian Osman
99ddd2a98d Remove (unused) geometry shader support
Bug: skia:8451 skia:10827
Change-Id: I5b38a1d72cd4558f8e2a92aaf9b12f05efce0923
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442683
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-08-27 19:41:10 +00:00
John Stiles
8d13084535 Migrate function-body finalization out of IRGenerator.
This is a first step towards replacing `finalizeFunction` with a
`FunctionDefinition::Convert` method living outside of the IRGenerator.

Previously this code would assert that we had no early returns from a
vertex-program main() method; this has been turned into an error.
(The original assertion was also tied to fRTFlip, because the *problem*
with early-returns in main is tied to the lack of RTFlip fixups, but
we fundamentally don't allow early returns, so it makes more sense to
just universally disallow it.)

Change-Id: Iba0742f7ef3cbc83995ea130fec1eb1ef2556c44
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442691
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-08-27 18:51:52 +00:00
John Stiles
842ee5bd8d Fix assertion when calling a built-in function at global scope.
The fuzzer invented a much more elaborate example, but I was able to
winnow it down to a simple otherwise-normal test case. This also fixes
a latent DSL bug; DSL functions were not updating the list of referenced
intrinsics, so the compiler might emit finished programs that called
built-in functions that didn't exist in the code.

Change-Id: I095bb566b9db9f87cbe9460732c300b7973eb112
Bug: oss-fuzz:37659
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442325
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-08-27 17:05:21 +00:00
John Stiles
bb8cf5804c Fix invalid variable ref-kind discovered by fuzzer.
No-op arithmetic simplification will convert expressions like `x += 0`
to `x`. When making this simplification, we will also downgrade the ref-
kind of `x` from "write" to "read" since the new expression is no longer
an assignment.

The fuzzer discovered that the ref-kind downgrade was too aggressive,
and would also traverse into nested subexpressions and downgrade them
as well. That is, for `x[y=z] += 0` would convert both `x` and `y`
into "read" references, which is incorrect; `y` is still being written
to.

The fuzzer managed to turn this mistake into an assertion by leveraging
a separate optimization. It added a leading, side-effect-less comma
expression for us to detect as worthless and eliminate. In doing so, we
clone the expression with the busted ref-kind, triggering an assertion.

Change-Id: I42fc31f6932f679ae875e2b49db2ad2f4e89e2cb
Bug: oss-fuzz:37677
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442536
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-08-27 14:01:21 +00:00
Robert Phillips
fdafc0cbe4 Move GrAtlasRenderTask to gpu/ops and skgpu::v1 namespace
Bug: skia:11837
Change-Id: I29352c0e2c9e987b28be983c3d519a88b589baf1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442022
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-08-25 21:39:11 +00:00
John Stiles
fbb905e70f Remove nearly-unused ASTNode description code.
The entire ASTNode is probably going away soon, but this is a decent
place to start. Checking the fuzzer logs, we had 0% coverage in here,
which makes sense because it's unreachable by any normal means.

Change-Id: I396464e3e613d46e990b629c4fc991c11f6110fa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442000
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>
2021-08-25 20:58:13 +00:00
Robert Phillips
3e87a8eda9 Move GrOpsTask to gpu/ops and the the skgpu::v1 namespace
Bug: skia:11837
Change-Id: I9ee6343b5144b02e8f455df0ea01b8199a8f14e1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440837
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
2021-08-25 18:32:17 +00:00
John Stiles
43ac7e6315 Fix parsing error with SPIR-V negating a uint.
Our SPIR-V code generator did not implement support for negating a uint.
However, this is something that GLSL allows (as does the rest of SkSL).
I checked glslang and it uses OpSNegate here. The SPIR-V docs indicate
that OpSNegate allows any type of integer, and the validator lets it
pass, so we now use OpSNegate here as well.

http://screen/33mkq92uxAT5Xu8
http://screen/4YBTh3gCWz8eZx7
http://screen/388HtXyytcN5vLZ

Change-Id: I8c142018fd5e162dcd051abe1bc5d69a6e034794
Bug: oss-fuzz:37627
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441880
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-08-25 17:52:50 +00:00
John Stiles
2d3f5e8f25 Demonstrate parsing error with structs and arrays.
The fuzzer detected a serious parsing error; a struct containing a
vardecl with multiple declarations would interpret arrays incorrectly.
An array would be applied to ALL variables in the decl after its initial
appearance. That is, `int w, x[10], y, z;` would be interpreted as
`int w, x[10], y[10], z[10];`. The fuzzer caught this by putting two
arrayed variables in a row; the second variable was interpreted as a
nested array, which led to an assertion.

This CL contains a simple hand-written test case demonstrating the bug,
with the fix coming in a followup.

Change-Id: I42d7372ba77fa1528ae24eb8c29a2e5903784139
Bug: oss-fuzz:37622
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441878
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-08-25 17:26:54 +00:00
John Stiles
f95048cc4d Report intermediate expressions as errors in For next-exprs.
We weren't coercing the expression because we don't care about its type,
but that allowed intermediate-expressions to pass through without
reporting an error. Now we coerce the expression to its present type,
which will always fail if the type is disallowed and succeed otherwise.

Change-Id: Ic0de0d17f0f5d56360575efe992ce4d74dec2a5a
Bug: oss-fuzz:37620
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441876
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-08-25 17:26:21 +00:00
Michael Ludwig
c1ed11d909 Move GrBlockAllocator and GrTBlockList to src/core/
src/gpu/GrBlockAllocator -> src/core/SkBlockAllocator
src/gpu/GrTBlockList -> src/core/SkTBlockList

Tests and references also renamed.

Bug: skia:12330
Change-Id: I5fad05faa3dcecd89a0a478dcf30c090ea7589f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441477
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
2021-08-25 13:21:32 +00:00
Brian Osman
b5282f0112 Migrate runtime effect sample tests to golden files
Much easier to maintain, especially with an upcoming change to the
sampling syntax.

Change-Id: I378811b7be0afcce5b7e68a942e7b46d96568155
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441518
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-08-24 15:19:44 +00:00
John Stiles
d77dda5bd5 Fix inliner bug discovered by fuzzer.
The inliner contained a type error when attempting to inline a function
that takes an array as input. The scratch copy of the array was created
as `float[123] var;` instead of `float var[123];`. This led to an
assertion in VarDeclaration::Make.

Change-Id: I5128fe71462bb59a015a7b4e59c1a74800828b16
Bug: oss-fuzz:37466
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441576
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>
2021-08-24 14:44:43 +00:00
Ethan Nicholas
553239bc12 Added reportPendingErrors to SkSL code generation
This fixes an assertion failure uncovered by the fuzzer.

Bug: oss-fuzz:37469
Change-Id: I626c003cfa8a0bc65851899df3a7695dbe29200b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441311
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-08-23 20:54:40 +00:00
John Stiles
3f37322d71 Fix const function-parameter assertion discovered by fuzzer.
During constant-folding, we baked in an assertion stating that any
const-typed variable reference ought to have an initial value, because
you can't declare a const variable without assigning a value. However,
function parameters are an exception to this rule! They are variable
references and are allowed to be const, but will not have an initial
value. (In this case, `const` just means you can't alter the value.)

In this case, all we needed to do was remove the assertion; we already
treated this case defensively and with the appropriate care.

Change-Id: I61242c6d08c59886c6992898f195771e6334f2b4
Bug: oss-fuzz:37465
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441239
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>
2021-08-23 19:28:29 +00:00
John Stiles
7253a201a3 Improve error-checking during array-size conversion.
The fuzzer noticed insufficient guards in IndexExpression::Convert when
converting an array size from an IntLiteral to a SKSL_INT. We had code
in IRGenerator which did this properly, so I moved our array-size
conversion logic into SkSLType and had IndexExpression share it.

Also, a variety of tests around similar error conditions were added.

Change-Id: I51529dea25f9029f81ae236511610069d66be29f
Bug: oss-fuzz:37462
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441236
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-08-23 14:35:00 +00:00
John Stiles
8f633ef2b5 Fix assertion discovered by fuzzer.
We now stop processing a var-declaration if its array-size expression is
invalid. Previously, we'd pass a null array-size expression into
convertVar, which would assert (but would fail cleanly afterwards).

Change-Id: I976f3326e32afbc7045a86d73c0dcb28f418a6f4
Bug: oss-fuzz:37457
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441079
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-08-23 14:23:46 +00:00
Robert Phillips
e453fa063d Move some v1-only gpu/tessellate files to gpu/ops
This CL just moves the files and renames them. It doesn't move them into the skgpu::v1 namespace.

Bug: skia:11837
Change-Id: Iab322d0dc5b5d1cfd32436785081539dc85c18d3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440776
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
2021-08-19 19:56:46 +00:00