Previously, when Graphite generated SkSL, it assumed all uniforms were
tightly packed, since SkUniform doesn't have a built-in Offset field.
Now we recalculate the offsets by reusing logic from the UniformManager.
UniformManager has been split into two parts; UniformOffsetCalculator
contains just the basics needed for offset calculation without keeping
a storage buffer or tracking uniform expectations. Since the offset
calculation logic is the same, we should get the same layout for our
SkSL as we get when laying out uniforms to begin with.
Change-Id: I9c55b3255d2228dfdd45e106518bb6896bc78c88
Bug: skia:13405
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556596
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Added code to remove code within switch statements due to break, return,
and continue statements. The logic is applied conservatively and only
among the statements of an individual switch-case statement without
affecting other cases.
Bug: skia:13484
Change-Id: Id5b936ca91e562a5180a31a039a85de9e093961d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556376
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
If the maximum dimension of a glyph is zero in the drawing of last
resort, then skip this SubRun.
Bug: oss-fuzz:48695
Bug: oss-fuzz:48690
Change-Id: Icdca979c4152494f7028a482e33224664d97f4ae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556597
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Originally, I had to convince myself that these three calls can never
legitimately produce nullptr - that's basically covered by the calls
to blender_requires_shader that would prevent these code paths from
happening. (The source FP is always non-null. The dst FP is nullptr
for two of these, but in both cases we can't reach the call if the
blender is actually kDst).
With the recent change to GrBlendFragmentProcessor, the reasoning is
much simpler: no blender should ever return nullptr on success,
regardless of input.
Bug: skia:13459
Change-Id: I3e097f96f83e45a9bac283b2aec579f012ffb4c1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551891
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Previously, the code for converting an SkSLType to a string only
existed in Ganesh, in GrGLSL.cpp/.h. This has now been moved to
SkSLTypeShared.cpp, and the Ganesh file was removed entirely.
Now that a cpp exists, I also moved some rarely-used utility function
bodies out of the header.
Change-Id: Id37ac54dabfe8b1264a2662a00c9780a1ecae2ba
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556602
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: Ie82c8a280cbc3997d0b65327535f0eaf0694bdfc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556601
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
The FP factory would apply optimizations based on the mode, and return
one of the passed-in FPs. We use 'nullptr' as the 'src' FP when
constructing the blender's FP in make_effect_fp. This means that we get
back 'nullptr' from a src-mode blender. Later, we interpret that as
src-over (the default for an unset blender). Oops.
The new test variant would previously fail, before the fix to the FP.
The tweak to the FP technically eliminates an optimization, but it's one
that only applies to blending happening in the shader (eg, compose
shader, or runtime effects using a blender), and only when the blender
is one of the trivial modes. The resulting shader will still optimize
down, it just involves a bit of extra work before that happens. This
shouldn't have any long-term performance impact, particularly on
important scenarios.
Change-Id: Id5c6a6ca8a263b35c2dca3c41171748cffd41adb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556599
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Pass an optional label parameter to GrBackendTexture and pass
it to MakeWrapped of GrGLTexture because almost all textures
that are coming clients are things where the client owns the
gl texture and passes it into Skia as a wrapped texture via a
GrBackendTexture.
Bug: chromium:1164111
Change-Id: I4bfddda956c72b53d0070595ef3268ee1a2b747f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/555597
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
These querries aren't actually used in this CL but will be used in the
follow up CL that adds newer intel bots.
Bug: skia:13401
Change-Id: I67b8e07fc66d0515e41e9a66616964235ace6568
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556598
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This reverts commit 5117907413.
Reason for revert: blocking G3 roll and red bots
Original change's description:
> Add ToolUtils helper for generating GM UI sliders for variable fonts
>
> Use this new tool in the COLRv1 GM test for creating sliders in the GM
> UI if the test font provides variable axes. Update GM
> "fontscalerdistortable" as well.
>
> Preparation for testing variable COLRv1 fonts while developing
> this feature in FreeType.
>
> Bug: chromium:1311241
> Change-Id: I55419d6dc058f420a567d8a50cca5d719206daf4
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/555476
> Reviewed-by: Ben Wagner <bungeman@google.com>
> Reviewed-by: Florin Malita <fmalita@google.com>
> Commit-Queue: Ben Wagner <bungeman@google.com>
> Commit-Queue: Dominik Röttsches <drott@chromium.org>
Bug: chromium:1311241
Change-Id: Icdd27da3b5c074964ca07fc6eee3e59aa90234b4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556656
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Robert Phillips <robertphillips@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Use this new tool in the COLRv1 GM test for creating sliders in the GM
UI if the test font provides variable axes. Update GM
"fontscalerdistortable" as well.
Preparation for testing variable COLRv1 fonts while developing
this feature in FreeType.
Bug: chromium:1311241
Change-Id: I55419d6dc058f420a567d8a50cca5d719206daf4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/555476
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
This checks the padding of a matrix followed by a scalar/vector, and the
padding of a scalar/vector followed by a matrix. The checks support
all mixes of 16- and 32-bit sized elements.
After some investigation, this CL also removes some TODOs in
`get_ubo_aligned_offset`. Our uniform system does not support structs,
which is a large source of disparity between layouts. With structs
removed from the equation, the only difference between layouts seems to
be related to std140 array padding. (std430 and Metal seem to be
entirely the same.)
Change-Id: I76c48ae1e597b98aad8a8f8495ab4a8ad262845b
Bug: skia:13478
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556356
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The old code would return a nullptr on failure or when doing strike
calculations. The rest of the code was not prepared to deal with a
nullptr. When I tried to modify the existing code to deal with a
nullptr, the code became too verbose. I switched to having MakeInAlloc
return an empty SubRunContainer instead of nullptr when performing
strike calculations only.
Bug: oss-fuzz:48695
Bug: oss-fuzz:48690
Change-Id: I633df3316f6f1c3a2b5749722f8c7a0d54ff7f8b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556361
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
CL[1] makes dmsaa fall back to the default path for d3d11 backend
on Intel gpus. As a complement, this patch adds other restrictions
to make CL[1] only work for Intel gen11 and gen12, which could avoid
any unknown performance regression on old Intel gpus.
[1] https://skia-review.googlesource.com/c/skia/+/545616
Change-Id: I08148d29d9605e692bdcd7208e4f11067156906d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/555296
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
The fuzzer figured out that #version 300 would let you declare a
nonsquare matrix uniform. Quite a bit of downstream code isn't ready for
that, yet. For now, just tighten things up so the var declaration checks
match the types supported by SkRuntimeEffect::Uniform.
Bug: oss-fuzz:48829
Change-Id: I63daf3dfa7deb795901f19553805cf2351378620
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556359
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
We have unit tests that require api 26 to run, but we have no vulkan
bots that would run them. API 26 is supported on all O devices and I
don't think we'll ever want to run Vulkan on previous android devices.
So I think it is fine for us to just test at api 26 for all our test
bots.
Change-Id: I8f92af6504960b7b688281ad71f5f307fdf57f49
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556028
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This reverts commit eaecd17d7a.
Reason for revert: works fine in practice, but with a debugger attached, the MTL built-in debug pipeline assertions trigger and report size issues e.g.:
Vertex Function(vertexMain): argument _anonInterface0[0] from buffer(0) with offset(0) and length(72) has space for 72 bytes, but argument has a length(80).
Fragment Function(fragmentMain): argument _anonInterface0[0] from buffer(0) with offset(0) and length(72) has space for 72 bytes, but argument has a length(80).
Original change's description:
> Simplify uniform padding in Metal (Ganesh).
>
> Previously, we would pad Metal uniforms to the nearest 16-byte size if
> they contained a float3, float4, or matrix type. This does not appear
> to be necessary (all tests pass without this level of padding).
>
> Since Metal is C++ based, it does have *some* struct padding, based on
> the basic type in the structure with the highest bit-width. Rather
> than track this amount, we just assume that it is 8 and round up
> Metal uniform blocks to the nearest 8-byte size. This will ~never be
> larger than our previous padding, since a typical Skia shader will
> generally always include a float2 uniform or larger (e.g. RTFlip is
> a float2), and will probably be tighter than before since most shaders
> include a color (float3/float4) or matrix uniform.
>
> Change-Id: Ic8dd49f33cb81a24a6415e9ba6e91c9f6faeb1b1
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556216
> Commit-Queue: John Stiles <johnstiles@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
Change-Id: Ib7749d09e603fb91b1c3f9ff706512b0205d2f16
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556357
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
This does not yet support gzip'ed svgDoc. DirectWrite just hands back
the raw svgDoc data here which may be in any format. It does not attempt
to check for the gzip header or flate the contents. Skia will eventually
need to do this itself for full support. At the moment glyphs which are
backed by a gzip'ed svgDoc will fall back to outlines.
Change-Id: I34a92b2285189ecdbdbc6a8b2a668bd4935bca15
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541061
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Thankfully, this was already working; no changes in the uniform manager
were needed.
Change-Id: Ic2c4807e8efa63a05127d6f96d8a58ce785bbc1e
Bug: skia:13478
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556316
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, we would treat double values outside [-FLT_MAX, FLT_MAX]
as finite. In practice, this introduces many hazards; any place in the
code which handled the double value as a float would silently convert
the value to infinity. This includes high-traffic calls like
Literal::MakeFloat.
Note that the if checks are structured in a slightly awkward way to
ensure that NaNs are treated as non-finite.
The original buggy behavior can be seen at http://review.skia.org/556078
Change-Id: Ic126afe57c3d6c7aa3edf9c8f7e339abc5f77739
Bug: oss-fuzz:48592
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556080
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, we would pad Metal uniforms to the nearest 16-byte size if
they contained a float3, float4, or matrix type. This does not appear
to be necessary (all tests pass without this level of padding).
Since Metal is C++ based, it does have *some* struct padding, based on
the basic type in the structure with the highest bit-width. Rather
than track this amount, we just assume that it is 8 and round up
Metal uniform blocks to the nearest 8-byte size. This will ~never be
larger than our previous padding, since a typical Skia shader will
generally always include a float2 uniform or larger (e.g. RTFlip is
a float2), and will probably be tighter than before since most shaders
include a color (float3/float4) or matrix uniform.
Change-Id: Ic8dd49f33cb81a24a6415e9ba6e91c9f6faeb1b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556216
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
It was easy to construct a buffer that indicated enormous amounts of
memory had to be allocated. An easy guard is to bail out if the buffer
can't possibly fill those buffers.
Simplify the code a bit: Four years ago (well past the earliest
supported SKP version), we stopped writing out triangle fan data (by
converting to tri-lists at construction time). Remove the
deserialization support, which makes the code easier to follow.
Bug: oss-fuzz:48228
Bug: oss-fuzz:48231
Change-Id: I941da595a250f940316a48cb54caeaec47768973
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556021
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Avoid strange issues with path case insensitivity on Windows with
os.path.normcase.
Change-Id: I9327ba9c22cf5e3ff66b49af56bedd223b24efd4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556025
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
The OT-SVG specification allows individual svgDoc inside the SVG table
to be optionaly gzip'ed. Add a glyph to SampleSVG.ttf which uses a
compressed svgDoc so that this can be tested.
This font crashes vanilla FreeType 2.12.0 and 2.12.1. This issue was
fixed with [0] and backported to Debian in 2.12.1+dfsg-3 [1].
[0] c26872ed59
[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1013094
Change-Id: I45e115a743b8aa4d545f34c9668597d22e0a2bf4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545779
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This reverts commit 7f8303d25a.
Reason for revert: red Graphite bots
Original change's description:
> Guard against divide-by-zero in drawing of last resort
>
> Exit early if there will be a divide by zero or if no pixels are
> to be drawn. Report that some glyphs have been excluded.
>
> Bug: oss-fuzz:48695
> Bug: oss-fuzz:48690
> Change-Id: Ifdb0fad656ffc27bac7253035c7cd05ee96c274c
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556018
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
Bug: oss-fuzz:48695
Bug: oss-fuzz:48690
Change-Id: I3342fc75ff3ba48db941f71d4abdfbfbe62fa589
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556081
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Robert Phillips <robertphillips@google.com>
FreeType reports that the type of a font is TrueType when the font is
logically TrueType. However, the underlying data may not be TrueType and
may be encoded in woff or woff2. The raw woff or woff2 data should not
be embedded in the PDF. If the tables are accessed individually it is
possible to subset them into a TrueType font. However, the subsetters
are not currently capable of handling a font as tables so fall back to
Type3 for now.
Change-Id: I5235ad02fd73fd88759dc30adfcf80aa0e4d2feb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543921
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This reverts commit 9da66d2a57.
Reason for revert: Seeing if this is blocking the G3 roll
Original change's description:
> [ganesh][dawn] Use GrRingBuffer for uniform buffer allocation
>
> The Dawn backend used its own GrDawnRingBuffer type to manage uniform
> buffer slices which always made new wgpu::Buffer allocations on-demand.
> It now uses the GrRingBuffer type for this purpose.
>
> While the Dawn backend does not use mapped buffers for uniform data
> uploads and buffers are ref-counted and freed by Dawn automatically
> after use, using GrRingBuffer allows it to share the same
> GrResourceProvider infrastructure as the other backends.
>
> Change-Id: Id2697f306f9ce3d5c6c85745b135585b092b6fb0
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/553525
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Arman Uguray <armansito@google.com>
Change-Id: Id9960b0b2bdf9ecab5889ef2050425e422e86090
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556079
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Exit early if there will be a divide by zero or if no pixels are
to be drawn. Report that some glyphs have been excluded.
Bug: oss-fuzz:48695
Bug: oss-fuzz:48690
Change-Id: Ifdb0fad656ffc27bac7253035c7cd05ee96c274c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556018
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
While reading through this code, I realized we implemented the same
function-argument stringizing loop three times. (A fourth version in
`description()` is similar, but prints the expressions, not types.)
Change-Id: I78610cdf6412b2d08984ac701e6421c356f25a83
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/556076
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
SkTypes.h was found exporting <stddef.h> and <stdint.h>.
Thus if a Skia file includes SkTypes.h, it did not need
to include either of those files to use things like
size_t or uint8_t respectively. [1]
This also resulted in strange IWYU warnings like:
warning: size_t is defined in "include/core/SkTypes.h", which isn't directly #included.
Thus, this CL removes those additional exports, and as a result,
adds many more includes of <cstddef> and <cstdint>.
The second change this CL is to not use the default IWYU
mappings which are hard-coded into IWYU [2]. These defaults
are valid, but make suggestions for the C version of
headers and not the C++ ones (i.e. <stddef.h> over <cstddef>).
To make these recommendations align better with our preferred
style (the C++ ones), we use IWYU with --no_default_mappings
and then have to expand our existing mappings to better deal
with how IWYU would resolve some of these headers.
[1] https://codereview.chromium.org/1207893002/#msg49
[2] 4c0f396159/iwyu_include_picker.cc (L1221-L1234)
Change-Id: Iafebe190f8f3e86f252147b9f538c182bcc34af4
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/555516
Auto-Submit: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The fuzzer managed to create a NaN using a carefully-crafted mix of
intrinsics and constant folding. (`cosh(421)` is a very large double,
which becomes +Inf when cast to float, which is then multiplied by 0;
zero times infinity is NaN.)
Our code which checked to see if a value is in range of an int did not
consider NaNs and their always-false behavior, so it incorrectly
decided that NaN was in range. This CL reverses the check so that a NaN
will not pass, but all other values will behave the same.
Followup CLs should probably also tighten up the folding/optimizer
behavior so that NaNs/Infs are not created at all.
Change-Id: Idd2b0447ebe115e00bdba63ca7ff655f6c902fc6
Bug: oss-fuzz:48592
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/555009
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
