The sized type printf macros like PRId32 contain only the conversion
format specifier. The introductory '%', flags, width, precision, and
length modifier are separate and must be specified if desired.
Change-Id: I8a88d9ee8bebe0275eb712728e146393d7837280
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311441
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
The as<T>() function asserts that the Expression is of the correct kind
before performing the downcast, and is also generally easier to read
as function calls flow naturally from left-to-right, and C-style casts
don't.
This CL updates several Expression downcasts in SkSL to the as<T>
syntax, but is not intended to exhaustively replace them all (although
that would be ideal). In places where we SkASSERTed the expression's
fKind immediately before a cast, the assert has been removed because it
would be redundant with the behavior of as<T>().
Change-Id: I67e7de2697aee4d9d09e6594a9abc5f2ecacc887
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311114
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Hopefully, this will let Chrome track down promiseImage fulfillment
mismatches.
Bug: 1116848
Change-Id: Ia1e5d6f7af4e2808ae4adfad85f4e96c1ea4fbd2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311096
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@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>
Since this doc was written, we've made several changes to how gradients
work under the hood:
- Colorizers receive the t interpolant via explicit-coordinate sampling,
not via sk_InColor
- Unused components passed to the colorizer are guaranteed to be zero
instead of potentially arbitrary values
- We no longer have a dedicated colorizer that is responsible for
sampling a generated texture map; GrTextureEffect is used directly
- Rewrote the section about how layout processors encode their return
value to hopefully be more clear to a reader unfamiliar with gradients
Change-Id: I037b5718c9f67a1bece96a9b6a4daea79ccdcd80
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311076
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
We now only support Wuffs v0.3.
Change-Id: Iaace012977ab88922ccdf2a929dc4c809bbb4047
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311016
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Leon Scroggins <scroggo@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>
This brings the GrSmallPathAtlasMgr into closer correspondence with
the GrAtlasMgr.
It also centralizes where we update the GrSmallPathShapeData's
atlas information.
Change-Id: I892be262a85b3878dbd7f71b0503208943203d46
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310476
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Inline small routines from the AtlasLocator.
With these routines inlined, the benchmark drops from 402ns -> 302ns.
Over the past couple of CL the total drop is 502ns -> 302ns.
Change-Id: I7de899c20dc6759db1b70c036e319a56da44a13b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310758
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@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>
There is no way to get a variable with that builtin identifier (it's
going away entirely soon), so no need for the protocol to send it to
the FP.
Bug: skia:10619
Change-Id: Icd1744d8573ded2a6fdf3c1901f46df0c91aab14
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310760
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Now that we're using GrBuffers for uploads, we need to match the size
of the upload buffer a little better to avoid wasting memory, so this
switches to a using either a power-of-two size, or the midpoint between
two powers-of-two.
Also, the mechanism of adding the staging buffers to the current
command buffer was not being invoked when calling submit for a cross-
context texture. Switching to submitToGpu() should handle this.
Change-Id: I8e0e5ae7e4dd00b5969d5e5554e3a2bff5758e81
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310339
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Change-Id: I17653e036ceb5f505284ae64cf6918b3bd2e02ce
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310762
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@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>
Split out the non-color-no-clip case for future specialization. This
routine will handle the vast majority of the glyph drawing. Split out
the generalized case too.
This change alone has a 20% improvement for performance due to fewer
register spills.
Change-Id: I89650c29ca273ac6e30266c2419029a7f177b029
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310516
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
The original code had the fill vertex method handling the clip, but
each glyph was clipped individually. Signal no clipping at all is need
when the sub run is entirely enclosed in the clip rect.
Change-Id: Ifc7174fe7af967c9ab7240b2c7e2c0b062d7e259
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310556
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
There are basically 3 parts to this change.
1) Implement GrVkGpu::xferBarrier virtual
2) Plumbing the need of an xferBarrier from ProgramInfo when getting a
render pass.
3) Tracking the need for an xferBarrier on GrOpsTask via
GrProcessorSet::Analysis
Bug: skia:10409
Change-Id: I6ab8f36719b3a4db576535eb6ed1c579ae34b7a4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310439
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This is a reland of 166cd52cee
with iOS simulator builds disabled.
Original change's description:
> move conditions for JIT into SkVM.h
>
> This makes it easier to keep in sync with the implementation in
> SkVM.cpp, and it's easier to handle here once than in each build system.
>
> Change-Id: I429f90cfc16f35a0d7e0b0fde176bc013aa7db18
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310263
> 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>
Change-Id: I82837c3d959769f26197f2402d3b911e71a35675
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310465
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
The self dependencies allow us to use xfer barriers for advanced blends.
If we don't have the advanced blend extension the self dependency render
passes will also have an input attachment on them which can be used to
read the dst value in the shader.
Also has some renaming of previously plumbed values to better match what
we are doing.
Bug: skia:10409
Change-Id: I3b343064627921b8dc3debeeb6869b0f4b2dcc42
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310337
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
This reverts commit 166cd52cee.
Reason for revert: iOS simulator was on unintentionally
Original change's description:
> move conditions for JIT into SkVM.h
>
> This makes it easier to keep in sync with the implementation in
> SkVM.cpp, and it's easier to handle here once than in each build system.
>
> Change-Id: I429f90cfc16f35a0d7e0b0fde176bc013aa7db18
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310263
> 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>
TBR=mtklein@google.com,herb@google.com,reed@google.com
Change-Id: I1ae283be09906603c207079cbf6f1e296a4835f8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310462
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This isn't particularly important. It just struck me that if we're
going to be byte-thrifty, might as well go all the way.
Change-Id: I4d137731ac06643c50e6bc66cc4654abad732cea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310325
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
There was a time when we used to #define VECTOR(...) to make
it easy to add opcodes, but not since
https://skia-review.googlesource.com/c/skia/+/299682
Oddly we never #undef'd VECTOR_MATRIX.
Change-Id: I4a68d29a0ee6c34ee421e1bc4e76155d110de6fd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310324
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This isn't used anywhere since we no have GrWaitRenderTask.
Change-Id: I31e5f89fabf547346886edba5db1fac819096146
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310377
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Greg Daniel <egdaniel@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>
This staging flag is no longer used by Chrome.
Bug: skia:104662
Change-Id: Ib788b3bb4a975e496d72729fa47fb52952ab6a12
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310336
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Auto-Submit: Adlai Holler <adlai@google.com>
Having the logic to emit, parse, and re-emit this boilerplate spread
across the generator and FP was unnecessary. Having it emitted directly
is simpler, and will allow some flexibility when we change the signature
of runtime effect SkSL.
Change-Id: I7ebd5e62bfe1e41035c4fb9efadb427be915b9f8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310158
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This makes it easier to keep in sync with the implementation in
SkVM.cpp, and it's easier to handle here once than in each build system.
Change-Id: I429f90cfc16f35a0d7e0b0fde176bc013aa7db18
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310263
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>
Relocate under modules/audioplayer and package as a standalone
component.
Change-Id: If9dc72bb0abe170049a514c9931186703a3c138a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310058
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Mike Reed <reed@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>
When this assert happens in a Chromium bug report it just states
"assert(false)". While the line number is present the line numbers
aren't always stable or easy to track down over time, so having the
condition that actually asserted present gives better certainty about
where in the code and which condition failed to hold in relation to the
crash report.
Change-Id: I7b0deb05c1736ba90c7642c90d8be44d2d17e9f4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310156
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This is easier to read than casting nullptr to the desired type.
Change-Id: I19eb85f96dc4e66316e36c1839b8df25f39fa608
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310178
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This makes the add() interface more closely aligned with
takeOwnershipOfXxxxx().
Change-Id: Iafd116fa86bd8ccfe07b730ebcd74d5304967878
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310069
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The calling code guards against < .5 radii if not stroke-only.
Change-Id: Ic6075e2bef3989a02f1f87085dba9cecdd93d825
Bug: chromium:1114968
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310057
Auto-Submit: Brian Salomon <bsalomon@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Use Queue::WriteBuffer() for uniform uploads instead of the staging
manager. Utilize the fUniformsDirty mechanism in GrUniformDataMangaer
to cache and reuse the most recent BindGroup uploaded.
Doing this required moving bind group creation and setting to
GrDawnProgramDataManager::uploadUniformBuffers(), and passing the
RenderPassEncoder down to setUniformData(). setTextures() was
similarly modified (for symmetry).
Change-Id: I841a0cf710b10d5025ea895d3a109f05d2966639
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309663
Commit-Queue: Stephen White <senorblanco@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Stephen White <senorblanco@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>
Suspect this pathway isn't used, and we dont want to support it
in a future refactor requiring a GrDirectContext in getROPixels.
Bug: skia:104662
Change-Id: I9492fa7a6b85306d187c2eeab2463f305bd17dd0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309720
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@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>
