Closed Bug 1551088 Opened 6 years ago Closed 5 years ago

Add accelerated RGB to RGBA/BGRA expansion methods

Categories

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

Product:
Core
Component:
Graphics: ImageLib
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla71
Tracking Flags:
Tracking Status
firefox68 --- wontfix
firefox71 --- fixed

People

(Reporter: aosmond, Assigned: aosmond)

References

(Blocks 1 open bug)

Details

Attachments

(8 files)

Bug 1551088 - Part 1. Expose SwizzleRow and PremultiplyRow variants.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1551088 - Part 2. Add unpack methods to convert from RGB to RGBX/BGRX.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1551088 - Part 3. Add SSSE3 and AVX2 implementations of unpacking.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1551088 - Part 4. Add gtests for PremultiplyRow and SwizzleRow.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1551088 - Part 5. Expose SurfaceFilter input row directly to avoid copy if possible.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1551088 - Part 6. Make image decoders use accelerated methods for swizzling and premultiplication.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1551088 - Part 7. Add gtests for SwizzleFilter.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1551088 - Part 8. Add reftest annotations for slight premultiplication differences.
47 bytes, text/x-phabricator-request
Details | Review

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.

Assignee: nobody → aosmond
Priority: -- → P3
Blocks: 1541900
Type: defect → enhancement

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.

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.

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).

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.

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.

Pushed by aosmond@mozilla.com: 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

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....

Flags: needinfo?(aosmond)
Pushed by aosmond@mozilla.com: 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
Flags: needinfo?(aosmond)
Pushed by aosmond@mozilla.com: 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
Blocks: 1583647
Flags: needinfo?(aosmond)
Pushed by malexandru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d0ab25c226a7 Update fuzzy range for background/border-image-repeat-round-2.html a=fix CLOSED TREE
Backout by csabou@mozilla.com: https://hg.mozilla.org/mozilla-central/rev/ab34396aa1bb Backed out 9 changesets for causing Bug 1583848. a=backout
Status: RESOLVED → REOPENED
status-firefox71: fixed → ---
Flags: needinfo?(aosmond)
Resolution: FIXED → ---
Target Milestone: mozilla71 → ---

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.

Flags: needinfo?(aosmond)
Pushed by aosmond@mozilla.com: 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

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

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

See Also: → 1616030
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: