1551088 - Add accelerated RGB to RGBA/BGRA expansion methods

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, enhancement, P3)

Description

[Andrew Osmond [:aosmond] (he/him)]

We use gfxPackedPixel to expand from RGB to BGRA in the image decoders. Isolated testing of algorithms I've written using SSSE3 and AVX2 suggests it is 6x to 11x faster than plain old C per-pixel copying. This is similar in nature to gfx/2d/Swizzle-*.cpp methods, although if we slot it into that infrastructure, it would be better if we expose methods to get a function pointer designed for row by row conversion like how imagelib does decoding and avoid setup overhead/branches in SwizzleData on every row.

These new methods should support both RGB to RGBA and RGB to BGRA to facilitate bug 1541900.

Comment 1

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1551088 - Part 1. Expose SwizzleRow and PremultiplyRow variants.

The image decoders produce surfaces row by row, so a variant to get a
function pointer to perform swizzle/premultiply operations makes more
ergonomic sense.

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1551088 - Part 2. Add unpack methods to convert from RGB to RGBX/BGRX.

Some image decoders (e.g. PNG) may have a native representation of the
data as RGB, and do not have accelerated methods to transform from RGB
to RGBX/BGRX. Exposing this as part of the swizzle/premultiply methods
allows us to write accelerated versions ourselves in a later patch in
this series.

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1551088 - Part 3. Add SSSE3 and AVX2 implementations of unpacking.

These variants perform significantly faster than the C implementations
according to local testing and that in treeherder. Image decoding is as
much as 40% faster in the most simple cases (solid green PNG image).

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1551088 - Part 4. Add gtests for PremultiplyRow and SwizzleRow.

Comment 5

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1551088 - Part 5. Expose SurfaceFilter input row directly to avoid copy if possible.

Some filters can do the copy of the given data into the working buffer
as part of the filter operation. For those that cannot, we will just
copy the data first, and then advance the row.

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1551088 - Part 6. Make image decoders use accelerated methods for swizzling and premultiplication.

The PNG decoder lacks fast implementations for swizzling/unpacking
inside the library, and both PNG and WebP may need to perform
premultiplication due to the alpha channel. This patch adds a new filter
allowing us to take advantage of our accelerated implementations to
perform these transformations on their behalf.

Comment 7

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1551088 - Part 7. Add gtests for SwizzleFilter.

Comment 8

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1551088 - Part 8. Add reftest annotations for slight premultiplication differences.

Comment 9

