Publicly expose one accelerated swizzle.

Publicly expose one accelerated swizzle.

SkOpts is intentionally internal, but we want to give Chrome access
to those pieces that would seriously benefit them.
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1822363002

Committed: https://skia.googlesource.com/skia/+/734351d2555024a679c4e4f5e13e8be2329803d2

[tomhudson, 2016-03-23 16:58:58]

Description was changed from

==========
Publicly expose one accelerated swizzle.

SkOpts is intentionally internal, but we want to give Chrome access
to those pieces that would seriously benefit them.
==========

to

==========
Publicly expose one accelerated swizzle.

SkOpts is intentionally internal, but we want to give Chrome access
to those pieces that would seriously benefit them.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[tomhudson, 2016-03-23 17:00:34]

tomhudson@google.com changed reviewers:
+ reed@google.com

[tomhudson, 2016-03-23 17:00:34]

reed@: public API change

Matt's seen huge speedups on some platforms with SIMD; while I'm cleaning up
pepper video code in Chrome I'd like to let them use our implementation.

[mtklein, 2016-03-23 17:02:34]

mtklein@google.com changed reviewers:
+ mtklein@google.com

[mtklein, 2016-03-23 17:02:35]

Maybe you should write a test.

https://codereview.chromium.org/1822363002/diff/1/include/core/SkSwizzle.h
File include/core/SkSwizzle.h (right):

https://codereview.chromium.org/1822363002/diff/1/include/core/SkSwizzle.h#ne...
include/core/SkSwizzle.h:12: void SkSwizzleRGBAToBGRA(uint32_t* src, void* dest,
int count);
Let's write (uint32_t* dest, const uint32_t* src, int count).

https://codereview.chromium.org/1822363002/diff/1/src/core/SkSwizzle.cpp
File src/core/SkSwizzle.cpp (right):

https://codereview.chromium.org/1822363002/diff/1/src/core/SkSwizzle.cpp#newc...
src/core/SkSwizzle.cpp:13: SkOpts::RGBA_to_BGRA(src, dest, count);
This has src and dest swapped.

[reed1, 2016-03-23 17:03:23]

https://codereview.chromium.org/1822363002/diff/1/include/core/SkSwizzle.h
File include/core/SkSwizzle.h (right):

https://codereview.chromium.org/1822363002/diff/1/include/core/SkSwizzle.h#ne...
include/core/SkSwizzle.h:12: void SkSwizzleRGBAToBGRA(uint32_t* src, void* dest,
int count);
// Swap the R and B bytes for each pixel/int32

Should the name reflect the symmetric/agnostic nature of the swizzle?

[mtklein, 2016-03-23 17:07:29]

On 2016/03/23 17:03:23, reed1 wrote:
> https://codereview.chromium.org/1822363002/diff/1/include/core/SkSwizzle.h
> File include/core/SkSwizzle.h (right):
> 
>
https://codereview.chromium.org/1822363002/diff/1/include/core/SkSwizzle.h#ne...
> include/core/SkSwizzle.h:12: void SkSwizzleRGBAToBGRA(uint32_t* src, void*
dest,
> int count);
> // Swap the R and B bytes for each pixel/int32
> 
> Should the name reflect the symmetric/agnostic nature of the swizzle?

Yeah, inside swizzler we often call this "swaprb".

[tomhudson, 2016-03-23 17:25:50]

Renamed to SkSwapRB(), reordered and retyped parameters, added test to
SwizzlerTest.

https://codereview.chromium.org/1822363002/diff/1/include/core/SkSwizzle.h
File include/core/SkSwizzle.h (right):

https://codereview.chromium.org/1822363002/diff/1/include/core/SkSwizzle.h#ne...
include/core/SkSwizzle.h:12: void SkSwizzleRGBAToBGRA(uint32_t* src, void* dest,
int count);
On 2016/03/23 17:02:34, mtklein wrote:
> Let's write (uint32_t* dest, const uint32_t* src, int count).

Done.

https://codereview.chromium.org/1822363002/diff/1/include/core/SkSwizzle.h#ne...
include/core/SkSwizzle.h:12: void SkSwizzleRGBAToBGRA(uint32_t* src, void* dest,
int count);
On 2016/03/23 17:03:23, reed1 wrote:
> // Swap the R and B bytes for each pixel/int32
> 
> Should the name reflect the symmetric/agnostic nature of the swizzle?

Done.

https://codereview.chromium.org/1822363002/diff/1/src/core/SkSwizzle.cpp
File src/core/SkSwizzle.cpp (right):

https://codereview.chromium.org/1822363002/diff/1/src/core/SkSwizzle.cpp#newc...
src/core/SkSwizzle.cpp:13: SkOpts::RGBA_to_BGRA(src, dest, count);
On 2016/03/23 17:02:34, mtklein wrote:
> This has src and dest swapped.

Done.

[reed1, 2016-03-23 17:34:17]

lgtm

api lgtm

[mtklein, 2016-03-23 17:35:13]

https://codereview.chromium.org/1822363002/diff/20001/src/core/SkSwizzle.cpp
File src/core/SkSwizzle.cpp (right):

https://codereview.chromium.org/1822363002/diff/20001/src/core/SkSwizzle.cpp#...
src/core/SkSwizzle.cpp:13: SkOpts::RGBA_to_BGRA(dest, src, count);
I think we usually use a 4 space indent.

https://codereview.chromium.org/1822363002/diff/20001/tests/SwizzlerTest.cpp
File tests/SwizzlerTest.cpp (right):

https://codereview.chromium.org/1822363002/diff/20001/tests/SwizzlerTest.cpp#...
tests/SwizzlerTest.cpp:166: for (int c = 0; c <= 255; c++) {
We making these public too?

[tomhudson, 2016-03-23 17:38:48]

https://codereview.chromium.org/1822363002/diff/20001/src/core/SkSwizzle.cpp
File src/core/SkSwizzle.cpp (right):

https://codereview.chromium.org/1822363002/diff/20001/src/core/SkSwizzle.cpp#...
src/core/SkSwizzle.cpp:13: SkOpts::RGBA_to_BGRA(dest, src, count);
On 2016/03/23 17:35:13, mtklein wrote:
> I think we usually use a 4 space indent.

Too much Chromium coding.
Done.

https://codereview.chromium.org/1822363002/diff/20001/tests/SwizzlerTest.cpp
File tests/SwizzlerTest.cpp (right):

https://codereview.chromium.org/1822363002/diff/20001/tests/SwizzlerTest.cpp#...
tests/SwizzlerTest.cpp:166: for (int c = 0; c <= 255; c++) {
On 2016/03/23 17:35:13, mtklein wrote:
> We making these public too?

My bad.
Done.

[tomhudson, 2016-03-23 17:39:37]

The CQ bit was checked by tomhudson@google.com

[tomhudson, 2016-03-23 17:39:37]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1822363002/#ps40001
(title: "mtklein second review")

[commit-bot: I haz the power, 2016-03-23 17:39:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1822363002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1822363002/40001

[commit-bot: I haz the power, 2016-03-23 17:51:23]

Description was changed from

==========
Publicly expose one accelerated swizzle.

SkOpts is intentionally internal, but we want to give Chrome access
to those pieces that would seriously benefit them.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Publicly expose one accelerated swizzle.

SkOpts is intentionally internal, but we want to give Chrome access
to those pieces that would seriously benefit them.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/734351d2555024a679c4e4f5e13e8be2329803d2
==========

[commit-bot: I haz the power, 2016-03-23 17:51:24]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/734351d2555024a679c4e4f5e13e8be2329803d2