This verifies that compiled SkSL code generates a reasonable trace.
Change-Id: Ia81a694460eaac261dfb9287b64f0afec013f76c
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/473145
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>
This provides a callback mechanism for the interpreter when a trace
opcode is encountered. The callbacks are only invoked when the trace
mask is enabled.
Change-Id: I55db22e18106ae09e4ab0a503533d830282c772c
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/473139
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Exceptionally large control points might require us to chop the path
very deeply. This CL converts the recursive chopping to loop with a
stack of control points on the heap, in order to avoid the risk of
stack overflow. It also adds a bail condition to avoid getting stuck
in endless recursion due to fp32 precision issues.
Bug: chromium:1266446
Change-Id: I005be4bc29de51d3c89f04b5d6c553a921a92aa3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/473197
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This associates the input SkSL source with our debug info. It is
serialized out to .trace files. This will actually let us display the
code associated with `trace_line`.
Bug: skia:12614
Change-Id: I6c10af6fa6dd97ed8411f3460e103f442722b7f6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/472999
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The SkVMDebugInfo now includes an skvm::Coord that the caller is
responsible for filling in before converting the program. If it is set,
the value indicates the device coordinates that should be traced. If it
is unset, debug traces will not be emitted at all.
Within the SkVMCodeGenerator, we now have a new traceMask() call which
combines the current execution mask with the trace mask. Tracing opcodes
now pass the result of traceMask() instead of mask(). This will limit
trace data to the selected pixel, instead of tracing the entire draw.
Bug: skia:12614
Change-Id: I1f8ce7d18a31aa60a139a4a7335c2a285d6aee60
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/472798
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We no longer indicate functions by their line number, which can be
ambiguous. The debug info now includes a list of function names which we
can refer to by index, and the `trace_call` opcode references functions
by their index in this list.
Change-Id: I4bdf2a6439637a07b116831fd2981f342c19ea4a
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/472796
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This fails exactly as it should, but we had no test for it.
Change-Id: I0aa3307c444f2c9bc3512ff43b784a56a7c09856
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/472449
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This wasn't meaningful, and made some error reporting worse.
Change-Id: I5e72b5aca5d3e159b8439fa9809290d75e44cbe2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/472656
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The `eval` methods take a shader/blender/colorFilter, and we assumed
when assembling the ChildCall expression that the child expression would
be a VariableReference because opaque objects don't participate in
normal expressions. However, comma-expressions were allowed to contain
opaque types. GLSL doesn't allow opaque types in comma-expressions:
http://screen/8YW59tYDUbBh9eW
Now we disallow them as well.
Change-Id: Iaf88ef7bddb5cc8f1f1e23b515174dfc291e00c7
Bug: oss-fuzz:41072
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/472446
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
`void` behaves differently from opaque types in several situations, such
as function return types, and errors involving `void` should not call it
an opaque type. I've fixed all the places where we relied on `isOpaque`
to return catch void types.
Change-Id: I359c00f836be6e56cb0373beff60cf2ff830f77b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/472451
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Mysteriously, I had written a test which put arrays of void inside a
struct, but had neglected to include the non-array case. It causes an
okay-not-great error (referring to void as an "opaque type").
Change-Id: Id20a9d3512d29aecea81d46877dce708b7b2f973
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/472450
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I209119e6c74ca54dd6021b6dec4775fc7b66adeb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/472448
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We should, of course, detect this and report an error.
Change-Id: I42b3be6e714a1f367d3251842506a384f2afe019
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/472447
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is done for webview which needs to know whether it is drawing
directly into the main window surface or in a layer.
Change-Id: I0a3f6f81bcf65907f9d51651035bae912c3a34ac
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/468616
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This is split out of Michael's CL:
https://skia-review.googlesource.com/c/skia/+/468459
The CommandBufferTest gets a bit shortchanged in this CL but most of those changes should wait until the GraphicsPipelineDesc is expanded anyways.
Bug: skia:12466
Change-Id: Idba20354a4ab158c2bc46f490da23b8f5fcd55b4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/472336
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This allows us to dump a trace to disk and read it back in again later.
This uses JSON to ensure a debug trace will stay readable across
platforms, builds, etc.
Change-Id: I4e5011b5023cf14f422ab130e9697c125c2b6e45
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471765
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, we'd report them as an overflowed integer.
Change-Id: Ia3632b4bc880829fb04b08a002d7ce9523567a54
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/472056
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is required by the ES2 standard: http://screen/Qysv4fPW5r5LA9e
This actually already worked fine because `strtoull` natively recognizes
octal values without any work on our part. However, we lacked a test.
Change-Id: I3033de899918abe99c63a9b7b79bd4c3374ee315
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471716
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The CPU blitters use signed 32-bit offsets when gathering source pixels,
so any image larger than that can't be fully indexed. Instead, we'd wrap
around and sample from invalid memory. This does put a new (smaller)
limit on valid image sizes, but it seems unlikely to impact any client.
Bug: chromium:1264705
Change-Id: I21b088c11b49e6410b1f237cbfb6b1639cb22ca0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471764
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
We should only be using include/private for types that actually are
needed in include/ files. I moved everything that was just in src to
new src/*Types.h files. This actually left nothing in GraphiteTypesPriv
for now, so I deleted it. If we actually need private types that are
part of include/ we can add it back later.
Bug: skia:12466
Change-Id: I180fc2032afd47bc6c5e8b9ccd4954b5d2451fc7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471758
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
There are definitely some missing getters on other functions we'll need
on BackendTexture. But for this CL I just wanted to get the basics and
ctor working. Other functions can be added as they are needed.
Bug: skia:12633
Change-Id: I668828138df943a432efcec6b1134756e681a582
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469820
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
The only type of expressions that getConstantSubexpression could ever
return are Literal and nullptr. getConstantValue now returns an
optional<double>; nullopt indicates a non-constant value in the slot.
This simplifies most use cases, and allows us to get rid of some extra
"zero" and "one" Literal objects in some of our Constructor classes.
This change fixes a recent fuzzer issue. The fuzzer had discovered that
calling `getConstantSubexpression` on a ConstructorCompoundCast that
contained a compile-time-constant value would return literals of the
wrong type (the cast was not applied). By nesting repeated matrix casts,
this type confusion could be turned into an assertion.
Change-Id: Icee69219e6db2822ffdfab4e5ccdaff54584a4b6
Bug: oss-fuzz:41000
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471376
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This assigns a human-readable name to a debug slot. The slot map is
emitted into skslc output files, and will be used in the future to
display human-readable names in the debugger.
Change-Id: I288358de305239005faa5814bd1d77a38b5e05b0
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470400
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>
This reverts commit 3aaed99930.
Reason for revert: removing changes to PrecisionQualifiers
Original change's description:
> Revert "Fix Metal codegen error with structs containing compound types."
>
> This reverts commit 2a6c41571b.
>
> Reason for revert: causing Mali G7x failures on tree
>
> Original change's description:
> > Fix Metal codegen error with structs containing compound types.
> >
> > While working on an unrelated test, I accidentally triggered a bug in
> > Metal code generation. Our struct-equality helper functions did not
> > properly handle vector fields. Wrapping each comparison in `all(...)`
> > fixes the problem. (all() on a scalar is allowed and does nothing.)
> >
> > Our struct comparison tests now include a vector and a matrix.
> >
> > Change-Id: I59061ae9c3c3ab2c2dbdcb5257bc23e2257152af
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470399
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
>
> Change-Id: Ieb5d5a1839978fb82525863488e9d54fdf44adbd
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471097
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Reviewed-by: Chris Dalton <csmartdalton@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: I8ee90df3de075cf82c0fcf3b4787577b09bb1a70
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471156
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 2a6c41571b.
Reason for revert: causing Mali G7x failures on tree
Original change's description:
> Fix Metal codegen error with structs containing compound types.
>
> While working on an unrelated test, I accidentally triggered a bug in
> Metal code generation. Our struct-equality helper functions did not
> properly handle vector fields. Wrapping each comparison in `all(...)`
> fixes the problem. (all() on a scalar is allowed and does nothing.)
>
> Our struct comparison tests now include a vector and a matrix.
>
> Change-Id: I59061ae9c3c3ab2c2dbdcb5257bc23e2257152af
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470399
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: Ieb5d5a1839978fb82525863488e9d54fdf44adbd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471097
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
While working on an unrelated test, I accidentally triggered a bug in
Metal code generation. Our struct-equality helper functions did not
properly handle vector fields. Wrapping each comparison in `all(...)`
fixes the problem. (all() on a scalar is allowed and does nothing.)
Our struct comparison tests now include a vector and a matrix.
Change-Id: I59061ae9c3c3ab2c2dbdcb5257bc23e2257152af
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470399
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
RenderPassTask should only represent a single RenderPass with possible
subpasses (represented by DrawPasses). This reconstructs the RenderPass
setup to match this thinking. RenderPassDesc now stores TextureProxys for
the attachments, since we may not have instantiated the Textures by the
time we create the RenderPassTask.
Bug: skia:12466
Change-Id: I4f8cab77a297aaf76bea7dd1cc3afd00143b8103
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470460
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
When calling ProgramToSkVM, you can now pass in a pointer to an
SkVMDebugInfo struct. Passing in this pointer now enables trace opcodes,
removing the need for a dedicated `fSkVMDebugTrace` flag.
In this CL, the struct is empty, but in followup CLs it will contain
mapping tables that can be used to translate trace instructions into
human-readable results (e.g. slot numbers to variable names).
Change-Id: I6b38ec559723babed1f6d2efc4c5c2579c3b99db
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470398
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Previously, none of our `runtime` tests relied on the input coordinate
in any way, so all of the logic was hoisted above the main loop in every
test. This CL adds an artificial reliance on the input coordinate so
that we have at least some SkVM tests with real code in the main loop.
This lets us see debug trace instructions interleaved with real code.
The input coordinate is clamped against a known uniform value
(`colorGreen` always contains 0101) so that the final test output
remains consistent in practice.
Additionally, I noticed that this test was only enabled in ES3, but
it doesn't seem to have anything ES3-specific in it, so it's now
enabled across the board.
Change-Id: Ie82f40b1060edb6071e300040ac59fb7d27094b0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470397
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Our inliner would ignore any functions with `inout` parameters, because
inlining them properly was more complex than just leaving the function
call. However, real-world code can sometimes contain helper functions
that have `inout` params that are never used at all (e.g. an uber-shader
with some features turned off).
We now read the ProgramUsage and check to see whether or not the
`inout`-qualified parameter is actually modified. If it's never changed,
the function now remains a candidate for inlining.
Change-Id: I92e494f94cc070801cb9aa28bd13faa689b806b6
Bug: skia:12636
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470299
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Yesterday's implementation was close but I realized later that it wasn't
quite ideal.
- Array index-folding was gated on `isCompileTimeConstant`, which is too
strict. The real limitation is `hasSideEffects`. If an array contains
a side-effecting expression, we should leave it alone. Otherwise it
is safe to pluck out an element from the array and toss the rest.
- Matrix index-folding was gated on `getConstantSubexpression` for the
extracted elements, but did not check the other elements at all. This
was too lenient; we now only proceed to the folding step if
`hasSideEffects` returns false.
I added some tests to verify the final behavior and also discovered a
small related issue. Diagonal matrices were not substituting literals
in for constant-values, which inhibited folding as well and would break
constant-expression evaluation. This is now fixed.
Change-Id: Idda32fd8643c1f32ba21475251cd4d4dd7cea94c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470396
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Functions which don't write to their out params should be safe to
inline, but we currently don't recognize this.
Change-Id: I753e48067c7be4473675ef6c95e61af17dc5ae41
Bug: skia:12636
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470298
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
We never used it internally, but the shaders used by Filament rely on
it. It doesn't exist in ES2 so this doesn't affect Runtime Effects.
Change-Id: Idb2afb15ff160b950ad02101bf6381a5d5c56468
Bug: skia:12635, skia:11209
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470156
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Indexing into a constant matrix is a constant expression, so we are
obligated to support it for ES2 compatibility.
Change-Id: Ibe1e5bac39d9a88ce0222997a38e8b6952fdb336
Bug: skia:12472
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469819
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We should support constant-expressions involving matrices (GLSL ES2
does, WebGL does). We currently don't. We do properly report out-of-
range indexing, but we don't optimize away valid matrix index
expressions or allow matrices to be indexed in a constant-expression
context.
Change-Id: If58aa4c5f15abef421a412957072f3617b4176df
Bug: skia:12472
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469818
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, SkSL was unable to resolve the constant expression `x[y]`
for a constant-array `x` and a constant-integer-scalar `y`. Now, if `x`
and `y` are known, we can replace `x[y]` with the indexed array element.
Note that we need to be careful here, as it's not a valid optimization
to eliminate array elements that have side effects. We preserve side-
effecting expressions using the comma operator.
Change-Id: I5721337eb42b48c0b05f919c1cadfae19dd3b84f
Bug: skia:12472
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469839
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Previously, we didn't have tests which leveraged constant-evaluation of
array indexing (because we didn't support it), and our test files
commingled constant-indexing into vectors with constant-indexing into
arrays.
The test files now separate vector- and array-handling into separate
tests, and a ton of new cases have been added to ArrayFolding. The
ArrayFolding tests now require constant-evaluation of array indexing,
so they fail in this CL, but will be fixed in the followup CL.
Change-Id: I3b663e743d97d6db80627bc9b7808f88c99917a7
Bug: skia:12472
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469528
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Handle kColor_GradientType (which is returned by asGradient
for color shaders) as a straight color paint server.
Bug: skia:12622
Change-Id: I46fa21ed23a7824d67fc0460f92d649743e1b1f2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469900
Commit-Queue: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Tyler Denniston <tdenniston@google.com>
This is unnecessary - the child FPs don't pay attention to the input
color anyway. Simplify OverrideInput while we're here - no one uses it
in the specialized form.
Change-Id: I07cd7ccafd3451e0436b61362a7f2cecfd5633ab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469896
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12466
Change-Id: Icf59f2412a66f3f339635014f4b1e4c44d3c1619
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469523
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
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>
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 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>
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>
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>
