This reverts commit 9f899ac9ed.
Reason for revert: broke ios bot?
Original change's description:
> Remove SkImage_Base::refPinnedImage(), use refView() instead.
>
> Also use refView() for lazy/raster images as well rather than creating
> a GrTextureProducer class outside the image.
>
> Bug: skia:11208
>
> Change-Id: Iee628c337bc1b4cfcccd78eaba98589757bb55ab
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360980
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
TBR=egdaniel@google.com,bsalomon@google.com,robertphillips@google.com
Change-Id: Iab4851f5cefdca853e16c030ec67238c55c69aa2
Cq-Include-Trybots: luci.skia.skia.primary:Test-iOS-Clang-iPhone6-GPU-PowerVRGX6450-arm64-Release-All-Metal
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/361838
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Change-Id: Ibe9c2a3fb52b1a18895c1a61bdb296351b989537
Bug: skia:5039
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/361456
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Previously, a uniform not wrapped in an interface block would report a
SPIR-V error:
"Variables identified with the Uniform storage class are
used to access transparent buffer backed resources. Such variables must
be typed as OpTypeStruct, or an array of this type..."
Now, the SPIR-V code generator automatically detects such global
variables and synthesizes a struct named _UniformBuffer to hold them.
When these variables are accessed, an OpAccessChain instruction is added
to grab the variable out of the struct.
Change-Id: I5e852d4de01b866c291506cc8cf6eb547f097d66
Bug: skia:11225
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360776
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I2c1c09d07a12e0382633e7e12750a4672f21bbda
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360416
Auto-Submit: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Previously, structs that were defined as part of a variable declaration
would end up declared similarly in the generated code. Now, global
variable declarations that include a struct definition generate two
separate program elements.
Bug: skia:11228
Change-Id: Id7ddde6931fe07a250c2c9c46153879005535fb3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/361359
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Also use refView() for lazy/raster images as well rather than creating
a GrTextureProducer class outside the image.
Bug: skia:11208
Change-Id: Iee628c337bc1b4cfcccd78eaba98589757bb55ab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360980
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This allows interface blocks in Metal to compile even if
`layout(binding=...)` is not specified. It will also be used in SPIR-V
in the followup CL, when an interface block is automatically synthesized
for top-level uniforms.
This CL also reorganizes the unit tests around uniforms a bit.
Change-Id: Ia898c536b454dda6f51677e232a8f6e6c3606022
Bug: skia:11225
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360778
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Simplifies the op to always triangulate the inner fan. We ensure this
will always work by using breadcrumb triangles. Also removes the
fail-on-non-simple mode from GrInnerFanTriangulator since it isn't
being used anymore.
Bug: skia:10419
Change-Id: Idb849712bf2bc4e5ef785bc3f9b8db03119d230e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359565
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Pre-cleanup as I start looking at how structs are parsed and handled in
the IR.
Bug: skia:11228
Change-Id: I6334d1073211cbbdf69ddffa8df420c45fd59fcc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/361059
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: Ibc995e908e5b4f8d1516e13d56854a4fcf5cc809
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360556
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This existed to enable shader-based versions of the old bleed GM that no
longer exist. It is currently making it harder to simplify image->view
consolidation without accidentally limiting the testing. This is because
an attempt to create a texture from a bitmap will succeed if max texture
size is not overridden by the test.
Bug: skia:11208
Change-Id: I432d1d2ab66c1e888c9d77583b3c9a9d673e7e8f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360609
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This is used to decide when we can keep the convex bit during
path.transform. This new version is much faster than the old,
but can return a false negative if there are multiple contours.
However, the caller now only calls this if the path is known to
be convex, so there should never be multiple contours.
This, along with
https://skia-review.googlesource.com/c/skia/+/359837
make path.transform() approx 2x faster
Bug: skia:11227
Change-Id: Ied7ca97d87b9224a3ecd9b77b22b823964809a52
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359916
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Marks its methods const and lifts the breadcrumb list out into
function arguments. This is one more step toward the final vision
where GrTriangulator just has an allocator and control knobs, and
everything else is functional.
Bug: skia:10419
Change-Id: I77341c045d481da49ebfee06de5dfc7a2a8a07be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360956
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Bug: skia:9310
Change-Id: I387f0251f05a2b6f2bc5a759f608d5766ed11ce2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357285
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Switch away from views, and add support for simulating
used-proxies. The latter will be used in an upcoming CL.
Bug: skia:10877
Change-Id: I7897516dc53c075a286cce8f31075d8cc93abccf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360604
Commit-Queue: Robert Phillips <robertphillips@google.com>
Auto-Submit: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Removes a use case of SkImage_Base::view().
This forces a RGBA flattening image which seems like an undesirable
side effect. If clients need the textures backing these image types
we should provide a query for GrYUVABackendTextures instead that
could return the whole set.
Bug: skia:11208
Bug: skia:9570
Change-Id: I085e5212b69fef7a53fb1606aecfb6f7f0e7c740
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360418
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
These have a kind of neat way of encoding the lane index, using the Q
bit to pick the lower or upper 64 bits of the register, then the S bit
to pick the 32-bit lane within those 64 bits. Usually Q=1 distinguishes
a 128-bit op from a Q=0 64-bit op, so its repurposing here is at first
surprising, but actually very fitting.
I'd eventually like load64/128 to use these like this:
Reg tmp0 = alloc_tmp(2),
tmp1 = (Reg)(tmp0+1);
if (scalar) { a->ld24s(tmp0, arg[immA], 0); }
else { a->ld24s(tmp0, arg[immA] ); }
mark_tmp_as_dst(tmp0, tmp1);
where the mechanism to track up to four registers per value and
implement mark_tmp_as_dst(...) for more than one argument is what I'm
still working on.
Change-Id: I944e571de19f65d41f462406ce35f0f2a35bb381
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360700
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Change-Id: I0c343167e63f147f21afee805ea934bd5f161024
Bug: chromium:1170700
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360608
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Part of effort to consolidate access to GPU SkImage texture proxies.
Bug: skia:11208
Change-Id: Icfcf6fea6be6f05220a5ddd1482f88dafe1cbd9d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359836
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Previously, we reimplemented get_storage_class almost verbatim in
writeGlobalVar. A few tiny tweaks allowed this logic to be deduplicated.
Change-Id: I9f7bb24c53f5249ea4e2e87ea8d18ff7b7150ed8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360598
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
SkTHashMap->remove does not like if you try to remove
an entry that isn't in there. In that case, we don't
want to do anything.
Bug: skia:10877
Change-Id: Ib87aac7a132c78915ec18724f847c44c2c3efa80
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360596
Auto-Submit: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
Change-Id: Ia0bc034a9360c03d74d97923f4a963aaa09fb58e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360577
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>
The old code only handled swizzles as the outermost expression(s) in an
lvalue. The new code removes that restriction, and puts all the logic in
writeStore, which is the only place it needs to exist.
The newly added test asserted before, and now passes.
Bug: skia:11178
Change-Id: I8083d9d478ad4dc993cb963d34a97c10965831b5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358956
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Evaluating either kind of expression now works like all other
expressions - evaluate the inner part, then work with the resulting
values. Added unit tests for both of these that previously failed.
With this change, writeVariableExpression is only used for
VariableReference expressions, so adjust that, too.
Reland now safe, after fix to Value::operator[]
This reverts commit 1ea6d6051e.
Bug: skia:11178
Cq-Include-Trybots: luci.skia.skia.primary:Test-Debian10-GCC-GCE-CPU-AVX2-x86-Debug-All-Docker
Change-Id: I14782fcdfef33a47a46334447c5847976721b21f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359564
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
An upcoming change to SkVMGenerator triggers a gcc-8 x86-only bug. This
change prevents that, by limiting the conversions back and forth from
size_t to int (particularly here, where the original value is also
stored using a bitfield).
Change-Id: Ie7f8f3322dd387b0627ab2b2325ce7d82ce6be39
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359563
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
It's very easy to add intrinsics to the VM generator this way, (and
there are others that can't be written in SkSL itself), so this makes
everything consistent.
Bug: skia:10913
Change-Id: I1fcd0047136459f1f6aee2cf4fd87e875edb7e3d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348879
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This creates a helper function, _entrypoint, which invokes main() and
assigns its result into sk_FragColor. We also make sure to prevent
sk_FragColor from being dead-stripped from the code during IR
generation.
At present this is useful for allowing our SkSL test shaders to compile.
Change-Id: I2d7fab0e1959a77778ffdb18ca569e869bcaeece
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358525
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This lets us use descriptive names like `colorRed` and `colorGreen`
instead of `half4(1,0,0,1)` and `half4(0,1,0,1)`. It also lets us use
actual unknown values instead of synthesizing sorta-kinda-unknowns by
calling sqrt.
Change-Id: I61481c33b7ff42182955777b05cfa5fcc13e0efc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359567
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This allows uniforms to be specified without an explicit `layout(set=N)`
modifier. They will assume a default set value instead.
This turns out to fix a handful of tests in Metal/SPIR-V which were
written with GLSL in mind, or adapted from real generated GLSL code, and
didn't have layout information specified on their uniforms. It will also
make it easier to write SkSL tests using uniforms that can compile
either as a runtime effect or as plain Metal/SPIR-V code.
Change-Id: Id79ec06f278b913a45c09c2e6211195dc98b42c0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359838
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
CoreText reports weight on a [-1.0, 1.0] scale with 0 being 'regular'.
Unfortunately the mapping from the [1, 1000] scale more commonly used
is not uniform and also varies between system fonts and fonts created
from data. For system fonts the NSFontWeight values will be used if
available, but there is no corresponding API for fonts created from
data. Previously the values for fonts created from data were hardcoded,
but the values have changed again recently. Directly read these values
by varying the weight in the font data, creating a CTFontDescriptor from
this data, and then using the value reported by CoreText.
Bug: skia:11176
Change-Id: I071e2ff7ac3f676c8395b13aa82dde7a97fdf2ce
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358535
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Previously, we would emit nothing at all, but that is not actually
valid if the Block is a child statement (e.g. the body of a loop).
Now we emit braces for empty blocks, even if the block was unscoped.
Change-Id: I456a8d7d306a3e59d85e39f80b9f15fe3347ea19
Bug: skia:11218
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359562
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This reverts commit 1eea1ea8c1.
Reason for revert: fixed implicit copy cons
Original change's description:
> Revert "Write pixels goes through GrRenderTask system."
>
> This reverts commit 27efe6cb1e.
>
> Reason for revert: wasm compile
>
> Original change's description:
> > Write pixels goes through GrRenderTask system.
> >
> > The specific motivation is to remove some uses of GrResourceProvider
> > making textures with data in lazy callbacks. But it's a general
> > improvement that could allow use cases like writePixels in DDL
> > recordings.
> >
> > Bug: skia:11204
> >
> > Change-Id: Ic55c3f75976a1d3a7d93981e21be75a3053ef069
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356845
> > Reviewed-by: Adlai Holler <adlai@google.com>
> > Commit-Queue: Brian Salomon <bsalomon@google.com>
>
> TBR=bsalomon@google.com,adlai@google.com
>
> Change-Id: I116caf1e4dd9015270b9d4f810bd26e0e30a6497
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:11204
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359559
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,adlai@google.com
# Not skipping CQ checks because this is a reland.
Bug: skia:11204
Change-Id: I7d8f92415995f03301ffb147500d972e6bd17640
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359561
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Adds a new op, GrPathStencilFillOp, that is a greatly simplified
rewrite of GrPathTessellateOp without inner fan triangulation. This op
is a very simple Redbook method built on tessellation. For now we
leave GrPathTessellateOp as-is. The next CL will change it to do inner
fan triangulation exclusively.
Bug: skia:10419
Change-Id: Ia67f4ab038a541e6454f5bf304ecd3c1d8805427
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357138
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
and add a default implementation of onResetContext (which only the GL backend implements)
Change-Id: I20bb4cd2a60dba3200fc8ef4c9e4fd6a9f54ceef
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359497
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Change-Id: I4e771083e90f3c60b61f7ce7c8e6697e7bf7c7e1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358518
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 27efe6cb1e.
Reason for revert: wasm compile
Original change's description:
> Write pixels goes through GrRenderTask system.
>
> The specific motivation is to remove some uses of GrResourceProvider
> making textures with data in lazy callbacks. But it's a general
> improvement that could allow use cases like writePixels in DDL
> recordings.
>
> Bug: skia:11204
>
> Change-Id: Ic55c3f75976a1d3a7d93981e21be75a3053ef069
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356845
> Reviewed-by: Adlai Holler <adlai@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,adlai@google.com
Change-Id: I116caf1e4dd9015270b9d4f810bd26e0e30a6497
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11204
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359559
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
The specific motivation is to remove some uses of GrResourceProvider
making textures with data in lazy callbacks. But it's a general
improvement that could allow use cases like writePixels in DDL
recordings.
Bug: skia:11204
Change-Id: Ic55c3f75976a1d3a7d93981e21be75a3053ef069
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356845
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
In GLSL and SkSL, control statements don't require explicit braces
around single-statement children. That is, the `match = true` child
statement here doesn't need to be braced.
if (condition) match = true;
Because there are no braces, we never create a Block or a dedicated
SymbolTable here. This is normally not a problem, but the fuzzer
discovered that it can dump things into the symbol table inside a child
statement:
if (condition) int newSymbol;
This becomes problematic because the symbol name now outlives its block.
This means `newSymbol` can be referred to later, which should be illegal
(and can cause the optimizer to blow up since the structure is bogus).
There doesn't seem to be any reason to allow this code to compile; the
user can add an explicit scope here to make it reasonable, and it's
(almost) meaningless to declare a symbol that's instantly going to fall
out of scope. This code is now rejected with an error message.
Change-Id: I44778e5b59652d345b10eecd4c88efbf7d86a5e0
Bug: oss-fuzz:29849
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358960
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The prior CL 358522 was reverted due to relying on
moving SkArenaAlloc which does not work.
This folds in cleanup CL 358156 also.
Bug: skia:10877
Change-Id: I0a29848f0fccb7d6a8f18d307b9f6b6a0311c83a
Cq-Include-Trybots: luci.skia.skia.primary:Build-Debian10-EMCC-wasm-Release-WasmGMTests
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358527
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Splitting its name in half makes it difficult to find when searching the
codebase, for almost no benefit.
Change-Id: I6f1558558bfc6801c19a12116324c35f42111442
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359136
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
The previous change caused varDeclarations() to sometimes return an
expression-statement. This only made sense in the context of being
called from Parser::statement(). Other places which called
varDeclarations() expect vardecls and nothing else.
Change-Id: I562657cadfa20dcd77b527f2dc43dca0c6bf389f
Bug: oss-fuzz:29845
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358528
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit f619079545.
Reason for revert: Some bots unhappy.
Original change's description:
> Fix field access and indexing of complex expressions
>
> Evaluating either kind of expression now works like all other
> expressions - evaluate the inner part, then work with the resulting
> values. Added unit tests for both of these that previously failed.
>
> With this change, writeVariableExpression is only used for
> VariableReference expressions, so adjust that, too.
>
> Bug: skia:11178
> Change-Id: Ia595be473b55f4bb03ec25897f9929835177257c
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358529
> Reviewed-by: Mike Klein <mtklein@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
TBR=mtklein@google.com,brianosman@google.com,johnstiles@google.com
Change-Id: I56776139f9164b24b35a93307774e9b12c50054e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11178
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358959
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Part of the larger work to remove drawImage from canvas (and in general
simplify our apis w.r.t. older patterns like skbitmap).
Change-Id: If208927e1d46256519036c42e68aec3d3c809a82
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358836
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Change-Id: I59821d748768d74b251d9339787f193c23e3d4dc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358526
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Evaluating either kind of expression now works like all other
expressions - evaluate the inner part, then work with the resulting
values. Added unit tests for both of these that previously failed.
With this change, writeVariableExpression is only used for
VariableReference expressions, so adjust that, too.
Bug: skia:11178
Change-Id: Ia595be473b55f4bb03ec25897f9929835177257c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358529
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
There are SkColorTypes (e.g. kBGR_101010x) that don't have Gr equivs.
When a SkPixmap is converted to GrPixmap we will have a kUnknown color
type.
Bug: chromium:1164705
Change-Id: Iefa2c8363582433a531c5501aa251c571b5a6eab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358521
Auto-Submit: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This reverts commit 3b5b7d1178.
Reason for revert: Arenas cant be moved
Original change's description:
> Merge adjacent GrOpsTasks with same target together
>
> This allows the ops tasks to make one render pass instead of multiple.
> The only case where this merging is needed is as a result of
> reordering (reduceOpsTaskSplitting).
>
> Bug: skia:10877
> Change-Id: Ia967ead6efc43f7d2c1da58f770d3987da690cda
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353656
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Adlai Holler <adlai@google.com>
TBR=egdaniel@google.com,robertphillips@google.com,adlai@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: skia:10877
Change-Id: I6d1813ee1c28bc43dffb0a5f26dba4a5c41c637f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358522
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
This attempt to retain a std::move'd value was inadvertently left in
place in a previous CL. It's harmless, but should have been removed.
Change-Id: I485e0f14d921fe6c87a62520c7478a28e8ce28d8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358517
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Will be needed for repositionable DDLs.
Change-Id: I6dacf51bd36cfbe1c54a9350987f17a6d18cf3c4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357776
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Allows creation of an image directly from a RTE. Caller provides the
effect input bindings, image info for the image, and optional local
matrix.
The info's alpha type and colorspace are tags for the output image. No
alpha type or color space conversions are applied to the output of RTE.
CPU does not yet support making kUnpremul images.
Bug: chromium:1151490
Change-Id: I69babc9dbbce4431d756cd7a3eef4753e727d6fe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357284
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
For the tessellation op this allows it to use either the record-time
or flush-time allocator instead of making its own.
Bug: skia:10419
Change-Id: Ida13283feffda1976bd8fd221d5be0013a5de990
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356516
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Temporarily puts inner fan triangulation behind a flag so we can
disable it for the atlas. The next CL will do the same thing by
creating an op that doesn't triangulate, but making the functional
change in this CL allows us to triage the diffs ahead of time. It will
also ensure the next CL doesn't introduce any others diffs.
Bug: skia:10419
Change-Id: Ia498957c53e83fe40aa797cadace171902dbf548
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357137
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
We can load into two or four adjacent registers from alloc_tmp(N) and
then simply name the right one as our output, discarding the others.
This still has the major TODO of returning all two/four registers at
once, eliminating the immB selector and the redundant memory traffic.
Expect changes towards support for up to 4 Vals per Op next week.
Change-Id: I94846d15ac59d4018c1c9d136c17833e5091f8cf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357305
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This allows us to write SkSL shaders which are valid both for use as
Runtime Effect, and for compilation with skslc targeting Metal.
Change-Id: I74e125d81865d4092e657a7d9948d2e72054bda5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357777
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>
(and Ternary for good measure)
Change-Id: I4afa121d54ab9ba8d0814693ce53da7cb73ef340
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353626
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Added asserts that verify we don't try to emit the same struct or array
with two different memory layout rules. Some code paths were failing to
inspect the associated variable, leading to incorrect errors about the
attached offsets of members.
Added a test case that triggered that error, and also triggers the new
asserts.
Then, fixed the underlying cause: writing out the struct definition as a
side effect of accessing a member in getLValue().
Bug: skia:11205
Change-Id: I6e5fb76ea918ec9ff10425f2d519ddbc54404b27
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357436
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This allows the ops tasks to make one render pass instead of multiple.
The only case where this merging is needed is as a result of
reordering (reduceOpsTaskSplitting).
Bug: skia:10877
Change-Id: Ia967ead6efc43f7d2c1da58f770d3987da690cda
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353656
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
1. Removes clamping of spotlight's specular exponent, which is not the
specified behavior of feSpotLight in SVG.
(note: we never clamped a specular lighting effect's
specular exponent/shininess parameter, although SVG 1.1 does clamp
that; that is the client's responsibility).
2. Fixes a bug in the GPU implementation of scale factor for spot lights.
3. Saturate computed lighting color after multiplying with color scale,
instead of just saturating the color scale (allows high intensity
lights to saturate to white which is more reasonable approximation of
an HDR effect stored in an LDR color).
Note: fixes 1 and 2 were originally addressed in https://bugs.chromium.org/p/chromium/issues/detail?id=472849
but the change was reverted for layout failure reasons and it was never
relanded after rebasing.
Note2: most of the layout test rebaselines necessary for chrome are
minor and related to fix 2. The exception is svg/dynamic-updates/SVGFESpecularLightingElement-dom-kernelUnitLength-attr.html
which is the result of fix 3. It took a little digging, but I believe
that fix 3 actually makes that generated SVG more correct (it's original
expected image was "wrong").
In that test, it has a specular constant of 4 (which is a multiplier)
and a shininess of 1 (which makes it practically a "diffuse" specular).
So basically we are multiplying the greenYellow color by 4 for most of
the image that has a normal pointing out of the screen. Firefox renders
a similarly yellow oversaturated appearance instead of clamping to the
base greenYellow.
Reading the feSpecularLighting spec, there is no saturation that is
specified where it had been performed before this change. Instead, all
that is mentioned is that the results of a given filter have to be pinned
to the color channel range (e.g. the last step).
Bug: skia:11007, skia:11057, skia:11153
Change-Id: I82e4a6f1742fecea59816fda75eb931c2a51d3e1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355496
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Tyler Denniston <tdenniston@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
We now search for a block of N adjacent registers that minimizes the
number of spills, where unspillable registers are counted as if
infinitely expensive.
On arm64, store64 now uses alloc_tmp(2) and st2.4s, and similarly
store128 uses alloc_tmp(4) and st4.4s.
For the purposes of arm64 instructions we could allow the block to wrap
around the register file mod 32, but I figured that probably wasn't
necessary and might be confusing to follow. If we're right on the edge
some day we could circle back.
I'm not sure yet if our register-pair use cases on x86-64 will ever care
that the registers are adjacent, but it doesn't hurt to start that way.
This should behave pretty much the same when N=1 except that we're not
doing any interesting tie-breaking when all registers are occupied. I
left a TODO, but I bet we'll never feel the need to follow up on it.
Change-Id: Ibbdd42858a6daf61401c638435617bfb37d1899c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357300
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This allows us to roll the Parser back to an earlier state if we need
to do so. This includes:
- rewinding the lexer
- restoring the previous Pushback node
- backing out AST nodes
- backing out errors
This functionality is used to back out of parsing a vardecl if we
discover mid-stream that it is actually an expression statement that
coincidentally starts with the name of a type.
Change-Id: Ia5feb45019693931c1e6870e3ff7a5398924c863
Bug: skia:11198
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356997
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The lexer can be reset to an earlier state simply by overwriting its
offset to a previous position.
Change-Id: I571c7981dbec3c43a894fe599f2e2a167bc380df
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356976
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This should not cause any functional changes, and is just a prerequisite
for upcoming DSL work.
Change-Id: Iea165d3b7ede39ccc9cf5f5d78f623bc883b391e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356816
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Change-Id: Ib150e6d6d3de34a85ce8051eea843ab3b2d7ab75
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356921
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Also cleanup some of the duplicate code in SkRecords
Bug: skia:7650
Change-Id: I4d3167a892c126c19a54002beab25c9a6c96fa5d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357000
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Rather than copying these triangles twice, we can run "pathToPolys"
during onPrePrepare and "polysToTriangles" during onPrepare.
Also adds a benchmark for normal and inner-fan triangulation.
Bug: skia:10419
Change-Id: Id301afde5de11d93ae026e75e42ac03a50867687
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355177
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This was the last remaining user of ByteCode. The skvm solution
is faster, and lets us delete the ByteCode system.
Testing on 15 instances of sinusoidal_emitter (90k particles):
- ByteCode ~9 ms
- ByteCode (older, optimized): ~5.5 ms
- skvm ~2.1 ms
Change-Id: Ia2e5c9ab2d36c97e59af28a6f989bf212889e439
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356919
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Like _globals, it's not actually necessary to indirect through a
separate pointer at all. The output struct is now passed by reference
and the additional pointer variable is removed.
(Additionally, renamed _skGlobals back to _globals.)
Change-Id: Id089a20cb751cdaedc48462a52da78ee43783611
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355632
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
I think it makes sense for the class functions to not have internal side
effects. Meaning, you can call pathToPolys and polysToTriangles all you
want without worrying about internal state.
Bug: skia:10419
Change-Id: I5e2136719bbf65a6a8e4c032c1c1326f0a9a98c3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356496
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Replace "pathToTriangles" with "pathToPolys, polysToTriangles". This
will allow the tessellator to do pathToPolys in onPrePrepare and
polysToTriangles in onPrepare.
Don't make countPoints and polysToTriangles virtual. This is a step
toward moving the outer mesh of GrAATriangulator back out of the
class, which seems to match the style better.
Bug: skia:10419
Change-Id: Id6d22dcc2da0af84b9cb46fb36ead4c2e30d5c32
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355176
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This GM tests out the GrTextureEffect with non-normalizable textures.
Change-Id: I5b0ffc43241a29d64516d07a4388668f224ffefe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355676
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
The SPIR-V code generator would sometimes invoke helper functions in an
unsequenced way, causing the output to vary across different compilers
or optimization settings. This CL explicitly sequences these operations,
which should make the output consistent across platforms.
Change-Id: Iaaf75b80e7495768d73dd6afa5b6d03f9cf3f262
Bug: skia:11175
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356844
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>
Just more internal cleanup/reorg, including passing around the (few)
fields we use from paint, rather than an entire paint, in the legacy
shader context. This was motivated (partly) to ensure that we don't
accidentally refer to getFilterQuality() downstream.
Bug: skia:7650
Change-Id: I69031dcb74e02c77bc4d6166fdb377262ae77046
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356842
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
I wrote this looking for places where we treat w differently from z.
None of these are an urgent problem today, but 2) could become one, and
all good to fix if only for consistency.
1) Recycle w's register when it dies in the interpreter.
If we don't recycle w's register slot when w dies, we'll keep
thinking w's alive and keep that slot around in case it's used.
This is only an inefficiency, but nice to avoid when we can.
2) Don't allow spilling w when looking for an unused register.
This is a JIT bug fix, avoiding the possibility of clobbering w with
dst(),x,y,z or a temporary when we need to use it. Doesn't look like
it's happening today, but could sneak in.
3) Allow aliasing w and dst().
This is for completeness, moot today... store128 is the only op using
w and doesn't use dst(), let alone try to alias them.
Change-Id: I1f22e67f8793be018282bc3e793412e1d1abd7a5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356797
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This isn't really how I intended to use these, but we might as well
check if the registers happen to already be lined up how we'd like them.
And it happens quite often... I didn't gather detailed numbers, but both
sides of the "are we lined up?" conditions are being hit lots and lots
of times, in both store64 and store128.
Also rewrote the scalar flow of store128 to mirror store64. The old
code was just fine, but this makes it easier to follow the conditions of
when we can use st4. (This does remind me that we could also use
single-lane st2/st4/ld2/ld4 instructions to handle the scalar paths.)
Change-Id: Id2c94f68e5ea14031f7a23bdc76583dff4a7b65f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356436
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This should be the same amount of work in Assembler::bytes()
with one fewer field to track... might as well!
Change-Id: I54ca919983f11d218255817dbc4db292e7972409
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356796
Auto-Submit: Mike Klein <mtklein@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Herb Derby <herb@google.com>
We use our own custom lexer now.
Change-Id: I6a2a408094ba37c2eef7092f5a5caa107938613b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356476
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This simplifies our world and opens the door for more optimization.
Bug: skia:10877
Change-Id: I3c721f12a23bfa73dbdf1e02d9c77d7c6a889aa0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356309
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This is a reland of 97c476ecb7
Original change's description:
> Disable the MSAA atlas mode for CCPR
>
> We have a long term path rendering plan that uses dynamic MSAA instead.
> This CL is a test to see if we can drop support for CCPR now.
>
> Change-Id: I1bff3ca3143a6b453b65a7932a1805c195922805
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354036
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
Change-Id: Ied3390d7df0b01d5e9d565247f5aed0addb5ab8f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356336
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
We were replacing points with the intersection of opposite edges.
Because of the distance tolerance we're using that may fall outside
of the original quad. Detect those cases and use averages of
intersection points instead.
Bug: chromium:1167277
Change-Id: I36b172f19339839bb21c060ddfe8109c184e9327
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356311
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
store64 and store128 will use the st?.4s instructions,
and load64/load128 the ld?.4s. The tricky bit for both
of course is that they load and store more than a single
register, and that those registers need to be adjacent.
Change-Id: I613d06cbcc6e00bfc16b1a2c88412dbbbb1c55ed
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356344
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
The previous implementation assumed that SkSL expressions do not have
side effects and so treated either side of a Boolean expression as
short-circuitable. That is, `foo() && false` and `false && foo()` would
both be optimized to `false`, eliminating the `foo()` call.
We now check for side effects first. An expression like `expr && false`
can only be optimized to `false` if `expr` has no side effects. (If
`expr` does have side effects, the expression is left as-is.)
Change-Id: I473cf026a8afe35d6a8d9518498f2b26d8996e60
Bug: skia:11162
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356357
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>
Additionally, restructure the unit test to return a color (green for
pass, red for fail).
Change-Id: Ib1bb6bd8771c72cc751d8d2c65cc14a693166d4c
Bug: skia:11112
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356301
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: I81c9a6ddcd65d40ebced18c7cfb1cad51066ba7b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356297
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
No longer used and troublesome to use, so remove.
Change-Id: Idad9dd3325443f7e546465abf551de9784d9f829
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352512
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
external callers
This should not result in any functional changes.
Change-Id: I5ef0301cf63bee561871c0110fda2692849b53d8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356102
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
The inliner can generate slightly tighter code if we avoid creating
helper variables inside a scoped block. This saves one temp variable
copy.
Change-Id: Iea9ecda24fa296eb489166887fd8ab50ec7176e6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355982
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
No functionality changes, but the new variants of the methods make them
more accessible from upcoming DSL code.
Change-Id: Iaa9f1fab31cb7db00007b00d7d3b88ff5b9696b6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356103
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This exposed a preexisting error: GrGrSLTypesAreSupported.fp used non-
square matrices, which are not actually present in GrSLType. The test
has been updated to use square matrices.
Change-Id: Ib51141cc14a0c3fcd1c3c3abf378f190d457b95f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356077
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
We already had support for &&, ||, ^^ but somehow the common cases of
== and != were not implemented in the constant-folder.
This CL also updates the test to return a green/red color on success or
failure, instead of assigning arbitrary numbers into sk_FragColor that
don't mean anything. The long-term plan is to signal success or failure
of each test by color code; we can display these colors as swatches in a
GM slide for testing purposes.
Change-Id: I0810108b3c6b656a60cd8aa64ceefd765eff0157
Bug: skia:11112
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355984
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Migrate clients to use drawImage(... sampling)
rather than relying on paint's filter-quality.
This CL just gets started, introducing the build-flag.
Bug: skia:7650
Change-Id: I4afdd964c6f805058afee0a8f3a6887d501ad42b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356076
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Reed <reed@google.com>
store128() has been lowered into SkVM Ops strangely (two interlocking
64-bit stores) only because of SkVM's limit of three arguments per Op.
With four arguments we can lower store128() in a straightforward way.
Perhaps surprisingly, I've left the implementations of store128 fairly
naive, with narrower stores than having all this data together in one
place allows. I do want to follow up here, but not so much because the
speed of store128 is important, rather more so because getting the tools
in place for idiomatic store128 implementations will lead us down a path
with great knock-on effects for more interesting features.
We'll need four adjacent temporary registers to use the ARM-idiomatic
st2.4s/st4.4s approaches for store64/store128, and the idiomatic x86
implementations need multiple temporary registers too. Once we're able
to manage multiple adjacent registers as a unit, we'll be able to
stretch the idea to things like load64/load128 returning 2 or 4
registers worth of data from a single Op. And the ultimate goal is in
Half-is-fp16 mode, where we'll be able to fill one register with 16-bit
float/int/mask data and spread any 32-bit data across a register pair.
Change-Id: Ieb20d8b7d00e9d806cb27fd30ebfd50ae9317da7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355936
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
These types are supported by SkSL but were mysteriously absent from
GrSL.
Change-Id: Id3479a23b1ddee0604362ed8c12da0eea7c6fa56
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355981
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This clip code will be shared with direct-to-op text drawing
path.
Change-Id: I6744a93cb75b555e91b3b016d5604d3ba81b02fe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355977
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This reverts commit b7e836cee9.
Change-Id: I3c39a928ba4a9a2863b616f2a500975294b03860
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355980
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This reverts commit ebf569004f.
Reason for revert: std::clamp is c++17
Original change's description:
> Support indexing by loop variables in SkVMGenerator
>
> Bug: skia:11096
> Change-Id: I25a91bacf1c3455ac67422fb0e59b9b152c2054a
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354667
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Mike Klein <mtklein@google.com>
TBR=mtklein@google.com,brianosman@google.com,johnstiles@google.com
Change-Id: I0590cf7fe626fb59be3381b5e8eb66a9a2a9e8cb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11096
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356056
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This reverts commit f1650efc55.
Reason for revert: fix for cpu/ddl configs
Original change's description:
> Revert "Test all YUVA image factories with different encoded origins."
>
> This reverts commit 2ba80af000.
>
> Reason for revert: new test fails ddl and cpu configs on imggen
>
> Original change's description:
> > Test all YUVA image factories with different encoded origins.
> >
> > Now that SkImage_GpuYUVA stores a GrYUVATextureProxies it supports
> > encoded origins.
> >
> > Modify wacky_yuv_format GMs to use different origins and remove
> > restriction in SkImage::MakeFromYUVAPixmaps.
> >
> > Bug: skia:10632
> > Change-Id: I02477d592b7baba164944d629eeac48223698c10
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353623
> > Reviewed-by: Jim Van Verth <jvanverth@google.com>
> > Commit-Queue: Brian Salomon <bsalomon@google.com>
>
> TBR=jvanverth@google.com,bsalomon@google.com
>
> Change-Id: If909ee4769cc1c74e1682a5e2870ec85a83f65c5
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:10632
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354661
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
TBR=jvanverth@google.com,bsalomon@google.com
# Not skipping CQ checks because this is a reland.
Bug: skia:10632
Change-Id: Iafe79ab5b3ce0ff9e3a4007e5d8fbc44edded196
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355630
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:11096
Change-Id: I25a91bacf1c3455ac67422fb0e59b9b152c2054a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354667
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This reduces complexity and will also allow us to add new ops that
take advantage of this same core logic.
Bug: skia:10419
Change-Id: I4ec8717a6d9510dea967d11467eeea0b5b7c7f4c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354296
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This is a pure refactor, making room for a fourth argument per Op.
I'll use this to make store128 work more naturally, taking all
128 bits of its input at once, rather than spreading across two strided
64-bit stores as done today. I've left this for a second CL because
this refactor on its own seemed error-prone enough.
Change-Id: I704e4f2e165b0bfd0276af921d97bf2cc01e8550
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355659
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
I've been looking to simplify and expand the kinds of instructions
we can support, and one easy early simplification is to break the
union of regs y,z and immy,immz into separate fields.
To further reinforce how unrelated they are, rename to immA,immB.
With the guaranteed two immediates available, we can simplify the
representation of some ops to use the immediates more naturally.
Change-Id: Iad139b5d77e8464e71f1360d881c1ed2e0957db8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355658
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Bug: chromium:1162942
Change-Id: Idc1dcb725ff9eae651b84de2fe792b188dcd1c1b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354671
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
`globalStruct` is now named `_skGlobals` and is passed around directly
by reference, with no additional helper variable (`_globals`) at all.
Change-Id: Icc5566d2212afd14a4d43700e89f50bedcc8b45f
Bug: skia:11168
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355717
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Dug in with instruments and yeah there's substantial work to do here.
Bug: skia:10877
Change-Id: I0f6978e0d16385e09e6017a0532fdcf9ba9a5d0c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355716
Auto-Submit: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This reverts commit 2ba80af000.
Reason for revert: new test fails ddl and cpu configs on imggen
Original change's description:
> Test all YUVA image factories with different encoded origins.
>
> Now that SkImage_GpuYUVA stores a GrYUVATextureProxies it supports
> encoded origins.
>
> Modify wacky_yuv_format GMs to use different origins and remove
> restriction in SkImage::MakeFromYUVAPixmaps.
>
> Bug: skia:10632
> Change-Id: I02477d592b7baba164944d629eeac48223698c10
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353623
> Reviewed-by: Jim Van Verth <jvanverth@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
TBR=jvanverth@google.com,bsalomon@google.com
Change-Id: If909ee4769cc1c74e1682a5e2870ec85a83f65c5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10632
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354661
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Sometimes our cache of convexity is wrong -- still trying to figure out
what do do about that.
Change-Id: Ie36292b87c4d07fc18643cc91f93be00c577b2ca
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355616
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Mike Reed <reed@google.com>
- Remove unexplained special case where we avoided coercing uints to int
- Replaced the set<int> of case values with SkTHashSet<SKSL_INT>. Should
be faster and more type-correct.
Change-Id: I3286bd50253cc7a1ff6a550dc429bdfebbb8d8c5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354670
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 Inliner likes to move function bodies around; after inlining, code
can inadvertently move upwards, above ProgramElements that the code
relies on. We work around this by always emitting functions last.
Change-Id: Ie5486cc3a79a478920342fb9f578d575486fb4cf
Bug: skia:11186
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354669
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This was originally added so that some clients (BitmapFactory,
BitmapRegionDecoder) could ignore exif for backwards compatibility, and
others (ImageDecoder) could respect it.
With the addition of NDK APIs for decoding all frames of an animated
image, hwui/ImageDecoder will handle compositing frames, including
handling the orientation, so it may as well always handle it.
This removes tests for SkAndroidCodec that are no longer applicable.
Android already has tests for most of them.
AndroidCodec_sampledOrientation is recreated in
ag/Ieda439910ae52e609f0710d424503616d99ae5c7.
Change-Id: Ibd280986892176f284895d543f2f50bca22d196b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344763
Commit-Queue: Leon Scroggins <scroggo@google.com>
Auto-Submit: Leon Scroggins <scroggo@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
This enforces an even stricter version of the rules from GLSL ES 1.0
Appendix A, Section 5. Essentially, indices (to arrays, vectors,
matrices) must be made of literals, loop indices, and expressions made
of those two.
Bug: skia:10837
Bug: skia:11096
Change-Id: I437a5ed64da58e24d5991ddbde68859f5214e98b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354665
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
As far as I know, there shouldn't be a way to introduce a struct or enum
other than at global scope; the keywords are not accepted inside a
function body. In fact, I wasn't able to find a way to exercise these
code paths in practice. But we now have concrete assurance that any
possible type can be cloned into a symbol table safely; all Types are
either built-in (available everywhere by design) or are clonable.
Change-Id: I4b006b6cab995b3e598b683736ab9689828629c9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354664
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The inliner needs to clone Types from one SymbolTable to another when
cloning blocks of code. However, it seems like a poor division of
responsibility for the inliner to need to know how to clone every Type
correctly. It makes more sense for this logic to exist within the Type
class itself.
Change-Id: I4a383d5e22d5084eb35992a0b5c24865d2030282
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354662
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 97c476ecb7.
Reason for revert: tree breakage - Test-Debian10-Clang-GCE-GPU-SwiftShader-x86_64-Debug-All-SwiftShader
Original change's description:
> Disable the MSAA atlas mode for CCPR
>
> We have a long term path rendering plan that uses dynamic MSAA instead.
> This CL is a test to see if we can drop support for CCPR now.
>
> Change-Id: I1bff3ca3143a6b453b65a7932a1805c195922805
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354036
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
TBR=egdaniel@google.com,bsalomon@google.com,csmartdalton@google.com
Change-Id: I9f472807f743b8ddfee92800ee3b62609f2d4717
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354673
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: I62755dfcf5c8bb5bd7e0cc742e675aacf4afbb78
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354658
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We have a long term path rendering plan that uses dynamic MSAA instead.
This CL is a test to see if we can drop support for CCPR now.
Change-Id: I1bff3ca3143a6b453b65a7932a1805c195922805
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354036
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This CL limits the availability of functions like "MakeLiteralType" and
"MakeScalarType" to the BuiltinTypes class. This allows us to know,
definitively, that whenever we encounter these types, they are part of
the BuiltinTypes class and available anywhere in the program without
needing to be cloned.
The remaining MakeXxxxxxTypes in SkSL::Type are constructed dynamically
and injected into child symbol tables during parse/IR generation. These
types must be treated with special care when being moved or cloned:
- Arrays (MakeArrayType)
- Structs (MakeStructType)
- Enums (MakeEnumType)
Change-Id: I5490d6739c2a5ffdd54195f5a0b9b5be05d07953
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354878
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
No code or functionality changes.
Change-Id: I24317767570ae9ebbfea56f873d98709fb22d8b9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354876
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 inliner discovered that when a binary expression is inlined, its
type is not cloned into the destination's SymbolTable. This meant that
when the inlined-from function was later dead-stripped, the type pointer
would become dangling. Did a quick pass over inlineExpression and
inlineStatement and ensured that types are always copied.
Also found that `copy_if_needed` was making a copy of eligible types
each time one was encountered, instead of making one copy and reusing
it. This is fixed as well.
Change-Id: Iee3259ab038dfb04034bf0110af1909ccffec3de
Bug: oss-fuzz:29444
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354219
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Now that SkImage_GpuYUVA stores a GrYUVATextureProxies it supports
encoded origins.
Modify wacky_yuv_format GMs to use different origins and remove
restriction in SkImage::MakeFromYUVAPixmaps.
Bug: skia:10632
Change-Id: I02477d592b7baba164944d629eeac48223698c10
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353623
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Change-Id: I93d580e67cb66d388bd66b280ff229cb79e5b154
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354218
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Unlike TriangulateSimpleInnerPolygons, this new method *always*
succeeds. It works by introducing a concept of "breadcrumb" triangles.
The breadcrumb triangles serve as a glue that erases T-junctions
between the outer curves and inner polygon triangulation.
This CL also introduces a test to verify the breadcrumb triangles
actually guarantee to eliminate T-junctions. It works by cancelling
out shared edges in the combined breadcrumb triangles and inner
triangulation, and then verifying that the resulting edge list is
bit-for-bit identical to the outer edges from the original polygon(s).
Bug: skia:10419
Change-Id: Iea8ee2ca3701697246309dceb90e63279aabdfe4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353536
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Unsized arrays are now allowed in exactly one place: On the declaration
of an interface block. This satisfies the one existing use-case, which
is the gl_in (sk_in) declaration for geometry shaders. There is no other
useful scenario, and most of our backends don't support them anyway.
Several spots were using less strict checks when attaching sizes to
arrays, allowing for zero or negative-sized arrays, so those are all
fixed now.
The existing tests that initialize arrays are still a problem, because
Metal doesn't support that (neither does GLES2). Also, ArrayConstructors
has gone from generating an error in the Vulkan backend, to invalid
SPIR-V.
Bug: skia:11013
Bug: skia:11127
Change-Id: Ib08dfe9aeec96bf605661665d6f166419d27e8bc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353817
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: Ia3ac338bef376aa1649569b9ebd3f7feb23ffd52
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353936
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
These were meant to make it easier to pass constants to skvm::Builder,
but I think they've caused more problems than they solve.
This change is pretty much just mechanical, expanding all the existing
combinations of F32/F32a into (more) combinations of F32/float, and same
for I32/I32a/int. In general for an N-ary function we'll end up with
2^N-1 variants (1->1, 2->3, 3->7), requiring at least one I32/F32.
As before, some functions have fewer variants where using a constant
doesn't actually make sense (the condition for select, the denominator
for divide, etc).
Change-Id: Ic7bbd82e7f1452d604479cdde0b3b6a60ffe8b8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353839
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This better matches GLSL's type coercion behavior.
Change-Id: I73fcfd8a9e57fd4cdb1692074d73ebd8fb788ac2
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353712
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Literals are still flexible; we still allow `1` to coerce to float.
However, we no longer accept code like `int x = sqrt(2);` or
`int x = 0; float y = x;` without an explicit cast.
Change-Id: Ieb294a4877447e2336252f876e8bc489d1e4a59a
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353417
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: Icd8982d604881effee31cc1392e2717cb112d06d
Bug: skia:11172
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353629
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:10680
Change-Id: Ic77f7355866363ef476a93d8da180cf53207fa6d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353707
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
It's not sound to pass undefined (skvm::NA) values into select(),
but this is working today because the F32a argument is 'fixing' it.
The first time through this snippet updating fReturn value,
int i = 0;
for (skvm::Val& slot : currentFunction().fReturnValue) {
slot = select(returnsHere, f32(val[i]), f32(slot)).id;
i++;
}
the call to f32(slot) creates an F32{builder, NA}. We pass that to
select() and that argument's F32a(F32) constructor, resulting in
F32a{builder, NA, 0.0f}. Then when we need that as an F32, we resolve
it as splat(0.0f) because the F32a's id field is NA.
In short, best to remove F32a. :)
Added some SkASSERTs that would have caught this.
Change-Id: I67324cf20ad39ca555e69b9c407f379d14046043
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353838
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
I'm looking to phase out I32a/F32a, and rewriting this expression to
avoid select() makes it easier, making these types unused except in
SkVM.{h,cpp}.
There's no particular reason beyond making that refactor easier to do
this: SkVM can convert select(cond, splat(1), splat(0)) into cond & 1
itself, and once I'm done with removing I32a/F32a, if we prefer select
we should be able to rewrite this back as
dst[i] = skvm::select(i32(src[i]), 1, 0);
Change-Id: I562a112e54fdc2578802db02f6754c64a12798cc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353837
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Previously, `writeVectorConstructor` did not consider boolean types at
all when converting scalars to a different type. Now, this code reuses
the existing logic from `castScalarTo(Float|SignedInt|UnsignedInt)`
which supports Booleans. Added `castScalarToBoolean` to cover going in
the opposite direction.
Change-Id: I5479ab181b9b721db7fbff0bdc01718ce8f9f9b9
Bug: skia:11171
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353625
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I4eda7ffb010f236b39750da6134fb8db6fc26380
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353836
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This revealed a gap in our SPIR-V scalar constructor support;
typecasting a number to bool would lead to an ABORT.
Change-Id: Idac6d7ba34adfd214ed3cad8139e22d7170456f0
Bug: skia:11172
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353628
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: Ibb9dca7b8e04485924f6a7d8dd4b17df90d5f320
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353779
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: I350a6768ac124362b0d3e0f17e7a026265acf804
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353627
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
These need to change because type coercion in SkSL is about to become
more strict in a followup CL; we are disallowing expressions that mix
ints and floats without a cast.
Change-Id: Iff5e2820806b9419afdfcbf25d4a7f96f2eeeccb
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353416
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The test diffs look scary, but the only actual change is a minor
renumbering of IDs. The actual logic is the same.
Change-Id: I5ecc26c8581a4c01834932ff0291deba7d9e4618
Bug: skia:11171
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353622
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Such loops must be unrollable, so that's what we do.
Bug: skia:11094
Change-Id: I1b34917b6f2d015ae7867415d0120a5df0ffd618
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353619
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This is a reland of b60255033d
Original change's description:
> Push SkYUVAInfo into GrYUVToRGBEffect.
>
> Wrap up SkYUVAInfo and proxies into new type GrYUVATextureProxies.
>
> Bug: skia:10632
> Change-Id: Ic907d78a1a40af3c8ef838021749839c422d62dc
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353042
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Jim Van Verth <jvanverth@google.com>
Bug: skia:10632
Change-Id: I1878609153e3fc763620cb71a85d3b012f915155
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353621
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:11094
Change-Id: I68a08e79d29579901b74daca3c22f5112fbb3c8c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353356
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This makes it easier to see which shaders do not have an SkVM
implementation (tricolor for drawVertices, perlin noise) and lets us
reserve the default onProgram() for other uses, like mutually recursive
F32/Half onProgram(), just as we've done previously for color filters.
I've marked both tricolor and perlin noise as "TODO?" rather than just a
simple "TODO", mostly because they're both intriguing to handle in other
ways, perhaps in externalizable SkSL.
Change-Id: I6ccd14a85dee1519b10d53ecbfc1074916954eca
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353319
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Bug: skia:10419
Change-Id: I6e331a10622a4e2d17442910f9ed6b3af92614e2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353337
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This reverts commit b60255033d.
Reason for revert: GM needs fix for abandoned context
Original change's description:
> Push SkYUVAInfo into GrYUVToRGBEffect.
>
> Wrap up SkYUVAInfo and proxies into new type GrYUVATextureProxies.
>
> Bug: skia:10632
> Change-Id: Ic907d78a1a40af3c8ef838021749839c422d62dc
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353042
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Jim Van Verth <jvanverth@google.com>
TBR=jvanverth@google.com,bsalomon@google.com
Change-Id: Ia5a1121ed388ad04ef86121a3f7905772316a200
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10632
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353618
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: Brian Salomon <bsalomon@google.com>
Note that GLSL accepts these sorts of constructors natively, but Metal
and SPIR-V do not. In the generated IR we actually add a cast for
subexpressions where the type does not match. These casts can be seen in
the final output for both GLSL (where they are no-ops) and Metal/SPIR-V
(where they are essential).
This change exposed some missing SPIR-V functionality (vector casts do
not support bool types). This can be fixed up in a followup CL; these
casts were previously disallowed by SkSL entirely, so there won't be any
of them in existing code.
Change-Id: I54ae922e91b38bed032537496428747a081dc774
Bug: skia:11164, skia:11171
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353576
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
... now that bitmap
- converts to image with just bitmap.asImage()
- canvas *always* converts bitmaps to images before they are drawn
Bug: skia:10037
Change-Id: I24292f62e0fd072b3b810d974d0fe5c6d9b9a68d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353582
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Wrap up SkYUVAInfo and proxies into new type GrYUVATextureProxies.
Bug: skia:10632
Change-Id: Ic907d78a1a40af3c8ef838021749839c422d62dc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353042
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Change-Id: I9859081a14b110731f943e09fdd94dc10e0c9dfc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353580
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This is a reland of c0315a72d9
Original change's description:
> Scrub memory released from a GrMemoryPool in debug mode.
>
> In debug mode, we now overwrite released memory with 0xDD. This is
> intended to make use-after-free errors easier to catch while debugging.
>
> Change-Id: I04a4c5abcfef5f3f604a2430da15a8b5125239af
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352956
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: Id79cadb196868b467fb6d9107eb313bdc147b4f0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353419
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
On x86_64 we cap out at 6 arguments for JITing, but on arm64 we support
up to 7. This change avoids the assert when running SkSLInterpreter
unit tests that do hit 7 arguments.
Cq-Include-Trybots: luci.skia.skia.primary:Test-Android-Clang-Pixel-CPU-Snapdragon821-arm64-Debug-All-Android,Test-Android-Clang-Pixel-CPU-Snapdragon821-arm64-Debug-All-Android_ASAN
Change-Id: I17383679fb9bce2e4ce052bcac3c694e5af77124
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353537
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This adds basic operator overloading (everything but array indexing) and
related tests to the SkSL DSL.
Change-Id: Ic9fdc9a02a5496e2706d18fb435d838b4ee53ad7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353103
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Previously, we blindly trusted that the scalars inside a compile-time-
constant vector Constructor would be of matching type. (e.g. the scalars
inside a half3 constructor would always be of type half) This *should*
be true if we've done all our type-checking homework correctly, but we
would ABORT if anything went wrong. Now, we are less trusting and will
actually verify types and do the right thing if the scalar inside turns
out to be an int somehow. In practice, there's no real downside to
doing the conservative type-checks.
Change-Id: If0a255eb96a0ccfec7d0a261caad542cae93d078
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353556
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>
I started unpacking the mechanics of type coercion, and realized that
the second half of the function was looking up the Symbol for a Type
based on its name (Types are already Symbols), converting that Symbol
back into a Type (we started with a Type anyway), wrapping that Type
in a TypeReference, then calling that TypeReference (which always
calls convertConstructor).
This CL cuts out the middle steps and simply calls convertConstructor
directly. A test was added to confirm that an earlier error encountered
on the CQ is no longer occurring.
Change-Id: I76aae455a301afe4e67ef989d9dfe11f47ed36ae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353105
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
No changes expected -- only if chrome decides to opt into nearest
Change-Id: Ie493b6c814f3e1e603a116e59a777e5f6b16b370
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353358
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>
This reverts commit 9b0fc9c9c1.
Something ain't right here, so clear it all out to start fresh.
Change-Id: I22d1b4285db415ebd360c61fc5ecf890aa193e51
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353296
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Bug: skia:10419
Change-Id: Ib517c52bf2ed9e7dc9d1fa8491dd39dae99a6f77
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350492
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Bug: skia:10419
Change-Id: I43fc724de1476cbfda95b9ddd3611ec2ec547a37
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352900
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This is a reland of e931b838d3
Original change's description:
> Fix issues with MTLPixelFormatBGR10A2Unorm on older OSes.
>
> MTLPixelFormatBGR10A2Unorm isn't supported on older MacOS or iOS
> versions, but we still want to build those. This CL adds some
> availability checks, and works around array initialization by using a
> internally defined constant.
>
> Bug: skia:11160
> Change-Id: Ife04b0a467a5e0aa27f081cabb1936c5771b5679
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352742
> Commit-Queue: Jim Van Verth <jvanverth@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
Bug: skia:11160
Change-Id: I0d39bab3edeeb50315251be68744ac71ef7c194e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353077
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This reverts commit c0315a72d9.
Reason for revert: breaks
Test-Debian10-GCC-GCE-CPU-AVX2-x86-Debug-All-Docker
Original change's description:
> Scrub memory released from a GrMemoryPool in debug mode.
>
> In debug mode, we now overwrite released memory with 0xDD. This is
> intended to make use-after-free errors easier to catch while debugging.
>
> Change-Id: I04a4c5abcfef5f3f604a2430da15a8b5125239af
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352956
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,michaelludwig@google.com,johnstiles@google.com
Change-Id: I78096b49fcc0781b33a49fc12aa7f657f2430fed
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353097
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Current release value is smallish (512) and debug is terribly
inefficient (5). Bump to 4K for both release/debug builds.
(I think the small debug value was meant to artificially stress Expat
streaming, but it seems unnecessary nowadays).
Change-Id: I459afce5dde9789da84baaa68c4db9164911ffa1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351200
Reviewed-by: Tyler Denniston <tdenniston@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
The new signature is more amenable to being called from outside code,
in particular the DSL.
Change-Id: I4c7650b71f02471f27e348b0c44abe8511691db1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353038
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Previously, the only way to detect a literal Type was to explicitly
check against the values fFloatLiteral/fIntLiteral or compare the name
against $floatLiteral or $intLiteral.
Now, you can call type.isLiteral() to see if the type corresponds to a
literal, or call type.scalarTypeForLiteral() to promote to the
equivalent non-literal type (fFloat/fInt). Calling
type.scalarTypeForLiteral() on a non-literal type is safe and will just
return the input type unscathed.
Change-Id: Ib939dda9bcd43b3ef4159211c5349d71fc857b95
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353037
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit e931b838d3.
Reason for revert: Failing tests on iOS bots
Original change's description:
> Fix issues with MTLPixelFormatBGR10A2Unorm on older OSes.
>
> MTLPixelFormatBGR10A2Unorm isn't supported on older MacOS or iOS
> versions, but we still want to build those. This CL adds some
> availability checks, and works around array initialization by using a
> internally defined constant.
>
> Bug: skia:11160
> Change-Id: Ife04b0a467a5e0aa27f081cabb1936c5771b5679
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352742
> Commit-Queue: Jim Van Verth <jvanverth@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
TBR=egdaniel@google.com,jvanverth@google.com
Change-Id: I3e98529c249e235b53cd4da8dea764d5b9b71ceb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11160
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353040
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
MTLPixelFormatBGR10A2Unorm isn't supported on older MacOS or iOS
versions, but we still want to build those. This CL adds some
availability checks, and works around array initialization by using a
internally defined constant.
Bug: skia:11160
Change-Id: Ife04b0a467a5e0aa27f081cabb1936c5771b5679
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352742
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Bug: skia:10632
Change-Id: If4dd7779b0856f6d0b441381bf7f2f51527cdb9d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352497
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
In debug mode, we now overwrite released memory with 0xDD. This is
intended to make use-after-free errors easier to catch while debugging.
Change-Id: I04a4c5abcfef5f3f604a2430da15a8b5125239af
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352956
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This will allow errors to be reported outside of the IRGenerator more
easily (without passing around an ErrorReporter object).
Change-Id: I4bcb59fcd526599fa593fcb3b1de0a5ae64ab901
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352737
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This is the first test that used uniform data, so fix up how uniforms
work in the generic SkSL-to-SkVM function.
Bug: skia:11094
Bug: skia:11096
Change-Id: Ie391c1a6b8b68f0f4f014d7e767d7b5101341fab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352739
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:10419
Change-Id: I547e7c63b9d7ad9abeb6377f4a31d6d671bb5030
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350482
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Bug: skia:10419
Change-Id: I0394697fdc292cabeb81da508e1acdc74e2127ff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350257
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Bug: skia:10419
Change-Id: I6351513b365a190dd97312a0b3d436a44b8fb10a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351858
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Change-Id: I96b547de4fe4b73096fb26d0ef21a4e7555ca06a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352238
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:11127
Change-Id: I3a7fda8bb62f9cf9b6c83441703f537e75461d07
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352509
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Previously, these were in SkSL::Context directly. This change doesn't
remove them from the context entirely, but it gives them a dedicated
subclass and firewalls them off from the rest of the context.
Change-Id: I0c344bf7436a11b8494a5fe7542d0a4ef1ece964
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352502
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This currently leaves ARM devices as the only devices actively using
this feature.
Bug: skia:10979
Change-Id: Icf86b3f4ea12092d2c21b1fd8ffd37444e4a57f7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352501
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Generates a bounding rect that encompasses both shadows for a given path,
relative to the path local space.
Bug: skia:11146
Change-Id: Ib8edb1072ed9cbfe36285523330be95fdf661d6b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351922
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Yegor Jbanov <yjbanov@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This doesn't change any logic, just makes the IR generator a few
hundred lines shorter.
Change-Id: I92010191ee9283c33499c819d65fc85913f25824
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352121
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This optimization doesn't perceptibly improve the generated code; it
just replaces a binary expression with an equivalent unary one.
Change-Id: Ib6cd2732a22c26978665c57ee00d7b5e5d0a0aee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352123
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Is not used by a major client and can be implemented using runtime
effects for users who want this noise.
Bug: skia:10536
Change-Id: Iaa06e6e1406b808c7f8dc0f76621fecf2becabf5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352057
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Swizzle::constantPropagate is a limited form of this optimization:
https://osscs.corp.google.com/skia/skia/+/master:src/sksl/SkSLCompiler.cpp;l=1159
It predates this optimization code, though. It also flattened out an
unnecessary constructor that would have otherwise required an extra pass
to eliminate.
Updated the optimization code at L1159 to simplify away the unnecessary
constructor where possible, and then removed Swizzle::constantPropagate.
This has no effect on generated code.
Change-Id: I0f43d5c51761965230c853f309a6ef068f9aef77
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352120
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
By making the DDLTask admit that it is using its ddlTarget
we no longer need this tracking in the drawing manager.
Change-Id: I1696ee6c089e4c21abf0781eb7e3b0fa78b3dbb4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352176
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
With repositionable DDLs the surface proxies they target need
special handling (i.e., their backing resource size cannot
be known ahead of time).
I'll follow this up with a CL that removes the DDLTarget system from
the GrDrawingManager. AFAICT it is not longer needed.
Change-Id: I0d9189b94726fdf356d54c16de32d7e52e0d1451
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352116
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
- fix a TODO to use shared decomposeScale
- return null rather than empty shader on degenerate input (more clear)
Landing this *before* trying to rebaseline chrome's images
Change-Id: I621eaff4dd3d958a1555c470578550483e9eb760
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352257
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>
This is a reland of b05571f0b8
Original change's description:
> Disable tessellation when we don't have indirect draw support
>
> Avoids the polyfill that uses looping instanced draws. This has led to
> perf regressions on android.
>
> Bug: skia:11138 skia:11139 chromium:1163441
> Change-Id: I129bf96c6d8a3eaadc79bfca496c7d50189f737e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350738
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
Bug: skia:11138 skia:11139 chromium:1163441 skia:11152
Change-Id: I416dfd1f85aad78786a31ca6390ff40bbe906a19
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352095
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This makes them reusable for the AA triangulator when it gets its own
file.
Bug: skia:10419
Change-Id: Ibf77b9a77aabc57898c0526b3a6c02ab9c46e2cc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351876
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
This tripped us up at least once recently.
Change-Id: I902759929e359ce639557bb766b0c6593badc07b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352093
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Bug: skia:10419
Change-Id: I1d19ff225666d2dd53ff6c0b48eab635d717ed68
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351857
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Bug: skia:10680
Change-Id: I8697bdc157d250f3c390c7f49074318aa8c7bdab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351918
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Now that BoolLiterals can be constructed with a Type*, there's no need
to pass the context through.
Change-Id: Ic2003aa03f5c2428e73e81c95eb12c862700554a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352025
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This CL also centralizes how the SkSurfaceProxy is output.
Change-Id: Ibdba1535e65ef21ce206778a8d757ee341334ec0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352081
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
We were dropping the modifiers when re-emitting the function
declarations. We rarely noticed this, thanks to aggressive inlining in
our existing tests. Upcoming changes to the test suite (to disable
inlining) exposed this bug.
Change-Id: I773d5ad6f7694d4a491be525c05b730794216e43
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352039
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This makes almost all existing code read more clearly.
Change-Id: I314331d9aa2ecb4664557efc7972e1a820afc250
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352085
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Change-Id: I1168871f1c5765f47814f81f729b4fbaf2b7a3e4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352086
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Change-Id: Ia49b9dceccb83f8316698ba13d0920a988335010
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352040
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>
In the referenced bug, we encounter a bad op
(drawPoints but null for the points array).
We detect this (reader has the error) but we don't actually stop, so
we call the draw method with bad params.
Bug: oss-fuzz:29115
Change-Id: I73273d49e8b21251f0c9f367e6960d907e5a314c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352037
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>
Change SkBase64::decode interface completely and better document
how to use. Unfortunately there are users of ::decode (and they are
leaking) so will need to keep the old interface until users can be
updated.
Change-Id: I214b771136d78fef758c5d0d9ec302f956f6e4f0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351201
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Change-Id: I0bbda6a41391fc2a11dc812be5e9c0c0d14c4d75
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351921
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This CL enables in Vulkan a system similar to render to texture for GL
MSAA. This can have rendering artifacts if we break up the rendering to
an MSAA surface into two render passes since when we load for the second
render pass it fills all the samples with the resolved color from the
first render pass instead of just the covered samples. These artifacts
exists in the GL version as well. Locally we are seeing perf gains up to
2x factor which justifies the artifacts of using this technique.
Additionally, when we turn on reduce oplist splitting this will decrease
the times where this is even an issue.
This enables it for all devices, but most likely we will only see
improvements on tilers and will end up disabling this for desktops.
Also includes some minor fixes.
Bug: skia:10979
Change-Id: Ic7c3000e3ebed9f4a6351ab8f5637be9ee0194ff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344964
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
The CFG/definition map are no longer valid after replacing an expression
entirely. Swizzle-of-swizzle optimization was another case where the
optimizer would replace an expression wholesale, but failed to set the
needs-rescan flag.
Change-Id: Ida0363d738cd1d3ac2a48c824aa04065a7ca16b7
Bug: oss-fuzz:29085
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351776
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I4a130b0c776888587122a7ae25e1b32b10011d55
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351499
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
The code at this point doesn't do anything useful, but establishes some
of the basic types and patterns.
Change-Id: I580a9e75ffa3162879893450fb7d1f0905a10687
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350697
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I75f907ca673ee67f5d623b032128b97833070a0b
Bug: skia:10931
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351504
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
All errors need to be caught earlier, so turn any remaining TODO items
into asserts.
Bug: skia:11127
Change-Id: I6731f947233522df6397b3444c26d5ebc417c431
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351506
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This will flatten out expressions such as `!false` or `!true`. We
already had a similar fix-up at IR generation time which handled simple
cases, but this will catch more complicated ones like `!sk_Caps.xxxxx`
(since caps bits are only flattened out at constant propagation time).
Change-Id: I04282809d9a784266a64dbcafd097f3b0662806c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351497
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>
Same basic deal as skcms.
This new GM tests our treatment of color spaces as sources is consistent
with our treatment of color spaces as destinations. It looks good to me
now, and I have tested that this GM catches a "well-placed typo" in each
of the three implementations modified here.
Bug: chromium:1144260
Change-Id: I3eabc93bbd65855c60006751f68c171ccdce9d94
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351336
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
I've made y an F32 instead of an F32a to remind us that it's
usually best to multiply instead of divide by a constant.
Change-Id: I15c9ab50e51f5011eca977908168ed27f1de25b0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351337
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This reverts commit 75887a2321.
Reason for revert: Flutter crash
Original change's description:
> Move GrTriangulator internal struct definitions to the .h file
>
> This makes them reusable for the edge-AA code when we move it.
>
> Bug: skia:10419
> Change-Id: I20042a419617717214535d45fc92a8cae986fb33
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349356
> Reviewed-by: Jim Van Verth <jvanverth@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
TBR=jvanverth@google.com,robertphillips@google.com,csmartdalton@google.com
Change-Id: I7e317b01883afc9eab494f1a0846ece9e3272ec5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10419
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351477
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This is not actually necessary now that constantPropagate can fully
flatten out unary negation into its constant operands. The compilation
results don't change at all.
Change-Id: I7ab55bd3720413609d799dd866e1703973cb2626
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351202
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>
Currently this doesn't actually handle going over the memory
budget, but it does carve out space to do that later. This algorithm
can't handle everything yet but I want to get it landed for
more iteration as long as it's disabled.
Bug: skia:10877
Change-Id: I37942172345e8cfd6fc2c591a3788a10652377da
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345168
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Auto-Submit: Adlai Holler <adlai@google.com>
Previously, we used a union of (int64, float) to store the cached value.
However, the union-based code turned out to be more complex than just
type-punning the float's bits to an int via memcpy (which we needed to
do anyway). The union-based approach was also likely to be UB by the
letter of the law--we were creating float keys by storing into fFloat,
then checking the map by comparing equality on fInt.
Change-Id: I62c7ff4b5fab8bb1be8836c23f746ef254053b6f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350957
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This fixes SPIR-V code generation when encountering nested constructors
like `float3 v4 = float3(float2(1), 1.0);` as featured in our unit test
VectorConstructors.sksl.
Change-Id: I3a0c4b466b3cb17ba50bd264f899e59c55c768ed
Bug: skia:11141
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350032
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Like getIVecComponent or getFVecComponent, this retrieves the n'th
element of a Boolean compile-time constant vector. This will be used in
followup CLs.
Change-Id: Ib41c9c89cb773251e4c0d6cdcaea0437d8074e48
Bug: skia:11141
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350918
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit b05571f0b8.
Reason for revert: Android crash
Original change's description:
> Disable tessellation when we don't have indirect draw support
>
> Avoids the polyfill that uses looping instanced draws. This has led to
> perf regressions on android.
>
> Bug: skia:11138 skia:11139 chromium:1163441
> Change-Id: I129bf96c6d8a3eaadc79bfca496c7d50189f737e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350738
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
TBR=bsalomon@google.com,csmartdalton@google.com
Change-Id: Iebb473211ff618c1988aa5b7306e3e39efa353f5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11138 skia:11139 chromium:1163441
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351216
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This will be used in a followup CL to implement getBVecComponent. At
present it's not called.
Change-Id: Idd6f18314d0835af3946ea7458e6650384f505ea
Bug: skia:11141
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350703
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously ExternalValues were flexible, and could be used as raw values
(with the ability to chain access via dot notation), or they could be
callable. The only non-test use-case has been for functions (in
particles) for a long time. With the push towards SkVM, limiting
ourselves to this interface simplifies things: external functions are
basically custom intrinsics (and with the SkVM backend, they'll just get
access to the builder, and be able to do any math, as well as
loads/stores, etc).
By narrowing the feature set, we can rename everything to reflect that,
and it's overall clearer (the SkSL types now mirror FunctionReference
and FunctionCall directly, particularly in how they're handled by the
CFG and inliner).
Change-Id: Ib5dd34158ff85aae6c297408a92ace5485a08190
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350704
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This reverts commit bef02dca9d.
Reason for revert: PreAbandonGpuContext failing on tree
Original change's description:
> Fix GPU improved noise impl and add to perlinnoise GM.
>
> GPU was recently busted when switching alpha-color type swizzles from
> aaaa to 000a.
>
> There was no GM that exercised SkPerlinNoiseShader::MakeImprovedNoise.
>
> It draws wrong before and after this change with the CPU backend.
>
> Bug: skia:10536
> Change-Id: I514e304d022fcccae80699a99facafa8ce947e9f
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350916
> Auto-Submit: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
TBR=egdaniel@google.com,bsalomon@google.com
Change-Id: Iac635028b402e6008e3a8050bdfa66052d94fd10
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10536
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350956
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
GPU was recently busted when switching alpha-color type swizzles from
aaaa to 000a.
There was no GM that exercised SkPerlinNoiseShader::MakeImprovedNoise.
It draws wrong before and after this change with the CPU backend.
Bug: skia:10536
Change-Id: I514e304d022fcccae80699a99facafa8ce947e9f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350916
Auto-Submit: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Continue to test everything with the ByteCode interpreter, and run most
tests with the new SkSL-to-SkVM utilities, as well. A few tests rely on
features that aren't yet implemented (function calls, looping), and some
of the bespoke tests (that don't use the test() helpers) use even more
exotic features that need to be implemented or disallowed in the IR
generator. This is getting us closer to not needing ByteCode at all,
though.
Refactored a bunch of the helper code to reduce copy-paste among the
many different 'test' functions.
Change-Id: I138d4a24266f2d862742245c5ee895d86c01018e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350560
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Bug: skia:11134
Change-Id: I0b0a3f530452d5bb5c08da7f247728298c234dd7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350583
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Avoids the polyfill that uses looping instanced draws. This has led to
perf regressions on android.
Bug: skia:11138 skia:11139 chromium:1163441
Change-Id: I129bf96c6d8a3eaadc79bfca496c7d50189f737e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350738
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
We've been assuming that all Ops with the same arguments produce the
same value and deduplicating them, which results in a simple common
subexpression eliminator.
But we can't soundly dedup two identical loads with a store between;
that store could change the memory those loads read, producing different
values, as demonstrated by the first new unit test.
Then, by similar reasoning, it may first seem fine to deduplicate
stores, e.g.
store32 arg(0), v1
store32 arg(0), v1
That second store certainly does look redundant. But if we slot a
different store between, it's no longer redundant:
store32 arg(0), v1
store32 arg(0), v2
store32 arg(0), v1
If we dedup those two v1 stores, we'll skip the second and be left with
v2 in our buffer instead of v1. This is the second new unit test.
Now, uniform32 and gather ops also touch memory... are they safe to
dedup? Surprisingly, yes! Uniforms are easy: they're read-only. No
way to store to uniforms, so no intervening store can invalidate them.
Gathers are a little fuzzier, in that the buffer we gather from is
uniform in practice, but not strictly required to be... it's not
impossible to construct a program that gathers from a buffer that the
program also stores to, but you'd have to go out of your way to do it,
and it's not a pattern we use today, and SkVM does not provide the
synchronization primitives you'd need to make attempting that even
vaguely sensible. So gathers in practice can also be deduplicated.
In general it's safe to dedup an operation unless it touches _varying
memory_, i.e. loads and stores. uniform32 and gathers touch
non-varying memory, so they're safe, and while index is varying, it
doesn't touch memory.
Change-Id: Ia275f0ab2708d3f71e783164b419436b90f103a9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350608
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
I noticed is_always_varying() is a little wrong, and this new test demos
how. This isn't terribly important: in most practical situations
gathers will indeed be varying.
Change-Id: I456d4c7287147726c49ebb5af5af347c65cd21d4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350602
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Many of our shaders generate the same vector constant dozens of times,
e.g. Gaussian blur uses float4(1) repeatedly. This change avoids
re-emitting redundant vector constants.
Change-Id: I22a71cd8b2783fb997f52d485b49031f64ca6d96
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350701
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Also explicitly default all special methods of SkStrikeCache to get
nicer warnings.
Change-Id: I8856a905c6075ca52d4ad900ccd0dacc581cd485
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350637
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This makes them reusable for the edge-AA code when we move it.
Bug: skia:10419
Change-Id: I20042a419617717214535d45fc92a8cae986fb33
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349356
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
and update GrGLGpu::flushScissorRect's signature to match it.
Change-Id: I9082003934168306833f86d310b942a8d7527384
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350612
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Previously, we had constant-value deduplication, based on the SkSL type
of the constant. However, we were still generating redundant constants,
because we would emit a separate constant for Float(n) and Half(n), or
Int(n) and Short(n), even though we generate the exact same instruction
for these constants. We now deduplicate based on the type's number-kind,
separating constant literals into three categories: floats, signed ints,
and unsigned ints. This better matches our type-handling in
getActualType.
Change-Id: I5777d4b3d567839b7aa72dc8de76908c18fc387e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350031
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This CL adds additional variants of some IRGenerator functions to enable
them to be more easily called from upcoming DSL code. This should not
change functionality (other than a slightly different error message in
one case).
Change-Id: Ic727c194fca5111d4283007f4ec23653598a853b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350017
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
I plan to add at least one more gr_* class, so I figure it makes sense
to just keep all this similar things in one place.
Bug: skia:11136
Change-Id: I96d24dd094731e9694201e012fec37926cce564d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350639
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
The pattern appears to be that the variable that stores the uploaded
value has a "Prev" suffix.
Change-Id: I78ea449d75f5fd091c113cf08af919331467eb8d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350638
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Bug: skia:10979
Change-Id: I81b8692b0bb1582b60a9324492a782d04a09538f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344963
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
IntLiteral and FloatLiteral each had a pair of public constructors: one
which takes a Type (what type of Int/Float is this?), and another
constructor which takes a Context (assume the generic Float/Int type).
BoolLiteral had a public Context-based constructor, but its Type-based
constructor was private. The Type-based constructor is now public to
match its siblings. This allows us to create Booleans without needing
a Context&, if we have an fBool_Type, and fixes make_unique (which
doesn't like private constructors).
Change-Id: Iffb4980947a73b3a89aa1452cc90dce63620bd67
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350636
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: Ia4a1c38161046b94dc56a1a76704766f1e14aab7
Bug: skia:11131
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350019
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, the IR generator had code which could simplify conversion
constructors like `int(1.23)`. Separately, the optimizer's constant
propagation pass had its own separate implementation of these
simplifications as well.
This CL unifies the two implementations. Previously, the constant-
propagation pass version of the code only supported integer literals, so
this change also improves our code generation slightly.
Change-Id: I32c70a5f2aed210d03bef3166b1178a2d40cdabd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350024
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 237911a4d8.
Reason for revert: timeouts
Original change's description:
> Add more comprehensive test for GPU write pixels.
>
> Similar to existing SurfaceContextReadPixels but for writes. Tries all
> combinations of src/dst color type and alpha type for write pixels.
> Always reads back pixels for verification using the ImageInfo of the
> tested surface context.
>
> Bug: skia:8862
> Bug: skia:11130
>
> Change-Id: Id01f6aa511f00c4be47c32746dca872368cd5d82
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348886
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
TBR=egdaniel@google.com,bsalomon@google.com
Change-Id: I5498be0b20604e520ad887898695a81ca82936ca
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:8862
Bug: skia:11130
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350559
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
If using discardable msaa surfaces in vulkan, when we break up a
render pass for an inline upload, we must do a load msaa subpass for
the second render pass. However, if the original render pass did not
have this load subpass (e.g. clear or discard load op), then all the
GrProgramInfos for draws that end up in the second render pass will
have been recorded thinking they will be in a render pass with only 1
subpass. Thus we add an override flag to the makeDesc call to force
the actually VkPipeline that gets created to be created using a render
pass with 2 subpasses. We do lose the pre-compile abilities with this
approach, but inline uploads are very rare and already slow.
Ideally we would find a way to pull inline uploads into their own
GrRenderTasks so all this could be known ahead of time. But currently
we allow uploads to happen between draws of the same Op so it makes it
difficult to break up and even know when they'll occur at record time.
Bug: skia:10979
Change-Id: I247b007813dd0be280ce8a72fc6af70fe21f082d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344962
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
We rely on exact output from SkStringAppendScalar in our
Housekeeper "Generated Files" bot. However, we can't trust snprintf(%g)
to emit the same string for infinite and NaN values on every platform.
For instance, the bits 0xFFFFFFFF as a float are `-nan` on Linux and
`nan` on OS X. Infinity is represented as `inf` on Linux/Mac and
`1.#INF00` in Visual C++.
This CL standardizes on the strings `inf`, `-inf` and `nan` across all
platforms. This solves a GeneratedFiles issue in the followup CL:
http://screen/5RVdSnLmBupzpja
Change-Id: I648fd32571f8300998ec427dcb3d1e7d7215dbdd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350496
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Similar to existing SurfaceContextReadPixels but for writes. Tries all
combinations of src/dst color type and alpha type for write pixels.
Always reads back pixels for verification using the ImageInfo of the
tested surface context.
Bug: skia:8862
Bug: skia:11130
Change-Id: Id01f6aa511f00c4be47c32746dca872368cd5d82
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348886
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This simplifies the argument lists for functions and will also allow us
to extract the AA logic into its own subclass.
Bug: skia:10419
Change-Id: If51e86a7633da7a3ee9352c0236258a0a21f2ebe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347976
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Bug: oss-fuzz:29183
Change-Id: I60475e2bda42e98b79a4291839249158d2738c9c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350028
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Previously `number(boolean)` casts were converted to a ternary during
IR generation, and `boolean(number)` casts caused an error.
Metal and GLSL should support this cast as written. SPIR-V needed a
little bit of logic to handle converting the boolean to a number via
OpSelect.
Change-Id: I0069781e2b5a26a25c8625ab41c2392342bfd10d
Bug: skia:11131
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349066
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The new unit test demonstrates load/store reordering is error-prone.
At head we're allowing loads from a given pointer to reorder later than
a store to that same pointer, and boy, that's just not sound. In the
scenario constructed by the test we reorder this swap,
x = load32 X
y = load32 Y
store32 X y
store32 Y x
using schedule() (following Op argument data dependencies) into
y = load32 Y
store32 X y
x = load32 X
store32 Y x
which moves `x = load32 X` illegally past `store X y`.
We write `y` twice instead of swapping `x` and `y`.
It's not impossible to implement that extra reordering constraint: I
think it's easiest to think about by adding implicit use edges in
schedule() from stores to prior loads of the same pointer. But that'd
be a little complicated to implement, and doesn't handle aliasing at
all, so I decided to ponder on other approaches that handle a wider
range of programs or would have a simpler implementation to reason
about. I ended up walking through this rough chain of ideas:
0) reorder using only Op argument data dependencies (HEAD)
1) don't let load(ptr) pass store(ptr) (above)
2) don't let any load pass any store (allows aliasing)
3) don't reorder any Op that touches memory
4) don't reorder any Op, period.
This CL is 4). It's certainly the easiest and cheapest implementation.
It's not clear to me that we need this scheduling, and should we find we
really want it I'll come back and work back through the list until we
find something that meets our needs.
(Hoisting of uniforms is unaffected here.)
Change-Id: I7765b1d16202e0645b11295f7e30c5e09f2b7339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350256
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Fixes terminology inside the generator to correctly use 'arguments'
rather than 'parameters'. Retains the caller-supplied argument span, so
that generateCode can push changes to 'out' and 'inout' parameters back.
Adds a useful helper function for getting a named function from a
Program.
All of this is used more extensively in upcoming CLs that migrate
interpreter unit tests (and particles) to SkVM.
Change-Id: I9d9aef4b7c4daf93d822e22f813162f4ed2385be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350023
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The code didn't take into account that x and y might be different types.
(This bug was not actually harmful; type coercion allowed the code to
compile even with the wrong type. The float would be silently splatted
into a vec and the rest of the code would work as-is.)
Change-Id: Ib76bc733f76304e451ef9197421b4bc22e29e49c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348888
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