[Andrew Osmond [:aosmond] (he/him)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b4f2b72e3781b05a42f5c6f826d65feece9e2ef

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ec69be661551
Part 1. Expose SwizzleRow and PremultiplyRow variants. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/6da767c8d55c
Part 2. Add unpack methods to convert from RGB to RGBX/BGRX. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/91300f9f99bb
Part 3. Add SSSE3 and AVX2 implementations of unpacking. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/4685fc022257
Part 4. Add gtests for PremultiplyRow and SwizzleRow. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/4760b8b22ffd
Part 5. Expose SurfaceFilter input row directly to avoid copy if possible. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/b061b1bf8281
Part 6. Make image decoders use accelerated methods for swizzling and premultiplication. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/d7684ca35c0d
Part 7. Add gtests for SwizzleFilter. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/02d9dc4d39a5
Part 8. Add reftest annotations for slight premultiplication differences. r=tnikkel

Comment 11

[Cosmin Sabou [:CosminS]]

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&revision=02d9dc4d39a56b6b660aa2689c689f3bf9172d95&selectedJob=267796611

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=267796611&repo=autoland

Back-out link: https://hg.mozilla.org/integration/autoland/rev/a5c7b61b18bc9c55cfe2c27c3d521891b9b67adc

[task 2019-09-21T11:48:58.178Z] 11:48:58 INFO - ld: warning: directory not found for option '-L/builds/worker/workspace/src/gcc/lib64'
[task 2019-09-21T11:48:58.178Z] 11:48:58 INFO - ld: warning: could not create compact unwind for _ffi_call_unix64: does not use RBP or RSP based frame
[task 2019-09-21T11:48:58.178Z] 11:48:58 INFO - Undefined symbols for architecture x86_64:
[task 2019-09-21T11:48:58.178Z] 11:48:58 INFO - "__ZN7mozilla3gfx14UnpackRowRGB24ILb1EEEvPKhPhi", referenced from:
[task 2019-09-21T11:48:58.179Z] 11:48:58 INFO - __ZN7mozilla3gfx20UnpackRowRGB24_SSSE3ILb1EEEvPKhPhi in SwizzleSSSE3.o
[task 2019-09-21T11:48:58.179Z] 11:48:58 INFO - "__ZN7mozilla3gfx14UnpackRowRGB24ILb0EEEvPKhPhi", referenced from:
[task 2019-09-21T11:48:58.179Z] 11:48:58 INFO - __ZN7mozilla3gfx20UnpackRowRGB24_SSSE3ILb0EEEvPKhPhi in SwizzleSSSE3.o
[task 2019-09-21T11:48:58.179Z] 11:48:58 INFO - ld: symbol(s) not found for architecture x86_64
[task 2019-09-21T11:48:58.179Z] 11:48:58 INFO - clang-8: error: linker command failed with exit code 1 (use -v to see invocation)
[task 2019-09-21T11:48:58.179Z] 11:48:58 INFO - /builds/worker/workspace/build/src/config/rules.mk:657: recipe for target 'XUL' failed
[task 2019-09-21T11:48:58.179Z] 11:48:58 ERROR - make[4]: *** [XUL] Error 1
[task 2019-09-21T11:48:58.179Z] 11:48:58 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/toolkit/library/build'
[task 2019-09-21T11:48:58.179Z] 11:48:58 INFO - /builds/worker/workspace/build/src/config/recurse.mk:74: recipe for target 'toolkit/library/build/target' failed
[task 2019-09-21T11:48:58.179Z] 11:48:58 ERROR - make[3]: *** [toolkit/library/build/target] Error 2
[task 2019-09-21T11:48:58.179Z] 11:48:58 INFO - make[3]: *** Waiting for unfinished jobs....

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/229c3d631d2e
Part 1. Expose SwizzleRow and PremultiplyRow variants. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/a011360f5018
Part 2. Add unpack methods to convert from RGB to RGBX/BGRX. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/925e26e17abe
Part 3. Add SSSE3 and AVX2 implementations of unpacking. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/d4dadb6ef0bd
Part 4. Add gtests for PremultiplyRow and SwizzleRow. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/5fe283ceaa0b
Part 5. Expose SurfaceFilter input row directly to avoid copy if possible. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/b259c9fb5ad6
Part 6. Make image decoders use accelerated methods for swizzling and premultiplication. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/222e856b5bc3
Part 7. Add gtests for SwizzleFilter. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/b3760d42e3bf
Part 8. Add reftest annotations for slight premultiplication differences. r=tnikkel

Comment 13

[Oana Pop-Rus]

Backed out for causing reftest failures in SwizzleAVX2.cpp, SwizzleSSSE3.cpp and transform.cpp

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=success%2Csuperseded%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&searchStr=linux%2Cx64%2Casan%2Ctest-linux64-asan%2Fopt-gtest-1proc%2C%28gtest%29&tochange=11e85f65759250318980f75eca734509119110b7&fromchange=db132849d960d7219f9b3299dc5ca227dd768fba

Also: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Csuperseded%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&searchStr=linux%2Cx64%2Casan%2Creftests%2Ctest-linux64-asan%2Fopt-reftest-e10s-6%2Cr%28r6%29&tochange=11e85f65759250318980f75eca734509119110b7&fromchange=db132849d960d7219f9b3299dc5ca227dd768fba

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=268173857&repo=autoland&lineNumber=8797

Backout: https://hg.mozilla.org/integration/autoland/rev/33c93c2f60953a41d5c6a482b2cce83c0c84c4c7

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bed9c93eeb2d
Part 1. Expose SwizzleRow and PremultiplyRow variants. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/25a5f5563e9d
Part 2. Add unpack methods to convert from RGB to RGBX/BGRX. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/d90a571e581f
Part 3. Add SSSE3 and AVX2 implementations of unpacking. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/9e954d6765de
Part 4. Add gtests for PremultiplyRow and SwizzleRow. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/f77c43bcc75b
Part 5. Expose SurfaceFilter input row directly to avoid copy if possible. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/5d72c8de4daf
Part 6. Make image decoders use accelerated methods for swizzling and premultiplication. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/3e6f25b21f8c
Part 7. Add gtests for SwizzleFilter. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/9ef391e20fa6
Part 8. Add reftest annotations for slight premultiplication differences. r=tnikkel

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d0ab25c226a7
Update fuzzy range for background/border-image-repeat-round-2.html a=fix CLOSED TREE

Comment 16

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/bed9c93eeb2d
https://hg.mozilla.org/mozilla-central/rev/25a5f5563e9d
https://hg.mozilla.org/mozilla-central/rev/d90a571e581f
https://hg.mozilla.org/mozilla-central/rev/9e954d6765de
https://hg.mozilla.org/mozilla-central/rev/f77c43bcc75b
https://hg.mozilla.org/mozilla-central/rev/5d72c8de4daf
https://hg.mozilla.org/mozilla-central/rev/3e6f25b21f8c
https://hg.mozilla.org/mozilla-central/rev/9ef391e20fa6
https://hg.mozilla.org/mozilla-central/rev/d0ab25c226a7

Comment 17

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/ab34396aa1bb
Backed out 9 changesets for causing Bug 1583848. a=backout

Comment 18

[Andrew Osmond [:aosmond] (he/him)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=71156be9cee648a8b89b73e7f3c78a7dddcb220b

Fixed what would have been a big endian regression, as well as the premultiply issues that caused the original backout. I'll double check with the build from CI to confirm before relanding.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/28f92f90d560
Part 1. Expose SwizzleRow and PremultiplyRow variants. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/9565552bf989
Part 2. Add unpack methods to convert from RGB to RGBX/BGRX. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/dfdf98704a03
Part 3. Add SSSE3 and AVX2 implementations of unpacking. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/f6a6eb672d21
Part 4. Add gtests for PremultiplyRow and SwizzleRow. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/124159db74bb
Part 5. Expose SurfaceFilter input row directly to avoid copy if possible. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/7a6b89adb970
Part 6. Make image decoders use accelerated methods for swizzling and premultiplication. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/7005e7b0c756
Part 7. Add gtests for SwizzleFilter. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/c67cee7b6275
Part 8. Add reftest annotations for slight premultiplication differences. r=tnikkel

Comment 20

[Andrew Osmond [:aosmond] (he/him)]

There were xpcshell test failures in comment 18 which I resolved prior to landing.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4fecfdf872cbffec41ea33a0ed3670770dcff5e6

Comment 21

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/28f92f90d560
https://hg.mozilla.org/mozilla-central/rev/9565552bf989
https://hg.mozilla.org/mozilla-central/rev/dfdf98704a03
https://hg.mozilla.org/mozilla-central/rev/f6a6eb672d21
https://hg.mozilla.org/mozilla-central/rev/124159db74bb
https://hg.mozilla.org/mozilla-central/rev/7a6b89adb970
https://hg.mozilla.org/mozilla-central/rev/7005e7b0c756
https://hg.mozilla.org/mozilla-central/rev/c67cee7b6275