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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1998373002: Improve srcover_srgb_srgb implementation. (Closed)

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

Description

I have found a more efficient way of detecting 1 and 0 alpha in SSE2. In addition, I found a stall on an execution unit for the lea instruction and rearranged to code to avoid that. Before 1,362.01 LinearSrcOvericonstrip.pngVSkOptsSSE41 2,132.54 LinearSrcOvericonstrip.pngVSkOptsDefault 1,717.77 LinearSrcOvericonstrip.pngVSkOptsNonSimdCore 3,525.14 LinearSrcOvericonstrip.pngVSkOptsTrivial 11,181.78 LinearSrcOvericonstrip.pngVSkOptsBruteForce 644.77 LinearSrcOvermandrill_512.pngVSkOptsSSE41 682.51 LinearSrcOvermandrill_512.pngVSkOptsDefault 1,169.65 LinearSrcOvermandrill_512.pngVSkOptsNonSimdCore 2,486.45 LinearSrcOvermandrill_512.pngVSkOptsTrivial 11,635.94 LinearSrcOvermandrill_512.pngVSkOptsBruteForce 217.76 LinearSrcOverplane.pngVSkOptsSSE41 437.09 LinearSrcOverplane.pngVSkOptsDefault 275.91 LinearSrcOverplane.pngVSkOptsNonSimdCore 481.70 LinearSrcOverplane.pngVSkOptsTrivial 1,504.66 LinearSrcOverplane.pngVSkOptsBruteForce 323.90 LinearSrcOverbaby_tux.pngVSkOptsSSE41 497.49 LinearSrcOverbaby_tux.pngVSkOptsDefault 456.08 LinearSrcOverbaby_tux.pngVSkOptsNonSimdCore 786.46 LinearSrcOverbaby_tux.pngVSkOptsTrivial 2,554.65 LinearSrcOverbaby_tux.pngVSkOptsBruteForce 484.83 LinearSrcOveryellow_rose.pngVSkOptsSSE41 821.86 LinearSrcOveryellow_rose.pngVSkOptsDefault 655.37 LinearSrcOveryellow_rose.pngVSkOptsNonSimdCore 1,323.80 LinearSrcOveryellow_rose.pngVSkOptsTrivial 5,802.61 LinearSrcOveryellow_rose.pngVSkOptsBruteForce After changes to sse2 and sse4.1 1,343.12 LinearSrcOvericonstrip.pngVSkOptsSSE41 1,441.17 LinearSrcOvericonstrip.pngVSkOptsDefault 1,679.97 LinearSrcOvericonstrip.pngVSkOptsNonSimdCore 3,481.05 LinearSrcOvericonstrip.pngVSkOptsTrivial 10,979.99 LinearSrcOvericonstrip.pngVSkOptsBruteForce 574.17 LinearSrcOvermandrill_512.pngVSkOptsSSE41 641.40 LinearSrcOvermandrill_512.pngVSkOptsDefault 1,169.44 LinearSrcOvermandrill_512.pngVSkOptsNonSimdCore 2,359.84 LinearSrcOvermandrill_512.pngVSkOptsTrivial 12,106.02 LinearSrcOvermandrill_512.pngVSkOptsBruteForce 209.95 LinearSrcOverplane.pngVSkOptsSSE41 249.12 LinearSrcOverplane.pngVSkOptsDefault 270.36 LinearSrcOverplane.pngVSkOptsNonSimdCore 466.30 LinearSrcOverplane.pngVSkOptsTrivial 1,431.14 LinearSrcOverplane.pngVSkOptsBruteForce 309.70 LinearSrcOverbaby_tux.pngVSkOptsSSE41 354.86 LinearSrcOverbaby_tux.pngVSkOptsDefault 442.69 LinearSrcOverbaby_tux.pngVSkOptsNonSimdCore 764.12 LinearSrcOverbaby_tux.pngVSkOptsTrivial 2,756.16 LinearSrcOverbaby_tux.pngVSkOptsBruteForce 457.70 LinearSrcOveryellow_rose.pngVSkOptsSSE41 500.50 LinearSrcOveryellow_rose.pngVSkOptsDefault 677.84 LinearSrcOveryellow_rose.pngVSkOptsNonSimdCore 1,301.50 LinearSrcOveryellow_rose.pngVSkOptsTrivial 5,786.40 LinearSrcOveryellow_rose.pngVSkOptsBruteForce BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1998373002 CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot Committed: https://skia.googlesource.com/skia/+/074b48ecb5ed8f9b25039477794437ae853d85c4

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -34 lines) Patch
M src/opts/SkBlend_opts.h View 3 chunks +45 lines, -34 lines 2 comments Download

Messages

Total messages: 16 (9 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998373002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998373002/1
4 years, 7 months ago (2016-05-20 21:14:10 UTC) #5
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 7 months ago (2016-05-20 21:29:02 UTC) #7
fmalita_google_do_not_use
LGTM, AFAIU this code. (maybe we can get mtklein to postreview when he gets back)
4 years, 7 months ago (2016-05-23 20:26:07 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1998373002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1998373002/1
4 years, 7 months ago (2016-05-23 20:28:41 UTC) #11
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://skia.googlesource.com/skia/+/074b48ecb5ed8f9b25039477794437ae853d85c4
4 years, 7 months ago (2016-05-23 20:50:16 UTC) #13
mtklein
I take it you've noticed you've now made SSE2 unambiguously better than the rest? lgtm ...
4 years, 7 months ago (2016-05-24 12:15:37 UTC) #15
mtklein
4 years, 7 months ago (2016-05-24 13:00:13 UTC) #16
Message was sent while issue was closed. 
https://codereview.chromium.org/1998373002/diff/1/src/opts/SkBlend_opts.h
File src/opts/SkBlend_opts.h (right):

https://codereview.chromium.org/1998373002/diff/1/src/opts/SkBlend_opts.h#new...
src/opts/SkBlend_opts.h:192: __m128i opaque       =
_mm_cmplt_epi32(signedPixels, _mm_set1_epi32(0x7F000000));
I think we can make this logic clearer.

To start, I think it'd help readability to change `opaque` and `transparent` to
`not_opaque` and `not_transparent`.

From there, it looks like the return value is
    !( !opaque ^ !transparent )

That expression is unusual enough it'd probably benefit from a truth table:

    opaque, transparent  ~~~> result
    1, 1  ~~~> (impossible input)
    0, 1  ~~~> !( !0 ^ !1 ) -->  !( 1 ^ 0)  --> !(1) --> 0
    1, 0  ~~~> !( !1 ^ !0 ) --> 0 by symmetry
    0, 0  ~~~> !( !0 ^ !0 ) --> ! ( 1 ^ 1 ) -->  !(0) --> 1

Powered by Google App Engine
This is Rietveld 408576698