Note: works for well-formed poly-to-poly (perspective) transforms, but
doesn't support AE's degenerate corners semantics (concave/inverted
polys) at this point.
Bug: skia:10100
Change-Id: I5b3492b008302495b616867c139c6e5ad6dc57df
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/281595
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
bit_clear is at least useful as a special case for select(),
which helps with code readability.
Add is_NaN() and use these all together in sweep gradient.
Change-Id: I57a54f8956f85e0db0662b33f8446b8dc7342d8d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/281685
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
- new this-> convention: never use it when calling common public
Builder methods like splat(), bit_and(), etc like you'd see in
normal user code, but always use it when calling private methods
like this->push(), this->isImm(), this->allImm().
- use c++17 if-statements to scope this->allImm() variables tighter.
- check for x.id == y.id cases where applicable, including a tweak
to min() and max() to make them able to hit the special case.
- add special cases for I32 +,-,*, and remove an old unimportant
unit test that assumed we didn't fold these.
- add special cases for select(), and use select() in a few more
places where it's clearer and now just as efficient.
Change-Id: Idaac9250ac5a95a48d33eeba1cc4380c8c91629d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/281678
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
bit_clear() is just another bit_and(),
and bytes() is a way of expression pshufb
that we never really use (yet).
Can always add them back later, but there's
some extra complexity to think about for each
that I'd like to not think about now:
- common sub-expression elimination between bit_and and bit_clear
- large constant management JIT'ing bytes
Change-Id: I3a54afa963231fec1d5de949acc647e3430ed0d8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/281557
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Plumb layer style parsing, and extend existing DropShadowAdapter to
support both drop shadow style and drop shadow effect.
Change-Id: Id99a419dacd06dc38dc4cf84ff4ecb92218c45f7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/279020
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
When converting from Instructions to OptimizedInstructions
place instructions that reduce register pressure earlier in
the instruction list.
This change reduces some register pressure in SkVM, and
improves the bitmap_RGBA_8888_A_scale_bilerp benchmark by
about 5%.
Change-Id: If5f6385bd2f7720701d1c827265062b35491a790
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/276485
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
While I think trunc(mad(x, scale, 0.5)) is fine for doing our float
to fixed point conversions, round(mul(x, scale)) was kind of better
all around:
- better rounding than +0.5 and trunc
- faster when mad() is not an fma
- often now no need to use the constant 0.5f or have it in a register
- allows the mul() in to_unorm to use mul_f32_imm
Those last two points are key... this actually frees up 2 registers in
the x86 JIT when using to_unorm().
So I think maybe we can resurrect round and still guarantee our desired
intra-machine stability by committing to using instructions that follow
the current rounding mode, which is what [v]cvtps2dq inextricably uses.
Left some notes on the ARM impl... we're rounding to nearest even there,
which is probably the current mode anyway, but to be more correct we
need a slightly longer impl that rounds float->float then "truncates".
Unsure whether it matters in practice. Same deal in the unit test that
I added back, now testing negative and 0.5 cases too. The expectations
assume the current mode is nearest even.
I had the idea to resurrect this when I was looking at adding _imm Ops
for fma_f32. I noticed that the y and z arguments to an fma_f32 were by
far most likely to be constants, and when they are, they're by far likely
to both be constants, e.g. 255.0f & 0.5f from to_unorm(8,...).
llvm disassembly for SkVM_round unit test looks good:
~ $ llc -mcpu=haswell /tmp/skvm-jit-1231521224.bc -o -
.section __TEXT,__text,regular,pure_instructions
.macosx_version_min 10, 15
.globl "_skvm-jit-1231521224" ## -- Begin function skvm-jit-1231521224
.p2align 4, 0x90
"_skvm-jit-1231521224": ## @skvm-jit-1231521224
.cfi_startproc
cmpl $8, %edi
jl LBB0_3
.p2align 4, 0x90
LBB0_2: ## %loopK
## =>This Inner Loop Header: Depth=1
vcvtps2dq (%rsi), %ymm0
vmovupd %ymm0, (%rdx)
addl $-8, %edi
addq $32, %rsi
addq $32, %rdx
cmpl $8, %edi
jge LBB0_2
LBB0_3: ## %hoist1
xorl %eax, %eax
testl %edi, %edi
jle LBB0_6
.p2align 4, 0x90
LBB0_5: ## %loop1
## =>This Inner Loop Header: Depth=1
vcvtss2si (%rsi,%rax), %ecx
movl %ecx, (%rdx,%rax)
decl %edi
addq $4, %rax
testl %edi, %edi
jg LBB0_5
LBB0_6: ## %leave
vzeroupper
retq
.cfi_endproc
## -- End function
Change-Id: Ib59eb3fd8a6805397850d93226c6c6d37cc3ab84
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/276738
Auto-Submit: Mike Klein <mtklein@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Herb Derby <herb@google.com>
- hook up fmls.4s as fnma_f32
- add fneg.4s
- use fneg.4s + fmls.4s to impl fms_f32
- more tests to exercise these
Change-Id: I60173a5e4618ab968a9361e15334a1d63c001372
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/275412
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Add fms op and instruction generation. Do fms and fnma
instruction selection.
TODO: Add the ops to Arm
Change-Id: I7e53abd7f4752eb99c31dcbff1f2ea7cf28af6c9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/275197
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Peephole add(F32,F32) for an argument that is a mul().
As a flourish, only generate Op::fma_f32 on machines we know support
real fused mul-adds. This removes the ambiguity of whether Op::mad_f32
is an FMA or not; the new Op::fma_f32 is always an FMA, and otherwise
you'll just see ordinary mul-add. No more Op::mad_f32.
Change-Id: I38016a2430774583116d8d6a8ada677012c1a8fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/275138
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Herb Derby <herb@google.com>
We really only need to_unorm(),
and that's fine with trunc(mad(x, scale, 0.5)).
Change-Id: I1561c678501963a9ae53c22994fc906159fc7199
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/275075
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Implement all AE grouping modes: character/word/line/all.
-- character grouping was already supported (default mode)
-- for word and line grouping, expand the existing domain mapping logic
to also track cumulative advance and max(ascent) per span, then use
this info to compute anchor point boxes
-- for "all" grouping, the anchor point box coincides with the text box
(https://helpx.adobe.com/after-effects/using/animating-text.html#text_anchor_point_properties)
TBR=
Change-Id: I8564f1349d167d82c31862d8f7e57615cdae0dcf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/274201
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
In adition to transforms/opacity/etc, text animators can target
per-glyph opacity.
Change-Id: I6ab63a6e49a64beaf63fc955f0b672a5b8ba84ba
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/272886
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
When per-character 3D is enabled, text properties can be animated in
3 dimensions.
- position and scale become 3-value vectors
- in addition to existing "r" (really rz), rotation gains "rx" and "ry"
- instead of specializing for 3D, expand the existing structures to
handle both 3D and 2D modes
- also ensure that sksg::Transform does not flatten to SkMatrix
Change-Id: I426a7ee1ff38c1702deb85e9f1db80f6069f36d6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/272648
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
AE discards lines with baselines outside the paragraph box.
This aligns Skottie's behavior with AE for default/top-alignment
(but not for any of the custom vertical alignment modes).
Bug: skia:9933
Change-Id: Id0318f0744bf89580774e89494faf19bfb6f6d14
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/272376
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Stroking in Skia follows the SVG rules of adding end caps to degenerate
contours. Skip all degenerate contours and degenerate curves on contours
to avoid this.
Bug: skia:9820
Change-Id: I320beeeb3728f39c764729454dcb128a05524d35
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/268166
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
There are probably ways to make this more efficient by only optimizing
what's necessary (e.g. try JIT first, then interpreter only if it fails)
and some other performance improvements to make, but for now I want to
focus mostly on keeping things simple and correct.
The line between Builder::done() and Program::Program() is particularly
fuzzy and becoming fuzzier here, and I think that'll be something
that'll change eventually.
This makes SkVMTest debug dumps more portable, though perhaps less
useful. Might kill that feature soon now that SkVM is tested more
thoroughly in unit tests and GMs and bots and such.
Change-Id: Id9ce8daaf8570e5bea8b10f1a80b97f5b33d45dc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/269941
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Bug: b/135133301
Follow-on to 196f319b.
- Add SkCodec::getICCProfile to match the SkAndroidCodec version.
- Update comments on getPixels() regarding how the SkColorSpace on the
SkImageInfo is treated.
- Add two new images that have ICC profiles that do not map to an
SkColorSpace. Add a test to verify that they have the un-transformed
color we expect.
- Stop uploading ColorCodecSrc images decoded to a null SkColorSpace to
Gold. Though they may be correct, they do not match other images they're
compared against. The new test above verifies that we do not do color
conversion with a null SkColorSpace.
Change-Id: I08635e4262f16500fab32ef97511d305c2c06483
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/269236
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
This adds a specialization pass to Builder::optimize() and moves the
x86-specific _imm ops there, rewriting with the Builder API itself. I'm
only using the private Builder::push() call for the moment, but that's
enough to make me feel confident that this is a good way forward: it's
still all going through CSE that way.
We're still doing this any time we're on x86, not when targeting the
JIT, but that'll come next, see the new TODOs. It's mildly better for
the interpreter to not use the _imm ops, but this is really all still
warmup for optimizations with less mild opinions.
I'm not proud of the switch/goto impl but it's the clearest I found.
Change-Id: I30594b403832343528b95967724fd50324cd79d1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/269232
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Kind of brewing a big refactor here, to give me some room between
skvm::Builder and skvm::Program to do optimizations, bakend
specializations and analysis.
As a warmup, I'm trying to split up today's Builder::Instruction into
two forms, first just what the user requested in Builder (this stays
Builder::Instruction) then a new type representing any transformation or
analysis we've done to it (OptimizedInstruction).
Roughly six important optimizations happen in SkVM today, in this order:
1) constant folding
2) backend-specific instruction specialization
3) common sub-expression elimination
4) reordering + dead code elimination
5) loop invariant and lifetime analysis
6) register assignment
At head 1-5 all happen in Builder, and 2 is particularly
awkward to have there (e.g. mul_f32 -> mul_f32_imm).
6 happens in Program per-backend, and that seems healthy.
As of this CL, 1-3 happen in Builder, 4-5 now on this middle
OptimizedInstruction format, and 6 still in Program.
I'd like to get to the point where 1 stays in Builder, 2-5 all happen on
this middle IR, and 6 stays in Program. That ought to let me do things
like turn mul_f32 -> mul_f32_imm when it's good to and still benefit
from things like common sub-expression elimination and code reordering
happening after that trnasformation.
And then, I hope that's also a good spot to do more complicated
transformations, like lowering gather8 into gather32 plus some fix up
when targeting an x86 JIT but not anywhere else. Today's Builder is too
early to know whether we should do this or not, and in Program it's
actually kind of awkward to do this sort of thing while also doing
having to do register assignment. Some middle might be right.
Change-Id: I9c00268a084f07fbab88d05eb441f1957a0d7c67
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/269181
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Similar to existing ADBE Easy Levels2, but provides separate mapping
controls per channel.
Change-Id: Ibc58c58e1e8cb8793d6eb819998c1804ccbbf859
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/268936
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
The GM exercises the compressed image formats using externally created resources
Note: the original image for the new flower resources can be found on Wikimedia Commons and has a "CC0 1.0 Universal Public Domain Dedication" license.
Bug: skia:9680
Change-Id: I6c5f9a12fcbbecdc3ba548dbb078bc21522073fe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/267836
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Also fix a couple of custom props issues:
- solid layer colors were not dispatched
- text values were not sync'ed
TBR=
Change-Id: I827f8c1d8c8bb73b03f05de15e1c7c96753a631e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/264936
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
extract() can generate silly instruction patterns like
v0 = ...
v1 = shr v0 24
v2 = bit_and v1 FF
v3 = whatever v2 ...
This CL skips those pointless bit_ands when we see the
mask is an immediate and (0xFFFFFFFF>>shift) == mask.
Change-Id: I2bb3847fbb2efdf24d024870ac37b37bb8f9aa3c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/263101
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
- Remove extract... it's not going to have any special impl.
I've left it on skvm::Builder as an inline compound method.
- Add no-op shift short circuits.
- Add immediate ops for bit_{and,or,xor,clear}.
This comes from me noticing that the masks for extract today are always
immediates, and then when I started converting it to be (I32, int shift,
int mask), I realized it might be even better to break it up into its
component pieces. There's no backend that can do extract any better
than shift-then-mask, so might as well leave it that way so we can
dedup, reorder, and specialize those micro ops.
Will follow up soon to get this all JITing again,
and these can-we-JIT test changes will be reverted.
Change-Id: I0835bcd825e417104ccc7efc79e9a0f2f4897841
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/263217
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
- Add instruction numbers to program dumps.
- Dump the program when an assertion fails,
and print the failing condition or an optional
other value (e.g. if alpha outside [0,1], print alpha).
With all that and the new commented assert enabled, I'm seeing that
sometimes we get a bilerp alpha of 0x3f800001, just a little more than
1.0f. Fix still tbd.
Change-Id: I2c20e41ae370d8cd2963e2dbf0fd91aa0fd50061
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/262808
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
With the recent transition to creating fonts from data as CTFonts and
dropping variation support from macOS 10.11 and earlier, it is now
possible to reliably make variation clones and get the axis information.
Change-Id: Ia9a0922ac94a29e1508d2e74d4ce973751044866
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/259421
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Ben Wagner <bungeman@google.com>
Currently, we treat track matte source layers (tagged with td:1) as single-shot mask triggers:
we apply once to the following layer, then move on.
But track mattes can cascade: a layer with a matte can itself be applied as a track matte for the
following layer.
Also, for matte/masking purposes, only the layer content is being considered (ignoring blend mode
and any masks applied to the matte itself).
To support this, refactor the layer attachment code:
- instead of tracking the presence of a single-shot matte source, always track
previous layer content trees
- instead of triggering matte attachment in the presence of a matte source, trigger based on
the matte *target* property (tt: X)
- log errors on unknown matte modes
Change-Id: I6c71d4007e1e27d3f3a139344bbf367d7bc6e29d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/259820
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Precomp layers can have a different size vs. main composition.
Instead of relying on the global animation (main comp) size, use the
current (pre)comp size when setting up cameras.
Change-Id: I54106375fb39dde2bfd11e14a38e5ec3e7190764
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/258156
Commit-Queue: Florin Malita <fmalita@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>
Auto-Submit: Florin Malita <fmalita@chromium.org>
Reviewed-by: Mike Reed <reed@google.com>
Provides functionality similar to AE property maps
Change-Id: I1705706a6b7e25fbab55465f2e20d0b145330b0b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/255977
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Currently just for image drawable, but going to use this for
references to other kinds of data in bindings, too.
Change-Id: Ic6673530013337bbaadd2d3f1c040626ec24ffb8
Bug: skia:9513
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/256776
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
This adds a bunch of tests for ops that can all be evaluated directly in
skvm::Builder. You can see the sort of effect this has by looking at
the diffs for SkVMTest.expected... lots of `v3 = sub_f32 v2 v2`
transformed to `v3 = splat 0 (0)` and that sort of thing.
My favorite part is handling many assert_true() calls at compile time!
While the old inter-Op code parallels aren't as clear now, these new
early-out tests kind of work like comments explaining each op. I find
that nice. I found it hard to parse so many uses of the word "splat" so
I did go back to isImm() from isSplat(), and added allImm() to test for
and read several immediates all at once.
Some of this is less C++17 than I'd like. :/
Change-Id: Ie8187d5d184195e3c0c92d613508fb708c28302f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/255814
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
So far Skottie has been assuming all cameras are two-node (have a point
of interest).
AE also supports one-node cameras, where the camera does not auto-orient
towards a POI but starts off perpendicular to the z == 0 plane.
(https://helpx.adobe.com/after-effects/how-to/camera-animation.html)
Change-Id: Id565de7d8feb9a762940ac372c1bbbcce2e2dfc6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/254559
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Lots of x86 instructions can take their right hand side argument from
memory directly rather than a register. We can use this to avoid the
need to allocate a register for many constants.
The strategy in this CL is one of several I've been stewing over, the
simplest of those strategies I think. There are some trade offs
particularly on ARM; this naive ARM implementation means we'll load&op
every time, even though the load part of the operation can logically be
hoisted. From here on I'm going to just briefly enumerate a few other
approaches that allow the optimization on x86 and still allow the
immediate splats to hoist on ARM.
1) don't do it on ARM
A very simple approach is to simply not perform this optimization on
ARM. ARM has more vector registers than x86, and so register pressure
is lower there. We're going to end up with splatted constants in
registers anyway, so maybe just let that happen the normal way instead
of some roundabout complicated hack like I'll talk about in 2). The
only downside in my mind is that this approach would make high-level
program descriptions platform dependent, which isn't so bad, but it's
been nice to be able to compare and diff debug dumps.
2) split Op::splat up
The next less-simple approach to this problem could fix this by
splitting splats into two Ops internally, one inner Op::immediate that
guantees at least the constant is in memory and is compatible with
immediate-aware Ops like mul_f32_imm, and an outer Op::constant that
depends on that Op::immediate and further guarantees that constant has
been broadcast into a register to be compatible with non-immediate-aware
ops like div_f32. When building a program, immediate-aware ops would
peek for Op::constants as they do today for Op::splats, but instead of
embedding the immediate themselves, they'd replace their dependency with
the inner Op::immediate.
On x86 these new Ops would work just as advertised, with Op::immediate a
runtime no-op, Op::constant the usual vbroadcastss. On ARM
Op::immediate needs to go all the way and splat out a register to make
the constant compatible with immediate-aware ops, and the Op::constant
becomes a noop now instead. All this comes together to let the
Op::immediate splat hoist up out of the loop while still feeding
Op::mul_f32_imm and co. It's a rather complicated approach to solving
this issue, but I might want to explore it just to see how bad it is.
3) do it inside the x86 JIT
The conceptually best approach is to find a way to do this peepholing
only inside the JIT only on x86, avoiding the need for new
Op::mul_f32_imm and co. ARM and the interpreter don't benefit from this
peephole, so the x86 JIT is the logical owner of this optimization.
Finding a clean way to do this without too much disruption is the least
baked idea I've got here, though I think the most desirable long-term.
Cq-Include-Trybots: skia.primary:Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-SK_USE_SKVM_BLITTER,Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Release-All-SK_USE_SKVM_BLITTER
Change-Id: Ie9c6336ed08b6fbeb89acf920a48a319f74f3643
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/254217
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
The matrices we're using can produce very slightly out of range color
channels. This gives surprising results when in shader blending is used
for color burn and color dodge. After this change we clamp the RGB
values to 0..1 before applying premul.
Adds a GM modeled on a blink layout test that shows the problem using
SkImageMakeFromYUVAPixmaps.
Bug: skia:9619
Change-Id: I446d39763a7f5a2f7c5f61d94d163927d851baa3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253879
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This does open us up to a little bit of possible inconsistency of
rounding when right on a x.5 (sometimes we'll +0.5 and trunc, sometimes
round to nearest, sometimes round according to the default mode which is
usually round to nearest) but I think that inconsistency may be worth
the free register not needing a splat(0.5f) buys us.
A few invisible diffs.
Change-Id: I9af092c937ccf7c5891c2ab3cb298d217e4a9e9f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253725
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
This plumbs through round but doesn't use it. I want that change to be
its own CL. It's nice to have assembler support and the name changes
even if I revert using round.
Change-Id: I6d67ec5c63546069eb7cc1c91599b599bafcda66
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/253724
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Refactor as a single interpolating loop, based on careful selection
of lerp coefficients.
Change-Id: I58786cddb2f042b53dcbac80c2346736429be102
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252858
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Mike Reed <reed@google.com>
Change-Id: Iea0f804b1b2fed9e663e45c33fb54a91b10fd07b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252652
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Observed AE layer parenting semantics:
* layers are flagged as either 2D or 3D
* camera applies to 3D layers, but not to 2D layers
* parented 3D layers treat their ancestor transform chain as 3D (SkMatrix44)
* parented 2D layers treat their ancestor transform chain as 2D (SkMatrix, ignoring 3D components)
This means that for a given layer, we may need to build two distinct transform chains - depending
on the type of descendant layer being considered.
Furthermore, transforms are animatable and their animators are scoped to a layer controller. Since
we're potentially building two version of the transform node, we need to ensure all animators for
both of them are transferred to controller object (we still want to only instantiate a single layer
controller and render tree to avoid duplication). IOW, all dependent layer transforms need to be
considered before "sealing off" a given layer controller.
In order to avoid a layer dependency/topological sort, we can split off the transform tree
construction into a separate pass. High-level changes:
-- replace existing LayerAttachContext with CompositionBuilder
(holds LayerBuilders and other Composition-wide state)
-- replace LayerRec with LayerBuilder
(holds Layer-wide state and also caches transform nodes)
-- pass 1: for each LayerBuilder, transitively build and cache a transform chain
of a type (2d/3d) determined by the leaf (entry point) layer
-- pass 2: for each LayerBuilder, build the actual layer content render tree
and instantiate the layer controller objects
Bug: skia:8914
Change-Id: I9f7efcf4819424282fd3dda98f5621ba12fd001b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/251001
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Use `extra_cflags=["-DSK_CAPTURE_DRAW_TEXT_BLOB"]` to enable.
Change-Id: I1d6db478ee91696cdce090647b889c17a83a2718
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/250259
Commit-Queue: Hal Canary <halcanary@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Rewrite program instructions so that each value becomes available as
late as possible, just before it's used by another instruction. This
reorders blocks of instructions to reduce them number of temporary
registers in flight.
Take this example of the sort of program that we naturally write,
noting the registers needed as we progress down the right:
src = load32 ... (1)
sr = extract src ... (2)
sg = extract src ... (3)
sb = extract src ... (4)
sa = extract src ... (4, src dies)
dst = load32 ... (5)
dr = extract dst ... (6)
dg = extract dst ... (7)
db = extract dst ... (8)
da = extract dst ... (8, dst dies)
r = add sr dr (7, sr and dr die)
g = add sg dg (6, sg and dg die)
b = add sb db (5, sb and db die)
a = add sa da (4, sa and da die)
rg = pack r g ... (3, r and g die)
ba = pack b a ... (2, b and a die)
rgba = pack rg ba ... (1, rg and ba die)
store32 rgba ... (0, rgba dies)
That original ordering of the code needs 8 registers (perhaps with a
temporary 9th, but we'll ignore that here). This CL will rewrite the
program to something more like this by recursively issuing inputs only
once needed:
src = load32 ... (1)
sr = extract src ... (2)
dst = load32 ... (3)
dr = extract dst ... (4)
r = add sr dr (3, sr and dr die)
sg = extract src ... (4)
dg = extract dst ... (5)
g = add sg dg (4, sg and dg die)
rg = pack r g (3, r and g die)
sb = extract src ... (4)
db = extract dst ... (5)
b = add sb db (4, sb and db die)
sa = extract src ... (4, src dies)
da = extract dst ... (4, dst dies)
a = add sa da (3, sa and da die)
ba = pack b a (2, b and a die)
rgba = pack rg ba ... (1, rg and ba die)
store32 rgba ... (0)
That trims 3 registers off the example, just by reordering!
I've added the real version of this example to SkVMTest.cpp.
(Its 6th register comes from holding the 0xff byte mask used
by extract, in case you're curious).
I'll admit it's not exactly easy to work out how this reordering works
without a pen and paper or trial and error. I've tried to make the
implementation preserve the original program's order as much as makes
sense (i.e. when order is an otherwise arbitrary choice) to keep it
somewhat sane to follow.
This reordering naturally skips dead code, so pour one out for ☠️ .
We lose our cute dead code emoji marker, but on the other hand all code
downstream of Builder::done() can assume every instruction is live.
Change-Id: Iceffcd10fd7465eae51a39ef8eec7a7189766ba2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/249999
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Change-Id: I6d29290eb2962262bb080a86dc829c39986cae4f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/249226
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
- Copy effect state to particle uniforms before each script, so changes
from spawn or update are visible.
- Guard path binding against out of range access
- New effect that actually stresses both of these conditions
Change-Id: Ice6112793099e515438af8bb863e9e1bf03d08b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/249125
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Most hoisted values are used in the loop body (and that's really the
whole point of hoisting) but some are just temporaries to help produce
other hoisted values. This used_in_loop bit helps us distinguish the
two, and lets us recycle registers holding temporary hoisted values not
used in the loop.
The can-we-recycle logic now becomes:
- is this a real value?
- is it time for it to die?
- is it either not hoisted or a hoisted temporary?
The set-death-to-infinity approach for hoisted values is now gone. That
worked great for hoisted values used inside the loop, but was too
conservative for hoisted temporaries. This lifetime extension was
preventing us from recycling those registers, pinning enough registers
that we run out and fail to JIT.
Small amounts of refactoring to make this clearer:
- move the Instruction hash function definition near its operator==
- rename the two "hoist" variables to "can_hoist" for Instructions
and "try_hoisting" for the JIT approach
- add ↟ to mark hoisted temporaries, _really_ hoisted values.
There's some redundancy here between tracking the can_hoist bit, the
used_in_loop bit, and lifetime tracking. I think it should be true, for
instance, that !can_hoist && !used_in_loop implies an instruction is
dead code. I plan to continue refactoring lifetime analysis (in
particular reordering instructions to decrease register pressure) so
hopefully by the time I'm done that metadata will shake out a little
crisper.
Change-Id: I6460ca96d1cbec0315bed3c9a0774cd88ab5be26
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/248986
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Gives enough information to locate variables by name (using the same
scheme as glGetUniformLocation), and provide hints about type and size.
Bug: skia:9513
Change-Id: I9444f1042471967a79c9f05167dcdb78eca41bad
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/244502
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Simplify burst handling. Scripts should just add to burst (if
they want to handle programmatic bursting, as well).
Update most effects to handle dynamic updates to position better,
and add a sample effect meant to be used with mouse tracking.
Change-Id: Ia302e1d04e62e2b07974807c44067786cc10a8ad
Bug: skia:9513
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/248798
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This was only being used in one effect (and for no good reason). SkSL is
plenty powerful to re-implement something similar if required, at no
real performance cost.
Re-implemented the one effect that used it with simpler math in the
script, updated the copy of that effect in the gallery.
Docs-Preview: https://skia.org/?cl=247040
Change-Id: I68c86d6550dd4f003f6ba5ecd0febab37b86540b
Bug: skia:9513
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/247040
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Confetti mimics the look of a standard skottie asset
Change-Id: Iffeedeb24182c4ac2d3ec390614bc1861b821376
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/246518
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Also removed some older effects that weren't interesting, improved others,
cleaned up the unused functions in several, and renamed most of them to
reflect which feature they're demonstrating.
Change-Id: Ib44a00ec3d25e852a1d1661918137ba13d30c86b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/244119
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
* Added a new binding type, SkEffectBinding. This stores another
entire effect params structure (so the JSON is just nested).
The name is a callable value that spawns a new instance of
that effect, inheriting the parameters of the spawning effect
or particle (depending on which kind of script made the call).
* Broke up the monolithic update function into some helpers,
got some code reuse with the script calling logic.
* Unlike particle capacity, there is no upper limit on child
effects (yet), so it's easy to trigger runaway memory and
CPU consumption. Be careful.
* Added death scripts to effects and particles, which are a
common place to want to spawn sub-effects. Like spawn,
these run on each loop, but for one-shots they play at the
end. Even with loops, this is helpful for timing sub-effects
(see fireworks2.json).
* Finally, added a much more comprehensive example effect,
raincloud.json. This includes a total of three effects, to
generate a cloud, raindrops, and splashes when those drops
hit "the ground".
Change-Id: I3d7b72bcbb684642cd9723518b67ab1c7d7a538a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/242479
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This change adds another layer of complexity and control to
the particle system. There are now two code chunks: the old
code that's run per-particle, and new code that's run for
the effect itself. This allows for effect lifetime to be set
by the script (eg, randomly), as well as the emission rate.
Rate can vary over time (see pulse.json), and particles can
be emitted in bursts by setting the effect's burst field
(see fireworks.json).
Additionally, the effect has its own frame of reference and
color, which becomes the default state for newly emitted
particles. This allows synchronizing state across particles
in various interesting ways (see color in fireworks.json).
Change-Id: Iec2f7a3427ce1d6411ed7ef5b3023cbef2e8a134
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/240498
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
We're currently letting render context overrides (opacity, color
filters, blend mode, etc) spill down the descendent/mask content
tree.
This is not ideal, as mask content isolation breaks atomicity
assumptions for deferred overrides. Case in point: motion blur uses
SkBlendMode::kPlus to accumulate content "layers" - but since mask
content gets rendered into a separate layer, it fails to produce the
expected result.
The fix is to realize all context overrides on the top-level mask layer
(we already allocate this layer, so there's no reason to defer
downstream anyway).
Change-Id: Icbb7e403f90feecfae5846697f559a03d8aa4097
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/239036
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Change-Id: If99e1802c8187ebd98b67717d744c6695bb25900
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/238118
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Introduce a new hybrid valign extension, kVisualDownscaleToFit (sk_vj: 4):
- when the text shaped at the requested size fits within the box,
center vertically (same as kVisualCenter)
- otherwise, scale down until it fits (same as kVisualResizeToFit)
Change-Id: I8e096a49e2b87582e1bd42161657ec4ef561ebdf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/235601
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
This makes the register recycling checks a bit more
precise. At head we never recycle a register that's
holding a hoisted value, which is overly conservative.
We really should never recycle a register that's still
needed. By extending the lifetime of any hoisted value
that's used in the loop, we prevent that, while still
allowing hoisted values that are only used in hoisted
computation to be reused.
This takes just a small tweak in the JIT code (removing
the !hoisted({x,y,z}) checks), and a somewhat larger
refactoring in the interpreter, making both hoisted and
non-hoisted code go through the same recycling register
assignment flow.
There's one diff in the existing cases where we now
reuse a hoisted register, and I've added a second test
just to make sure it's covered explicitly.
Change-Id: I25b37ab1f1fea3042d7fd167529abc8fed1dddff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/233239
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Due to limitations in BodyMovin/AE JSX, full effect data is not
available (specifically the "channel range" property).
We only support static master hue, static master saturation and
static master lightness at this point.
This CL also introduces a new animation builder pattern:
DiscardableAdapterBase and attachDiscardableAdapter().
The former is a base class for adapters with full animator ownership.
This enables a) capturing raw adapter pointers in animator lambdas and
b) syncing to SG only once, after all local animators are updated).
The latter is a helper for managing adapter creation and optional
destruction (when all adapter properties are static we can discard it).
Change-Id: Iecc4b78830e5464e7958cb12cdfd75a61010aa25
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/231956
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Add missing comparison and selection ops, bit casts, 16-bit memory
operations, gathers, uniform loads, and fill in math holes where
reasonable. Update some names to be a bit more regular.
I think all instructions are implemented in the interpreter,
and many tested. More testing and JITs to follow.
Change-Id: I8cf377e8b72a86ac950e020892ce82b39e9d7277
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/229893
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Change-Id: If57fb79db8f8c5fd185fefaa202167c8082dd846
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/229921
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: Ic81b3433b485ca9ce0e60bd10ec12706e673ee89
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/229917
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This removes all of the fixed-function particle affector classes.
Instead, each particle effect just has two SkSL snippets, one for
spawn logic, and one for update logic. Each one gets an inout copy
of the particle struct. Ultimately, this makes the effects much
simpler and smaller, while also being far more flexible (you can
do whatever you want with any values you want). Finally, because
the interpreter is vectorized and a particular effect's scripts
are usually tuned to the specific behaviors desired, it's faster
on basically every effect I compared.
I re-created all of the old effects in the new system. Many just
use pure SkSL (no curves or anything). Some of the old curve and
path/text stuff was very handy, though - so those are now exposed
as external values in the interpreter. Basically, an effect can
have any number of named "bindings" that are a callable thing.
This can be a path, text (shortcut for making fancy paths), curve,
or color curve. The path ones return a float4 with position and
normal, the curves return one or four floats.
... and this transposes all of the particle data storage into
SoA form, so that it can use the much faster interpreter entry
point.
Change-Id: Iebe711c45994c4201041b12d171af976bc5e758e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/222057
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Even if a JIT ultimately doesn't end up hoisting any values, it's going
to want this information while it decides. Writing it in one place also
ensures we only get it wrong in one place...
I'm no_ extending the lifetime of hoisted instructions here in Builder.
That's something to leave to the backend so they have the flexibility of
which of these values to hoist, if any. If they don't hoist, they'll
need to know when the value dies.
Moving this information back here lets the test expectation goldens
reflect the hoist bit again too. Kind of nice.
Change-Id: Ib165ca898a97c1d822cb28fe24f15bae4d570a17
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/229024
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
I'm slowly refactoring my way to where hoisting and register assignment
are done in backend-specific ways, but this liveness analysis is always
going to be useful for each backend.
Use deaths() to restore friendly ☠️ dead code markers in test dumps.
Change-Id: I3ab94665bbbbf0788b0b27e00d644eba927dff47
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/228113
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Unlike all other Skottie effects, motion blur requires sampling at multiple
points on the timeline.
To support this:
1) Introduce MotionBlurEffect - a custom SG render node which can drive
the timeline of its subtree using an sksg::Animator.
2) Introduce MotionBlurController to swap for a regular LayerController
when needed. MotionBlurController dispatches time ticks to
MotionBlurEffect instead of directly to the layer animators.
The actual motion blur impl is based on
https://skia-review.googlesource.com/c/skia/+/221416.
Motion blur requires Lottie files exported with this BodyMovin patch:
https://github.com/bodymovin/bodymovin-extension/pull/15
Change-Id: I075e101ea91ec9aa300bac35ee810fd539f1aced
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/225416
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Noticed we were only dumping the final register
programs for the integer code. Might as well also
track the value programs.
Change-Id: I417c5c655b632691557bbbb136dcbd3f3167af9a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/225324
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
We used to rely solely on visual bounds for vertical alignment. That
had the downside of leading/trailing empty lines being ignored.
Then https://skia-review.googlesource.com/c/skia/+/220916 switched to
using typographical bounds. This approach produces results in line
with AE, but allows some glyphs to overflow the alignment boundary.
This CL introduces a hybrid approach:
1) for standard AE text alignment, continue to use typographical bounds
2) for Skottie VAlign extensions (sk_vj), use the union of typographical
and visual bounds - this should mitigate both issues mentioned above
Change-Id: Ifd3ccae3d721728ce67942206160ebe92056d3a2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/224188
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Avinash Parchuri <aparchur@google.com>
I was just reading the ARM docs and realized that their BIC ("BIt
Clear") is the same as SSE's ANDN ("AND Not") instruction. It's kind of
a neat little tool to have laying around... comes up more than you'd
think, and it's sometimes the clearest way to express what you're doing,
as in the changed program here where the comment is "mask away the low
bits". That's a bit_clear with a mask for what you want to clear away!
And the real reason to write this up is that I want to have a CL to
point to that shows how to add an instruction top to bottom.
Change-Id: I99690ed9c1009427b3986955e7ae6264de4d215c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/223120
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Instruction is the fundamental data, and Analysis derived from it.
The fields in Analysis are only* needed in Builder::done(), and this
split seems to help clarify what done() can tweak (Analysis) and what
it cannot (fProgram, Instructions). done() is now const.
No speed change as far as I can tell.
* As you may notice looking at the test expectations, making analysis
ephemeral means that dump() can no longer print the skull for dead code
or the arrow for hoisted. The register program that's also in the
expectation file still reflects both of these optimizations, so we're
not really losing any information. Just maybe less demo-friendly.
Change-Id: I79feb57558525591baf3faadeb59c418c12793f3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/223119
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This cuts the overhead bench from about 19µs to about 15µs.
The key insight here is that the only registers that might become
available after any given instruction are the ones that hold that
instruction's inputs. We can check when they become available
directly from the original Builder::Program, without needing a
side death schedule data structure.
Marking hoisted instructions as having life == program size
helps make this logic a little simpler to reason through.
Change-Id: Ifb9957f2d0e323e0e5d07996a2cc988f7c8b4c3f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/223117
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Implement radial wipe with a sweep gradient shader mask filter.
The implementation is slightly convoluted because edge feathering requires a real blur, which in turn requires content layer isolation.
So there are two distinct operation modes:
- no feather -> draw the content directly into the dest buffer, with the mask filter
deferred in SG context
- feather -> draw the content into a separate layer, then blend (dstOut) the composed
blur+shader mask on top
Change-Id: I253701aff42db8010ce463762252c262e2c5d92b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/222596
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
- 32x8 i32 add,sub,mul
- add I32_Naive bench/test builder to get better i32 mul coverage
- minor refactoring all over
Change-Id: I13cc19ff37a2da0bcff289ba51baac08f456d6c5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/222485
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
The motion tile phase is a one-dimensional shift, applied to every other
row or column (based on a selector property).
Implement using a masking shader (covering the static rows/cols),
and blend mode shader composition (srcIn for static/pass-through
rows/cols, and srcOut for phased rows/cols).
TBR=
Change-Id: I336c150e5d4900962dc2de801a4e1572cf4b5d59
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/221339
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
I'm staring at this assembly,
vmovups (%rsi), %ymm3
vpsrld $24, %ymm3, %ymm4
vpslld $16, %ymm4, %ymm15
vorps %ymm4, %ymm15, %ymm4
vpsubw %ymm4, %ymm0, %ymm4
Just knowing that could be
vmovups (%rsi), %ymm3
vpshufb 0x??(%rip), %ymm3, %ymm4
vpsubw %ymm4, %ymm0, %ymm4
That is, instead of shifting, shifting, and bit-oring
to create the 0a0a scale factor from ymm3, we could just
byte shuffle directly using some pre-baked control pattern
(stored at the end of the program like other constants)
pshufb lets you arbitrarily remix bytes from its argument and
zero bytes, and NEON has a similar family of vtbl instructions,
even including that same feature of injecting zeroes.
I think I've got this working, and the speedup is great,
from 0.19 to 0.16 ns/px for I32_SWAR, and
from 0.43 to 0.38 ns/px for I32.
Change-Id: Iab850275e826b4187f0efc9495a4b9eab4402c38
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/220871
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Yet another way to transform a layer, disguised as a distort effect.
TBR=
Change-Id: Ic2d5479fa6ae27b460de60875924f73f77fc7f71
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/221001
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Now that we've got shr_16x2, extract(..., 8, splat(0x00ff00ff)) is
better done as shr_16x2(..., 8). This swaps a 16-bit shift in for
the 32-bit shift, a wash, but lets us drop the bit_and at the end,
saving one whole instruction.
This places I32_SWAR a tiny little bit faster than the code in Opts,
like .19 ns/px vs .20 ns/px for Opts.
Change-Id: I4160dc03ecc8b855c0773a927f1510ad5cbb4b87
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/220856
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This is the final bunny I've got in my hat, I think...
Remembering that none of the s += d*invA adds can overflow,
we can use a single 32-bit add to add them all at once.
This means we don't have to unpack the src pixel into rb/ga
halves. We need only extract the alpha for invA.
This brings I32_SWAR even with the Opts code!
curr/maxrss loops min median mean max stddev samples config bench
36/36 MB 133 0.206ns 0.211ns 0.208ns 0.211ns 1% ▁▇▁█▁▇▁▇▁▇ nonrendering SkVM_4096_I32_SWAR
37/37 MB 152 0.432ns 0.432ns 0.434ns 0.444ns 1% ▃▁▁▁▁▃▁▁█▁ nonrendering SkVM_4096_I32
37/37 MB 50 0.781ns 0.794ns 0.815ns 0.895ns 5% ▆▂█▃▅▂▂▁▂▁ nonrendering SkVM_4096_F32
37/37 MB 76 0.773ns 0.78ns 0.804ns 0.907ns 6% ▄█▅▁▁▁▁▂▁▁ nonrendering SkVM_4096_RP
37/37 MB 268 0.201ns 0.203ns 0.203ns 0.204ns 0% █▇▆▆▆▆▁▆▆▆ nonrendering SkVM_4096_Opts
Change-Id: Ibf0a9c5d90b35f1e9cf7265868bd18b7e0a76c43
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/220805
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
I figure the easiest way to expose 16-bit operations
is to expose 16x2 pair operations... this means we
can continue to always work with the same size vector.
Switching from 32-bit multiplies to 16-bit multiplies
is going to deliver the most oomph... they cost roughly
half what 32-bit multiplies do on x86.
Speed now:
I32_SWAR: 0.27 ns/px
I32: 0.43 ns/px
F32: 0.76 ns/px
RP: 0.8 ns/px
Opts: 0.2 ns/px
Change-Id: I8350c71722a9bde714ba18f97b8687fe35cc749f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/220709
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
I just kind of remembered that if we're doing (xy+x)/256
and x is a destination channel and y is 255-sa, then you
can get the +x for free by multiplying by 256-sa instead.
(d * (255-sa) + d)
(d * (255-sa + 1))
(d * (256-sa) )
Duh. This is a trick we play in a lot of legacy code and
I've just now realized it's exactly equivalent to the trick
I want to play here... sigh.
Folding this math in kind of makes mul/mad_unorm8 moot.
Speed's getting good:
I32_SWAR: 0.3 ns/px
I32 : 0.55 ns/px
F32 : 0.8 ns/px
RP : 0.8 ns/px
Opts : 0.2 ns/px
Change-Id: I4d10db51ea80a3258c36e97b6b334ad253804613
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/220708
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
The mask-only special case for extract is wrong...
it never looked it its input!
This not only makes things correct-er, but oddly it also
makes them faster by breaking inter-loop data dependencies.
Disable tests for _I32... they're actually still broken
because of a much more systemic flaw in how I've evaluated
programs. The _F32 and _I32_SWAR JIT code and all interpreted
code is just getting lucky. o_O
While here, update the I32_SWAR code to use the same math as I32,
(x*y+x)/256 for unorm8 mul. This just helps keep me sane.
Change-Id: I1acc09adb84c426fca4b2be5ca8c2d46d9678dd8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/220577
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Add logic to adjust glyph positions based on animated tracking properties.
This adjustment is applied post-shaping (it doesn't observe the text box),
and requires line re-alignment - thus it is being processed per-line.
Change-Id: Id44a295032a48c7216f126cb02dd2d2d5cc18ae3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/220076
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Range selector's "Based On" property controls how range indices map
to glyphs: characters, characters-excluding-spaces, words, lines.
To support this feature:
- update SkottieShaper to track domain-relevant info per fragment
(fLineIndex, fIsWhitespace)
- update TextAdapter to build domain maps
(domain index -> fragment span)
- update RangeSelector to run its range indices through a domain map,
if present.
Change-Id: I80e713f6beaa2578aa0eae1d1ddae8e1e47d8d10
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/219859
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Ben Wagner <bungeman@google.com>
I used to have a dump of the value program before it was
translated to registers, but it went away a while ago.
This restores it.
Change-Id: I9b8bfcb124843cad4b0dc44bdf0a03e95a0c83d8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/219757
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Convert extract(x,bits,z) to be (x >> bits) & z,
now a more explicit parallel to pack().
This lets us eliminate the funky bit counting required from the old
instruction, but more saliently it makes it more likely that the masks
we AND with will be the same value.
Ultimately down at the x86 or ARM ISA level, the AND instructions don't
really benefit from having an immediate argument (while the shifts do).
We might as well treat the mask as a normal value, letting it get
commoned with identical values, loop hoisted, etc.
Change-Id: I48a38468b46f2c730574c025f412262296472447
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/219597
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The current implementation applies constant coverage (outside selector
range) based on computed integral edges.
But the integral range is clamped to the valid index domain and its
extremes are always assumed to have partial coverage - so we never get
to constant-blit the full buffer when the interval is outside, which
can yield incorrect coverage for the first/last fragments.
Update the constant coverage logic to operate in full domain coordinates.
Change-Id: I23902674fe5e822081fb8262167511df1cc3463e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/219206
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Introduce square/ramp/triangle/round/smooth shape generators,
and use them to seed the range selector coverage pipeline.
Change-Id: Ib7b94ceecd2ccf66820f4dd2443fdd62e2ac6a1b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218828
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Ben Wagner <bungeman@google.com>
At some point adding more and more complex instructions reduces
to the absurdity of SolveTheWholeProblem-The-Instruction, but
I think this one will come up often enough to still make sense.
mad() makes sense for unorm8 just about everywhere mad() makes
sense for f32.
This instruction won't matter to a JIT, but helps the interpreter.
Change-Id: Iace92296cffbb6fbc3acd1f853cb01c51792f796
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218716
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
I'm of two minds about this... it adds register pressure and really only
tends to hoist few instructions that are fairly cheap anway. On the
other hand, it's neat, it's easy to turn off (just set the initial
hoist value to false in Builder::push()) and it does deliver a
noticeable though slight performance improvement in the interpreter.
I think the final decision will probably come down to what we think
about maintainability?
Change-Id: Idd6346f70f03188917918406731154246a7c6fcb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218584
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This reverts commit 346f82c1c3.
Reason for revert: *SAN bots
Original change's description:
> print 1/K floats as fractions
>
> Change-Id: Id00cbd0950e77debb5ab5d45541dc0f8d13a3c42
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218338
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Mike Klein <mtklein@google.com>
TBR=mtklein@google.com,brianosman@google.com
Change-Id: Ic35cec97d2dc2c1e19dbdf8ea7b505ad75072da1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218529
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Change-Id: Id00cbd0950e77debb5ab5d45541dc0f8d13a3c42
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218338
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Each animator can have multiple range selectors, whose combined "coverage"
modulates how the animator props compose with other/initial props.
Since there can be multiple animators with different/arbitrary selectors,
we compute independent property values for each fragment.
Supported features:
- start, end, offset, amount
- units: percentage, index
- based-on: characters-only for now
- mode: add-only for now
- shape: square-only for now
Change-Id: If7fee46ffb29e1f92542822481ed699fd0b0b521
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218076
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Ben Wagner <bungeman@google.com>
Change-Id: Ib0d4f354787e413749fdda8b59ccc2f94472b0ce
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218243
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Kind of the flip side of pack.
Made slightly awkward by instructions having only one immediate...
calling _BitScanForward / __builtin_ctz() at runtime seems to work
fine, but it really could have been done at compile time.
Change-Id: Ic83fe8e0a1603fb9189598dcc26c842cc797bf45
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218241
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This instruction can lower to some useful SSE/NEON
instructions, and even if not, is a handy way to
express the frequent paring of << and |.
I32_SWAR: 2.3 -> 1.9
I32: 2.6 -> 2.4
F32: 5.1 -> 4.7
Change-Id: Ia169ad40f0aaef32417e05d9bf91c2d2542e7b5f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218238
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Another way for an interpreter to go faster
is to provide better instructions.
mul_unorm8 is one we use all the time.
Drops _I32 bench from ~3.6ns/px to ~2.6ns/px.
Change-Id: I9d08914c114048b79075796af9ec802236b35706
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218236
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Eliminate the duplicate functionality,
and better testing for the bench builders.
Change-Id: If20e52107738903f854aec431416e573d7a7d640
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218041
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This is mostly to test how easy rebaselining SkVMTest is.
Change-Id: I27ab6f6bb8b7e126327374009783afd86d416f55
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218039
Reviewed-by: Brian Osman <brianosman@google.com>
- keep expectations in resources/
- overwrite automatically if needed
so we can see the diff in Git
Change-Id: I2486b127ebcc7f40332fd0462e38b1af04d3e32b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218038
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
The interesting bit here is a change in glyph positioning:
AE text animator transforms are to be applied relative to the glyph position.
To support this behavior, update Shaper to externalize glyph positioning
when in fragmented mode. I.e. instead of baking glyph positions in blobs,
apply them at the scene graph transform level (such that they compose with
animated transforms correctly).
Change-Id: I9aeb5e6f8c1ec1a2c8b5351e8fc2a73d4bdf5cad
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/217556
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Limitations:
- no range selectors (applies to the whole text)
- only position, fill color and stroke color for now
Change-Id: I91e88a6107c5f66687c1c27f27a71be3914bde25
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/217386
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Ben Wagner <bungeman@google.com>
Introduce a new Shaper::Valign enum to support aligning the shaped text
visual bottom with the text box bottom.
This option corresponds to JSON prop sk_vj: 2.
kResizeToFit (used to be sk_vj: 2) is now bumped to sk_vj: 3.
Change-Id: Ib1621a21a42bfc21c99826e203c587a3fdc663dc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/215821
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
This is implemented at backend-neutral level and so misses some
opportunities to reduce the number of passes in the GPU backend.
Filter quality is interpreted as:
none - single nearest neighbor resampling
low - chain of bilinear resamplings. 2x up/down except for one
step which may be smaller than 2x.
medium - same as low
high - when both scale factors are up then same as low but with bicubic
filtering rather than linear. Otherwise, same as low.
Bug: skia:8962
Change-Id: I4467636c14b802d6a0d9b5c363c1ad9e87a1a44b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/213831
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Update sample effects to use that (and remove the need for the
hacky workaround "random -> frame" affector I was using).
Current perf on my workstation, 6k particles updating:
native: 0.67 ms
interp: 0.97 ms
Change-Id: I3a2168c210d7431ffffe2b87ab6adade69f1dce7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/214190
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: Ib440570ecbd46b5bc98d346592cbbb72f58ae85a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/212500
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Introduce a new SkottieShaper VAlign option (kResizeToFit), to scale the text
size for the best box fit.
The basic idea is to perform a binary search on the font size, until
the shaped text fits snuggly within the specified box. The search is
focused on height, as horizontal fitting is assumed to be handled in
SkShaper.
Change-Id: I56269e02dda7a34e4ef3b79c205ea651b909f370
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/212962
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
This allows for testing falling into various buckets in the gpu
fallbacks.
Change-Id: Ia0c319a6bdd03c5cdece1ce83ab228c1a3a7c46d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/199420
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Motivation: it would be a good idea if the API documentation examples
were checked into the skia repository, so we could make sure they
compile as part of the commit queue.
Fiddle would make/update a named fiddle each time it gets a new
commit of Skia, extracted from the code in the examples/ directory.
The docs would point at those named fiddles. Named fiddles have urls
in the form:
https://fiddle.skia.org/c/@Bitmap_000
Then we would stick a link to the example into the header documentation
like this:
/** Allocates the pixel memory for the bitmap, given its dimensions
and SkColorType. Returns true on success, where success means
either setPixels() or setPixelRef() was called.
@param bitmap SkBitmap containing SkImageInfo as input, and
SkPixelRef as output
@return true if SkPixelRef was allocated
@example https://fiddle.skia.org/c/@Bitmap_000
*/
bool allocPixelRef(SkBitmap* bitmap) override;
There are still around 200 disabled examples that need to be fixed
(these result from API changes since the author left).
Change-Id: I14a31348a9ccaaa31f65424b91e3a3533d2583a7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/198824
Commit-Queue: Hal Canary <halcanary@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Bug: skia:
Change-Id: I499262277ac1c8d92a39a66f6e846e248b102aef
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/197767
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
All curves (and path affectors) are driven by an SkParticleValue. The
value can derive its value from the current defaults (age of particle
or effect), or explicitly choose the other one, a random value, or any
other particle value. Values can be range adjusted and support repeat,
clamp, and mirror tiling.
Also fixed some more issues related to resource path in the slide GUI.
Bug: skia:
Change-Id: I4755018d5b57ae2d5ec400d541055ca4fb542978
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/196760
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:
Change-Id: Ie6485a11bb57fecef470d727dcf3b4fe5dff0b90
Reviewed-on: https://skia-review.googlesource.com/c/195582
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Added explicit Linear segment type, merge math evaluation helpers for
scalar and color curves. Add logic to visitFields that cuts down on the
serialized size of simple curves, and makes the GUI easier to work with.
Remove the curve plot from the GUI. It was incorrect (wrong points at
cubic handle locations), not terribly helpful, and difficult to
maintain.
Bug: skia:
Change-Id: I190cb5d118b1f4b910984e4df50ee3351c8be895
Reviewed-on: https://skia-review.googlesource.com/c/195884
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The other generator was never used (or useful). String-based serialization
of enums is quite helpful, though.
Bug: skia:
Change-Id: Ic9d58f8d20cfe7aba47722bd74f1e6f8f0f219e5
Reviewed-on: https://skia-review.googlesource.com/c/195368
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Add particle "frame" enum, to allow effects relative to local, world,
or velocity. Remove the "orient along velocity" and replace with a much
more general orientation affector (angle curve + frame). Add an angular
velocity affector to mirror the behavior of the linear velocity affector.
Bug: skia:
Change-Id: Ibbaaeb352c9547d00d81c7916d00148dd65ed2b9
Reviewed-on: https://skia-review.googlesource.com/c/195361
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Adjust reference frame for affector to be consistent (so angles are
counted clockwise from "up" in both local and world modes).
Bug: skia:
Change-Id: I643e1484bc0a58d1f1c0cfe35ac2ab37dc2ea409
Reviewed-on: https://skia-review.googlesource.com/c/194189
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Move to a model that only needs floats, and puts the age in the
SkParticleState struct. Add a better test case for spawn affector
animation, to verify that things are still working.
Bug: skia:
Change-Id: I97d99de5f5d4cb302b76116e67ecc93368fb1677
Reviewed-on: https://skia-review.googlesource.com/c/193580
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
- Collapsed the per-particle data into a single struct, and
use that to communicate with drawables, too. Let the drawables
manage allocation of xforms, colors, etc. Helpful for non-atlas
drawables, and just to keep the effect code simpler.
- Having all of the params in a single struct allows us to move
the remaining animated behaviors into affectors (color/frame).
- Added SkColorCurve, which works like SkCurve for SkColor4f.
Use that to create a color affector (rather than simple
start/end colors in the effect params).
- Also put the stable random in SkParticleState. This is going
to be necessary if/when we change affectors to operate on all
particles (rather than one at a time). Still need to move t
value into the particle struct (or eval it from the lifetime
params on demand).
Change-Id: Icf39116acbfd5d6e8eb91e9affbd8898d106211d
Reviewed-on: https://skia-review.googlesource.com/c/193473
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Added a simpler circle drawable, moved drawing code out so that frame
calculation is handled by the drawable. Fixed all the sample effects,
including some size adjustments to better create the intended effect.
Bug: skia:
Change-Id: I60af9cd6262ff98352ca8ceaf6768aef9c7e164c
Reviewed-on: https://skia-review.googlesource.com/c/193029
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
- Converted all linear force stuff into a single affector,
used at either spawn or update time appropriately.
The new affector can either set or adjust velocity.
- Converted lifetime to a curve.
- Removed SkRangedFloat, initial velocity params, etc.
Looks like a large addition, but that's mostly down to the
JSON getting bigger. There's a net reduction in LoC.
Bug: skia:
Change-Id: Iac7417f15f96d0313efd08c4b26dc3250b80fa77
Reviewed-on: https://skia-review.googlesource.com/c/192102
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Effects now have a duration, and can be played looped
or one-shot. Added a second list of affectors that are
applied at spawn vs. update.
Effects grab and store the SkRandom at construction,
so it no longer needs to be passed to update().
Bug: skia:
Change-Id: Ib54d60466e162e4d4b70fa64c1215fc01680d47a
Reviewed-on: https://skia-review.googlesource.com/c/191722
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This adds a new "Particles" slide to viewer, that allows
editing, loading, and saving particle effects. All of the
particle system code is in modules/particles.
There are many rough edges and some not-yet-finished changes
to generalize the model[1]. A rough overview:
- SkReflected.h implements a lightweight reflection system
for classes derived from SkReflected. Adding a new class
involves deriving from SkReflected, adding a macro to the
class declaration, and implementing visitFields(), which
simply calls a virtual on an SkFieldVisitor for each field.
Currently, emitters and affectors use this mechanism.
- SkParticleSerialization.h demonstrates two useful field
visitors - for serializing to and from JSON. The driver
code that uses those is directly in ParticlesSlide.
- SkParticleData.h and SkCurve.h define a variety of helper
types for talking about particles, both for parameterizing
individual values, and communicating about the state of a
particle among the effect, affectors, and emitters.
- SkParticleEffect.h defines the static data definition of
an effect (SkParticleEffectParams), as well as a running
instance of an effect (SkParticleEffect). The effect has
simple update() and draw() methods.
- ParticlesSlide.cpp adds a third field visitor to generate
GUIs for interactively editing the running effect.
---
1: The critical change I'd like to make is to remove all
special case behavior over time and at spawn (setting sprite
frames, size over time, color over time, etc...). Integration
is the only fixed function behavior. Everything else is driven
by two lists of affectors. One is applied at spawn time, using
the effect's lifetime to evaluate curves. This allows spawning
particles with different colors as the effect ages out, for
example. The second list is applied every frame to update
existing particles, and is driven by the particle's lifetime.
This allows particles to change color after being spawned, for
example.
With a small set of affectors using a single expressive curve
primitive (keyframed list of cubic curve segments), we can
have affectors that update color, size, velocity, position,
sprite frame, etc., and implement many complex behaviors.
Bug: skia:
Change-Id: Id9402bef22825d55d021c5a2f9e5e41791aabaf4
Reviewed-on: https://skia-review.googlesource.com/c/181404
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Reed <reed@google.com>
To facilitate demo code consolidation, introduce a custom property
manager which filters for node names starting with '$' and treats all
properties sharing the same name unitarily.
Update the Colorize GM to use this new helper.
Also revisit the PropertyObserver interface:
* aliases for client-facing value types
* introduce a new (decomposed) TransformPropertyValue, to replace component-wise setters
* consolidate the PropertyHandle interface to only expose get()/set()
Bug: skia:
Change-Id: I9aa9ee80c1fb57bbfbacab0fc3f017da909b24d9
Reviewed-on: https://skia-review.googlesource.com/c/173220
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Bug: skia:8461
According to skbug.com/7069, we should allow GIFs to use a transparent
index outside of the range of the color table. Add a test to verify
support for this.
The GIF is 2x2 with the following pixels:
-------------------------------------------------
| black | white |
-------------------------------------------------
| transparent | transparent |
-------------------------------------------------
The color table only has 2 entries (black and white), and the
transparent index is 2.
Change-Id: I16574a61e2982b6628c3eca96cb7b3e1f57d3b2a
Reviewed-on: https://skia-review.googlesource.com/c/161561
Reviewed-by: Nigel Tao <nigeltao@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Auto-Submit: Leon Scroggins <scroggo@google.com>
Extend the image asset provider API to support animated/multi-frame images.
Add a GM based on SkAnimCodecPlayer + animated public domain GIF
(source: https://giphy.com/explore/public-domain).
Bug: skia:
Change-Id: Iaa596e01a7626ca6574db1ebc90632f5a9a02bdc
Reviewed-on: https://skia-review.googlesource.com/159162
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Mike Reed <reed@google.com>
Sets up a fake web font loader which serves a local/resource font to
the sample text animation.
Also rename resources/skotty/skotty_* -> resources/skottie/skottie_*.
Change-Id: I4af5b24fc3cc5c63c78801979f9be56744047134
Reviewed-on: https://skia-review.googlesource.com/155881
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
This reverts commit 49894f450f.
Reason for revert: Breaking a CTS test on the Android roll:
java.lang.AssertionError: expected:<67043583> but was:<50266367>
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.failNotEquals(Assert.java:834)
at org.junit.Assert.assertEquals(Assert.java:645)
at org.junit.Assert.assertEquals(Assert.java:631)
at android.graphics.cts.BitmapColorSpaceTest.verifyGetPixel(BitmapColorSpaceTest.java:301)
at android.graphics.cts.BitmapColorSpaceTest.inColorSpaceP3ToSRGB(BitmapColorSpaceTest.java:612)
Expected: 3FF00FF Actual: 2FF00FF
Original change's description:
> Reland "Switch SkCodec to use skcms" plus fixes
>
> This reverts commit 33d5394d08,
> relanding 81886e8f94 as well as
> "Fix CMYK handling in JPEG codec" (commit
> f8ae5ce20c)
>
> Add a test based on the CTS test that failed in the original commit.
> purple-displayprofile.png is the image used in the CTS test, with the
> Android license.
>
> This also adds a fix for SkAndroidCodec, ensuring that we continue to
> use a wide gamut SkColorSpace for images that do not have a numerical
> transfer function and have a wide gamut. This includes a test, with
> wide-gamut.png, which was created with Photoshop and the profile
> "sRGB_Calibrated_Homogeneous.icc" from the skcms tree.
>
> Bug: skia:6839
> Bug: skia:8052
> Bug: skia:8278
>
> TBR=djsollen@google.com
> As with the original, no API change
>
> Change-Id: I4e5bba6a3151f9dc6491e8eda73d4de0535bd692
> Reviewed-on: https://skia-review.googlesource.com/149043
> Commit-Queue: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: Leon Scroggins <scroggo@google.com>
TBR=djsollen@google.com,mtklein@google.com,scroggo@google.com,brianosman@google.com
Change-Id: Ie71e1fecc26de8225d2fe603765c1e1e0d738634
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:6839, skia:8052, skia:8278
Reviewed-on: https://skia-review.googlesource.com/149262
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
This reverts commit 33d5394d08,
relanding 81886e8f94 as well as
"Fix CMYK handling in JPEG codec" (commit
f8ae5ce20c)
Add a test based on the CTS test that failed in the original commit.
purple-displayprofile.png is the image used in the CTS test, with the
Android license.
This also adds a fix for SkAndroidCodec, ensuring that we continue to
use a wide gamut SkColorSpace for images that do not have a numerical
transfer function and have a wide gamut. This includes a test, with
wide-gamut.png, which was created with Photoshop and the profile
"sRGB_Calibrated_Homogeneous.icc" from the skcms tree.
Bug: skia:6839
Bug: skia:8052
Bug: skia:8278
TBR=djsollen@google.com
As with the original, no API change
Change-Id: I4e5bba6a3151f9dc6491e8eda73d4de0535bd692
Reviewed-on: https://skia-review.googlesource.com/149043
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
This reverts commit 81886e8f94 and
f8ae5ce20c
("Fix CMYK handling in JPEG codec")
This fixes the Android build, which was failing a CTS test with this
change.
Bug: skia:6839
Bug: skia:8052
TBR=djsollen@google.com
As with the original, no API change
Change-Id: Ic744a610e9f431707f871de44f9f64040bc60d14
Reviewed-on: https://skia-review.googlesource.com/148810
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Bug: skia:6839
Bug: skia:8052
Create an skcms_Profile instead of an SkColorSpace when creating an
SkCodec. Eventually we'll move the SkImageInfo (and its SkColorSpace)
entirely to clients (e.g. SkAndroidCodec, SkCodecImageGenerator), but
for now, create it with SkEncodedInfo::makeImageInfo.
Create new SkEncodedInfo::Colors for the special PNG cases that we
want to map to specific SkColorTypes.
SkEncodedInfo:
- Add ICCProfile, which owns an skcms_ICCProfile
- FIXME: maybe we should have a single instance for
SRGB like SkColorSpace?
- Add kXAlpha_Color, for kAlpha_8. Since I'm not longer creating
an SkImageInfo (at least in SkPngCodec), it needs a way to pass
this info to the caller.
- Add k565_Color, for the same reason. Matt originally had this in
https://codereview.chromium.org/2212563003/#ps120001, but didn't
land that version. I like it though. Mike didn't like the bits
per component for 565, but it seems like a sensible hack, much
like the existing one for kAlpha_8
- Add width and height. These were removed for redundancy with
SkImageInfo, but it makes sense to have them here without it.
BUILD.gn:
- Build the new SkEncodedInfo.cpp
SkCodec:
- Remove the constructor with an SkImageInfo. Edit the other one
to drop width and height (now in SkEncodedInfo) and take a RHS
reference to SkEncodedInfo
- Create the SkImageInfo from fEncodedInfo (for now)
- Consolidate choosing skcms_AlphaFormat for Transform here
- Call conversionSupported from initializeColorXform, with a new
parameter for whether there is a color Xform, allowing SkJpegCodec
and SkHeifCodec to override that method instead of having another
method.
SkBmpCodec (etc)
- Adapt to the changes above
- Create a new SkEncodedInfo w/o profile for the swizzler.
SkPngCodec:
- use the new SkEncodedInfo::Colors rather than a custom SkImageInfo
SkRawCodec:
- Remove SkEncodedInfo from SkDngImage, which doesn't actually need it.
This is helpful since we don't know all the info yet.
- Rewrite gAdobeRGB_toXYZD50 as an skcms_Matrix3x3
SkWebpCodec:
- Remove premul_step computation, and simplify to just rely on
the base class' handling of applying the transform.
SkSwizzler:
- Add cases for the new SkEncodedInfo::Colors
TBR=reed@google.com
No public API changes. Only private/public members of SkCodec.h are
modified.
Change-Id: Ic0d3bb752b03f13be886b80331987aa5a5713fc0
Reviewed-on: https://skia-review.googlesource.com/136062
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:
Change-Id: Ic0e0ae23c88f86885583a304d90d1ce874ff14ed
Reviewed-on: https://skia-review.googlesource.com/139960
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This eliminates many GM diffs between gl and glsrgb.
Bug: skia:
Change-Id: Ia35ae9db3f9dba03c227f82d2ad7ab602100d2e9
Reviewed-on: https://skia-review.googlesource.com/139861
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
SkCanvas::drawVertices now supports overloads that take an array of bone deformation matrices.
SkVertices::MakeCopy and SkVertices::Builder now support two additional optional attributes, boneIndices and boneWeights.
Bug: skia:
Change-Id: I30a3b11691e7cdb13924907cc1401ff86d127aea
Reviewed-on: https://skia-review.googlesource.com/137221
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Ruiqi Mao <ruiqimao@google.com>
jpeg_skip_scanlines was incorrectly incrementing an internal counter,
resulting in an infinite loop. (This only occurs for certain types of
progressive images, using certain sample sizes.)
The fix is at 26f109290d.
This is included in tip-of-tree, which is unofficially 2.0.0, so go ahead and
update to it.
Add a test based on the original bug.
Bug: b/78329453
Change-Id: I5ade9924812324d58668c26f71cd622ef93f40a7
Reviewed-on: https://skia-review.googlesource.com/129459
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Bug: skia:
Change-Id: I43c1a86eead7701db052451c446f9ad59467f909
Reviewed-on: https://skia-review.googlesource.com/136610
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ruiqi Mao <ruiqimao@google.com>
improved third_party template to include headers as system headers for non-Windows machines
Bug: skia:
Change-Id: Id2fa74fc31b49f9b07cc83e7f60477c7ab4f8d83
Reviewed-on: https://skia-review.googlesource.com/135450
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The upperLeft and upperRight profiles are A2B/B2A-only, and so cannot be
represented as SkColorSpace using SkColorSpace::Make(skcms_ICCProfile).
They do parse fine, of course.
Change-Id: Iaf51911c2b06b985037d3d5e74b043fb344e320e
Reviewed-on: https://skia-review.googlesource.com/129653
Commit-Queue: Mike Klein <mtklein@chromium.org>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Mike Klein <mtklein@chromium.org>
Reviewed-by: Brian Osman <brianosman@google.com>
The lua files were moved at one point without the internal path
references being updated. This updates them so that the SampleLua slides
can be seen in their full glory.
Change-Id: I4dd09c847be48d80fa61d3b9864d34a4aa9bb2f5
Reviewed-on: https://skia-review.googlesource.com/129515
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
BUG=skia:7624
Change-Id: Id2b7449048591892ff802484d5e3745a7e1402bb
Reviewed-on: https://skia-review.googlesource.com/109521
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
CoreGraphics does not provide a means to get the original font data for a
CGFont, only the tables. As a result, Skia pieces the font data back
together when requested. The most awkward part of this is choosing the
first four bytes, and the CTFont suggestion seems to often be wrong.
This change doublechecks the selection of 'typ1', prefering to use 'OTTO'
if there are no 'TYP1' or 'CID ' tables. These sorts of fonts are
extremely old and unlikely to be in current use. It appears that CTFont
may report that it has this format if it is an 'OTTO' font with very few
glyphs. If Skia serializes such a font with 'typ1' as the first four
bytes, CoreGraphics will not create a CGFont from the resulting font data.
BUG=chromium:809763,skia:7630
Change-Id: I9979b9f0ebdd27c4ad0903e8ee6237241e755541
Reviewed-on: https://skia-review.googlesource.com/113306
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Bug: chromium:807324
Though these pngs are technically incorrect, many such PNGs exist, and
they are supported in Chromium. Ensure that users of SkCodec (e.g.
Android, Flutter) display them as well.
Change-Id: I2f1e573b4b7039cea81f96397cc0aa4cbc9461c3
Reviewed-on: https://skia-review.googlesource.com/111082
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Extend composition layers to support referencing external .json
animations ("$"<PATH> syntax).
This is a custom extension (not supported in BM/Lottie).
Also make skottie::Animation ref-counted, to facilitate sharing.
TBR=
Change-Id: I062d031e5868d759f3930dea9b261f9b3ec81684
Reviewed-on: https://skia-review.googlesource.com/109806
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Bug: oss-fuzz:6274
Even if a frame does not have enough LZW blocks to decode all rows,
(which is unknown until we actually decode them), it is marked complete
once there are no more LZW blocks.
When decoding, even if we've decoded all LZW blocks, check fRowsDecoded
to determine whether we've actually all the rows. Report the number of
rows decoded so that SkCodec can fill in the remaining ones.
Change-Id: I1d6e0c29e3c37649725836cf24a4a239e3266b76
Reviewed-on: https://skia-review.googlesource.com/106964
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Bug: os-fuzz:6288
Negating it is undefined, so don't try.
Change-Id: I055520b8036dd8b355e744114717e08d76206bc1
Reviewed-on: https://skia-review.googlesource.com/107062
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Bug: os-fuzz:6295
Change-Id: I0ea9a3c54d61d41f21f2e9b945ab83fa2beb00d8
Reviewed-on: https://skia-review.googlesource.com/107025
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Bug: b/63909536
ImageDecoder will respect the origin, but BitmapFactory will maintain
its current behavior of not respecting it. Add an option to respect it.
In addition, add support for reading the EXIF data from a WEBP. This
seems to be an uncommon use case, but is occasionally used when
converting from a JPEG. Add 8 WEBPs, all converted (with cwebp) from
their analogous JPEG files already checked in.
Change-Id: I38afca58c86fa99ee9ab7d1dc83aaa4f23132c11
Reviewed-on: https://skia-review.googlesource.com/95300
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Coarse workflow:
* Construction
1) build a Json tree
2) collect asset IDs (for preComp/image layer resolution)
3) "attach" pass
- traverse the Json tree
- build an SkSG dom, one fragment at a time
- attach "animator" objects to the dom, for each animated prop
4) done, we can throw away the Json tree
* For each animation tick
1) iterate over active animators and poke their respective dom nodes/attributes
2) revalidate the SkSG dom
3) draw the SkSG dom
Note: post construction, things are super-simple - we just poke SkSG DOM attributes
with interpolated values, and everything else is handled by SkSG (invalidation,
revalidation, render).
Change-Id: I96a02be7eb4fb4cb3831f59bf2b3908ea190c0dd
Reviewed-on: https://skia-review.googlesource.com/89420
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Our runtime definition of the XYZ matrix was fairly inaccurate. It also
didn't round-trip through ICC fixed point correctly. Now, constructing the
color space at runtime produces exactly the same matrix as constructing
the space from the ICC profile. And the values can then be serialized back
to ICC exactly. This eliminates the need for the snapping logic, too.
Bug: skia:
Change-Id: I69f4a9bfec3eeef153935e21ab3a0630794b1607
Reviewed-on: https://skia-review.googlesource.com/84840
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Brian Osman <brianosman@google.com>
Should make it easier to ask just for images.
Change-Id: If821743dc924c4bfbc6b2b2d29b14affde7b3afd
Reviewed-on: https://skia-review.googlesource.com/82684
Commit-Queue: Hal Canary <halcanary@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Part of the Typeface test is Android specific and tests that the Android
legacyMakeTypeface returns nullptr when called with a non-null family
name and there is no match. A new FontMgrAndroidLegacyMakeTypeface test
is introduced to test this and also properly test creating one with
SkFontMgr_Android_CustomFonts.
Change-Id: Iceb8b9b5490f5eeeaeac30a68daf2c4daac53c86
Reviewed-on: https://skia-review.googlesource.com/70660
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Ben Wagner <bungeman@google.com>
Bug: 769134
readByteArray can fail (due to not having enough available or due to the
wrong alignment). If it does, do not return an uninitialized block of
memory.
Further, drop the initial size check, which is covered by readByteArray.
Add a test.
Change-Id: Ia101697c5bb1ca3ae3df1795f37a74b2f602797d
Reviewed-on: https://skia-review.googlesource.com/52742
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Bug: chromium:712455
Change-Id: Ic9bb9b862abe01f112cc41d28589733460b15bc1
Reviewed-on: https://skia-review.googlesource.com/50181
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Previously, if ReadHeader returned false, it deleted the input stream.
But there are a couple of cases where ReadHeader creates an SkCodec and
then returns false. The SkCodec deletes the stream, and then so does
NewFromStream.
Make sure that we do not double delete by only deleting if no SkCodec
was created.
Add a test, so such a double delete will be caught by the bots.
Bug: b/37623797
Change-Id: I787422c9af58f0b92ad9e9ef9ad87c54a12f5e31
Reviewed-on: https://skia-review.googlesource.com/23620
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
SkCodec sets fRequiredFrame to be the earliest possible frame that a
given frame can depend on. e.g.
- Frame A fills the screen, Keep
- Frame B does not cover A, Keep
- Frame C covers B but not A, and is opaque
Frame C can depend on either A or B. SkCodec already reports that C
depends on A. This CL allows a client of SkCodec to use either A or
B to create C.
Also expose the DisposalMethod. Since any frame between A and C can
be used to create C except for DisposePrevious frames, the client
needs to be able to know the disposal method so they do not try to
use such a frame to create C.
Further, the disposal method can be used to give the client a better
idea whether they will continue to need a frame. (e.g. if frame i is
DisposePrevious and depends on i-1, the client may not want to steal
i-1 to create i, since i+1 may also depend on i-1.)
TODO: Share code for decoding prior frames between GIF and WEBP
Change-Id: I91a5ae22ba3d8dfbe0bde833fa67ae3da0d81ed6
Reviewed-on: https://skia-review.googlesource.com/13722
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Chris Blume <cblume@chromium.org>
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
If the ICO reports that it has a large BMP file embedded, do not
crash if we attempt to allocate too much memory.
Bug: b/38116746
Change-Id: I70eb66f5e4ffc15587007b398bbe843665eae500
Reviewed-on: https://skia-review.googlesource.com/18447
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
This one produces noticeable differences if we fail to dither.
Bug:720105
Change-Id: I208d0c8147f4cca1b484f2f55edc09ce1bef2dcb
Reviewed-on: https://skia-review.googlesource.com/18036
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Bug: skia:5526
Change-Id: I12bd44c7dabd72f215a34ce9e733bb7a29d3fc68
Reviewed-on: https://skia-review.googlesource.com/17763
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Change-Id: I889a3f508407178c457f675c321880d869caf9ea
Reviewed-on: https://skia-review.googlesource.com/17717
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
TBR=reed@google.com
(No change to the public API, but changed a header file)
SkWebpCodec:
- Implement onGetFrameCount, onGetFrameInfo, and onGetRepetitionCount
- Respect the alpha reported by libwebp. Although the spec states that
it is only a hint, the libwebp encoder uses it properly. Respecting
allows us to draw opaque images faster and decode them to 565. This
also matches other SkCodecs (and Chromium).
- onGetPixels:
- Decode the frame requested, recursively decoding required frame if
necessary
- When blending with a prior frame, use SkRasterPipeline
SkCodec:
- Move check for negative index to getFrameInfo
- Reset the colorXform if one is not needed
SkCodecAnimation:
- Add new blend enum, for WebP's (and APNG's) non-blending option
SkFrameHolder:
- New base classes for frames and the owner of the frames, allowing
code sharing between SkWebpCodec and SkGifCodec (particularly for
determining whether a frame has alpha and what frame it depends on)
- When moving items from SkGIFFrameContext, use Skia conventions (i.e.
int instead of unsigned)
- Rename "delay time" to "duration", to match e.g. SkFrameInfo::
fDuration
SkGifImageReader:
- Move pieces to SkFrameHolder, and adapt to changes made in the
process
- Make setAlphaAndRequiredFrame (now on the base class SkFrameHolder)
more general to support webp, and add support for frames that do not
blend
- Change SkGIFFrameContext from a struct to a class, to match how we
use the distinction elsewhere (i.e. struct is a small object with
public fields)
- Rework hasTransparentPixel (now hasTransparency, since it returns true
in some cases where there is not a transparent pixel) to better fit
with the modified setAlphaAndRequiredFrame. Also be more consistent
when there is no transparent pixel but no color map.
- Simplify an if condition that was previously simplified in 2d61e717
but accidentally got reverted in a4db9be6
CodecAnimTest:
- Test new animated webp files
- Rearrange the test to more cleanly print alpha type mismatches for
the first frame
resources:
- webp-animated.webp
- animated webp from Chromium
- blendBG.webp
- new webp file using bits of webp-animated-semitransparent4.webp
from Chromium
- tests required frame and alpha when using the non-blending mode
- frames have the following properties:
- Frame 0: no alpha, fills screen
- Frame 1: alpha, fills screen
- Frame 2: no alpha, fills screen
- Frame 3: alpha, fills screen, blendBG
- Frame 4: no alpha, fills screen, blendBG
- Frame 5: alpha, blendBG
- Frame 6: covers 4, has alpha, blendBG
- also used to test decoding to 565 if the new frame data has alpha
but blends onto an opaque frame
DM.cpp:
- Test animated images to non-native 8888 and unpremul
DMSrcSink.cpp:
- Do not test non-native 8888 decodes to f16 dst
- Test unpremul decodes to f16
- Copy a frame of an animated image prior to drawing, since in unpremul
mode, the DM code will premultiply first.
Bug: skia: 3315
Change-Id: I4e55ae2ee5bc095b37a743bdcfac644be603b980
Reviewed-on: https://skia-review.googlesource.com/16707
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
Bug: skia:
Change-Id: Ia4f892368fceda4a99490f5bd29851837a7a6927
Reviewed-on: https://skia-review.googlesource.com/17212
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Matt Sarett <msarett@google.com>
This instructs us on how to encode jpegs when the src
image has alpha. The original behavior is to ignore
the alpha channel. This CL adds the option to blend
the pixels onto opaque black.
Note that kBlendOnBlack and kIgnore are identical
unless the input alpha type is kUnpremul.
Bug: 713862
Bug: skia:1501
Change-Id: I4891c70bb0ccd83f7974c359bd40a2143b5c49ac
Reviewed-on: https://skia-review.googlesource.com/15817
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Matt Sarett <msarett@google.com>
Bug: 717935
Change-Id: Ibf15b815891eef5a0239bc408bcbfe7c8b1507c5
Reviewed-on: https://skia-review.googlesource.com/15301
Commit-Queue: Matt Sarett <msarett@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
In this case, the fuzzer thinks there is a bug because we are
returning kInvalidConversion for a corrupt png file.
Bug: skia:6550
Change-Id: I33f588442f5eaa8a4d642e9328750779f9a9ef5d
Reviewed-on: https://skia-review.googlesource.com/14324
Commit-Queue: Matt Sarett <msarett@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Prior to e132e7be5f, calling getFrameInfo
on an SkGifCodec which was truncated before fully parsing the local
color map of the first frame would hit an assert. Add a test to verify
that the problem has been fixed.
Modify box.gif to use a local color map for the first (and only) frame.
Change-Id: I905afe60c0c6ecc82e251e00ef82b2e5b22975a3
Reviewed-on: https://skia-review.googlesource.com/13320
Reviewed-by: Chris Blume <cblume@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Add SkCodec::FrameInfo::fAlphaType. The SkImageInfo for the SkCodec
specifies the SkAlphaType for the first frame, but the opacity can vary
from frame to frame.
When determining the required frame, also compute whether a frame has
alpha. Update how we determine the required frame, which had bugs.
(Update a test that had an incorrect required frame as a result.)
Add new test images covering cases that have been fixed:
- randPixelsAnim2.gif
It has the following frames:
A (keep)
B (keep) (subset)
C (disposePrevious) (covers B)
D (any) (does *not* cover B)
B and C depend on A, but D depends on B, since after disposing C, B
should be visible again.
- alphabetAnim.gif
Includes frames which fill the image size, with different disposal
methods and transparencies.
Change-Id: Ie086167711c4cac4931ed8c4ddaeb9c9b0b91fdb
Reviewed-on: https://skia-review.googlesource.com/9810
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
This limit matches the limit used by Chromium. I am not aware of any
real world BMPs that are larger than this (or even close to it), but
there are some invalid BMPs that are larger than this, leading to
crashes when we try to read a row.
BUG:34778578
BUG=skia:3617
Change-Id: I0f662e8d0d7bc0b084e86d0c9288b831e1b296d7
Reviewed-on: https://skia-review.googlesource.com/8966
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
An RLE bmp reports how many bytes it should contain. This number may be
incorrect, or it may be a very large number. Previously, we buffered
all bytes in a single allocation. Instead, use a fixed size buffer and
only read what fits into the buffer. We already have code to refill the
buffer if there is more data, so rely on that to keep reading.
Choose an arbitrary size for the buffer. It is larger than the maximum
possible number of bytes we need to read at once.
Add a test with a test image that reports a very large number for
the number of bytes it should contain. With the old method, we would
allocate 4 gigs of memory to decode this image, which is unnecessary
and may result in OOM.
BUG=b/33251605
Change-Id: I6d66eace626002725f62237617140cab99ce42f3
Reviewed-on: https://skia-review.googlesource.com/7028
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
If a frame does not have a valid transparent index and it covers the
prior frame, it does not really depend on that frame. Instead, it
depends on the frame that the prior frame depends on.
Determine this once we have parsed the local color map (if any), so a
transparent index out of range of the color map is not considered
valid.
Share code that determines whether a frame has a transparent pixel.
Add a test that we compute the dependencies correctly. randPixelsAnim.gif
has 13 frames. After the first, the frames cover all combinations of
- Whether the prior frame was keep, restoreBG or restoreToPrevious
- Whether the new frame covers the prior frame
- Whether the new frame has a transparent pixel
(It only does so when using a global color table. It may make sense to
expand the test to also cover using local color tables.)
The test caught a bug where we incorrectly reused an existing
SkColorTable for a different frame. Fix that bug by keeping track of
the transparent index associated with the current SkColorTable.
Change-Id: I3cf6be7f612990fa7a00d9e74d116d31bd227526
Reviewed-on: https://skia-review.googlesource.com/6402
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Recent changes (crrev.com/2045293002) made it so that a GIF may not
support index 8. In that case, make SkAndroidCodec not suggest index 8.
Add a test and a new test file. randPixelsOffset.gif is the same as
randPixels.gif, except its frame is offset. Since it does not have a
transparent index, we have to decode to kN32.
Change-Id: I1c09ab9094083de3dfc436632b3c26dbde1dccbd
Reviewed-on: https://skia-review.googlesource.com/6196
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
In SkBmpCodec, if the header size does not match a known header,
stop trying to create an SkCodec. We do not know of any BMPs with
arbitrarily sized headers, so this should not cause any real
regressions.
In addition, this fixes a bug where we attempt to read too much data
from a file. Since we attempt to read the header size in one read,
and a size reported by the "BMP" may be larger than SSIZE_MAX, this
will crash when reading from a file.
Add a test.
BUG:b/33651913
Change-Id: I0f3292db3124dc5ac5cbdbc07196bda130a49ba7
Reviewed-on: https://skia-review.googlesource.com/6150
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
When clearing due to SkCodecAnimation::RestoreBGColor_DisposalMethod,
intersect the frameRect with the image size to prevent clearing outside
the bounds of the allocated memory.
Add a test image, created by the fuzzer.
BUG=skia:6046
Change-Id: I43676d28f82abf093ef801752f3a9e881580924c
Reviewed-on: https://skia-review.googlesource.com/5860
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
Add a test
BUG=skia:3534
BUG=b/33300701
Change-Id: Ifb3a824a36998c5e626c4ad58466845f49d18ebf
Reviewed-on: https://skia-review.googlesource.com/5568
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
Add a new accessor to retrieve the repetition count.
Remove constants (and corresponding copyright) in SkCodecAnimation.
These may make sense for the calling code, but are not needed here.
kRepetitionCountInfinite corresponds to Blink's kAnimationLoopInfinite.
Move cLoopCountNotSeen to private. It is used to determine whether we
still need to parse. Add a new enum to the parse query - only parse
enough to determine the repetition count.
Unlike Chromium, SkGifCodec does not account for deleting the reader
(which SkGifCodec does not do) or failed decodes.
Add a test.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2447863002
Review-Url: https://codereview.chromium.org/2447863002
While investigating skbug.com/5883 I noticed that we use the color
table for the current frame even while recursively decoding frames that
the current frame depends on.
This CL updates fCurrColorTable, fCurrColorTableIsReal and fSwizzler
before decoding prior frames, and then sets them back afterwards.
Move telling the client about the color table into prepareToDecode,
since the other callers do not need to do so. (That is only necessary
for decoding to index 8, which is unsupported for frames with
dependencies.)
Add a test that exposes the bug. colorTables.gif has a local color
table in its second frame that does not match the global table used by
the first frame.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4208
Change-Id: Id2dc9e3283adfd92801d2f38726afa74574b1955
Reviewed-on: https://skia-review.googlesource.com/4208
Reviewed-by: Joost Ouwerling <joostouwerling@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
BUG:660838
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4200
Change-Id: Ib57eb3705d6fe638e3a9cb56788937fc7e282847
Reviewed-on: https://skia-review.googlesource.com/4200
Commit-Queue: Matt Sarett <msarett@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
The refactoring breaks off A2B0 tag support into a separate
subclass of SkColorSpace_Base, while keeping the current
(besides CLUT) functionality in a XYZTRC subclass.
ICC profile loading is now aware of this and creates the A2B0
subclass when SkColorSpace::NewICC() is called on a profile
in need of the A2B0 functionality.
The LabPCSDemo GM loads a .icc profile containing a LAB PCS and
then runs a Lab->XYZ conversion on an image using it so we can
display it and test out the A2B0 SkColorSpace functionality,
sans a/b/m-curves, as well as the Lab->XYZ conversion code.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2389983002
Review-Url: https://codereview.chromium.org/2389983002
Original Commit Message:
"""
Avoid integer overflow in SkIcoCodec
Definitely good to avoid overflow here.
FWIW, this looks to be harmless for Android's current use.
They will just fail later on when trying to allocate the
bitmap.
BUG=skia:5857
"""
With the new test, ASAN also caught an integer overflow
bug in SkImageInfo. Fix this as well.
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-Clang-GCE-CPU-AVX2-x86_64-Debug-ASAN;master.client.skia.android:Test-Android-Clang-AndroidOne-CPU-MT6582-arm-Debug-GN_Android
BUG=skia:5857
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=3568
Change-Id: I0d777a547850474ea6cea87e36efa05434e33635
Reviewed-on: https://skia-review.googlesource.com/3568
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Definitely good to avoid overflow here.
FWIW, this looks to be harmless for Android's current use.
They will just fail later on when trying to allocate the
bitmap.
BUG=skia:5857
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=3527
Change-Id: Ia1fb7d864d21ecdb127a1dd1a72cab8375cb43fb
Reviewed-on: https://skia-review.googlesource.com/3527
Commit-Queue: Matt Sarett <msarett@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
The hintgasp font was added specifically for the typefacerendering gm.
However, this gm didn't actually use the font, so now use it. In
addition this adds embedded bitmap strikes to the hintgasp font and the
gm is updated to test these as well.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2140863002
NOTREECHECKS=true
Test only change, does not affect users.
Review-Url: https://codereview.chromium.org/2140863002
(1) Draw triangles onto a D50 color gamut so they are
actually in the correct place.
(2) Add options to display canonical sRGB and Adobe RGB
gamuts.
(3) Label the R, G, and B points of the color gamut.
(4) Decode and reencode the input image without color
correction, so we can compare to the corrected version.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2017153002
Review-Url: https://codereview.chromium.org/2017153002
This will allow me to test and visualize some assumptions
on parsing and applying color profiles. Also, it should
help me find and fix bugs.
This is certainly not an optimized implementation, and, as
far as I know, it doesn't take any shortcuts to improve
performance. We'll probably want to do both of these
once we know where it fits in the pipeline.
Right now this test is only run on an arbitrary set of ~100
images from the top 10k skps. I'll continue to add more
"interesting" images and probably tweak the code as
necessary.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1995233003
Review-Url: https://codereview.chromium.org/1995233003
This reverts commit 554784cd85 and
1956b4ae1c
Reason for revert - ASAN failures, e.g. from https://uberchromegw.corp.google.com/i/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-MSAN/builds/2233/steps/perf_skia%20on%20Ubuntu/logs/stdio :
Uninitialized value was created by a heap allocation
0 0x7f69aa96f799 in operator new[](unsigned long) /b/work/skia/third_party/externals/llvm/out/../projects/compiler-rt/lib/msan/msan_new_delete.cc:37
1 0x7f69aaa315c1 in SkAutoTArray<unsigned int>::reset(int) /b/work/skia/out/Build-Ubuntu-GCC-x86_64-Debug-MSAN/Debug/../../../include/private/../private/SkTemplates.h:137:22
2 0x7f69aaa34ee9 in LinearSrcOverBench<SrcOverVSkOptsSSE41>::LinearSrcOverBench(char const*) /b/work/skia/out/Build-Ubuntu-GCC-x86_64-Debug-MSAN/Debug/../../../bench/SkBlend_optsBench.cpp:108:9
3 0x7f69aaa30cf2 in $_24::operator()(void*) const /b/work/skia/out/Build-Ubuntu-GCC-x86_64-Debug-MSAN/Debug/../../../bench/SkBlend_optsBench.cpp:167:1
4 0x7f69aaa30c87 in $_24::__invoke(void*) /b/work/skia/out/Build-Ubuntu-GCC-x86_64-Debug-MSAN/Debug/../../../bench/SkBlend_optsBench.cpp:167:1
5 0x7f69aaa68856 in BenchmarkStream::rawNext() /b/work/skia/out/Build-Ubuntu-GCC-x86_64-Debug-MSAN/Debug/../../../bench/nanobench.cpp:653:32
6 0x7f69aaa61467 in BenchmarkStream::next() /b/work/skia/out/Build-Ubuntu-GCC-x86_64-Debug-MSAN/Debug/../../../bench/nanobench.cpp:642:25
7 0x7f69aaa5b703 in nanobench_main() /b/work/skia/out/Build-Ubuntu-GCC-x86_64-Debug-MSAN/Debug/../../../bench/nanobench.cpp:1119:27
8 0x7f69aaa5e10d in main /b/work/skia/out/Build-Ubuntu-GCC-x86_64-Debug-MSAN/Debug/../../../bench/nanobench.cpp:1290:12
9 0x7f69a8c95ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
TBR=herb@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1969803002
CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
Review-Url: https://codereview.chromium.org/1969803002
Add this flag to SampleApp, and it will run with the specified restricted sequence
--sequence /skia/trunk/resources/nov-talk-sequence.txt
BUG=skia:
TBR=
Review URL: https://codereview.chromium.org/1410243009
