Last Comment Bug 386937 - ConvertPixel doesn't work as nsColor and gfx_color are not compatible...
: ConvertPixel doesn't work as nsColor and gfx_color are not compatible...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Alfred Kayser
:
Mentors:
Depends on:
Blocks: 490700 deadcode
  Show dependency treegraph
 
Reported: 2007-07-05 01:23 PDT by Alfred Kayser
Modified: 2011-08-16 07:48 PDT (History)
5 users (show)
alfredkayser: wanted1.9.2?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Simple removal of ConvertPixel (and DebugShowCairo...) (2.08 KB, patch)
2009-12-30 04:26 PST, Alfred Kayser
vladimir: review+
Details | Diff | Review

Description Alfred Kayser 2007-07-05 01:23:31 PDT
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 2009-04-29 15:01:00 PDT
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 2009-12-30 04:17:10 PST
Actually ConvertPixel is not used anywhere, so I propose to remove it before it gets misused.
Comment 3 Alfred Kayser 2009-12-30 04:26:36 PST
Created attachment 419553 [details] [diff] [review]
Simple removal of ConvertPixel (and DebugShowCairo...)
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-01-28 09:55:23 PST
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 2010-02-12 01:20:10 PST
Pushed: http://hg.mozilla.org/mozilla-central/rev/f319622cee4d
Comment 6 Marco Castelluccio [:marco] 2011-08-16 06:17:26 PDT
The patch landed, why the bug is still opened?

Note You need to log in before you can comment on or make changes to this bug.