This CL addresses the root cause of the fuzzer issue, by checking for
LayoutIsSupported before getting the MemoryLayout of a type. However,
this array ought to be detected as an error everywhere, as samplers are
opaque types; at present, this code compiles without error in GLSL and
Metal. This is an issue for followup CLs.
GLSL's actual support for arrays of samplers is interesting and probably
too nuanced for us to try to emulate:
https://www.khronos.org/opengl/wiki/Data_Type_(GLSL)#Opaque_arrays
"Under GLSL version 3.30, Sampler arrays (the only opaque type 3.30
provides) can be declared, but they can only be accessed by compile-time
integral Constant Expressions. So you cannot loop over an array of
samplers, no matter what the array initializer, offset and comparison
expressions are.
Under GLSL 4.00 and above, array indices leading to an opaque value can
be accessed by non-compile-time constants, but these index values must
be dynamically uniform. The value of those indices must be the same
value, in the same execution order, regardless of any non-uniform
parameter values, for all shader invocations in the invocation group."
Change-Id: Ib382f5c3b563f996b3c8f1eb6b021b6d31fa9ce7
Bug: oss-fuzz:28107
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339159
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, GLSL and Metal code generators would emit a struct wherever
the type was first used in the code, regardless of where it was
originally defined or what scope the type needs to live in. This CL adds
a ProgramElement for struct definitions, so that structs will now appear
at the top-level as they were originally defined. In the case of Metal,
some special handling is also needed to handle the Globals struct
properly.
Not yet fully supported:
- No special handling for structs declared inside functions yet
- No support for structs in separate scopes with overlapping names
The severity of the remaining issues depends mostly on whether we want
to support structs inside functions in Runtime Effects.
Change-Id: Ia95d4529506cb3fa6da63f5cb548199a93e1c0c5
Bug: skia:10922, skia:10923, skia:10925, skia:10926
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338600
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This test verifies that dead-stripping works on both built-in and user
functions, if their function call is optimized away.
Change-Id: I3125a34640c69de43c383343cd00d97e5a32ac60
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338836
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Enums are an SkSL-only concept--when we output code, we emit plain
IntLiterals--so the fix is simply to ignore the Enum program element
when we encounter it. This is what GLSLCodeGen does as well.
Also added a unit test to confirm that enums work normally, and that
enums are subject to optimization and static-comparison checks just as
ints would be.
Change-Id: Ic4f8da7a27983add9eb41b936d46f6638d22bd4b
Bug: skia:11003
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338800
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
There were a surprisingly small number of dedicated SPIR-V tests.
SkSLSPIRVBadOffset was the only test that didn't already exist in the
golden outputs, although it actually contained two tests.
The SPIRVTest.cpp file has been converted to SPIRVTestbed.cpp, which can
be used for local debugging of SPIR-V issues via dm (like GLSLTestbed
and MetalTestbed).
Change-Id: I978d8a7cf5735af7f537113d2b9411ce42cfcf88
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338756
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
^^ is not an operator in Metal. != can be used for the same purpose.
Change-Id: If75b000076ebe0aa81d0ab354a8ae33e6ed52101
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339156
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>
Sorta introduced here: https://skia-review.googlesource.com/c/skia/+/338318
When we switched the controller from filter-quality to sampling, we
'continued' the old behavior of forcing bilerp when sampling a miplevel.
This matched the old enum, but the new code should respect whatever
fFilter the client has set.
Change-Id: I44bac879c1d3c880d8dbfac241e8511397de6641
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339117
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>
Another small step in removing SkCanvas::flush
Change-Id: I6f3bec367e582754b8463b7bfe6a2542436ca829
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335647
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
5850c748b4..fcb6b5a5c5
2020-11-26 syoussefi@chromium.org Vulkan: Fix precision transformation for geometry shaders
2020-11-26 syoussefi@chromium.org Vulkan: Implement multisampled incomplete textures
2020-11-26 jmadill@chromium.org More cleanups to run_code_generation.
2020-11-26 syoussefi@chromium.org Vulkan: Experimentally enable geometry shaders
2020-11-26 aleino@nvidia.com Vulkan: Work around Nvidia depth clamping bug
2020-11-26 syoussefi@chromium.org Vulkan: Move SPIR-V generators to base transformer class
2020-11-26 angle-autoroll@skia-public.iam.gserviceaccount.com Roll Vulkan-ValidationLayers from fc78e93ce895 to 1835944edcfa (9 revisions)
2020-11-26 angle-autoroll@skia-public.iam.gserviceaccount.com Roll Chromium from 4b04564d4cb2 to 730f0fe9e50a (373 revisions)
If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/angle-skia-autoroll
Please CC michaelludwig@google.com on the revert to ensure that a human
is aware of the problem.
To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug
Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/master/autoroll/README.md
Cq-Include-Trybots: skia/skia.primary:Build-Debian10-Clang-x86_64-Release-ANGLE;skia/skia.primary:Test-Win10-Clang-AlphaR2-GPU-RadeonR9M470X-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-Golo-GPU-QuadroP400-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-NUC5i7RYH-GPU-IntelIris6100-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-NUC6i5SYK-GPU-IntelIris540-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-NUC8i5BEK-GPU-IntelIris655-x86_64-Debug-All-ANGLE;skia/skia.primary:Test-Win10-Clang-NUCD34010WYKH-GPU-IntelHD4400-x86_64-Debug-All-ANGLE
Tbr: michaelludwig@google.com
Change-Id: I098ba4466a3aa30d55441f0ca692394082ce970a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338997
Reviewed-by: skia-autoroll <skia-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: skia-autoroll <skia-autoroll@skia-public.iam.gserviceaccount.com>
Needed to pass assumptions in cc_unittests.
Bug: skia:11004
Change-Id: Ie70ab06908c24f88a72c6a0e07033e1be4f6a7cf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338936
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Goal is to isolate where we look at the quality enum to the fewest
number of places (e.g. single place in doStages) -- to make it clearer
how we'll be able to eliminate it going forward.
Change-Id: If60df3ac14692d0841a23665ce1baa69a55ac041
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338318
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
- easy: ceil, floor, sqrt
- index is our first arm64 instruction to need a temporary,
but other than that is pretty simple, just N - iota as usual.
With Op::index now supported, `viewer --slide GM_runtime_shader`
frame time drops from ~1ms to ~0.24ms.
I accidentally swapped in a float-subtract for an int-subtract and
everything worked fine. o_O
Change-Id: I44c51506a6a9014b398d6943bb0e3712e4e52445
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338661
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Guard flag has been added to clients
Change-Id: Ib61a48781f5dbd52279c8f4257ba3e22fb2704e0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338596
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Uniforms in practice are always pointers or 32-bit ints or floats, so
these are essentially dead code. The change to SkVMBlitter.cpp is the
only interesting change, and I think it makes more sense now than
before. The program will need float coverage in the end, so might as
well feed it one directly.
Change-Id: I7f1e77731cf10ccc35595012a6df4f9e54a0dad8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338631
Commit-Queue: Mike Reed <reed@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Now that I've been reminded that half-float compute is real and no
longer just a dream, Q14 kind of pales in comparison, and just gets in
my way when working on SkVM.
As usual I've left in assembler support and unit tests for those
instructions. The instructions are all pretty easy to keep working and
tested and don't get in the way, unlike the real "let's do Q14" stuff.
None of this Q14 code was hooked up to anything but unit tests, so no
capability lost here, and no diffs. As always, it'll be easy to restore
should we ever want to by looking at this CL.
Change-Id: Ia42a96652b381267a7c3ec563b5978efcfc717a7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338630
Auto-Submit: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Reed <reed@google.com>
I'm not using any of these, so nice to move them aside.
Change-Id: Id43c1606c2f9e6bba0d8f6bd7d2f8f5e02d5b762
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338629
Auto-Submit: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Bug: skia:10997
Change-Id: Ic6da0cbe6dd68009d888bc3174de913852559de7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338598
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Migrating chromium code to use SkImageFilters::Shader over
SkPaintImageFilter requires dithering to be preserved for its gradient
fills.
I debated always forcing it to true, but dithering was never turned on
for the turbulence filter and it's not necessary for const color shaders
Given that, I opted to just make it a parameter to the filter factory,
which seems okay since we're unlikely to embed dithering into SkShader
itself, it's a shading-related parameter of SkPaint, and if we migrate
to always dithering, then we can remove it.
Bug: skia:9310
Change-Id: I86f14969e2446f3a84e71e687cb263bcd44cf9d9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338156
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Change-Id: Ifc1f0921d983ee09d7bc2632aeca41689f1bf0c3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338603
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>
Conceptually these should be no-ops, but hopefully could improve
performance slightly when compiling very simple programs.
Change-Id: I87f560fa6af817e7cf39fa920d04fe62d40ca79b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338599
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:10991
Change-Id: Ie33a37109feba31acf675fc3a7739dcfc2c38668
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338601
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This reverts commit 8636e13c2d.
Reason for revert: recording canvases with really big float bounds could
produce a non-empty integer rect bounds that became empty after mapping
it to (0,0,w,h) for the device. This meant resetForNextPictures logic of
updating QR bounds directly from the input bounds allowed state to become
inconsistent with computeDevClipBounds().
PS1->PS4 shows the 1-liner to just compute bounds from the device. This
means that, for now, we preserve the behavior of setting the QR bounds to
be actually empty. skbug.com/10997 is added to fix the underlying issue
with recorders and excessively large float bounds. If that change landed
first, I'd be able to reland this w/o any modifications, but have decided
that it's better to have all locations that modify fQuickRejectBounds use
the exact same expression.
Original change's description:
> Revert "Move conservative bounds tracking from SkCanvas to SkNoPixelsDevice"
>
> This reverts commit 11a394759a.
>
> Reason for revert: assert during google3 tests.
>
> Original change's description:
> > Move conservative bounds tracking from SkCanvas to SkNoPixelsDevice
> >
> > Change-Id: I56670b4a4159e21eaa1a58a9a3ee439298d5aa8e
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335863
> > Reviewed-by: Mike Reed <reed@google.com>
> > Commit-Queue: Michael Ludwig <michaelludwig@google.com>
>
> TBR=mtklein@google.com,bsalomon@google.com,reed@google.com,michaelludwig@google.com
>
> Change-Id: I7c3a8797460113d9a8ef18d82bbbd64aba2f439c
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338316
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
TBR=mtklein@google.com,bsalomon@google.com,reed@google.com,michaelludwig@google.com
# Not skipping CQ checks because this is a reland.
Change-Id: I1b33e128b4fb4e06b8c7a6ee9b9dcc67202674d8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338322
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Still needs more testing before we can enable this all the time.
Bug: skia:10804
Change-Id: I6d8416fa072054d2f44e337641ed5d8379e81559
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337216
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Ran the following commands
from $SKIA_ROOT
go get go.skia.org/infra@e5c4a9cfc4
make -C infra/bots train
Change-Id: Id00ebbe564edfe9024e3291ff0726e92bc05965f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338217
Commit-Queue: Nathaniel Nifong <nifong@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
From the perspective of a SPIR-V opcode stream, ^^ is equivalent to !=,
so TK_LOGICALXOR can share the existing logic with TK_NEQ. (There are
differences in precedence and in supported types, but those were shaken
out at the IR-gen/compilation stages.)
Change-Id: I541a5ecfa603a07b256132fd1522f91941de6b20
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338351
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, we allowed unary minus on numbers and vectors (of any type).
Now, we allow them on numbers and vectors of numbers.
Also updated the Boolean arithmetic error test to cover scalars as well
as vectors.
Change-Id: Ie74d1f3bfc1e9353e04c6f8e468fa20e0cbba16f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338396
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
- remove an odd space
- "rounds" is fine, but "bits" is more precise
- after fixing a typo, four parallel hashes no
longer was any faster (or slower) than three,
so I consider that TODO now done
Change-Id: I7fff29640c1238229418bd8385b3b3aeae4ad68f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338621
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
GLSL does not allow most binary operations on bvec types; we can now
detect these and properly flag them as errors.
Note that `determine_binary_type` was also refactored. It originally
started with an enormous omni-switch over every possible Token type,
used to set various bools describing the type of binary expression at
hand. Instead of one big switch, this has been refactored into several
small switches in standalone functions that simply switch on the op and
immediately return true or false. Conceptually this seems like more
work (checking the op multiple times), but these tiny switches actually
boil down to little branchless shift-and-mask functions, so in practice
they should be quite efficient compared to the original omni-switch.
Change-Id: I81b473d98c65da1edd136f35fc8f656261f8930d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338346
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is a reland of cddfce2c24
Original change's description:
> Move GL's SkSL::Compiler to the GPU (like all other backends)
>
> This was the only backend that didn't store the compiler on the GrGpu,
> and also the only one that did lazy-instantiation. Trying to standardize
> this code a bit.
>
> Change-Id: Ibdd1bcc2dc9c3756b46a4c6f0543b5bb20fe135d
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337716
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I28cd2b20a86ca2cc34460cd494feff5b599f65bb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338597
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
These checks are made very frequently; it significantly eases
readability to have dedicated accessor methods, versus the verbose
`x.typeKind() == Type::TypeKind::kFoobar`.
Change-Id: I812b95f871cee436ccd3a5982c404f83563d44e5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338317
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This is very unlikely to occur in real-world code, as it's somewhat
nonsense to use the comma operator in this way. However, it's better to
fail cleanly than to assert.
Change-Id: I76481cd8a993cb1a798ee16956400a512efd4c15
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337636
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This new version always delivers the same results,
and I think can be simplified like this without
spoiling any of the bulk speed.
Change-Id: I20e42e58418e658278bb5db9472c39722b33160a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338339
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Fiddled with the logic a bit so that when we're in unit test mode, the
output still includes all of the SPIR-V (as well as the validation error
message), so that tracking them down is easier.
Bug: skia:10694
Change-Id: I15e7777af3d268a5952765dbe5d63612cad0ac07
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338320
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Perf graphs were showing a good amount of CPU time spent in
vkCmdClearAttachments. So we want to test if we get any wins by just
always doing draws instead.
Change-Id: If76a26cb0de411a2a1d1d17f17192b2d9fbdc459
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338319
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This is a reland of 0d5d956f7b
Original change's description:
> SkSL: Test/implement "geometric" intrinsics
>
> Bug: skia:10913
> Change-Id: Ie82354b05db141c8ab90b1a615ddfada4f71a98b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335049
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
Bug: skia:10913
Change-Id: I103dd2efbbab0efeac2be786d7e8f913d5c4b22a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338158
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
