386937 - ConvertPixel doesn't work as nsColor and gfx_color are not compatible...

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Alfred Kayser]

In, the code assumes that nscolor and the aPixel (gfx_color) formats are equal.
http://lxr.mozilla.org/mozilla/source/gfx/src/thebes/nsThebesDeviceContext.cpp#435
435 nsThebesDeviceContext::ConvertPixel(nscolor aColor, PRUint32 & aPixel)

In fact they are not.
nsColor uses ABGR and gfx_color (cairo) uses ARGB.
Furthermore, in gfxColor gfxRGBA() and ::Packed, allows all kinds of formats, while only PACKED_ABGR is really used.
GFX_PACKED_PIXEL(a,r,g,b) produces the cairo format: ARGB.

In order to remove all this confusion, 
I would propose to convert all to one format: PACKED_ARGB (for the PRUint32 packed version), which is compatible with the Cairo image surface encoding).
All other places don't really care about the order.
nsColor always uses macro's access the parts.
And gfxColor can also easily be made to only handle ARGB.

Comment 1

[Alfred Kayser]

Standardization to the Cairo format is also needed in qCMS.
Using one packed color format throughout the mozilla codebase will make live a lot easier.

Comment 2

[Alfred Kayser]

Actually ConvertPixel is not used anywhere, so I propose to remove it before it gets misused.

Comment 3

[Alfred Kayser]

Attachment: Simple removal of ConvertPixel (and DebugShowCairo...)

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 419553 [details] [diff] [review]
Simple removal of ConvertPixel (and DebugShowCairo...)

Not sure why you asked me to review this, I'm not a gfx peer and don't really know anything about cairo :)

Comment 5

[Alfred Kayser]

Pushed: http://hg.mozilla.org/mozilla-central/rev/f319622cee4d

Comment 6

[Marco Castelluccio [:marco]]

The patch landed, why the bug is still opened?