This allows the user to run up to a specified line (or until execution
is complete).
Change-Id: Ied34798d2df1b4a9e1d723c8d2eff3d579ca9b2d
Bug: skia:12747
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/485860
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously attribute offsets were always computed based on their
position in an attribute array and the stride was determined
by the offset and size of the last attribute.
Now a GP has the option to create attributes with explicit offsets
and specify an explicit vertex stride. All attributes must either
be implicit or explicit (enforced by assert).
GrGeometryProcessor::AttributeSet is now responsible for handling
implicitly determined attribute offsets and strides. The backends
no longer compute them.
Bug: skia:12720
Change-Id: I0211673dc70d4797c2d66b2555d8f5fb430be056
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/484736
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This is useful to know because breakpoints can only meaningfully be set
on a line of code which is reachable. In other words, we know the
entire execution path ahead of time, so we can tell you that some
branches aren't actually taken without needing to set a breakpoint at
all.
Change-Id: I72b2cddcf92fd6f75ea12d302230bae42b7112bc
Bug: skia:12666
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/485218
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
We now track each slot's scope depth when it is written, and when scopes
are closed, we remove variables from display which fall out of scope.
Note that we DON'T have a specific trace-op dedicated to setting a
variable's stack depth when it is first declared. This is okay because
the SkVM code generation always stores a zero to a variable's slots as
soon as the variable is declared. This will consistently initialize its
depth to the depth of its initial declaration.
Change-Id: I68f76dfe2930fcd415ba635206cce3d0b94d1aac
Bug: skia:12741
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/484564
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
We didn't have any test cases where a loop variable (other than a loop
induction variable) fell out of scope. Now we have a demonstration of
the trouble behavior.
Change-Id: Iec070748c93aaeab1dc41a46c42e33c78b73d7f5
Bug: skia:12741
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/484560
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
These will give the debugger enough information to discard variables as
they fall out of scope.
Change-Id: Ia400e82a3ca9cf0a51a72d819f897d346979226c
Bug: skia:12741
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/484556
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Note that the 2D cross product isn't defined. There are at least two
possible interpretations of what that might mean. This name makes it
clearer that we're asking for the length of the resulting vector, if
we computed the 3D cross product (assuming Z == 0 for both vectors).
It also eliminates name overlap between builtin functions and actual
intrinsics.
Change-Id: I24e8bc0ab2ec91aaace20f0dd3e8565c10bd44a0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/484440
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This CL adds the trace op and tests the builder commands, but it is not
yet hooked up to the SkVM code generator or debugger.
Change-Id: Iaa64293dfd0973e299eb480cb2b43672b0886c32
Bug: skia:12741
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/484439
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, the variable pane listed variables in slot order, which was
not very helpful (other than usually mapping to a top-to-bottom order in
the original source code). Now, the variable pane sorts variables to the
top when they are modified, and maintains their relative order
throughout execution. This is much easier to follow.
http://screen/C9odSHV4asFSLZH
Change-Id: Id86adf745207451a38a4dc46756f316e8d86bf81
Bug: skia:12666
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/483958
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Previously, return values like `[func].result` would stick around in the
Variables table indefinitely. This felt very counterintuitive. Now they
only appear for one step, then vanish.
Change-Id: Iedfc7d2ddf136111005b26aaefb380ffc6281d05
Bug: skia:12666
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/483605
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We already had this logic in SkVMDebugTrace::dump; it just needed to be
factored out so that it could be accessed directly.
Change-Id: Ib497b82a7722b8ac2285de74f1937379c2f9a51b
Bug: skia:12666
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/483600
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
By tracking slot writes using a dirty mask, and reporting these in the
VariableData, we will be able to highlight assigned-to variables in the
debugger.
Change-Id: I93a767095c9c1b6dedc55a08424649f83aa1c960
Bug: skia:12666
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/483598
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: Ib1ea2936f7a9431bc62c04b1931cf6da2c213b36
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/483496
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
We already had this logic in SkVMDebugTrace::dump; it just needed to be
factored out so that it could be accessed directly.
Change-Id: Idd6c92d23ab4dddc60fdc3c7b1693a0d89b1c992
Bug: skia:12666
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/483517
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Stepping out is extremely similar to stepping-over, so this was mostly
working already.
Change-Id: Ib1c1ba96b45a732500e72b6da4cbfe41a1f75307
Bug: skia:12666
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/483196
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: Id4fc455f0eb23aa8d9b6f5474be7f6eba71c48de
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482698
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This class is responsible for interpreting a debug trace and allowing it
to be stepped through, like a debugger. It tracks the current line
number, call stack, slot values, and associates slots with stack frames.
It supports stepping forward or stepping over (i.e., stepping to the
next line in the current function, hiding function calls).
Change-Id: I2b7d90c3b38b0006bebdfbf65a7bf678d5227d56
Bug: skia:12666
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482460
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Chromium has been using the remote glyph cache for a few years now.
It's time to give it a proper home.
This is an intermediate CL. The old .h file includes the new .h file.
After I change the include paths in Chromium, I will delete the old
file.
Change-Id: Iaf00c46aa0698326c0bdec9a0eed218bcc3e334e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482700
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This reverts commit 85cc1bece7.
Reason for revert: ends up not being useful after all
Original change's description:
> Trace function return values after function-exit.
>
> This will allow function return values to be easily seen when stepping
> "over." This has the unexpected side benefit of generating slightly
> fewer ops when a function has unoptimizable conditional returns.
>
> Change-Id: I48d23de635d3caaddff91aa595593d0371dfcdcb
> Bug: skia:12708
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/481076
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
Bug: skia:12708
Change-Id: I61ebb175b60d2060f6ad21b170238b37557b80be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482458
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Also fixes recursion end point update in recursive_edge_intersect, which
was causing us to search in the wrong area of the lines and falsely
report no intersection when there should have been one (which is then
detected by the topology check in intersectEdgePair() and more less
accurately corrected for).
Bug: chromium:1245359
Change-Id: I1d21c934a23c9b99c3767849ee19bc2f6c551d3f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482456
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
The debug-slot code didn't expect to encounter a void type.
Change-Id: Ied452b51e1cf90a0c0bc24770f82e711105b8e82
Bug: skia:12708
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482461
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Function return values exist at the boundary between stack frames and
will need slightly special treatment by the debugger.
Change-Id: I017b5713919f507074f072d040332325307c2e81
Bug: skia:12666
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482459
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12701
Change-Id: I51bfad4ff61f2ab32753e532eb5b6ee46d765303
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/481737
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
There was only one virtual method, so switch that to a bool stored in
the base class. The derived types exist as hints for the reader, and an
easy way to adjust how the new localToDevice is constructed.
With this change, we don't need SkSimpleMatrixProvider. SkMatrixProvider
is concrete, so we can use it directly. SkOverrideDeviceMatrixProvider
no longer needs the original provider for anything, so remove that
parameter. It now exists solely to inhibit the hitsPixelCenters flag.
Fix a few spots (SkParticleBinding, some sites in SkRuntimeEffect) where
we used SkSimpleMatrixProvider, even though the local coordinates being
passed did not obey the hits-pixel-centers constraints.
Most importantly, document how localToDeviceHitsPixelCenters works.
Change-Id: Ibe9060bac0822d0edf52a507d390bd198d8e6dbd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482176
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This entire API existed, but was unused (no longer connected to
drawVertices or runtime effects).
In theory, we could further simplify some of the matrix providers, but
more importantly - I have serious doubts about the correctness of
localToDeviceHitsPixelsCenters for most of them.
Change-Id: If5af182015dd96e5ed3353a117223e8dbbe17097
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/481683
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This fix appeared in prior CL patchsets, but got lost through rebases
and CL splits.
Change-Id: I1befff3830b40740f1e52e3e88f74484122027ce
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/481680
Reviewed-by: Julia Lavrova <jlavrova@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I57233c89fb27f09ca22509edeab79ea6b158b3c3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/481281
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
The current system short circuits out when an SkTextBlob will draw
anything based on its bounds, the bounds of the clip, and the state
of the paint.
A similar thing needs to happen, but the clip needs to be ignored.
Return the nullptr if the SkTextBlob would never draw to signal that
the Slug can be ignored.
Bug: chromium:1277583
Change-Id: I2d6641e03c88ae5f160c479c9e715ad8dc9f65b3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/481592
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Bug: skia:12701
Change-Id: If286f9398c099298b1a543a12d4a28ba70f7a20b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/479062
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This reverts commit 062652067b.
Reason for revert: design change removes need for this op
Original change's description:
> Add SkVM op `trace_done` to indicate completion of debug tracing.
>
> This op can be used to invoke a callback function and dump the log to
> disk when it is ready. SkRuntimeEffect doesn't have any other viable
> mechanisms for detecting that a paint has completed, AFAIK. We can
> wait for ~SkRTShader to occur, but there's no guarantee that this will
> happen quickly, and the SkPaint with the SkRTShader shader can be reused
> over and over again.
>
> Unlike other trace ops, this only needs a trace mask, not an execution
> mask (we are unconditionally done at the end of main).
>
> Change-Id: I6f7ee41f2005b65940d36dee892279d4f245509f
> Bug: skia:12708
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/479876
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:12708
Change-Id: Ic4c4f5dd72541195f07ca32035267a20a82536e7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/481577
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Color filters and blenders are currently not supported for tracing,
because we don't have access to the pixel coordinate, and so there is
nothing to compare the debug-trace coordinate with.
Change-Id: I7fe7fb4955b002432046ceef61c6f0a4c721a581
Bug: skia:12708
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/481278
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Added comments to explain the semantics (both what's expected when you
set the uniform, and what you see in the shader). The old name was
confusing, because it sounded like you got an sRGB color in the shader.
This is terse, but I think it's the cleanest syntax - and for embedding
clients, they can use C++ (etc.) API to require that color uniforms are
assigned from color types.
Bug: skia:10479
Change-Id: If00ea754060494aaa83001a5b357687953de8a5f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/480577
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This will allow function return values to be easily seen when stepping
"over." This has the unexpected side benefit of generating slightly
fewer ops when a function has unoptimizable conditional returns.
Change-Id: I48d23de635d3caaddff91aa595593d0371dfcdcb
Bug: skia:12708
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/481076
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Matrices now include [C][R] notation and vectors indicate .x/.y/.z/.w.
Previously this information was missing, so you couldn't tell which slot
of a variable was being altered.
Change-Id: I590614372b1fc98e95e8fb5ebc3d318c449a8ee1
Bug: skia:12708
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/480798
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This op can be used to invoke a callback function and dump the log to
disk when it is ready. SkRuntimeEffect doesn't have any other viable
mechanisms for detecting that a paint has completed, AFAIK. We can
wait for ~SkRTShader to occur, but there's no guarantee that this will
happen quickly, and the SkPaint with the SkRTShader shader can be reused
over and over again.
Unlike other trace ops, this only needs a trace mask, not an execution
mask (we are unconditionally done at the end of main).
Change-Id: I6f7ee41f2005b65940d36dee892279d4f245509f
Bug: skia:12708
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/479876
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
SkRuntimeEffect needs an API for generating debug traces. This means
that we will need references to debug traces inside a public header.
Rather than reference SkVMDebugInfo directly, we now have a simpler
base class for debug traces. This is better suited to landing in
include/.
I've also renamed SkVMDebugInfo to SkVMDebugTrace for consistency, since
it now contains all the trace data. (When it was first added, it only
had the slot info.)
Change-Id: Ibaa4dedf9a17b9462b4f233a28a7b875d0317892
Bug: skia:12708
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/480356
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This creates SkImageShader instances that do not perform color space
conversion, nor do they premultiply their pixels (if they started out
unpremultiplied). These are useful for runtime shaders that want
non-color inputs (like normal maps or lookup tables).
Includes GM that demonstrates lack of color conversion, and lack of
premultiplication in the context of a lighting shader.
Bug: skia:10479
Change-Id: Ic07aa8b8d3407ae5f81bc075648fdcba6d4cce29
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/477299
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
dump() now emits the most important part of the debug trace: the actual
trace itself. This will make tests easier to write and reason about.
Change-Id: I414e26b80a05c9f7ea21956d84c5a682a5f3c274
Bug: skia:12708
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/479703
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
I ran into a snag while trying to hook up SkRuntimeEffect with debug
tracing. Runtime effects only have access to a skvm::Builder, and are
never exposed to the full skvm::Program. SkVMBlitter is responsible for
assembling the full skvm::Program, but is oblivious to runtime effects
and nested skvm sub-programs. Additionally, multiple runtime effects can
(and often do) coexist within a paint.
This CL changes how debug traces are enabled. skvm::Program no longer
has a `attachDebugTrace` method. Instead, this method lives on the
skvm::Builder. Calling `attachDebugTrace` generates a "trace-hook ID"
(which is actually an index into a vector of TraceHook pointers).
Every trace opcode now includes this trace hook ID. When the Builder
assembles a final Program, it copies the TraceHooks into the Program.
The skvm interpreter uses the trace hook ID on the op to dispatch a
trace command to its associated TraceHook.
From a user perspective, this doesn't change very much, but it does
mean that the SkVM Code Generator now supplies a TraceHook for us
(since it adds trace ops to the Builder and needs to know the proper
trace-hook ID).
Change-Id: I8bd5fea24f477f81470fae8ba41be45f76949407
Bug: skia:12708
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/479597
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Use that to preserve the alpha type of (unpremul) SkImages when
serializing. This ensures that the new GM works correctly in
serialize-8888, and is required for an upcoming "raw" image feature.
Change-Id: I747803c2928cbb5872d4c97a421701248d8f6a51
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/478957
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Previously, the trace opcodes took a single mask argument, which was
computed as `execution mask & trace mask`. This led to extra bit_ands in
the output, as this value would be need to be recalculated every time
the execution mask changed.
To reduce this cost on program size, the trace ops now take two mask
arguments and require that both must be true. We have four register
slots at our disposal in an Op, which is more than we need, so this
doesn't really cost us anything.
(As an extra minor optimization, if one of the masks is "always-on", we
optimize it away. This avoids burning a register just to hold a ~0
immediate value.)
Change-Id: I9eb71292a1983e71b03c7ac842534beb3d6bbf17
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/478456
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Add an array which represents the trace itself into the SkVMDebugInfo.
Since we expect complex programs to emit a lot of trace data, we try to
keep the JSON representation here compact. Ops and their data are
represented as a compact array of integers. Data fields can be left
unspecified; these are assumed to be zero.
Change-Id: Ia52ad280ab5989496eb495c4efb8b99aa72cda5f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/477983
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
switch is no longer an ES3-specific feature.
Change-Id: Ic878a77268e517e17699c2e35a37da6b0a7765dd
Bug: skia:12450
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452320
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This also hides the ctors for the respective classes so you can't just
arbitrarily make one outside of the factories.
Change-Id: If31ac8ea9b54c9e10c162081251e77d7e9d07147
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/478956
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
With this CL, rt adjust is handled automatically by the backend of the
CommandBuffer when setViewport is called, so DrawPass and the
CommandBufferTest are updated to configure that (and the geometry of
command buffer test is rewritten to be in Skia's coord system instead
of NDC).
Currently, the metal command buffer uses push constants to set the
rtAdjust uniform. Hypothetically, D3D12 could use the root descriptor,
etc. There is some coupling here between the intrinsics the SkSL
programming environment operates in, and the backends configuring the
pipelines, but I don't think it's unreasonable and it seems cleaner
to me than having DrawPass bind and upload uniforms for all of the
intrinsics.
Cq-Include-Trybots: luci.skia.skia.primary:Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-Graphite,Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Debug-All-ASAN_Graphite,Build-Mac-Clang-arm64-Release-iOS_Graphite,Build-Mac-Clang-arm64-Debug-iOS_Graphite,Build-Mac-Clang-arm64-Debug-Graphite_NoGpu,Build-Mac-Clang-arm64-Debug-Graphite,Build-Mac-Clang-arm64-Debug-ASAN_Graphite,Build-Mac-Clang-arm64-Release-Graphite
Bug: skia:12466
Change-Id: Id91e9ffc31688886c5bf3ee6134567070820207a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/478656
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Passes the Transform for each draw to the RenderStep, to both
writeVertices and writeUniforms, since any given RenderStep could
choose to transform vertices on the CPU, pack the matrix as instance
attributes, or upload it as a uniform.
Also updates UniformManager to take the source data as const void*
Cq-Include-Trybots: luci.skia.skia.primary:Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-Graphite,Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Debug-All-ASAN_Graphite,Build-Mac-Clang-arm64-Release-iOS_Graphite,Build-Mac-Clang-arm64-Debug-iOS_Graphite,Build-Mac-Clang-arm64-Debug-Graphite_NoGpu,Build-Mac-Clang-arm64-Debug-Graphite,Build-Mac-Clang-arm64-Debug-ASAN_Graphite,Build-Mac-Clang-arm64-Release-Graphite
Bug: skia:12466
Change-Id: I7ac40af0b7c123d068478f5672dda455c0bbbfb5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/478376
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
This demonstrates how much additional non-trace code can be generated
when debug traces are turned on. A followup CL adds a setting to disable
`trace_var`, which eliminates a significant percentage of the additional
ops (at the cost of removing valuable debug info).
Change-Id: I238e28e6f6531f1dbccfef8f1dcd24a1e8481669
Bug: skia:12692
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/478416
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Julia Lavrova <jlavrova@google.com>
Bug: skia:12466
Change-Id: I85d23e73a37a368ed68c81191d072014df110080
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/475645
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Cq-Include-Trybots: luci.skia.skia.primary:Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-Graphite,Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Debug-All-ASAN_Graphite,Build-Mac-Clang-arm64-Release-iOS_Graphite,Build-Mac-Clang-arm64-Release-Graphite,Build-Mac-Clang-arm64-Debug-iOS_Graphite,Build-Mac-Clang-arm64-Debug-Graphite_NoGpu,Build-Mac-Clang-arm64-Debug-Graphite,Build-Mac-Clang-arm64-Debug-ASAN_Graphite
Bug: skia:12466
Change-Id: If1d4c29a434a1ad3445ceadb98bd7f3b8abb5ee4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/475639
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Bug: skia:12633
Change-Id: Ida78c4145423376dc0267096a1d758b74144fd0c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/477139
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This shouldn't change code generation when debug traces are disabled.
When they are enabled, we now get trace_var opcodes emitted for every
return statement. Internally, this required a fair amount of refactoring
around how return values are passed around, but it should all be
functionally equivalent.
Change-Id: Ieb9d9c75399109186f905e0499d8fe6e2fc2067c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/477981
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>
Given how badly these fail (even on recent NVIDIA drivers), I'm prepared
to ignore the conformance suite on this one and just disallow this in
SkSL. For now, this disables these tests so that they don't crash for
anyone running an NV GPU.
Bug: skia:12443
Change-Id: I990eaa47acf0e23f4f0b6e90e136712d837b7f6d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/477477
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This new image filter constructor enables SkRuntimeEffects to be
used as shaders within the ImageFilter DAG. The shader is created
lazily using the SkRuntimeShaderBuilder enabling the resulting
shader to consume the previous stage of the ImageFilter graph.
This relands commit ad9d774c1f after
fixing the threading issue found by the TSAN bot.
Change-Id: I06dc4cf6a304b14804c55531a5bc3f2afa34e219
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/477156
Auto-Submit: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
SkRuntimeEffect users will be insulated from skvm and the Builder, so
they won't be able to make an skvm::Coord directly. Replace this with
an SkIPoint, and do the conversion inside the SkVM code generator.
This also removes the requirement of assigning a trace coordinate; if
no trace coordinate is set, (0, 0) will be used.
Change-Id: I97be025d0ea146a86655cc3cdb76726f3bbc2324
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474556
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I7fa9b4da48f993143ade3b8030e2c67831cf0ffd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/475257
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reusing the same opcode for enter and exit didn't have any real upside,
and forced us to deal with a fake immediate-value.
Change-Id: I35cab4d196f55b6f5e3040074c731b298720d62b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/477157
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
I should have realized the fuzzer would find this assert when I added
it. Now the front-end rejects these layout qualifiers on both struct
fields and interface block fields. LayoutInInterfaceBlock.sksl is a
reformatted version of the fuzzer input. LayoutInStruct is hand-crafted
to trigger the same failure on a different code path. Both would
previously assert in the SPIRV generator. Now, neither one gets that
far.
Bug: oss-fuzz:41347
Change-Id: Iff69d8f5482da7b772e9331c4fd2d58e89813c46
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/476396
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
SkFontStyle needs to be updated to take scalar weight and width, and
separate slant and italic into separate scalar values. Since there are
currently issues with serialization of SkFontStyle, start with the
serialization and deserialization before moving these changes up into
SkFontStyle and its users itself.
Bug: oss-fuzz:24962,skia:6844
Change-Id: Ic121d95fe1309ee04dbd6b1fe72d9b1d01b52258
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313476
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Bug: skia:12466
Change-Id: I84d017eda7964a19ddd36570cd46e8d98ba2f50f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474936
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This reverts commit b1f450bb2b.
Reason for revert: Hmm, bot disagrees.
Original change's description:
> Update some skvm test outputs
>
> Not sure how these were wrong in the repo for so long - these all should
> have changed like this when the optimization of commutative operations
> landed: reviews.skia.org/473239
>
> Change-Id: I3035d4f29c208096f83184b4393023666e10ab92
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/476316
> Auto-Submit: Brian Osman <brianosman@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I9dc808927c73a979f4cde5c7035b109d8db808f6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/476397
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Also renames UniformCache::findOrCreate() to insert(), and has it return
the assigned id. ExtractCombo() now does not take a UniformCache, since
it just has to return the CPU data extracted from the PaintParams. The
caching/id assignment is the responsibility of the DrawPass.
Updates unit tests and DrawPass to use this instead.
Bug: skia:12466
Change-Id: Ib363fcad799f62d0ad31bb9d3e4c995e191cae80
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/475798
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Not sure how these were wrong in the repo for so long - these all should
have changed like this when the optimization of commutative operations
landed: reviews.skia.org/473239
Change-Id: I3035d4f29c208096f83184b4393023666e10ab92
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/476316
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This reverts commit ad9d774c1f.
Reason for revert: Task Test-Debian10-Clang-NUC7i5BNK-GPU-IntelIris640-x86_64-Release-All-DDL3_TSAN has been failing since this CL landed.
Original change's description:
> Add public API support for SkImageFilters::RuntimeShader
>
> This new image filter constructor enables SkRuntimeEffects to be
> used as shaders within the ImageFilter DAG. The shader is created
> lazily using the SkRuntimeShaderBuilder enabling the resulting
> shader to consume the previous stage of the ImageFilter graph.
>
> Change-Id: I5d6917e34a8e5fdd053399f15a1e2cc7409e686f
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470459
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Derek Sollenberger <djsollen@google.com>
Change-Id: I0367a5c480df109c6116eb168792fe3c2fc58807
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/475644
Auto-Submit: Leandro Lovisolo <lovisolo@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Leandro Lovisolo <lovisolo@google.com>
This is a reland of 9372ef0228
Original change's description:
> Restrict where 'binding' and 'set' can appear
>
> In SPIRV, these are an error when applied to struct members. Some of our
> tests were triggering that because we had free-floating uniforms
> decorated this way (and we coalesce those into members of an interface
> block).
>
> Now, we only allow those layout qualifiers on variable types that will
> remain top-level constructs in the back-end.
>
> Bug: skia:12670
> Change-Id: I73e69cecf6237a1c1180ad38d9b5d52ea80316fb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474218
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:12670
Change-Id: I01c0323bba7ce0bddea5f9fb907e2b60e6b812d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/475156
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This new image filter constructor enables SkRuntimeEffects to be
used as shaders within the ImageFilter DAG. The shader is created
lazily using the SkRuntimeShaderBuilder enabling the resulting
shader to consume the previous stage of the ImageFilter graph.
Change-Id: I5d6917e34a8e5fdd053399f15a1e2cc7409e686f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470459
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
Moves Attribute into its own header. I did not move AttributeSet/Iter
out of GraphicsPipelineDesc because RenderStep should be able to just
rely on SkSpan<Attribute>. Since they are fixed definitions, there
shouldn't be a need to support skipping over uninitialized attrs. Once
GraphicsPipelineDesc just points to a RenderStep, AttributeSet/Iter can
be deleted.
Bug: skia:12466
Change-Id: I3b01b6283669fc02dc9cd0521bb236a9d0daa5eb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474136
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Change-Id: I94ded218e822fe50687e1efff7d46bcc4c1f1293
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471780
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This reverts commit 9372ef0228.
Reason for revert: Unhappy bots
Original change's description:
> Restrict where 'binding' and 'set' can appear
>
> In SPIRV, these are an error when applied to struct members. Some of our
> tests were triggering that because we had free-floating uniforms
> decorated this way (and we coalesce those into members of an interface
> block).
>
> Now, we only allow those layout qualifiers on variable types that will
> remain top-level constructs in the back-end.
>
> Bug: skia:12670
> Change-Id: I73e69cecf6237a1c1180ad38d9b5d52ea80316fb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474218
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:12670
Change-Id: Ie518192d9a52fc896e615ec08ce0674ad683ec61
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/475099
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
In SPIRV, these are an error when applied to struct members. Some of our
tests were triggering that because we had free-floating uniforms
decorated this way (and we coalesce those into members of an interface
block).
Now, we only allow those layout qualifiers on variable types that will
remain top-level constructs in the back-end.
Bug: skia:12670
Change-Id: I73e69cecf6237a1c1180ad38d9b5d52ea80316fb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474218
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:12524
Change-Id: I2664c8ea707a40724bcf916e907fe28d1fa276d6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474357
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This is redundant information now that we have SkVMSlotInfo.
Change-Id: Ia05b1eaa722023e719042c83255708aa9debed61
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/473777
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This adds new validation rules that we were breaking.
Binding and DescriptorSet can't be applied to push constants, nor to
struct members.
Bug: skia:12670
Bug: chromium:1270328
Change-Id: I332f77717b08d9945c8e5b79c5bf649a8f5f2043
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474056
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:12559
Change-Id: I76b225c9ca81264a15869324007d774d210053b5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/473416
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Previously, we would avoid emitting redundant trace_vars by checking to
see if the slot is being assigned to the exact same Var.ID. However,
this had the potential to eliminate useful trace_vars:
- At the start of execution, slots all contain 0. Code which explicitly
assigned a zero into a slot would not be shown in a trace. (So things
like `color = half4(0,1,0,1)` would only emit traces for color.ga.)
- A function call's parameter slots are reused every time it is called,
so calling a function twice would only emit traces for the parameters
that aren't the same Val.ID as the previous call.
- A VarDeclaration inside a loop reuses its slot each time through the
loop, even though conceptually it's a "new" variable.
We now track a slot's "written-to" status. At the start of execution,
no slots have been "written-to". These slots will always emit a
trace_var opcode (fixing the first issue). Also, issuing a function
call or declaring a variable will reset the "written-to" status of the
associated slots (fixing the second and third issues).
When the debugger is not in use, the written-to field is unused.
Change-Id: I482a86cb6e90d0f85dd2a161e984f212782a7b4d
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/473776
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We now canonicalize commutative operations by ordering their value IDs.
The lower-numbered value ID is always placed first into a commutative
instruction. In other words, this instruction:
bit_and result, v7, v5
Would be silently converted to this:
bit_and result, v5, v7
This will allow these two logically-equivalent instructions to be
deduplicated:
bit_and result, v7, v5
bit_and result, v5, v7
Of course, deduplicating these ops can unlock additional free CSE/DCE.
The affected instructions are listed in http://review.skia.org/473238
Change-Id: Ib9beb79d6b72d7903184aaa9a53e8e5a02ae126d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/473239
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
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>
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>
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>
* 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>
This will be used to populate a trace buffer for the SkSL debugger.
See http://go/sksl-tracing for details and rationale.
Change-Id: I4c218c65ff01c339cf460e97e41566860a694720
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/468436
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Also updates MtlRenderPipeline to be MtlGraphicsPipeline to match the
type it extends, although we could choose to have it stay matched with
the backend object (MTLRenderPipeline) that it wraps instead.
Bug: skia:12466
Change-Id: Ida118e68a93d737d21edca15a59f5e64e17b5fd0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467780
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This reverts commit b61804e94c.
Reason for revert: chromium perf regression
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
Bug: chromium:1267108
Change-Id: Ic2b9d6cc029e1519be928615585a0b22c7037b56
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/468276
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Much like http://review.skia.org/467759, this CL defensively guards
against programs which consume more space than is reasonable. Globals
exist outside of functions, so they wouldn't be caught by the stack size
checks.
Change-Id: I035f27d57bc329508820a729a1e367ecaadfe156
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467760
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Functions that declare variables totaling more than 100,000 slots will
now generate an error.
This is only a partial mitigation to the problem, as a sophisticated
attack could still chain/nest multiple functions together to consume
extremely large amounts of stack. However, this mitigation is still more
sophisticated than our peers; both WebGL and glslang are susceptible to
similar problems, and in the general case (ES3+ with full flow control)
it's intractable.
Change-Id: I153c75267c017a23f59fe9e59f6e391197ee6101
Bug: oss-fuzz:40304, oss-fuzz:40694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467759
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The skstd::optional already does this correctly, but it appears to be
one of the guarantees of this class which isn't tested.
Change-Id: Iedd9004496f10c1ec56da4ebc5e246880c9a457f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467781
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
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>
It's a performance foot-gun today (disables raster pipeline), and will
hopefully be unnecessary in the future.
Change-Id: I241484286388e4bd2a429a6d22e3b96ecf975cec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467456
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Add a getter to SkTypeface to provide information on whether it
needs access to the foreground color (in COLR fonts) so that the
strike cache can determine caching requirements.
Since remote SkTypefaces do not have access to table information,
implement this as a serializable flag, with font-blob backed
implementations being able to return this based on whether they
have a COLR table or not, starting with FreeType.
Preparation for supporting foreground color in FreeType COLR
rasterisation.
Bug: skia:12576
Change-Id: I7e71b0ec12e17f652ab7b43adffc43bc780ce2e7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/466936
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
The fuzzer triggered this error in a strange way that involves parsing a
TK_INVALID token. The fuzzer's original input used \xFF bytes in the
shader text to do this. I replaced these with the ` character since it
behaved the same, but allows our test inputs to remain basic ASCII.
The root problem is that `cast_expression`, part of no-op arithmetic
simplification, can now fail because expressions like `int(4000000000)`
no longer get past Constructor::Convert. Previously we had assumed
`cast_expression` could never return null; now we check its result for
null before using it.
Change-Id: I7335395bab0daf1f788b0c7c154904b2372ae13f
Bug: oss-fuzz:40660
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467316
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>
Updated ReturnsValueOnEveryPathES3 to remove overlap with the ES2 tests,
and fixed some broken cases. Disabled the ReturnValueOnEveryPathES3 test
on Intel + Windows because switch statements on Intel + Windows are
pretty broken.
Change-Id: Id93e8af1ef7bf11fd74ef12a464c77d56cc032a0
Bug: skia:11209, skia:12465
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467078
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Cleaning out old TODOs.
Change-Id: Ia54bffab5145d61dbacc3da5617e0e3293a6ddf2
Bug: skia:11209
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467076
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
It's possible to write code containing errors that are only apparent
once the inliner runs. For instance, a function which takes a short and
returns its negative it is valid for most inputs, but undefined for
-32768 (because +32768 does not fit in a short). A function which takes
floats and casts them to ints is valid for many inputs, but not valid if
you pass in 5 billion.
This CL restructures our out-of-range integer error detection to report
errors cleanly in these cases instead of asserting. It also refactors
the range checking code to be usable in situations where we don't yet
have a Literal expression.
Change-Id: I98f0be63bf9afbbf1ab90233fa86d380cfae42b4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/466439
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I01d82447658c7acc5fe9eb230eb7020b49fa6c4f
Bug: skia:12498
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/466447
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The code was original target for LLVM 10. I have updated it
to use LLVM 14. This does not fully complete the LLVM code because
there are still missing ops: array32, from_fp16, to_fp16,
load64, store64, load128, store128.
args.gn:
extra_cflags = [
"-DSKVM_LLVM",
"-isystem",
"../llvm-project/include",
]
extra_ldflags = [
"-L../llvm-project/lib",
"-lLLVM",
]
Running:
env DYLD_LIBRARY_PATH=/Users/herb/bin/llvm/lib ./out/SkVM-LLVM/dm
Change-Id: I79e3d3d62b29ed8bdad22890d4db5bc160ba4bf6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/460316
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This is a more cautious attempt of https://skia-review.googlesource.com/c/skia/+/464876
GrAlignTo renamed to SkAlignTo and moved to SkTypes.h
(with the rest of our align helpers).
GR_MAKE_BITFIELD_OPS and GR_DECL_BITFIELD_OPS_FRIENDS
renamed to SK_* and moved to private/SkMacros.h
This avoids our public includes using a src header file
and messing with SkUtils.h, which apparently was used
by Android. That should be cleaned up in a separate effort.
Change-Id: I86d5e1fb6a7834034534266a6c340bc7757f9abb
Bug: skia:12584
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/466176
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I7512491f55c10118f0ab058500f6ce9b5b8545cd
Bug: oss-fuzz:40557
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/466296
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>
This is a reland of 75e1f4c028
Original change's description:
> Lift recursive curve culling out of tessellators
>
> We need recursive chopping/culling logic in order to draw
> astronomically large paths. But rather than do that at the same time
> the tessellators chop curves, this CL moves that logic into an
> SkPath -> SkPath transformation that runs ahead of time (and only if
> the path is extremely large to begin with). This will enable us to
> remove recursion from the tessellators and quickly determine ahead of
> time the size of buffers they need.
>
> Bug: skia:12524
> Change-Id: Ib2800fb23054f1548501811203173e58273fbc83
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463936
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
Bug: skia:12524
Change-Id: Idf54f0c2bddaaddc9fc17bee99c910f3961682a5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465361
Reviewed-by: Greg Daniel <egdaniel@google.com>
Auto-Submit: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This will hopefully improve performance on lower-end GPUs.
Change-Id: I9c2ee6dc31acd08bec0bfb5f59edc3cf90163f9e
Bug: skia:12339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465078
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: Ic30c48dce0cb0072f07defcdb0b9e60b94f50818
Bug: oss-fuzz:40479
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465392
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>
The fuzzer discovered that SkSL could create an out-of-range int literal
by casting from a floating point literal. We were only doing range
checks when the starting literal was an integer. Since we now assert
when an out-of-range int literal is created (as of
http://review.skia.org/464124), the fuzzer can detect this error.
Change-Id: Ie66f60ddbe7b4fbe5b648c17292c59a4ba079716
Bug: oss-fuzz:40456
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465385
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 guards against unexpected results when dfdy is used in complex
expressions. In practice, I'm not aware of this causing any trouble.
Change-Id: Ia476e57936969d248273856a94d5c403b47c29b4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465379
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This guards against unexpected results when dfdy is used in complex
expressions. In practice, I'm not aware of this causing any trouble.
Change-Id: I639bef465d7907049d79681a49f9be67b4c435a6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465378
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 9fc189f1cb.
Reason for revert: shader compile failure on AndroidOne-GPU-Mali400MP2 devices
Original change's description:
> Wrap 'u_rtFlip.y * dfdy()' in parentheses.
>
> This guards against unexpected results when dfdy is used in complex
> expressions. In practice, I'm not aware of this causing any trouble.
>
> Change-Id: I58d4762871481fdb4c173b570e4d5d6edf657af7
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465077
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: Idfaa9316d657717d5ee7117837c9cc9c3d4ee189
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465377
Auto-Submit: Greg Daniel <egdaniel@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
This reverts commit 65a726bb49.
Reason for revert: You cannot include a src file in an include file. This ends up using c++17 features in our includes. Breaks rolls.
Original change's description:
> Move GR_MAKE_BITFIELD_OPS and GrAlignTo to non-GPU files
>
> These have been renamed SK_MAKE_BITFIELD_OPS and SkAlignTo
> because nothing seemed particularly GPU/Ganesh specific to them.
>
> I moved the latter to SkTypes.h because we have other align
> code there and former to src/SkUtils.h because I didn't know
> where else it should go.
>
> The primary motivation was removing the GrTypesPriv.h
> include from src/core/SkBlockAllocator.h. I had attempted
> some amount of #if SK_SUPPORT_GPU, but that's not as clean
> here because both our CPU and GPU backends use the
> SkBlockAllocator (as far as I could tell).
>
> This also moves sk_memset* from SkUtils.h to SkOpts.h, because
> SkOpts.h requires bringing in RasterPipeline, which seemed
> like overkill.
>
> Change-Id: I5163ef5064ad3840a15b7e873930d60e2620bf9d
> Bug: skia:12584
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464876
> Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:12584
Change-Id: I1b772bbbc6f150d737bb53fa4e5f45d1581929fa
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465376
Auto-Submit: Greg Daniel <egdaniel@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
This guards against unexpected results when dfdy is used in complex
expressions. In practice, I'm not aware of this causing any trouble.
Change-Id: I58d4762871481fdb4c173b570e4d5d6edf657af7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465077
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Bug: skia:12466
Change-Id: I12c64396791d8e0b8891b82927b1c8811a6e164f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464385
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
The SortKey will only hold the uniqueID of the program and uniform data. When actually executing the draw we will need to reconnect with the actual data.
Bug: skia:12466
Change-Id: Iea7f0a99d471ea7fe2a3864bdd60255b09289088
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464926
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
These have been renamed SK_MAKE_BITFIELD_OPS and SkAlignTo
because nothing seemed particularly GPU/Ganesh specific to them.
I moved the latter to SkTypes.h because we have other align
code there and former to src/SkUtils.h because I didn't know
where else it should go.
The primary motivation was removing the GrTypesPriv.h
include from src/core/SkBlockAllocator.h. I had attempted
some amount of #if SK_SUPPORT_GPU, but that's not as clean
here because both our CPU and GPU backends use the
SkBlockAllocator (as far as I could tell).
This also moves sk_memset* from SkUtils.h to SkOpts.h, because
SkOpts.h requires bringing in RasterPipeline, which seemed
like overkill.
Change-Id: I5163ef5064ad3840a15b7e873930d60e2620bf9d
Bug: skia:12584
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464876
Reviewed-by: Brian Osman <brianosman@google.com>
A recent CL (http://review.skia.org/464121) made it an error to coerce a
literal value to a type that cannot hold the value. The fuzzer found a
case where we assumed type-coercion of a literal would always succeed,
and failed to null-check the result. We now null-check the result.
Change-Id: Id97c6016e56c20ef724028f71bbf4688dde3c064
Bug: oss-fuzz:40428
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464919
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Yesterday's negation-related changes (http://review.skia.org/464123)
exposed a flaw that the fuzzer was able to exploit. We were previously
able to assume that `simplify_negation` would always return a non-null
expression; in some cases, that is no longer true.
Change-Id: Ia585232b0e35fafe0c642384a59ef94ce743ffd5
Bug: oss-fuzz:40427
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464916
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>
Adds Attributes and supporting types to RenderPipelineDesc so they can
be created for the RenderPipeline.
Bug: skia:12466
Change-Id: I7ed920ea6d44f27f7dace81d35cd967a8dea55de
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464377
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
With this change, we no longer have any SkSL tests which are able to
make a Literal integer that overflows its type. Literal::MakeInt now
asserts that its value is within bounds. I look forward to the fuzzer's
inevitable attempts to trigger these assertions.
Change-Id: I7b15e862caaf65984d33f5d72d2c1de816d1d292
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464124
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This was mistakenly using dFdx in some portions (copy-paste error).
Change-Id: Ifb159b3c44185d9166c10725b24002a28a0895b2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464381
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 is a reland of 8a2a020ef4
Original change's description:
> Create a common Tessellation.h header
>
> This header and corresponding implementation will contain common
> definitions and subroutines for tessellation code.
>
> Bug: skia:12524
> Change-Id: Ib29b444177f284acb88a3d5644936674c48c0b89
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463437
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
Bug: skia:12524
Change-Id: I874fed63cdab50df841e8a5d25d8c822690b5af8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464294
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Bug: skia:12466
Change-Id: If633ce39c8f45b1ee3c042b5b72d7e0f95ca5c19
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459597
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Previously, we would create a Literal with the negated value even if it
was outside the type's minimum/maximum values. Error reporting would
happen elsewhere, if at all (e.g. during assignment or coercion).
Change-Id: I020a93daf2b0f5741fb805a58a690489d7578dab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464123
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Previously, we would create SkSL literals of ints that didn't fit into
an int. This change causes a few errors to report differently. (In
particular, we no longer create global variables containing values that
wouldn't fit in that variable, so those symbols are invalid later.)
Change-Id: I29d219e853126ea78dd2d2a6d8a69b23ef2b06b8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464121
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 8a2a020ef4.
Reason for revert: breaking chrome roll. Needs change in chrome to land first
Original change's description:
> Create a common Tessellation.h header
>
> This header and corresponding implementation will contain common
> definitions and subroutines for tessellation code.
>
> Bug: skia:12524
> Change-Id: Ib29b444177f284acb88a3d5644936674c48c0b89
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463437
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
Bug: skia:12524
Change-Id: Ifd7e81eed8c337506d67ecd0fd2501eedf2e4408
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464297
Auto-Submit: Greg Daniel <egdaniel@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
This reverts commit 75e1f4c028.
Reason for revert: need to revert parent CL
Original change's description:
> Lift recursive curve culling out of tessellators
>
> We need recursive chopping/culling logic in order to draw
> astronomically large paths. But rather than do that at the same time
> the tessellators chop curves, this CL moves that logic into an
> SkPath -> SkPath transformation that runs ahead of time (and only if
> the path is extremely large to begin with). This will enable us to
> remove recursion from the tessellators and quickly determine ahead of
> time the size of buffers they need.
>
> Bug: skia:12524
> Change-Id: Ib2800fb23054f1548501811203173e58273fbc83
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463936
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
Bug: skia:12524
Change-Id: Ib075e8398767fd7b9f49e50ecee98ce47a2ad6b1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464296
Auto-Submit: Greg Daniel <egdaniel@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This was causing errors in UBSAN when compiling some of our existing
SkSL tests.
Change-Id: I66f22607094df77d47ff70948a139c77feae8624
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464118
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We need recursive chopping/culling logic in order to draw
astronomically large paths. But rather than do that at the same time
the tessellators chop curves, this CL moves that logic into an
SkPath -> SkPath transformation that runs ahead of time (and only if
the path is extremely large to begin with). This will enable us to
remove recursion from the tessellators and quickly determine ahead of
time the size of buffers they need.
Bug: skia:12524
Change-Id: Ib2800fb23054f1548501811203173e58273fbc83
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463936
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This header and corresponding implementation will contain common
definitions and subroutines for tessellation code.
Bug: skia:12524
Change-Id: Ib29b444177f284acb88a3d5644936674c48c0b89
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463437
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
On linux we need to support drm format modifiers on vulkan images. When
we have a modifier it can restrict the allowed usages/features of the
VkImage.
The current use case we have for images coming into skia that have
these modifiers are from video decoders. All these images are only used
as sampled images in draws and they have no filtering applied.
Therefore, for now, instead of tracking all format and modifier pairs
to know what is supported, we internally set these images to be external
which already restricts their use to basic in shader sampling.
Additionally in chrome, all these images are coming in ycbcr conversions
already which we treat as external.
Bug: skia:12336
Change-Id: I59a564f937f49a6d906efe954b24cebe5c7470ff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441298
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Bug: skia:12466
Change-Id: Ie4c1973427e96d84b77f28edb5a4d6f1d53ef8aa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463316
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
These are not very interesting right now, because the in and out types
boil down to the same thing (int/int, float/float). When half-
precision types are enabled, these helpers will be more useful. They
will return an array which casts each element from int-to-short or
float-to-half (or vice versa).
Change-Id: Ida716ddd27d370ba33fd23f17a1b07fa5a201e40
Bug: skia:12339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463337
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
* Keep RenderPass-related routines together
* Rename setRenderPipeline to bindRenderPipeline
* Pass resources as const& to platform-specific routines, to avoid
copies.
Bug: skia:12466
Change-Id: I4c3ea006263ace4a050b6c86f70163f5fd5e254a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463336
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
The inverse, outerProduct and matrixCompMult polyfill functions in Metal
were written assuming that all float matrices would use the `float`
type. They now use a template so that `half` matrices will work too.
Change-Id: I7696c8ad1e4aaffbd71c56b9245485e74cd96c5a
Bug: skia:12339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463338
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:12466
Change-Id: If90c17634de9caa2ac3a928187226339523d53eb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/462879
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
This class can now be shared with Graphite.
Bug: skia:12524
Change-Id: I7841410b3e8e111a12298efe0a1898a33295873a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/462556
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Also adds test of texture copyback to CommandBufferTest, and
endEncoding() to BlitCommandEncoder.
Bug: skia:12466
Change-Id: I57e4646aacc1d950cc16b70d7feeced1a04e6867
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/462078
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Bug: skia:12466
Change-Id: I5c0c919d4523258a11b4545346093953a4f09c7e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/461597
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This helps remove all Ganesh dependencies from GrVertexWriter so it can
be shared with Graphite.
Bug: skia:12466
Change-Id: I1ede1cfc48d501cb1e028bca1e8590e6c92f4e4d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/462316
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
At this point, it seems like this was a mis-diagnosis of the underlying
issue around dual-source blending (and its interaction with other blend
state).
Change-Id: I11af0c9b70c32e14c353848db3d6adbfe5f08225
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/462176
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
After reporting the error, we convert the reserved word to an identifier
as this led to the best error reporting. (This avoids double error
reporting or strange cascading errors.)
Change-Id: I67209bc342fe794287baeaaaf34fa77afd4ac26b
Bug: skia:12560
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/462096
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:12559
Change-Id: I91e0b46702ca361737459d64d1cdb57f8dd2f64e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/461697
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12466
Change-Id: I7eb240985ce0c267bbc7be040e0f812407d31435
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/461699
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Auto-Submit: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This class is only meant to be used with Wang's formulas, so it
belongs in their name namespace.
Bug: skia:12524
Change-Id: Ib1667781b947a7f052af103b7bf9084d09a690de
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/461567
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Tessellation code can just exist in the skgpu namespace.
Bug: skia:12524
Change-Id: I44d473c5f82d608bb6cd3bb9df239c4f4028da4f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/461557
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This lets clients access and use the same RGB to YUV matrices that we
use internally.
Inspired by https://crrev.com/c/3223983
Bug: skia:12545
Change-Id: I7f70f56e721819c3d33da447eff2e572b7774c51
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/461476
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Bug: skia:12466
Change-Id: I2e0421191d05c3e0f457425e0dab49c2afb9c2b9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/461477
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Auto-Submit: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Switches Shape to store kRect types as skgpu::Rect instead of SkRect,
although it can be constructed or set from an SkRect. It returns a Rect
from rect() and bounds() now.
Also switches out the line to be constructed from float2's, and re-use
the fRect storage to store the first segment vertex in topLeft and
the second vertex in botRight.
Bug: skia:12466
Change-Id: I1032fb6f222b0853305186c0e34091b014ae0cea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/460577
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Implements a BSP tree with NEON/SSE optimizations that tracks
non-overlapping regions. This object can be used batch sets of paths
into non-overlapping draws. Performance on AppleM1 with our existing
benchmarks looks very promising:
desk_motionmarkarcs.skp 1227 paths -> 69 draws 450us
desk_motionmarksuits.skp 1417 paths -> 26 draws 201us
desk_chalkboard.skp 1940 paths -> 11 draws 84us
desk_ynevsvg.skp 859 paths -> 10 draws 31us
desk_micrographygirl.skp 318 paths -> 29 draws 11us
Bug: skia:12466
Change-Id: I847a93ed726dea10cb403cb76e578bd81eb920d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/460298
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Bug: skia:12466
Change-Id: I89d41509b8158aabac5250f035115954e8ae7f4b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/460655
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Bug: skia:12466
Change-Id: Ibb99647cf151fb45901aee340d60914611c2ba75
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/461096
Auto-Submit: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Bug: skia:12466
Change-Id: Ibbb4894c6a46dd8853909214da85314f9abc3959
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459880
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This completes the long process of moving all of IRGenerator's code and
data into better homes and finally kills the class altogether. There is
a lot of #include churn here due to poor header hygiene in the past;
IRGenerator.h included a ton of stuff and almost everything included
IRGenerator, allowing us to inadvertently be sloppy with our includes.
Change-Id: I70d854e57dec7bd9a311b72f9f72d978d354da98
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459936
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:12466
Change-Id: I69222b60d421d8deebcb977124ee0f5cb06f34dd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/460638
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Adds test of command buffer submission with GPU recording (if enabled).
Also adds programmatic GPU recording support.
Bug: skia:12466
Change-Id: I85aba9f7e5e09f62f2ddc7040d1ff84bb9b4ae09
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/460337
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Change-Id: I894bfa01e7cf58f140423554d0200b6c66beef35
Bug: oss-fuzz:39998
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459883
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is supported in GLSL ES3. (Strangely, vector operator! isn't.)
Previously, this was flagged as an error: http://review.skia.org/459885
Change-Id: I2c4299159fff58fefe8bd131c8d317cd82974a62
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459886
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>
At present, we only detect four errors here. We should detect six.
Change-Id: I226854ab930a273695c42cf2f7bdb1d5cd97e50b
Bug: oss-fuzz:39998
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459882
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>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
See http://review.skia.org/460037 for an example of the existing
behavior. Const variables are constant-expressions and should be allowed
here.
Change-Id: I41383d79668785f270b7825485e9f6fa56c553c1
Bug: skia:12549
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/460036
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>
Transform.h is named Transform_graphite.h to avoid collision with
Transform.h in skottie/src
Bug: skia:12466
Change-Id: I017eee2bffe1a0aad605221e67540f732c2ee6b9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459719
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
A handful of older drivers choke on weird matrix construction layouts.
The tests have been disabled on those bots. The GLSL docs and
conformance tests make it clear that such ctors were intended to work,
even if the drivers don't necessarily handle them properly.
Change-Id: Id9d4bb541482fd08344e78087286d8e829e7ff6b
Bug: skia:12443
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459559
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
We are making src/gpu/tessellate independent of v1, and more represent
math and types that just know how to tessellate, so this seems like the
right place again for this file.
Bug: skia:12524
Change-Id: I773de3c0f93a57ce7ee16b44480667013f32ebd5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459136
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
We now detect attribute, varying, precision and invariant as reserved.
Change-Id: I8c90655a70b1bad31bf6143c3fdcb2ce582320b1
Bug: skia:12484
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459479
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
`samplerCube` is a type which we don't support at all. It has been added
to the reserved-word list.
`textureCube` was in our list of built-in types, but was not actually
used in any way; it wasn't actually added to the root or private symbol
tables, and was totally unreferenced by the code. It's been deleted.
Change-Id: I4f79ce5d40ac6ebdb2a7067fa60cc79e316b01b6
Bug: skia:12484
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459123
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This CL adds a few more exceptions to our ES2 test import, and adds the
dm code which actually runs the tests.
Change-Id: If6691dd35931f4f10262d3a1eff020c2c347ca59
Bug: skia:12484
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459124
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This reverts commit eb68973c2f.
Reason for revert: ES2 conformance test checks this
Original change's description:
> Disallow matrix ctors which overflow a column.
>
> The GLSL spec allows matrix constructors containing vectors that would
> split between multiple columns of the matrix. However, in practice, this
> does not actually work well on a lot of GPUs!
>
> - "cast not allowed", "internal error":
> Tegra 3
> Quadro P400
> GTX 660
> GTX 960
> - Compiles, but generates wrong result:
> RadeonR9M470X
> RadeonHD7770
>
> Since this isn't a pattern we expect to see in user code, we now report
> it as an error at compile time. mat2(vec4) is treated as an exceptional
> case and still allowed.
>
> Change-Id: Id6925984a2d1ec948aec4defcc790a197a96cf86
> Bug: skia:12443
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449518
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:12443
Change-Id: I5a32744c88b9b830ad657488824c8c7dd0b0a652
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458056
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Previously, in public code, private types didn't exist anywhere in the
symbol table chain, and those names were free for the taking. Now, we
register them as invalid types in the public symbol table. This prevents
them from being used as variable names, and gives a more explicit error
if you try to use them as a type.
Change-Id: I9a943bf923639b72cbf36b1acf4b4fbe70982786
Bug: skia:12538
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459119
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>
Change-Id: Ie853618fb496a77ffb79d6669f87048260df68b7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458979
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This is just a bunch of renaming, and for now the tessellate/shaders
directory remains untouched. The next steps will be to clean up and
remove all Ganesh v1 dependencies from tessellation.
Bug: skia:12524
Change-Id: I8cc166c0c78f9fb160de807131fa53fcc0765818
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458876
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
With this CL we can run as:
dm --src gm skp tests --config grmtl -v --nocpu --nogpu
and not get all the non-Graphite unit tests.
Bug: skia:12466
Change-Id: Ib3f04f315fe4b5731a54e4c72979a0c1e00baf24
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/457898
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
GLSL treats builtin types and user-defined types differently; `int` and
`float` are keywords and cannot be used to name variables. However, it's
fine for a user type like `struct xyz` to be hidden by a variable
`int xyz` or even `xyz xyz` (i.e., a variable of type `struct xyz` named
`xyz`).
We now honor that distinction and include tests for it. This will fix
several ES2 conformance tests (local_struct_variable_hides_struct_type,
local_int_variable_hides_struct_type, etc.).
Change-Id: I7a45c70707087f9f355ce5b06b032fed16683f3e
Bug: skia:12527
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458721
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This fixes GLSL ES2 conformance test `array`.
Change-Id: I6ebee9253e1e8c394d9ddb6899e3a0940b7a38ef
Bug: skia:12495
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458718
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This is a reland of 35a74eab5d
Added guard for SKNX_NO_SIMD. I guess they don't want speedy goodness.
Original change's description:
> add a scaled uint32x4_t divided by uint32_t to SkVx
>
> This extracts the divide used in SkImageBlurFilter.cpp, and
> encapsulates it into ScaledDividerU32. It generates results that
> are with in +/- 1 of the rounded answer generated by doubles.
>
> I have added hand coded implementations for sse and for neon to
> hopefully to avoid code generation problems.
>
> Bug: skia:12522
>
> Change-Id: Ia7372d45895c799f69f8c0fd9fdea5efac321139
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458216
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
Bug: skia:12522
Change-Id: I9833a98f159827f483147c8155f1b92b7a7130ed
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458716
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Implements a Rect type whose standard operations use float4. The intent
of this class is to make calculations on draw bounds as fast as
possible.
Change-Id: I3bdb219b242bb7097809507c345b613670ff386e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/457136
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This reverts commit 35a74eab5d.
Reason for revert: Breaks Google3
Original change's description:
> add a scaled uint32x4_t divided by uint32_t to SkVx
>
> This extracts the divide used in SkImageBlurFilter.cpp, and
> encapsulates it into ScaledDividerU32. It generates results that
> are with in +/- 1 of the rounded answer generated by doubles.
>
> I have added hand coded implementations for sse and for neon to
> hopefully to avoid code generation problems.
>
> Bug: skia:12522
>
> Change-Id: Ia7372d45895c799f69f8c0fd9fdea5efac321139
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458216
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
Bug: skia:12522
Change-Id: Id5d6968c813322dfc68e549e2f3afea7da9a0e18
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458258
Auto-Submit: Herb Derby <herb@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Just having an skpu namespace should be enough.
Change-Id: I3026ece828aee7a57f8c5de2252ebc79d97baf7a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458636
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This extracts the divide used in SkImageBlurFilter.cpp, and
encapsulates it into ScaledDividerU32. It generates results that
are with in +/- 1 of the rounded answer generated by doubles.
I have added hand coded implementations for sse and for neon to
hopefully to avoid code generation problems.
Bug: skia:12522
Change-Id: Ia7372d45895c799f69f8c0fd9fdea5efac321139
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458216
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
SkSL treated these two functions as distinct, even though they are not:
void func(in float x);
void func(float x);
The `in` modifier on a function parameter is the default state, making
these two prototypes functionally identical. We now strip off an `in`
modifier on a function definition. This gives us three potential states
for each param: nothing (meaning `in`), `out`, and `inout`.
Change-Id: Id2acb53ecaca98f86a7f6a83e0b9a375f9abe2b8
Bug: skia:12525
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458257
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>
Bug: skia:12515
Change-Id: I8db3501c129d93fc1eb822c90840119a7a7f2b4b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/457478
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This is in prep for compiling with -std=c++14 and -Wno-c++17-extensions
when building with clang. Chrome has encountered problems with
third_party headers that are included both in Skia and other Chrome
sources that produce different code based on whether preprocessor macros
indicate a C++14 or C++17 compilation.
In C++17 they are already inline implicitly. When compiling with C++14
we can get linker errors unless they're explicitly inlined or defined
outside the class. With -Wno-c++17-extensions we can explicitly inline
them in the C++14 build because the warning that would be generated
about using a C++17 language extension is suppressed.
We cannot do this in public headers because we support compiling with
C++14 without suppressing the C++17 language extension warnings.
Bug: chromium:1257145
Change-Id: Iaf5f4c62a398f98dd4ca9b7dfb86f2d5cab21d66
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/457498
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:12466
Change-Id: I401a185d818a964327d323b9ebcd0850ec0b1c9b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/457318
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This reverts commit 01b02956c7.
Reason for revert: Codegen regressions
Original change's description:
> Add convenient "xyzw" accessors and swizzles to skvx
>
> Change-Id: Ic300285d10679a4e34190ab7b6b08bd1f6d80330
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/454309
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
Bug: skia:12515
Change-Id: Id853e4d9e25c6d2ae622668ef064e1b2b078b824
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/457476
Auto-Submit: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Change-Id: Ic300285d10679a4e34190ab7b6b08bd1f6d80330
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/454309
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
* Rename TFlagsMask<> to Mask<>
* Make Mask<> implicitly convertible to bool
* Rename SKGPU_MAKE_BITFIELD_CLASS_OPS to SKGPU_MAKE_MASK_OPS
* Remove the macro for non-class enums
Change-Id: I86059335d731f34c9c9c20a4cd6d5491543aa3b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/456816
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
As DSLWriter evolved, it ended up performing two unrelated duties:
1. Holding per-thread state which is referenced throughout SkSL
(including non-DSL code)
2. DSL-specific utility functions
There's no reason for those to be contained in the same class, and
there's also no reason the per-thread state should be held in a
DSL-specific class. This breaks out an SkSL::ThreadContext class to hold
the state info, leaving the DSL utility functions behind in DSLWriter
(which should perhaps be renamed, but that's a job for a future CL).
Change-Id: Iccd45314bd9b37d4a1d3e27920e32a50c0b07d7c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/456736
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
The fuzzer has found that it can get timeouts in SkVM by nesting loops
very very deeply, then at the bottom of the chain, making an inside-out
loop that runs for zero iterations. This has a calculated unrolled-size
of zero, but SkVM would still think hard about unrolling the (ultimately
empty) outer loops.
SkSL now optimizes away unrollable loops that run for zero iteratinons,
as well as empty unrollable loops. This should eliminate the fuzzer's
troublesome construct entirely.
Change-Id: Ic3ef7b7a6a9fc7ee7fb13eb7bd7f34c9bff57448
Bug: oss-fuzz:39661
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/456469
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 will allow a uniform array to be used in make_unrolled_colorizer
(or anywhere else that we might need one).
Change-Id: I9246f8d3121353b029fd36924fb874d838e67d9c
Bug: skia:8401
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/456238
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The fuzzer has been poking various holes in DSL by intentionally
creating illegal types (e.g. private or not ES2-compatible), then
finding ways to use those types, e.g. constructors or swizzles.
Previously we were mitigating those by calling `reportIllegalTypes` at
the locations where the type was used. Now, we detect the illegal type
usage at the source, and return a poison DSLType. This prevents the
illegal type from leaking out at all, and stops the problem at its
source. It also allows us to remove calls to `reportIllegalTypes`
sprinkled through the code, as those are now redundant.
Change-Id: Id50b50f72849111d80f76e4fdc2cb6094d3009bd
Bug: oss-fuzz:39597
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/455999
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
These weren't used anywhere in our test suite.
Change-Id: I35e8607ad2dbddf8f403668bd2b2636a8964d304
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/455777
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
`Type::isPrivate` now works properly even on structs and arrays, so we
don't need two separate methods anymore.
Change-Id: Ic3e16e1315ebb0c8cec575f109af7e472a11ac8c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/455660
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 is a reland of 36f53ec7e1
Original change's description:
> Disallow constructors of ES3 types in ES2 code.
>
> The fuzzer found that we constructed TypeReferences without first
> checking for disallowed tyoes. (In fact, TypeReference creation had no
> error checking at all; it didn't even have Convert/Make functions.)
>
> Added proper Convert/Make to TypeReference, and used those calls to
> report errors or cause assertions if trying to make a TypeReference to a
> type that the program did not support.
>
> (While tracking down this bug, I added strict-ES2 type assertions to our
> constructor IR nodes as well. This helped pinpoint the error and seem
> reasonable to leave in, just in case.)
>
> Change-Id: I896b68ae9d3d9e1f30d7eba9fa594617ab851c74
> Bug: oss-fuzz:39540
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/455498
> 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>
Bug: oss-fuzz:39540
Change-Id: Id8e323c22b18726214613b6061c08873048b7c69
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/455617
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This turns out to work fine, but we didn't cover it in any test case.
Change-Id: I98c40dc023bc9f0739beeb6e4163cde087a0be99
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/455499
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
