1500709 - Primitives used for premultiplying are implemented wrong for big endian architectures

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect, P3)

Description

[A. Wilcox [:awilfox]]

Attachment: Screenshot_20181016_184952.png

gfx/2d/Swizzle.cpp has code that is meant to handle swizzling on big endian architectures.  However, it is implemented incorrectly; the values and constants used for BE are wildly incorrect.  This results in compile-time warnings such as:

 0:31.71 In file included from /var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/gfx/2d/Unified_cpp_gfx_2d2.cpp:47:0:
 0:31.71 /var/clean/mozppc/mozilla-unified/gfx/2d/Swizzle.cpp: In instantiation of ‘void mozilla::gfx::PremultiplyFallback(const uint8_t*, int32_t, uint8_t*, int32_t, mozilla::gfx::IntSize) [with bool aSwapRB = false; bool aOpaqueAlpha = false; unsigned int aSrcRGBShift = 24u; unsigned int aSrcAShift = 16u; unsigned int aDstRGBShift = 24u; unsigned int aDstAShift = 16u; uint8_t = unsigned char; int32_t = int; mozilla::gfx::IntSize = mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>]’:
 0:31.71 /var/clean/mozppc/mozilla-unified/gfx/2d/Swizzle.cpp:334:3:   required from here
 0:31.71 /var/clean/mozppc/mozilla-unified/gfx/2d/Swizzle.cpp:221:36: warning: result of ‘(65280 << 24u)’ requires 41 bits to represent, but ‘int’ only has 32 bits [-Wshift-overflow=]
 0:31.71        uint32_t g = color & (0xFF00 << aSrcRGBShift);
 0:31.71                             ~~~~~~~~^~~~~~~~~~~~~~~~
 0:31.71 /var/clean/mozppc/mozilla-unified/gfx/2d/Swizzle.cpp:222:25: warning: result of ‘(65280 << 24u)’ requires 41 bits to represent, but ‘int’ only has 32 bits [-Wshift-overflow=]
 0:31.72        g = g*a + (0xFF00 << aSrcRGBShift);
 0:31.72                  ~~~~~~~~^~~~~~~~~~~~~~~~
 0:31.72 /var/clean/mozppc/mozilla-unified/gfx/2d/Swizzle.cpp:223:38: warning: result of ‘(16711680 << 24u)’ requires 49 bits to represent, but ‘int’ only has 32 bits [-Wshift-overflow=]
 0:31.72        g = (g + (g >> 8)) & (0xFF0000 << aSrcRGBShift);
 0:31.72                             ~~~~~~~~~~^~~~~~~~~~~~~~~~
 0:31.72 /var/clean/mozppc/mozilla-unified/gfx/2d/Swizzle.cpp:230:14: warning: right shift count >= width of type [-Wshift-count-overflow]
 0:31.72           (rb >> (8 - aDstRGBShift)) |
 0:31.72           ~~~~^~~~~~~~~~~~~~~~~~~~~~

[ and so on, for each instantiation of the template ]

It also results in run-time failure because the colours become more corrupt with every composition, eventually resulting in blocks of wrong colours (see attached PNG) and then finally a crash from the assertion in VerifyRGBXCorners.

Comment 1

[A. Wilcox [:awilfox]]

Attachment: Implement bit-shift constants for swizzle methods correctly on big endian CPUs.

This patch corrects the error and allows BasicCompositor to function flawlessly on big endian CPUs.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0f7573deea1
fix big-endian RGBBitShift in Swizzle. r=me

Comment 3

[Lee Salzman [:lsalzman]]

Comment on attachment 9018838 [details] [diff] [review]
Implement bit-shift constants for swizzle methods correctly on big endian CPUs.

Thank you for catching this. This patch is semantically okay, but there is a simpler way to make this work. I just went ahead and pushed in my own patch so we can get the fix in quickly.

Comment 4

[A. Wilcox [:awilfox]]

Great, thanks so much for landing this.

Comment 5

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/f0f7573deea1