Commit Graph

94 Commits

Author SHA1 Message Date
Brian Osman
20fad32064 SkSL: Hide most integral types from runtime effects
'int' is the only integral type that exists in GLSL ES 1.0 (and it's not
really guaranteed to be an integer). This enforces the same restriction
on runtime effects - no unsigned integers, and no short or byte types.

Bug: skia:11093
Change-Id: I938f1e0e125dc8347507f428b46b51c66033c752
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347046
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-12-23 18:28:17 +00:00
Brian Osman
c0f2b64342 SkSL: Hide non-square matrices from runtime effects
These don't exist in our minimum spec (GLSL ES 1.0)

Bug: skia:11093
Change-Id: Ia2d871199fff2a98dcd517c1eebe46decb0c2dfb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346657
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-12-22 19:31:42 +00:00
Brian Osman
d6f2338ab1 SkSL IR normalization: Convert while loops to for loops
Bug: skia:11095
Change-Id: Icd69df40675e5ecde5004e04a7dcd78eedf8343c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344765
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2020-12-16 23:03:23 +00:00
Brian Osman
ff44584b47 SkSL: Disallow '%' and '%=' on non-integral types
Bug: skia:11072
Change-Id: Ic24e40bfea5bf1d2d14c0f681632228a5ecc7104
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342929
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
2020-12-10 22:19:49 +00:00
John Stiles
1b27c3d7a3 Check array bounds when a constant array index is used.
This sort of error would be detected by most backend compilers. This
case was also detected by the bytecode generator. It's easy for us to do
a similar check during SkSL IR generation and report the error sooner.

Also, `convertIndex` had migrated a few hundred lines away from
`convertIndexExpression`, so I moved it back to live next to its parent.

Change-Id: I715d3abf42581782b55ba60df30d0296355667d4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341377
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-12-07 18:17:30 +00:00
John Stiles
076e9a2f34 Disallow returning array types in SkSL.
This is illegal in older versions of GLSL and in Metal. We now fail at
SkSL compilation time and properly report the error.

Change-Id: I6ddaeabff5386a1ed6ca3eb8703a6035476ec77a
Bug: skia:11021
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339298
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-12-03 16:27:39 +00:00
Brian Osman
0006ad01ce Stop cloning builtin functions
Previously, any builtin functions would be optimized as a side-effect of
optimizing programs that used them. Now that shared elements aren't
being optimized in that way, we explicitly optimize any shared modules
when they are first created. We don't remove dead elements, but we
we do substitute settings, simplify, and inline.

Bug: skia:10905
Change-Id: I701b5e9f52fb880ef3e6f4c67694d08602f47e95
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336440
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-11-20 15:02:54 +00:00
Brian Osman
1f71f433ff Always enable SkSL's ByteCodeGenerator, disable interpreter in Google3
The ByteCodeGenerator is needed for SkSL-via-skvm, but almost no one
needs the ByteCode interpreter.

Bug: b/172773885
Change-Id: Ia7b6768dbc00c6c78b971ba50f0b702536bbd5b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336016
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-11-18 21:30:45 +00:00
Brian Osman
d7e7659cad Move GrShaderCaps from Program::Settings to Compiler
This ties the caps to the compiler instance, paving the way for
pre-optimizing the shared code. Most of the time, the compiler is
created and owned the GPU instance, so this is fine. For runtime
effects, we now use the shared (device-agnostic) compiler instance
for the first compile, even on GPU. It's configured with caps that
apply no workarounds. We pass the user's SkSL to the backend as
cleanly as possible, and then apply any workarounds once it's part
of the full program.

Bug: skia:10905
Bug: skia:10868
Change-Id: Ifcf8d7ebda5d43ad8e180f06700a261811da83de
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331493
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-11-04 19:38:33 +00:00
Mike Klein
51dc28505f allow early returns
For posterity, here's my initial, wrong thinking:

    If we squint at "return foo" and read it as "result = foo; goto
    end_of_program", and then remember we can always skip forward jumps (and
    where's further forward than end of program?), early returns turn out to
    work just like a store.

The reason this is wrong is that by the time we reach a final return,
the entire mask stack has been popped back down to its original default
ffffffff (active) state.  But that return shouldn't override any prior
returns.  So that scheme isn't quite right.

Instead we accumulate the result by disabling updates to lanes that have
already returned.  By the time we're done, all lanes should have hit
_some_ active return, now asserted.

Bug: skia:10852
Change-Id: I27b05f04a60ff4a5f2fe5f59bf398c3f7224a41b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/327457
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-10-20 14:33:31 +00:00
Brian Osman
401a366eb1 Fix swizzle-of-swizzle lvalues in ByteCodeGenerator
Bug: skia:10785
Change-Id: I01708af63d7e2ffc160022074ea9ff2b3c69eab5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320638
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-09-29 20:34:58 +00:00
Ethan Nicholas
7bd6043029 moved SkSL Block's data into IRNode
Change-Id: Id1066c7c2aeffce47fcd6823cb01a58b52e450c2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319676
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-09-25 19:33:19 +00:00
Brian Osman
32d53550c8 Remove unsafe compiler methods related to external values
We don't want to be polluting the global namespace with external values,
especially when the typical/recommended way to use the Compiler is with
a single long-lived instance. Force client code to manage ownership (the
only non-unit-test case was already doing this), and pass external
values to convertProgram, so they can be added to the Program's symbol
table.

Change-Id: If4c1db5e48a62e2cf4333b8d80420f2dfede27ab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319125
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-09-24 14:12:08 +00:00
John Stiles
759880ab05 Pass non-temporary Program::Settings to convertProgram.
This will fix ASAN errors such as these:
https://logs.chromium.org/logs/skia/4e924a3651708511/+/steps/symbolized_dm/0/stdout

Change-Id: I49756df6e077cc8813f51f70082c88d861fa337f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316451
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2020-09-11 16:37:34 +00:00
John Stiles
7571f9e490 Replace 'typedef xxxxx INHERITED' with 'using INHERITED = xxxx;'.
Mechanically updated via Xcode "Replace Regular Expression":

  typedef (.*) INHERITED;
    -->
  using INHERITED = $1;

The ClangTidy approach generated an even larger CL which would have
required a significant amount of hand-tweaking to be usable.

Change-Id: I671dc9d9efdf6d60151325c8d4d13fad7e10a15b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314999
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-03 03:41:26 +00:00
John Stiles
534d79979d Fix constness of ExternalValue* inside ExternalValueReference.
Objects in the symbol table are intentionally constant. However, when
converting "ExternalValue" AST nodes into symbols via symbol table
lookup, IRGenerator::convertIdentifier was casting away constness
because ExternalValueReference held a non-const pointer. Fixing this
involved significant ripple-effect additions of "const" throughout the
ExternalValue class and its subclasses.

These changes generally appear to be benign, but one interesting edge
case is `ExternalValue::write`, which intuitively does not seem to make
sense as a const method. However, invoking `write` should not alter the
ExternalValue object itself; rather, it is intended to alter the
*external value* that is being referenced. (In practice, nothing invokes
write() anyway except for one unit test, which continues to pass.)

This issue was discovered while converting casts to `as<T>()` calls.

Change-Id: I8ff6a477e475833d2a99c72f1c79c766b57767ee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311276
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-08-18 13:44:05 +00:00
John Stiles
c1c3c6d70d Enable ClangTidy flag bugprone-suspicious-string-compare.
https://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-string-compare.html

Find suspicious usage of runtime string comparison functions.
This check is valid in C and C++.

Checks for calls with implicit comparator and proposed to
explicitly add it:

  if (strcmp(...))       // Implicitly compare to zero
  if (!strcmp(...))      // Won't warn
  if (strcmp(...) != 0)  // Won't warn

Checks that compare function results (i,e, strcmp) are compared to valid
constant. The resulting value is

  <  0    when lower than,
  >  0    when greater than,
  == 0    when equals.

A common mistake is to compare the result to 1 or -1:

  if (strcmp(...) == -1)  // Incorrect usage of the returned value.

Additionally, the check warns if the results value is implicitly cast
to a suspicious non-integer type. It’s happening when the returned
value is used in a wrong context:

  if (strcmp(...) < 0.)  // Incorrect usage of the returned value.

Change-Id: I001b88d06cc4f3eb5846103885be675f9b78e126
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310761
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2020-08-16 03:54:08 +00:00
Ethan Nicholas
e8ad02c6e9 Revert "Reland "Revert "Omit dead SkSL functions"""
This reverts commit a15f2bfb6f.

TBR=bsalomon@google.com, brianosman@google.com

Change-Id: I64cc7830281ee69bf3377c5b6daa3d96ccf4769e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/293976
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2020-06-04 02:46:55 +00:00
Ethan Nicholas
a15f2bfb6f Reland "Revert "Omit dead SkSL functions""
This reverts commit fd1173ac71.

Reason for revert: TSAN failure: https://chromium-swarm.appspot.com/task?id=4c93823229fa6a10

Original change's description:
> Revert "Revert "Omit dead SkSL functions""
> 
> This reverts commit 7c969f26bc.
> 
> Change-Id: I8fb99f271e2ecaeb83d570cc2d2cf8851bc776f0
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/293338
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

TBR=bsalomon@google.com,brianosman@google.com,ethannicholas@google.com

# Not skipping CQ checks because this is a reland.

Change-Id: Iab3ee5a336074676ebd0b761922ececf189752ef
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/293843
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2020-06-03 17:14:18 +00:00
Ethan Nicholas
fd1173ac71 Revert "Revert "Omit dead SkSL functions""
This reverts commit 7c969f26bc.

Change-Id: I8fb99f271e2ecaeb83d570cc2d2cf8851bc776f0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/293338
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2020-06-03 14:22:18 +00:00
Ethan Nicholas
7c969f26bc Revert "Omit dead SkSL functions"
This reverts commit 97fe0cbed2.

Reason for revert: ASAN failures

Original change's description:
> Omit dead SkSL functions
> 
> Now that SkSL inlines functions, dead functions are very common. This
> change causes them to be omitted from the final output.
> 
> Change-Id: Ie466a3f748812eff1a368498365c89d73ab0b7be
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/292684
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

TBR=bsalomon@google.com,brianosman@google.com,ethannicholas@google.com

Change-Id: Id20c5be67dd574d30d6f978ba610e43aa5018416
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/293241
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2020-06-01 15:00:00 +00:00
Ethan Nicholas
97fe0cbed2 Omit dead SkSL functions
Now that SkSL inlines functions, dead functions are very common. This
change causes them to be omitted from the final output.

Change-Id: Ie466a3f748812eff1a368498365c89d73ab0b7be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/292684
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2020-06-01 14:27:38 +00:00
Mike Reed
3ef77ddf9e clean up public m44 and camera api
saveCamera() is no longer experimental

In a separate CL, will stage changes to concat virtual to take M44.

Change-Id: Iaf37ce2f24ab1223c54aeb1e79eaebf18f87fece
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/281589
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Reed <reed@google.com>
2020-04-06 15:34:17 +00:00
Brian Osman
b08cc024cd Switch back to stack-based SkSL interpreter
It's slower, but code size is quite a bit smaller, memory usage is
smaller, and we think that mapping it to SkVM is just as easy.

This effectively reverts all of the following commits:

"Fix gcc9 warning around size of memset."
https://skia-review.googlesource.com/c/skia/+/279861

"Remove unused (and misleading) 'instruction' from SkSLInterpreter.h"
https://skia-review.googlesource.com/c/skia/+/278177

"Interpreter: Fix intrinsics when called with vector types"
https://skia-review.googlesource.com/c/skia/+/272721

"Make it easier to add vector versions of byte code instructions"
https://skia-review.googlesource.com/c/skia/+/272527

"Interpreter: Support returns from runStriped"
https://skia-review.googlesource.com/c/skia/+/268941

"add SkSLInterpreter vector instructions"
https://skia-review.googlesource.com/c/skia/+/266560

"Fix crash when editing particle scripts"
https://skia-review.googlesource.com/c/skia/+/269487

"Revert "Revert "Complete rewrite of the SkSL interpreter"""
https://skia-review.googlesource.com/c/skia/+/266205

Change-Id: I4258596399c4ca94489d4faf8aacfec88afeee13
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/281205
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-04-03 17:20:04 +00:00
Brian Osman
2e19af063b Revert "Move interpreter disassemble to out-of-line member of ByteCode"
This reverts commit 1b0124fec6.

Reason for revert: Check generated files bot is unhappy.

Original change's description:
> Move interpreter disassemble to out-of-line member of ByteCode
> 
> Now it returns a string (rather than just calling printf).
> 
> Adds GUI view of particle effect byte code (for fun), and fixes the
> unit tests that called ByteCodeFunction::disassemble, which wasn't
> doing anything.
> 
> Change-Id: Ide3fd933cf14832feae7ff9e0fdc1ae8f24a28d4
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/273878
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>

TBR=brianosman@google.com,ethannicholas@google.com

Change-Id: I478a93769a3e1a72a339853d6d41865dba8bbe66
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/275800
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-03-07 17:44:05 +00:00
Brian Osman
1b0124fec6 Move interpreter disassemble to out-of-line member of ByteCode
Now it returns a string (rather than just calling printf).

Adds GUI view of particle effect byte code (for fun), and fixes the
unit tests that called ByteCodeFunction::disassemble, which wasn't
doing anything.

Change-Id: Ide3fd933cf14832feae7ff9e0fdc1ae8f24a28d4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/273878
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-03-06 22:16:11 +00:00
Brian Osman
15c98cbfef Revert "Add most important intrinsics to the interpreter"
This reverts commit 838007f1ff.

Reason for revert: Significant RAM regression detected by Chrome.

Original change's description:
> Add most important intrinsics to the interpreter
> 
> Started with component-wise comparison and mix builtins.
> 
> Implemented min, max, clamp, and saturate using those.
> Moved dot to SkSL as well. Because we now have builtins
> implemented using other (intrinsic) builtins, I had to
> split the include file in two - this lets the intrinsics
> be marked so we can call them from the second phase of
> builtins that are written in SkSL.
> 
> Given that the comparisons and kSelect are now used by
> these, I added vector versions of those instructions.
> I also switched the kSelect args to match GLSL mix(),
> mostly so the logic of mapping intrinsic arguments to
> instruction register args remains simple.
> 
> Inspired by the (never-landed):
> https://skia-review.googlesource.com/c/skia/+/230739
> 
> Change-Id: Iecb0a7e8dc633625ff2cada7fb962bf2137fa881
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/272516
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>

TBR=bsalomon@google.com,brianosman@google.com,ethannicholas@google.com,reed@google.com

Change-Id: I931a0ccc254b55339c9b077543a0daaf28146b19
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/273800
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-02-27 18:37:15 +00:00
Brian Osman
838007f1ff Add most important intrinsics to the interpreter
Started with component-wise comparison and mix builtins.

Implemented min, max, clamp, and saturate using those.
Moved dot to SkSL as well. Because we now have builtins
implemented using other (intrinsic) builtins, I had to
split the include file in two - this lets the intrinsics
be marked so we can call them from the second phase of
builtins that are written in SkSL.

Given that the comparisons and kSelect are now used by
these, I added vector versions of those instructions.
I also switched the kSelect args to match GLSL mix(),
mostly so the logic of mapping intrinsic arguments to
instruction register args remains simple.

Inspired by the (never-landed):
https://skia-review.googlesource.com/c/skia/+/230739

Change-Id: Iecb0a7e8dc633625ff2cada7fb962bf2137fa881
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/272516
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-02-26 20:27:36 +00:00
Brian Osman
dc2a97774b Interpreter: Fix intrinsics when called with vector types
Change-Id: I0296ff5a68b934f6bc7152a66f57ef045fc94daf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/272721
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-02-26 14:23:45 +00:00
Mike Reed
46f5c5f08b Make SkM44 public
Need to migrate clients from private/ to core/ include
Unexperimentalize concat44() methods on SkCanvas

Change-Id: I64b8816722a9d93316cb8b8691d2d9a3e36f167f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/272464
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Mike Reed <reed@google.com>
2020-02-21 15:50:31 +00:00
Brian Osman
8c80b19936 Interpreter: Support returns from runStriped
Change-Id: Id84c3fb35cb61fa839691471d03a44152964bedb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/268941
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-02-08 02:09:04 +00:00
Ethan Nicholas
b962eff76c Revert "Revert "Complete rewrite of the SkSL interpreter""
This reverts commit 99c54f0290.

Change-Id: I010ac4fdb6c5b6bfbdf63f4dcac5dbf962b0ad9c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/266205
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2020-01-24 14:42:37 +00:00
Ben Wagner
470e0ac14a Revert "Revert "Revert "Complete rewrite of the SkSL interpreter"""
This reverts commit 7deb1c26ba.

Revert "maybe fixed?"

This reverts commit 7ad3f229c7.

Revert "removed extraneous change"

This reverts commit 682f299aa8.

Revert "test change"

This reverts commit 5f40986cef.

Revert "derp"

This reverts commit 4f830b8df3.

Revert "let's see what happens"

This reverts commit d5290563f0.

Change-Id: Ib3c13c2a6ade9fc42382509d036e212c7fe50cc6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/265979
Reviewed-by: Ben Wagner aka dogben <benjaminwagner@google.com>
2020-01-22 21:59:48 +00:00
Ethan Nicholas
5f40986cef test change 2020-01-22 16:45:31 -05:00
Ethan Nicholas
7deb1c26ba Revert "Revert "Complete rewrite of the SkSL interpreter""
This reverts commit 99c54f0290.
2020-01-22 16:44:31 -05:00
Mike Reed
b26b4e7340 Revert "Revert "use SkM44 internally""
This reverts commit f79aacba2b.

Fix: had transposed when converting from colormatrix to m44

Change-Id: I6bc81d0c50bb1bf8d771e4dfa0c25c39de265b1e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/265765
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
2020-01-22 20:19:16 +00:00
Mike Reed
f79aacba2b Revert "use SkM44 internally"
This reverts commit 295cdf8775.

Reason for revert: wacky gm colors, must have busted colormatrixfilter

Original change's description:
> use SkM44 internally
> 
> Today we use SkM44 in canvas, and SkMatrix44 in sksl (and colormatrix).
> This CL tries to move core to use a single type.
> 
> SkMatrix44 has callers in android and chrome, is loaded with double/float
> variants in its API. I am suggesting moving to a private, clean-start,
> API for internal use. SkM44 is much faster, and has a leaner API.
> 
> If eventually we can migrate clients to a shared impl/api, great. For now,
> I want to have one we are free to optimize/use as we see fit without
> worrying about changing client results.
> 
> Change-Id: Id782ac1cc8b8d7f6621970e44e1f9729964d2a94
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/265299
> Commit-Queue: Mike Reed <reed@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>

TBR=mtklein@google.com,bsalomon@google.com,brianosman@google.com,fmalita@chromium.org,reed@google.com

Change-Id: I35fcd636f1b57001bb65684e78523b6a94cfebee
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/265764
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
2020-01-22 18:05:34 +00:00
Mike Reed
295cdf8775 use SkM44 internally
Today we use SkM44 in canvas, and SkMatrix44 in sksl (and colormatrix).
This CL tries to move core to use a single type.

SkMatrix44 has callers in android and chrome, is loaded with double/float
variants in its API. I am suggesting moving to a private, clean-start,
API for internal use. SkM44 is much faster, and has a leaner API.

If eventually we can migrate clients to a shared impl/api, great. For now,
I want to have one we are free to optimize/use as we see fit without
worrying about changing client results.

Change-Id: Id782ac1cc8b8d7f6621970e44e1f9729964d2a94
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/265299
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
2020-01-22 17:39:15 +00:00
Mike Reed
55f6fc3064 IWYU -- colorspace no longer includes matrix44
Change-Id: I659552466940b76a339caaf124700303806fd082
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/265456
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-01-21 21:37:32 +00:00
Ethan Nicholas
99c54f0290 Revert "Complete rewrite of the SkSL interpreter"
This reverts commit 2cde3a1320.

Reason for revert: breaking the Chrome roll

Original change's description:
> Complete rewrite of the SkSL interpreter
> 
> Change-Id: Idf4037b04c22f8ace5c1ef16c7a28d8c3df92e91
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/250817
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

TBR=brianosman@google.com,ethannicholas@google.com

