Other code outside this namespace will want to use this method soon.
Bug: skia:10419
Change-Id: Ib155f224866fd333b8f9a4b78e6c9e51ac0600df
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311936
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Adds methods to measure the rotation angles of quadratics and cubics,
and to chop curves at locations that divide the rotation angle in half.
Bug: skia:10419
Change-Id: I840e12034fc66e1a459de875fefda07a27a78335
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308880
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Now that approxOpCount() can recurse into sub pictures,
adding ~0 will overflow its total count.
New tests fail at head, pass now.
Change-Id: Icdb7d8a062739724207c9a6369734510f00f2ffd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311774
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
It's been replaced with a `uniform half4` in tests where it was needed,
which tends to generate similar code overall.
Change-Id: Iacc062efaeae37ebecf78505abe0fc563b69ceb0
Bug: skia:10619
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311099
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 original code assumed that the switch value and case value would
both be IntLiterals, and asserted if this were not true. However, in
sufficiently complicated cases, the switch value can sometimes be a
Constructor containing a scalar.
It's possible that there's a fix to constant propagation which would
flatten out this Constructor to just a scalar, but that has eluded me
for today, and this approach does seem to solve the issue. (Also, it's
surprising that the optimizer does not notice that it can simplify
`half4(1.0, 2.0, 3.0, 4.0).yyyy`.)
Change-Id: Id490b15c724dd174c448665a19242cc80cdce213
Bug: skia:10623
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311636
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I5b1baf2bc3aafa03c1d0cace4f7c09a09cf4a084
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311112
Reviewed-by: Julia Lavrova <jlavrova@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Allow the user of SkCustomTypefaceBuilder to set the SkFontStyle of the
resulting SkTypeface. This allows users to build font families.
Fix the Font_flatten test to actually work (instead of relying on the
magic behavior of nullptr for SkTypeface), add a test with the custom
typeface, and reduce the number of times the inner loop runs from
302,400 times to 4,032 times so that the test finishes in a reasonable
amount of time.
Bug: skia:10630
Change-Id: I0b5e939552ee4a9a1249eefbb7a7279a59b38e5a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311596
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Xiao Yu <xster@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
While writing test code for skia:10623, I encountered a separate bug in
the SkSL compiler. Specifically, the codegen would assert when trying to
emit a function that returned an int. There were several basic types
missing from `glsltype_string`. I've added them, and created a unit
tests to exercise a subset of them.
Change-Id: Ied6aaacc67f472998cb6673490c4d98ce98886d2
Bug: skia:10631
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311453
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>
Objects in the symbol table are intentionally constant. However, when
converting "ExternalValue" AST nodes into symbols via symbol table
lookup, IRGenerator::convertIdentifier was casting away constness
because ExternalValueReference held a non-const pointer. Fixing this
involved significant ripple-effect additions of "const" throughout the
ExternalValue class and its subclasses.
These changes generally appear to be benign, but one interesting edge
case is `ExternalValue::write`, which intuitively does not seem to make
sense as a const method. However, invoking `write` should not alter the
ExternalValue object itself; rather, it is intended to alter the
*external value* that is being referenced. (In practice, nothing invokes
write() anyway except for one unit test, which continues to pass.)
This issue was discovered while converting casts to `as<T>()` calls.
Change-Id: I8ff6a477e475833d2a99c72f1c79c766b57767ee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311276
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
These are very easy to mis-read, and they don't make much sense.
Swizzles should refer to the base expression at least once.
Change-Id: I4c2740c7439a11b51b2eb41263c6435e3aefad35
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310376
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Just wrapping the backend texture shouldn't invalidate any parameters.
Change-Id: I02859e519b2c8e9612533b2230da753d5fff97b7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/261084
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
- reset, findLastOf: these were entirely unused
- find: already existed in C++11's std::string, can just be inherited
- defaulted constructors: in C++11, these can be inherited directly from
std::string via 'using'
Change-Id: I1772ef04e49ab905aaada2ec38c1abeb9a8e26bf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311039
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
To my surprise, this even works with homegrown smart pointers (such as
SkTLazy).
https://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-smartptr-get.html
Find and remove redundant calls to smart pointer’s .get() method.
Examples:
ptr.get()->Foo() ==> ptr->Foo()
*ptr.get() ==> *ptr
*ptr->get() ==> **ptr
if (ptr.get() == nullptr) ... => if (ptr == nullptr) ...
Change-Id: I8ff541e0229656b4d8e875c8053a7e6138302547
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310976
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
https://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-string-compare.html
Find suspicious usage of runtime string comparison functions.
This check is valid in C and C++.
Checks for calls with implicit comparator and proposed to
explicitly add it:
if (strcmp(...)) // Implicitly compare to zero
if (!strcmp(...)) // Won't warn
if (strcmp(...) != 0) // Won't warn
Checks that compare function results (i,e, strcmp) are compared to valid
constant. The resulting value is
< 0 when lower than,
> 0 when greater than,
== 0 when equals.
A common mistake is to compare the result to 1 or -1:
if (strcmp(...) == -1) // Incorrect usage of the returned value.
Additionally, the check warns if the results value is implicitly cast
to a suspicious non-integer type. It’s happening when the returned
value is used in a wrong context:
if (strcmp(...) < 0.) // Incorrect usage of the returned value.
Change-Id: I001b88d06cc4f3eb5846103885be675f9b78e126
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310761
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Previously, the inner type was ignored.
Change-Id: I51d251fc38358ef889b5a3f85d5f2d23bd8cf4c5
Bug: skia:10615
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310657
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>
These tests verify that switches and enums only work with constant
integral values. Floats or uniforms should be rejected with an easy-to-
understand error message.
Change-Id: Ib634cb1ca1734a4b66ba53a3476e9ee539a63e3e
Bug: oss-fuzz:24889, skia:10615
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310396
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>
It's possible to construct a case value expression that's a compile time
constant, but fails to produce a value from getConstantInt. MSAN noticed
us using the uninitialized integer. It's now initialized, but also never
used in the failure case: We make getConstantInt return status, and give
better error messages in the two places it's used.
Bug: oss-fuzz:24889
Change-Id: I88e4e5b7bd1caeea1cf53f9b1d6f345dd8a5326f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310296
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Inlined return statements emit two statements--an assignment, then a
break. If scope braces are otherwise missing, the break statement will
end up in the wrong place. i.e.:
Mistranslated:
if (foo) return bar; --> if (foo) out = bar; break;
Translated properly:
if (foo) return bar; --> if (foo) { out = bar; break; }
Change-Id: Id992abf64ad09e6752f64c71cb556f05c868a453
Bug: skia:10607
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310177
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-override.html
Adds override (introduced in C++11) to overridden virtual functions and
removes virtual from those functions as it is not required.
virtual on non base class implementations was used to help indicate to
the user that a function was virtual. C++ compilers did not use the
presence of this to signify an overridden function.
Change-Id: If66d8919358f72a4035190caf8d7569268037a9a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310160
Commit-Queue: Mike Klein <mtklein@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
The new unit test SkASSERT()s a hash collision at head
(but passes in release builds) and does not SkASSERT()
with this patch to SkColorSpace.cpp.
Bug: chromium:1113865
Change-Id: Ibc05af4145a92bbd15c7d5e06ece9d269bd7a242
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310110
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We do not have a good mechanism to break out of a nested for/while loop
or switch statement. Our inlining approach generates incorrect code in
those cases, and there's no apparent fix, since we don't have a
mechanism for subverting control flow like 'goto'. When we detect this
case, disable inlining for that function.
Change-Id: Ic4180b367a3895806b0cc36872155185138826e1
Bug: skia:10606
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310063
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>
Prior to this CL, if we failed to create the DDL Recorder's target
proxy while creating the target surface we could create an invalid
DDL.
The specific repro case involved too big of an target proxy but
many other scenarios could result in the same behavior.
Bug: 1105903
Change-Id: I519a072600c168aa590fbe920f4029d08fe29e6f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309777
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Bug: skia:10369
Add SkEncodeImageWithNDK, mirroring the CG and WIC versions, for
encoding with the NDK APIs added to R.
Rename SK_ENABLE_NDK_DECODING to SK_ENABLE_NDK_IMAGES and use it for
both encoding and decoding.
Move code for converting to/from NDK types into a common location.
Update encode_platform.cpp to use NDK encoding APIs when available and
to use both types of webp (lossy and lossless). Add tests specifically
for the new implementation.
Update NdkDecodeTest to use ToolUtils::equal_pixels for comparing
pixels.
Change-Id: Ic62f89af27372ccce90b8e028e01c388a135a68c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308800
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
`onXxxxx` methods in Skia generally appear to be non-public, which I did
not realize until after originally implementing this task.
Followup CLs will update the non-gencode `onDumpInfo` methods to be
private as well.
Change-Id: I807eee132b080f72f6b040e1ca7f687be25dff11
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309883
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This CL also marks `GrProcessor::dumpInfo` as final. This prevents a
subclass from mistakenly overriding `dumpInfo` instead of `onDumpInfo`.
`onDumpInfo` is responsible for providing the same data as `dumpInfo`,
except that the FP name is automatically prepended.
Change-Id: I2b44c30a01bc65e9d88321cc21651a94e20074c6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309793
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
and
"Guard gencode dumpInfo() calls with GR_TEST_UTILS, instead of SK_DEBUG"
and
"Remove dumpInfo() entirely when GR_TEST_UTILS is off."
This is a reland of 26900788ef
and b5e8a2533a
and d7b09c4c3a
and unit tests are now fixed.
Original change's description:
> Guard dumpInfo() calls with GR_TEST_UTILS, instead of SK_DEBUG.
>
> (One exception: the `dumpInfo` in GrVkCommandPool is wrapped with
> SK_TRACE_MANAGED_RESOURCES to match its peers in GrVkManagedResource.)
>
> Change-Id: I6cf55fa2bb5687f79a2cc0c2a9c02a629bfd4f8e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309556
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
Change-Id: I16e6606082a814b4fa5ef58d1096fc915f5ac704
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309721
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Forgot to add these when I fixed the bug. Verified that these tests
failed without the fix.
Bug: skia:10593
Change-Id: Ia20fad3cd8e5b0f63ca19946b8314eed49bec2bf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309716
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
I created this problem with my fix to crbug.com/1108475
Bug: 1112259
Change-Id: Ieb001fa61bd9ed68dcae43e7d511b4581f16ef0f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308638
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
These radii are treacherous.
Added a regression test derived from the path found by the fuzzer.
Bug: chromium:1111169
Change-Id: I92fde0c31057ea8adc864336438c0af2869b0374
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309306
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
SkRRect::computeType() can fail to produce an isValid() SkRRect,
with the regression test added here.
We can...
- land something like this that forces a good rect value,
- drop the assert,
- or maybe explore deeper and fix this?
Change-Id: I6d7402a0a4af2141ce45e82d287bfb4806aeb0ec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309379
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This is a reland of a1df23c8b7
Original change's description:
> Implement `dumpInfo` for .fp files.
>
> Change-Id: I40f6c1a02e194f090e67a0e3f2d7d83cd2317efd
> Bug: skia:8434
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309139
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
Bug: skia:8434
Change-Id: If485635440b800f8a282c871a1c5f2801608d3c7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309660
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit a1df23c8b7.
Reason for revert: Needs c++17 library as written
Original change's description:
> Implement `dumpInfo` for .fp files.
>
> Change-Id: I40f6c1a02e194f090e67a0e3f2d7d83cd2317efd
> Bug: skia:8434
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309139
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,johnstiles@google.com
Change-Id: I09b98e83735bc30ec8a2e313e4b76a9eb6a7631a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:8434
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309656
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I40f6c1a02e194f090e67a0e3f2d7d83cd2317efd
Bug: skia:8434
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309139
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
These doubles are sometimes used as doubles and sometimes floats,
but it doesn't make sense to use values that can't be a float as one.
Just skip those in the test combinatorics.
Change-Id: Ibfdb699cac80b260258b164f95361110eb85c152
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309483
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
These should work fine through SkVMBlitter,
and I'm not really sure why we didn't add them
to SkRasterPipeline.
Bug: chromium:1113777
Change-Id: Id89f82e6a53bd49d88a9562309acf0ab53ea5ec5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309472
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Profiling showed that repeated calls to `input_texel_color` were
actually a very expensive part of the test. Fortunately, we can expose
the texture's pixel buffer to the unit test method and easily reclaim
that performance.
Change-Id: I2c1cdd57a3e14dc859bdf03d8131137ca81364ff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309437
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The fuzzer has a found a pathological case where the maxRadius
needs to be reduce by 17 ulps.
Reduce the larger radius an ulp at a time until the sum fits.
Change-Id: I9cc4528667e7f9e902eff34d447fd4040a09742e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309417
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This is a net reduction in code size because this idea was already
implemented separately in two places:
- dump_fragment_processor_tree (GrProcessorSet)
- describe_fp (ProcessorTest)
This consolidates the implementations. This CL also fixes a handful of
dumpInfo() methods that were not sufficiently descriptive--e.g. the FP
name was missing, or the implementation was just buggy.
Change-Id: If34ac46c97e9ae431c7c64b1247fc619703580b2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309324
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit d169e1915c.
Reason for revert: broke chrome via code generator
Original change's description:
> Migrate GrSurfaceContext readPixels to take direct context
>
> After this lands we'll proceed up the stack and add the direct
> context requirement to the public API and SkImage.
>
> Bug: skia:104662
> Change-Id: I4b2d779a7fcd65eec68e631757821ac8e136ddba
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309044
> Commit-Queue: Adlai Holler <adlai@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
TBR=robertphillips@google.com,adlai@google.com
Change-Id: I6126f2dca4bc902c903512ac486e22841cc472e5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:104662
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309281
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
Runtime effects previously allowed two kinds of global input variables:
'in' variables could be bool, int, or float. 'uniform' could be float,
vector, or matrix. Uniform variables worked like you'd expect, but 'in'
variables were baked into the program statically. There was a large
amount of machinery to make this work, and it meant that 'in' variables
needed to have values before we could make decisions about program
caching, and before we could catch some errors. It was also essentially
syntactic sugar over the client just inserting the value into their SkSL
as a string. Finally: No one was using the feature.
To simplify the mental model, and make the API much more predictable,
this CL removes 'in' variables entirely. We no longer need to
"specialize" runtime effect programs, which means we can catch more
errors up front (those not detected until optimization). All of the API
that referred to "inputs" (the previous term that unified 'in' and
'uniform') now just refers to "uniforms".
Bug: skia:10593
Change-Id: I971f620d868b259e652b3114f0b497c2620f4b0c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309050
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
After this lands we'll proceed up the stack and add the direct
context requirement to the public API and SkImage.
Bug: skia:104662
Change-Id: I4b2d779a7fcd65eec68e631757821ac8e136ddba
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309044
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This is causing failures on the status dashboard.
Change-Id: I29e8fb5bef72282dbe6fafb5402607ed48aae707
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309136
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Opted the Galaxy S20 out of the test to allow Tryjobs to pass.
Change-Id: I8d4637a23f36edb012c96b7059b184c231c0436d
Bug: skia:10384, skia:10595
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309119
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Previously, MakeChildFP avoided infinite recursion by rejecting any FP
that took inputs. MakeChildFP now generates random inputs up to a
user-supplied tree depth.
The ProcessorOptimizationValidationTest test has been updated to
test up to a tree depth of 3. The ProcessorCloneTest has been left at
a tree depth of 1 due to a bug that only appears on Galaxy S20/Mali G77.
The Mali bug doesn't appear to be related to FP cloning, but probably
deserves further analysis. (It appears that on this device, these
processors hooked together in sequence render a tiny bit differently
each time: DitherEffect -> RectBlurEffect -> ImprovedPerlinNoise. By
visual inspection it looks like the dither varies on each draw.)
Change-Id: Ib8f619eb7a8a9c9254080303504c20065ff35453
Bug: skia:10384, skia:10595
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308556
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, this test would synthesize a random FP, call clone(), render
both, and the compare the results and report differences.
Now, if a difference is discovered, this test will re-synthesize the
originally-created random FP from scratch without using clone().
Instead, we call TestCreate again using the same random seed.
Then we can render and compare that output against the original as well.
This will allow to better differentiate failures that were actually
caused by clone(), versus failures caused by other types of
inconsistency.
If the regenerated version still mismatches, there are a variety of
potential explanations:
- the FP's TestCreate() does not always generate the same FP from a
given seed
- the FP's Make() does not always generate the same FP when given the
same inputs
- the FP itself generates inconsistent pixels (shader UB?)
- the driver has a bug
Change-Id: I71701c0f3d33a08f6ee926313782620487d336bd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309076
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
https://clang.llvm.org/extra/clang-tidy/checks/performance-unnecessary-copy-initialization.html
Finds local variable declarations that are initialized using the copy
constructor of a non-trivially-copyable type but it would suffice to
obtain a const reference.
The check is only applied if it is safe to replace the copy by a const
reference. This is the case when the variable is const qualified or when
it is only used as a const, i.e. only const methods or operators are
invoked on it, or it is used as const reference or value argument in
constructors or function calls.
Change-Id: I1261410deccd8ea64e85edec53fbd5360940e587
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308759
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Follow-on CLs will push higher up in SkDraw, so that everywhere today
we have to cons-up (with the associated mallocs) a temp SkPath we can
replace it with a stack-based SPath...
- drawRect
- drawOval
- drawRRect
- drawLine(s)
(similar to how this CL already handled quads and triangles)
Bug: skia:10566
Change-Id: I882b4f4c60e80235ca83c86c926e905b269a7afd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307784
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Reed <reed@google.com>
SkArenaAlloc has three fields that are used only for reset. Make a
subclass called SkArenaAllocWithReset which has the three
fields, and has the reset functionality.
An example of a reset() that is used instead of using a better scope
is PathOpsAngleAfter in PathOpsAngleTest.cpp.
Change-Id: Ie1965d128dfb7df9e022f4d18460d3f75f33e1a6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307348
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Herb Derby <herb@google.com>
https://clang.llvm.org/extra/clang-tidy/checks/readability-const-return-type.html
`const` on a non-pointer/reference return type typically doesn't add
value and can have negative side effects. (i.e., returning a
`const std::string` isn't meaningfully different from returning a
`std::string`, but can sometimes inhibit move-related optimizations.)
In Skia's case, the priv() functions are a notable exception where const
return types are intentional and valuable. These calls have been marked
with NOLINT to exclude them from the check.
This check does not affect pointer and reference returns, where
constness is important.
Change-Id: I86cab92332f164e5ab710b4127182eec99831d7d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308564
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Discovered via ClangTidy `readability-static-accessed-through-instance`.
Change-Id: I646e3293853e65b5cc2419c8687c035aab4b669a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308659
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Unlike GLSL/SkSL, Metal does not natively support casting an array from
one size to another; we need to synthesize a helper function which will
assemble a new matrix from the values in the old matrix and the
identity.
Previously, our matrix-conversion helpers understood how to glom
together an arbitrary collection of scalars/vectors/matrices into a
matrix containing a matching number of scalars, but it would fail when
given a matrix of unequal size.
Change-Id: I35eb161ed7c17b982b00ecceb7b525cbfb8f3bcb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308190
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 fixes a large number of SkSL namespaces which were labeled as if
they were anonymous, and also a handful of other mislabeled namespaces.
Missing namespace-end comments have been added throughout.
A number of diffs are just indentation-related (adjusting 1- or 3-
space indents to 2-space).
Change-Id: I6c62052a0d3aea4ae12ca07e0c2a8587b2fce4ec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308503
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
We were only respecting this feature in runtime shaders. Note that use
of any tagged matrices will cause color filter creation to fail, but
color transformation is a totally sensible thing to want in a color
filter.
Change-Id: I482226b287ab794cb341367fce453381cb581966
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308507
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Some of these checks are currently redundant (we don't allow color
filters to have children right now). But the next CL will re-add that
capability, and the unit tests here will ensure we don't re-break things
by allowing child-sampling to violate the color filter invariant.
Change-Id: I54c10d8b1d1e376c13347296765185d42b9f644a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308285
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Android migration landed in Android CL 12234077
Chrome migration is landing in Chrome CL 2335812
Note: makeFromCompressedTexture is not used by Chrome.
Bug: skia:104662
Change-Id: Ibbe6d412cf22e87188926383d10b21f780208e48
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305102
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>
This function isn't used by Chrome so we migrate directly.
Flutter migration is at https://github.com/flutter/engine/pull/20235
Bug: skia:104662
Change-Id: I9d875acdbd162f50a6d86b3a4cae3f400e4dd38f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305180
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Change-Id: I9ed98859814e462c63ab29b94f0365ccc57d2e9b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307706
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
This reverts commit 07438b0cda.
Bug: skia:10369
Bug: skia:10371
This will allow Skia clients developing for Android 11+ to rely on
Android's NDK APIs for decoding, which will allow them to decode
without including their own decoding libraries (e.g. libjpeg-turbo).
Using these APIs also provides support for static HEIF images.
Run ImageGenSrc in kPlatform_Mode on Android to verify decoding
visually.
Add tests and a grayscale png.
Update some test bots running Android R to specify ndk_api so they will
run the new code.
Change-Id: I4ca07d832dbd6a9d8cff0faea975fd70da00718f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308185
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
This reverts commit cfef980939.
Reason for revert: Breaking Google3 roll
Original change's description:
> Add an SkImageGenerator that uses NDK APIs
>
> Bug: skia:10369
> Bug: skia:10371
>
> This will allow Skia clients developing for Android 11+ to rely on
> Android's NDK APIs for decoding, which will allow them to decode
> without including their own decoding libraries (e.g. libjpeg-turbo).
> Using these APIs also provides support for static HEIF images.
>
> Run ImageGenSrc in kPlatform_Mode on Android to verify decoding
> visually.
>
> Add tests and a grayscale png.
>
> Update some test bots running Android R to specify ndk_api so they will
> run the new code.
>
> Change-Id: Ica782339b2414d472ede0b61729a127ce41892a5
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305689
> Reviewed-by: Mike Klein <mtklein@google.com>
> Commit-Queue: Leon Scroggins <scroggo@google.com>
TBR=djsollen@google.com,mtklein@google.com,scroggo@google.com,brianosman@google.com,reed@google.com
Change-Id: Ifed506a76a0ff5903d101c1bf7330d319b8376a6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10369
Bug: skia:10371
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308180
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Bug: skia:10369
Bug: skia:10371
This will allow Skia clients developing for Android 11+ to rely on
Android's NDK APIs for decoding, which will allow them to decode
without including their own decoding libraries (e.g. libjpeg-turbo).
Using these APIs also provides support for static HEIF images.
Run ImageGenSrc in kPlatform_Mode on Android to verify decoding
visually.
Add tests and a grayscale png.
Update some test bots running Android R to specify ndk_api so they will
run the new code.
Change-Id: Ica782339b2414d472ede0b61729a127ce41892a5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305689
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
SkFontDescriptor and its serialization and usage are updated to better
reflect how variations should be serialized. In addition this begins
teasing apart SkFontData since it is now mostly an artifact of how the
FreeType port works than anything else.
This also removes SkTypeface::MakeFromFontData since it is no longer
used and since SkFontData (which it takes as a parameter) was never
public. SkFontMgr::makeFromFontData now only exists to support older
skps and may be removed in the future.
Change-Id: I266bd5e87de85788661cdf5c571592ea1f2ae669
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307344
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
RRect needs 8 radii, 4 corners x XY, so should pass 8 values, not 4.
Change-Id: I997cbf2a61e6ffb32e4a13d5c95d16cb172152be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307789
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Change-Id: I68bb6bb239074d7cf657f56acf6c970771af62f8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307717
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
This adds support for things like "foo.xxx1", which is equivalent to
"float4(foo, foo, foo, 1)".
Change-Id: Id52111917e30d7dd8ba2e0633074b64d7ac6c72a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306727
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
The majority of existing call sites were automatically updated using
clang-tidy -fix. A small handful required a manual update,
e.g. CppCodeGen.
This check is a bit lenient, and in particular will not flag cases like
`std::unique_ptr<Base>(new Derived())` which is still pretty common
throughout our codebase. This CL does not attempt to replace all the
cases that ClangTidy does not flag.
Change-Id: I5eba48ef880e25d22de80f321a68c389ba769e36
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307459
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Need to notify the underlying pathref if we've made oval or rrect
Bug: skia:9000
Change-Id: I57a801f1fb446b99634d7b028249a812a5a978f1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307516
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
We only want SkSL to contain fp variables at global scope, and to sample
directly from references to those global variables. Anything else will
confuse our sample-usage analysis, and often leads to asserts in the CPP
or pipeline-stage generators.
Bug: skia:10514
Change-Id: Ie1ef10821c1b2a946a92d050fea45d95569bc934
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304599
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The repro case for this bug is where we have ~700 non-AA texture ops
with a few coverage-AA texture ops sprinkled throughout. In the old
system this could result in the quad limits being exceeded bc the
AA type for the entire run was believed to be non-AA during the creation
phase only to find that it was actually coverage-AA at execution time.
This problem manifested in both the bulk creation method and the
1-by-1/chaining creation method.
Note: in the original repro case every texture op has its own separate
texture so the problem manifests in the chaining case.
Bug: 1108475
Change-Id: I8f08fa4d5db5dbfe4a28145737895b655a145b08
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306605
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Change-Id: I82f9bffc73dcc4c07050199c755120cc964b7198
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304858
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This speeds up compiler construction, because we no longer have to parse
and process a bunch of SkSL source code during startup.
Change-Id: I6d6bd9b5ce78b1661be691708ab84bf399c6df8b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305717
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This will allow us to enable the ClangTidy check
performance-for-range-copy.
Change-Id: I11f152ffe458f5f353da8715ffd2fd47cf4e71a7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306946
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Added vpinsrd to make it easy, and fixed a comment on vpinsrb's tests.
Nothing too tricky, just the naive implementations.
The hardest part was getting all the data to the right places.
No diffs!
Change-Id: Ie4c1f1e429abfa75ca80a93d108061287d5ace80
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306872
Commit-Queue: Herb Derby <herb@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This noop refactor ports the lane-immediate idea to the 64-bit loads and
to 128-bit stores, and renames to simple load64, load128, store128.
The store128 part of this change is _slightly_ sneaky in that we pack
the argument pointer index and the lane both into int immz, but there's
plenty of space there: lane needs 1 bit, and the argument pointer index
needs maybe 3 or 4 max.
Change-Id: I3fa01bf31312b8a69c7e287d649470ba15a8ea40
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306810
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
More recontexting for SkImage. Chrome flag in CL 2323135.
Flutter migration landed in https://github.com/flutter/engine/pull/19962
Bug: skia:104662
Change-Id: Id725eb130310639457ba90f378ecdb334dd5f3cd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306182
Auto-Submit: Adlai Holler <adlai@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This allows us to JIT functions taking one more argument.
Our x86 JIT was the weak link: LLVM handles arbitrary numbers
of arguments, and our aarch64 JIT handles up to seven already.
I plan to pass one more source varying sometimes, and in extremely
unusual cases it we could need six pointers:
1) uniforms
2) destination
3) 8-bit coverage plane
4) 8-bit multiply plane
5) 8-bit add plane
6) source varying
Those varyings 3-5 are all indexable off the coverage pointer 3) if we
know the dimensions, so I could be convinced that we should only pass
one there, making our maximum number of arguments four. I'll be looking
at that independently, but it doesn't hurt to have our capability to go
up to six either way.
Change-Id: Id7a5b88e382a95bb560633e95c5be273b7ea67d1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306241
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
The 3 bits that represent rsp are used as a signaling mechanism to
choose between compact base + disp encoding and full base + disp +
scale*index encoding, one byte longer. If a base is encoded as rsp in
the MOD R/M byte, an SIB byte follows.
r12 shares its 3 bottom bits with rsp, so we need to treat it like rsp
when deciding whether or not we need an SIB byte. (As usual registers
r8-15's distinguishing upper bit is carried by a REX/VEX prefix.)
rsp is also treated as a special signal in the index field of the SIB
byte, meaning essentially scale=0, and as a result it's not possible to
use rsp as an index. It _is_ possible to use r12 as an index, with a
test added here. This worked without any code change. It seems the
index=rsp -> scale=0 signal is triggered by all four bits of the index
registger, including the X upper bit from REX/VEX.
I have found these charts useful:
https://wiki.osdev.org/X86-64_Instruction_Encoding#32.2F64-bit_addressing_2
Looking at those charts I noticed that rbp/r13 are also special cases,
so I'm eyeing them warily and will avoid using them for now.
Change-Id: Id78f826a39c060b03000eae7c50c642ef44d57db
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306237
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Since subsetting may require rasterizing/resolving the generator, we may
also need access to the GrDirectContext. To simplify apis, rely on
makeSubset() for that.
Related: https://skia-review.googlesource.com/c/skia/+/305970
Change-Id: I1980e3c823fb6cf54f197c350942c2f82b03e20f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306136
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Bug: chromium:1101491
Bug: b/161896447
Found using
git grep -wiEIl \ '(he)|(she)|(his)|(hers)|(him)|(her)|(guy)|(guys)'
Change-Id: I6b91853de067fd4c2e84f7ec70275522ce6c8bfc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306186
Commit-Queue: Leon Scroggins <scroggo@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Auto-Submit: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
They have been replaced with equivalent constants (and given the
requisite k prefix).
Change-Id: I70907eec234e0861cc97aac1ca03086faca42f9a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306160
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Reed <reed@google.com>
No longer needed as GrSurface and GrRenderTarget are private.
Change-Id: I2ec653b2d9daa115233bb7eaa1f2b7f880772c0a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305730
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Bug: chromium:1101491
Bug: b/161896447
Switch to more inclusive language, like "main", or remove where
simply unnecessary.
Change-Id: I36ef6ec631eb991f54f42b98887333f07c0984c2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306060
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
It's hard to imagine a scenario where it makes sense to recompile a
shader for every possible point or rect.
Change-Id: Ic4ca9a4826c4ae4b604705549170503c8ec580a6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306075
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, floats would be converted to int32 using a C-style cast when
added to a processor key, truncating any fractional component of the
value. This would cause two processors compiled with different floats to
generate the same key.
The float is now sk_bit_cast to a uint32 instead; this will preserve the
uniqueness of all values.
Additionally, the CPP generator will now abort if asked to add an
unsupported type to the processor key. Previously, it would do a C-style
cast to int32 for all unrecognized types and hope for the best.
Change-Id: I270ae8b3036497a9e9a77747255f763d9aad1927
Bug: skia:10486
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305572
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This should be the last batch of tests. All the remaining uses of
GrContext should be resolved when SkImage no longer requires a context.
Change-Id: I47eeb3b74c28f483c20d9bec4daecbdb6d2cb982
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305541
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
We seed the builder with the imageinfo of an image, but after it is
built we then have to "attach" it to an image (or more than one).
Check that the mip chain is compatible with the parent image we are
attaching to.
Change-Id: Iead6aee54861fdd4910f38f89a0364ed80c341d8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305680
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Fixes a bug with name conflicts in the final SkSL.
Bug: skia:10526
Change-Id: Ic238f89dd778c186e775ecbaabfbaed9e426274f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305563
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Change-Id: Iaa0829d72d0da1469df2da23102ff0e3572b641b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305556
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 existed because GrTexture used to be public.
Change-Id: I5e507084ae12058a20481b517b9130b41c793d29
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305521
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
* Combine find and makeMRU.
* Switch from SkMutex to SkSpinLock
In the gm 'paragraph_$' this reduces acquiring the lock from 1.2% to 0.7%
as measured by instruments and nanobench.
Change-Id: I33e3af31825f175c9de42f001acf68ffe3623a8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305564
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
GrTAllocator implies relatively limited use cases, while GrTBlockLinkedList
helps clarify the underlying data structure (and its associated advantages
and disadvantages). I am not beholden to the name, so am happy to have
a discussion on alternatives like GrTLinkedList or GrTBlockList or
GrTBlockArray.
Change-Id: I5b10801d8593991d5e804c4074a81efb1dd110ee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304396
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Only works on raster backend for the now.
Bug: skia:10344
Change-Id: I2c82d5345ae83a2bb2744ab27e3d971c57ea989e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303271
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Did some cleanup to remove repetition that was distracting from the
thing being tested.
Change-Id: Ie385c6ec2d1325a1bd0ba5c2270e7f2ddd1d24b2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305076
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
vpermps (added here) makes this very easy,
with an index controlling what 32-bit values go where.
A index of the form {0,2,4,6|?,?,?,?} will put the 4 low 32-bit halves
of 4 64-bit values in lanes 0,1,2,3. We can use that twice to get all 8
low halves, then our new vperm2f128 to put them together. Conveniently
vpermps can also load directly from memory:
vpermps (%rdi), {0,2,4,6|?,?,?,?}, lo
vpermps 32(%rdi), {0,2,4,6|?,?,?,?}, hi
vperm2f128 0x20, lo,hi, dst
We don't care what those top four indices are for load64_lo, so we'll
use them as the indices for load64_hi. That makes the full index
{0,2,4,6|1,3,5,7}, and load64_hi will just vpermf128 the other 128-bits
of lo/hi:
vpermps (%rdi), {?,?,?,?|1,3,5,7}, lo
vpermps 32(%rdi), {?,?,?,?|1,3,5,7}, hi
vperm2f128 0x31, lo,hi, dst
vpermps needs its index in a register, so we use a temporary for that.
Our logical lo can alias dst, and hi can alias that index, so it's just
one extra temporary register in the end.
Change-Id: Ie6a4efbf12ddada45dd09c0f580fa7350cf3019e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305171
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Add some packing instructions to make it possible.
The gist is that we've got
r(x) = {a,b,c,d|e,f,g,h}
r(y) = {i,j,k,l|m,n,o,p}
where r(x) holds each low 32-bit half of a 64-bit value,
and r(y) holds the high halves. We want to write
a,i,b,j,c,k,d,l,e,m,...
So first the vpunpck[lh]dq instructions produce
L = {a,i,b,j|e,m,f,n}
H = {c,k,d,l|g,o,h,p}
which gets us halfway there. The vperm2f128s select the low (0x20) or
high (0x31) 128-bit halves of L/H, so we end up writing to memory
dst+0: a,i,b,j,c,k,d,l
dst+32: e,m,f,n,g,o,h,p
Existing tests cover that store64 works.
Change-Id: Ic00ad9bdb448b79867584c27cf0114a42ed32379
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305156
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Does not add kNearest as a mip map mode yet.
Bug: skia:10344
Change-Id: Ida80cbf19e2b1eed5209d0680837fb45e54b4261
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303481
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Change-Id: Ic44e24057b95bb014504f02a736fb4341afc8971
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304856
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Change-Id: I82276e38ea4a5524127176eb5a34066b6cb06d88
Bug: skia:10217
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304799
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 423dd689df.
Reason for revert: had missed corner case when tail block was empty
Original change's description:
> Revert "Support moving blocks from one allocator to another"
>
> This reverts commit 0f064041bf.
>
> Reason for revert: unit test failures on some windows bots
>
> Original change's description:
> > Support moving blocks from one allocator to another
> >
> > Uses this new capability and the previously implemented reserve()
> > feature to implement efficient concatenation of GrTAllocators.
> >
> > Change-Id: I2aff42eaf75e74e3b595d3069b6a271fa7329465
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303665
> > Commit-Queue: Michael Ludwig <michaelludwig@google.com>
> > Reviewed-by: Robert Phillips <robertphillips@google.com>
>
> TBR=robertphillips@google.com,csmartdalton@google.com,michaelludwig@google.com
>
> Change-Id: I931f2462ecf6e04d40a671336d0de7d80efd313d
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304604
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
TBR=robertphillips@google.com,csmartdalton@google.com,michaelludwig@google.com
# Not skipping CQ checks because this is a reland.
Change-Id: Ia793c2f0e7ab2f3fd437871fa7fb4f56979f9ceb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304739
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Change-Id: Ic7799b3c5f4294cba9ff72f8c11a2ad285ab189f
Bug: skia:10217
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304738
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: Ia92dc2be9a25f334bdbc098564cf2332496677fa
Bug: skia:10217
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304296
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Three TODOs, all basically the same idea: divide-by-zero is not the only
way to produce non-finite results from a division. You can also divide
by very-near-zero, and maybe some other ways.
Added is_finite() to make this clear. is_finite() is almost as cheap as
the comparisons it replaces, so performance shouldn't be affected.
Change-Id: I0a803e9ab4e3286f4e10a13d3aacee370eaaa803
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304669
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Also mipMapped params to GrBackendTexture functions to mipmapped
GrBackendTexture::hasMipMaps -> GrBackendTexture::hasMipmaps
Misc test vars fMipMapped -> fMipmapped
Change-Id: Ic0651d14fc106c21b0ab45529875b95ed8dc2dfd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304598
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This reverts commit 0f064041bf.
Reason for revert: unit test failures on some windows bots
Original change's description:
> Support moving blocks from one allocator to another
>
> Uses this new capability and the previously implemented reserve()
> feature to implement efficient concatenation of GrTAllocators.
>
> Change-Id: I2aff42eaf75e74e3b595d3069b6a271fa7329465
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303665
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
TBR=robertphillips@google.com,csmartdalton@google.com,michaelludwig@google.com
Change-Id: I931f2462ecf6e04d40a671336d0de7d80efd313d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304604
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
For consistency with other enums and public APIs.
Change-Id: I026da5529f11051693cae5691c7ad92fad5ed446
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304597
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Uses this new capability and the previously implemented reserve()
feature to implement efficient concatenation of GrTAllocators.
Change-Id: I2aff42eaf75e74e3b595d3069b6a271fa7329465
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303665
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Change-Id: Ia2cfbca8982b57399b6681cbb4501c2933ab4df7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304576
Auto-Submit: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This can optimize cases in a allocate-release loop that moves back
and forth across the end of one block and the start of another. Before
this would malloc a new block and then delete it.
It also enables reserve() in higher-level data collections w/o blocking
bytes left in the current tail block.
Change-Id: Ide16e9038384fcb188164fc9620a8295f6880b9f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303268
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Previously, these would produce a "valid" effect object, but it wouldn't
draw anything.
Bug: oss-fuzz:24070
Change-Id: I17d0ed1710196853da0694cac9f4c260312700a9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304064
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
The current algorithm runs an exponentially-increasing number of trials
based on the number of children supported by the fragment processor and
has become a large drag on test times. This version runs a fixed number
of trials to determine which optimization bits are able to appear, and
then continues running trials until each potential optimization has been
demonstrated successfully five times.
The algorithm doesn't attempt to check interactions between the various
optimization bits (e.g. a hypothetical bug that might only occur when
two optimizations interact with one another) but hopefully the minimum
of 100 successful trials is enough to shake out most issues.
Change-Id: I4eba7ace84739027a5aea8f8f895b44c4532b816
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304059
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
To extract metadata and validate the shader, we've added several
iterations over all program elements. This CL rearranges things
to iterate once (*). Variable conversion is moved to a separate
loop later, to help with nesting and readability.
Removes hard-to-read asserts. These were validating things enforced
by both the IR generator and unit tests.
*: Technically, there are additional implicit iterations when we call
SkSL::Analysis functions. Folding all of this into a single pass
would be even better, but much more complicated.
Change-Id: I4f5aa649e74094e94c365ad20ef2ac96082285cd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303924
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Mostly this is a lot of plumbing of sk_sp around instead of const*.
This does allow the d3d and vk backends to hold refs to the GrBuffers that
are bound on a command buffer. This means that our buffer alloc pools will
not try to reuse this buffers until the gpu is done with them. Previously
vk and d3d will sniff out if one of these buffers was being used again
while still active on the gpu and rip out the internal backend buffer and
allocate a new one which is not cheap. We see a lot of perf wins from
not doing this.
Change-Id: I9ffe649151ee43066dce620bd3e2763b029a9811
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303583
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Previously, this test layered its paints by calling
`addColorFragmentProcessor` multiple times. We intend to remove support
for multiple color FPs on a GrPaint in the near future, so we use input
fragment processors instead to generate the same result.
Change-Id: I33e82ce0067183189e69b2af0fe0c228d1d60f14
Bug: skia:10217
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303479
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
The helper class allows us to regenerate a "random" fragment processor
with the same seed as many times as desired. This lets us check the
`compatibleWithCoverageAsAlpha` optimization without needing to clone
the input FP; instead, we can actually generate the FP under test three
times in a row, with the same random seed.
In a followup CL (http://review.skia.org/303479/) we will also leverage
this helper class to regenerate the FP under test with the same random
seed, but with a different input.
Change-Id: I1cd83082a949d555f7898970c8a1cc3002818286
Bug: skia:10384
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303657
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This adds load/store ops for 64-bit values, with two load64 instructions
returning the low and high 32-bits each, and store64 taking both.
These are implemented in the interpreter and tested but not yet JIT'd
or hooked up for loading and storing 64-bit PixelFormats. Hopefully
those two CLs to follow shortly.
Change-Id: I7e5fc3f0ee5a421adc9fb355d0b6b661f424b505
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303380
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
