Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(156)

Issue 1138893002: Plus xfermode using Sk4px. (Closed)

Created:
5 years, 7 months ago by mtklein_C
Modified:
5 years, 7 months ago
Reviewers:
mtklein, reed1
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Target Ref:
refs/heads/master
Project:
skia
Visibility:
Public.

Description

Plus xfermode using Sk4px. Xfermode_Plus runs 4-5x faster. We expect mixed_xfermodes to have a small diff. This is because kFoldCoverageIntoSrcAlpha was incorrectly set to true. This implementation handily beats the Sk4f impl, the portable impl, and the existing SSE2 impl. Reading the SkXfermodes_opts_SSE2.cpp file, I'm pretty confident that we'll be able to beat all SSE2 impls. I believe this impl will beat or match the existing NEON impl too, but that may not be true for more complicated xfermodes. They can take advantage of transposing ARGBARGB... to AAAARRRR.... cheaply and I haven't figured out an abstraction for that yet that doesn't screw SSE. Adds: - MapDstSrc() to Sk4px - saturatedAdd() to SkNi (only implemented as far as it's used). - div255Narrow() BUG=skia: Committed: https://skia.googlesource.com/skia/+/6cbf18c70bf99f58b2bb1c49cdf8d41be561fee4

Patch Set 1 #

Patch Set 2 : update assert #

Patch Set 3 : aa too #

Patch Set 4 : Can't actually fold coverage and alpha for Plus. #

Patch Set 5 : remove idea #

Patch Set 6 : split div255Narrow into div255RoundNarrow and div255TruncNarrow #

Patch Set 7 : modulate too #

Patch Set 8 : screen #

Patch Set 9 : guard #

Unified diffs Side-by-side diffs Delta from patch set Stats (+139 lines, -1 line) Patch
M src/core/Sk4px.h View 1 2 3 4 5 2 chunks +72 lines, -0 lines 0 comments Download
M src/core/SkNx.h View 1 2 chunks +10 lines, -0 lines 0 comments Download
M src/core/SkXfermode.cpp View 1 2 3 4 5 6 7 8 7 chunks +55 lines, -1 line 0 comments Download
M src/opts/SkNx_sse.h View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 33 (15 generated)
mtklein
5 years, 7 months ago (2015-05-12 14:36:03 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1138893002/20001
5 years, 7 months ago (2015-05-12 14:36:17 UTC) #4
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 7 months ago (2015-05-12 14:42:09 UTC) #6
reed1
very pretty -- lets try landing w/ the aa version.
5 years, 7 months ago (2015-05-12 15:45:59 UTC) #7
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1138893002/70001
5 years, 7 months ago (2015-05-12 18:12:11 UTC) #9
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1138893002/90001
5 years, 7 months ago (2015-05-12 18:50:22 UTC) #12
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 7 months ago (2015-05-12 19:03:23 UTC) #14
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1138893002/110001
5 years, 7 months ago (2015-05-12 19:30:32 UTC) #16
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 7 months ago (2015-05-12 19:36:31 UTC) #18
reed1
purty -- lgtm (assuming we can guard for blink and eventually test on neon)
5 years, 7 months ago (2015-05-12 20:32:06 UTC) #19
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1138893002/130001
5 years, 7 months ago (2015-05-12 20:34:59 UTC) #21
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 7 months ago (2015-05-12 20:41:03 UTC) #23
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1138893002/150001
5 years, 7 months ago (2015-05-12 20:44:58 UTC) #26
reed1
5 years, 7 months ago (2015-05-12 20:48:01 UTC) #27
reed1
lgtm
5 years, 7 months ago (2015-05-12 20:48:08 UTC) #28
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 7 months ago (2015-05-12 20:51:10 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1138893002/150001
5 years, 7 months ago (2015-05-12 22:47:55 UTC) #32
commit-bot: I haz the power
5 years, 7 months ago (2015-05-12 22:48:14 UTC) #33
Message was sent while issue was closed.
Committed patchset #9 (id:150001) as
https://skia.googlesource.com/skia/+/6cbf18c70bf99f58b2bb1c49cdf8d41be561fee4

Powered by Google App Engine
This is Rietveld 408576698