Change-Id: If0fbc78118173e0cacbe1e01cabe3331e35aa49e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/265516
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2020-01-21 16:07:08 +00:00
Ethan Nicholas
2cde3a1320 Complete rewrite of the SkSL interpreter
Change-Id: Idf4037b04c22f8ace5c1ef16c7a28d8c3df92e91
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/250817
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-01-21 14:49:59 +00:00
Brian Osman
9b8b4558d2 Change ByteCode and ByteCodeFunction to classes
The lack of encapsulation was finally starting to bother me. Had to
change the Interpreter namespace to a struct so that it could be
friended, but otherwise this was a nice and simple cleanup.

Also updated the comments on the two run functions, and renamed
fInputSlots to fUniformSlots, to reflect recent clarification
around in vs. uniform.

Change-Id: I24bbc59778b3ab6448bffcf98133d5c149a060a9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/244883
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2019-09-30 18:02:24 +00:00
Brian Osman
b23d66e10a Interpreter: Lots of minor cleanup/refactoring
- Update the parameter lists to both run and runStriped so
  that they're in the same (sane) order, named consistently,
  and always take counts with pointer arguments.
- Add the same count-based safety checks to run that were
  already in runStriped.
- Remove the N parameter to run, it was only used to run
  things one-at-a-time (other than one spot in unit tests),
  and it simplifies the code quite a bit. If you want to run
  multiple times, use the striped version. I also moved that
  functions 'N' earlier in the parameter list, to make the
  pattern of the remaining parameters clearer.
- Remove an interpreter benchmark class that was never used.

Change-Id: Ibff0a47bdb2d29d095a0addd27e65ab13cb80fce
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/244716
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2019-09-27 15:02:16 +00:00
Ethan Nicholas
31cff27607 Revert "Revert "remove 'in uniform' support from GrSkSLFP, make rules more clear""
This reverts commit 85705c1b3b.

Change-Id: If189dafce53491728296a4292c76af55b05835ac
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/244509
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2019-09-26 18:11:45 +00:00
Ethan Nicholas
85705c1b3b Revert "remove 'in uniform' support from GrSkSLFP, make rules more clear"
This reverts commit ac18a5ca60.

Reason for revert: breaking Chrome roll

Original change's description:
> remove 'in uniform' support from GrSkSLFP, make rules more clear
> 
> Bug: skia:
> Change-Id: Iaa4d33c1bfb295d87343411ba6aacc8fae68ef9c
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/244300
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

TBR=brianosman@google.com,ethannicholas@google.com

Change-Id: I6e53f5197c751d961abfa21861b940d4168de213
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/244508
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2019-09-26 16:04:26 +00:00
Ethan Nicholas
ac18a5ca60 remove 'in uniform' support from GrSkSLFP, make rules more clear
Bug: skia:
Change-Id: Iaa4d33c1bfb295d87343411ba6aacc8fae68ef9c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/244300
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2019-09-26 14:05:10 +00:00
Brian Osman
73fb39c9b0 SkSL: Support bitwise negation on unsigned integers
Change-Id: I5558891882923b4e554a8b97a87da6bc4386b645
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/243817
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2019-09-24 20:50:43 +00:00
Ethan Nicholas
6f62412c92 Fixed integer divide / remainder in sksl interpreter
Our integer tests were broken: constant folding was optimizing away
the actual operations, so nothing was actually being tested. This
allowed us to not realize that integer divide / remainder didn't
work: vector division signals if any of divisor's lanes are zero,
and zeroes are common in masked-off lanes.

This replaces naive vector operations with loops and mask checks
for integer divide and remainder, and corrects the various broken
integer tests so they are actually doing stuff.

Change-Id: I6ffcad9e7b38a0bfd1604097f86d0faa24e1dbc7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/243698
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2019-09-24 17:35:42 +00:00
Brian Osman
4c2146f2ca Interpreter: Add bitwise shift operators
These only support fixed shift amounts (for now) ... because that's what
skvx supports ... because that's what various SIMD instructions support.

(We could loop over lanes, but the only need for this at the moment
is fine with constant shift).

Change-Id: I0e2c17084d019ff9b9a21476633adb59b3ce4bd6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/243656
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2019-09-24 14:56:36 +00:00
Brian Osman
e5bbce209c Interpreter: Add bitwise operators
Also, negation is unary, not binary.

Change-Id: Ic7c5a6f2ee0b2bbda89eef62999e4ebbc97dca12
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/243161
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2019-09-23 17:10:01 +00:00