Added gradient shader factories that take SkColor4f + SkColorSpace.
Modified Descriptor to only store SkColor4f + SkColorSpace.
Existing factories make use of helper code to convert SkColor and
forward to the new factories.
Bumped SKP version to handle new gradient serialization format.
I was toying with using half-float when serializing SkColor4f,
despite my aggressive packing of flags, this format is significantly
bigger.
Also added GM to use 4f factories. This GM should (and does)
look identical to the existing gradients GM.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2370063002
Review-Url: https://codereview.chromium.org/2370063002
This is a less generally applicable trick than I have previously hoped. The need to thread through contexts into each stage really means you can only include one context-dependent stage in each fused batch.
We can still manually fuse these, of course, as you can see in SkRasterPipelineBench. It's just that we can't really write a generic compile-time template to do it except for context-free stages. And since we can't write a generic version, and I have only this one specific use case right now, I've kept it quite specific to that use case.
This does work pretty well for this use case, though. Here's the fused clamp-then-store-565:
+0x00 pushq %rbp
+0x01 movq %rsp, %rbp
+0x04 movq 8(%rdi), %rax
+0x08 xorps %xmm4, %xmm4
+0x0b maxps %xmm4, %xmm3
+0x0e maxps %xmm4, %xmm0
+0x11 maxps %xmm4, %xmm1
+0x14 maxps %xmm4, %xmm2
+0x17 minps 4262818(%rip), %xmm3
+0x1e minps %xmm3, %xmm0
+0x21 minps %xmm3, %xmm1
+0x24 minps %xmm3, %xmm2
+0x27 movaps 4965378(%rip), %xmm3
+0x2e mulps %xmm3, %xmm0
+0x31 cvtps2dq %xmm0, %xmm0
+0x35 pslld $11, %xmm0
+0x3a mulps 4965375(%rip), %xmm1
+0x41 cvtps2dq %xmm1, %xmm1
+0x45 pslld $5, %xmm1
+0x4a mulps %xmm3, %xmm2
+0x4d cvtps2dq %xmm2, %xmm2
+0x51 orpd %xmm0, %xmm2
+0x55 orpd %xmm1, %xmm2
+0x59 pshufb 4474510(%rip), %xmm2
+0x62 movq %xmm2, (%rax,%rsi,2)
+0x67 popq %rbp
+0x68 retq
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2745
Change-Id: Ia7d66aecc6cbff154158d2600d7874feed1a76f6
Reviewed-on: https://skia-review.googlesource.com/2745
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
We used to step at a 4-pixel stride as long as possible, then run up to 3 times, one pixel at a time. Now replace those 1-at-a-time runs with a single tail stamp if there are 1-3 remaining pixels.
This style is simply more efficient: e.g. we'll blend and lerp once for 3 pixels instead of 3 times. This should make short blits significantly more efficient. It's also more future-oriented... AVX+ on Intel and SVE on ARM support masked loads and stores, so we can do the entire tail in one direct step.
This also makes it possible to re-arrange the code a bit to encapsulate each stage better. I think generally this code reads more clearly than the old code, but YMMV. I've arranged things so you write one function, but it's compiled into two specializations, one for tail=0 (Body) and one for tail>0 (Tail). It's pretty tidy.
For now I've just burned a register to pass around tail. It's 2 bits now, maybe soon 3 with AVX, and capped at 4 for even the craziest new toys, so there are plenty of places we can pack it if we want to get clever.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2717
Change-Id: I45852a3e5d4c5b5e9315302c46601aee0d32265f
Reviewed-on: https://skia-review.googlesource.com/2717
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reason for revert:
Regression on displacement GM when sRGB support is lacking.
Original issue's description:
> Tag checkerboard bitmaps as sRGB
>
> Significantly reduces the diff between legacy and sRGB/F16 on about 25
> GMs. This is just the biggest piece of low-hanging fruit. Many GMs create
> N32 raster surfaces to procedurally generate source textures, and I'd like
> to fix all of them. It's much easier to reason about the GMs (is sRGB
> doing the right thing) when everything is tagged like this - the only
> expected differences are due to filtering and blending.
>
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2368933003
>
> Committed: https://skia.googlesource.com/skia/+/fe843cea499ba163d53281425af210b1887d28e7TBR=mtklein@google.com,reed@google.com,robertphillips@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=skia:
Review-Url: https://codereview.chromium.org/2375063002
Unlike nanobench this tool has no purpose when built in Debug mode.
Just don't let it happen.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2718
Change-Id: Iaa7b8c44d46024485d4f5ce3d9c3e33d865b99d7
Reviewed-on: https://skia-review.googlesource.com/2718
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
Gradients (and other shaders) are going to end up serializing this
particular color space very frequently, so we want a shorthand way of
writing it out. I think it's also helpful to have a clearer way of
creating it (vs. NewNamed(kSRGB_Named)->makeLinearGamma()).
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2377763002
Review-Url: https://codereview.chromium.org/2377763002
Today if you use the simple SK_RASTER_STAGE interface to build a pipeline, each stage you add calls into a next stage. The last stage you add calls into a special backstop stage JustReturn that, well, just returns, ending the pipeline.
This adds last(), which cuts that last stage off the pipeline. Instead, the stage you add using last() returns directly, ending the pipeline itself without jumping into JustReturn.
This reduces the overhead of using the pipelined version of SkRasterPipelineBench from ~25% to ~20% on my desktop.
Also, add docs.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2713
Change-Id: I11469378e2765c6e34db52eb3eef648d6612da3f
Reviewed-on: https://skia-review.googlesource.com/2713
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
I'm not seeing any problems with these locally. Perhaps the bots have something to say.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2709
Change-Id: I6f0c7045c8f270efcd71d837f22a40e9f9d3e9b7
Reviewed-on: https://skia-review.googlesource.com/2709
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
I don't know _why_ Clang would like these .inc files to have a newline at the end of the file, but it seems a harmless way to silence the warning.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2711
Change-Id: I6c530ee5096c48c91ddf322aca916e70a0dd770b
Reviewed-on: https://skia-review.googlesource.com/2711
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
I've even found the code that's making this happen, just don't know why.
I've added a test to assert that it's safe to assume malloc() is 8-byte aligned.
Test should compile this time.
CQ_INCLUDE_TRYBOTS=master.client.skia.android:Test-Android-Clang-NexusPlayer-CPU-Moorefield-x86-Release-GN_Android-Trybot
Change-Id: I48714b99670c20704adf4f7f216da0d60d7d9bcd
Reviewed-on: https://skia-review.googlesource.com/2662
Reviewed-on: https://skia-review.googlesource.com/2703
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Mike Klein <mtklein@chromium.org>
This reverts commit If8a2898ab3a77571622eb125c97f676e029b902c.
Reason for revert:
../../../../../work/skia/tests/OverAlignedTest.cpp: In function 'void test_OverAligned(skiatest::Reporter*, sk_gpu_test::GrContextFactory*)':
../../../../../work/skia/tests/OverAlignedTest.cpp:19:33: error: invalid operands of types 'void*' and 'int' to binary 'operator&'
REPORTER_ASSERT(r, SkIsAlign8(p));
^
ninja: build stopped: subcommand failed.
Original issue's description:
> Focus -Wno-over-aligned to just 32-bit x86 Android.
>
> I've even found the code that's making this happen, just don't know why.
> I've added a test to assert that it's safe to assume malloc() is 8-byte aligned.
>
> BUG=skia:
>
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2662
>
> CQ_INCLUDE_TRYBOTS=master.client.skia.android:Test-Android-Clang-NexusPlayer-CPU-Moorefield-x86-Release-GN_Android-Trybot
>
TBR=mtklein@chromium.org,bungeman@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Change-Id: Ic9b30ce980d8d5155528a6f2b4e1913e5fa95dc0
Reviewed-on: https://skia-review.googlesource.com/2702
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
I've even found the code that's making this happen, just don't know why.
I've added a test to assert that it's safe to assume malloc() is 8-byte aligned.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2662
CQ_INCLUDE_TRYBOTS=master.client.skia.android:Test-Android-Clang-NexusPlayer-CPU-Moorefield-x86-Release-GN_Android-Trybot
Change-Id: If8a2898ab3a77571622eb125c97f676e029b902c
Reviewed-on: https://skia-review.googlesource.com/2662
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Reason for revert:
Let's see if reverting this helps the roll.
Original issue's description:
> My take on SkAlign changes.
>
> Like the other change, it makes SkAlignN(x) macros work for pointers, and makes the macros themselves just syntax sugar for SkAlign<N>(x). We can still decide if we want to sed away the macros independently.
>
> This just does it in a somewhat less repetitive way, and adds some tests.
>
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2368293002
>
> No public API changes.
> TBR=reed@google.com
>
> Committed: https://skia.googlesource.com/skia/+/e1a5f4e292384046678edc5c1e360b3e13dc118cTBR=cblume@chromium.org,mtklein@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:
Review-Url: https://codereview.chromium.org/2372083002
The warning looks to helpfully pre-warn about possible link failures, but it's warning exclusively in places where we're doing things right.
The worst that happens ignoring this warning is a missing-symbol linker error.
I've taken the opportunity to batch in a few other de-escalations:
- Wconditional-uninitialized is done better by MSAN
- It'll take some work to dig Wformat-literal out of our shader compiler, but nothing looks unsafe
- Most of Wshift-sign-overflow is 0xff << 24. Don't want to ban that.
- Wdeprecated is mostly warning about features C++11 technically deprecated that might be removed in later releases. Punt!
- Wcovered-switch-default is pretty much the opposite of what we want.
- Wshadow is triggering too often to fix quickly. Probably mostly false positives.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2620
Change-Id: I20a85a77d2e19ed05a536b23037bd988350f821e
Reviewed-on: https://skia-review.googlesource.com/2620
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
