Note: There are a few cases of duplicates in user autocompletion which are being worked on.

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

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: Alfred Kayser, Assigned: Alfred Kayser)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.9.2 ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

10 years ago
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.
(Assignee)

Comment 1

8 years ago
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.
Assignee: nobody → alfredkayser
Blocks: 490700

Updated

8 years ago
Component: Image: Painting → Image: Painting
Product: Core → Core Graveyard
(Assignee)

Comment 2

8 years ago
Actually ConvertPixel is not used anywhere, so I propose to remove it before it gets misused.
(Assignee)

Updated

8 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

8 years ago
Component: Image: Painting → GFX: Color Management
Product: Core Graveyard → Core
(Assignee)

Comment 3

8 years ago
Created attachment 419553 [details] [diff] [review]
Simple removal of ConvertPixel (and DebugShowCairo...)
Attachment #419553 - Flags: review?(vladimir)
(Assignee)

Updated

8 years ago
Attachment #419553 - Attachment is patch: true
Attachment #419553 - Attachment mime type: application/octet-stream → text/plain
(Assignee)

Updated

8 years ago
Flags: wanted1.9.2?
QA Contact: image.gfx → color-management
(Assignee)

Updated

8 years ago
Attachment #419553 - Flags: review?(vladimir) → review?(gavin.sharp)
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 :)
Attachment #419553 - Flags: review?(gavin.sharp)
(Assignee)

Updated

8 years ago
Attachment #419553 - Flags: review?(vladimir)
Attachment #419553 - Flags: review?(vladimir) → review+
(Assignee)

Comment 5

8 years ago
Pushed: http://hg.mozilla.org/mozilla-central/rev/f319622cee4d
(Assignee)

Updated

7 years ago
Blocks: 457262
The patch landed, why the bug is still opened?

Updated

6 years ago
Assignee: alfredkayser → nobody
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Component: GFX: Color Management → Graphics
QA Contact: color-management → thebes
Resolution: --- → FIXED

Updated

6 years ago
Assignee: nobody → alfredkayser
You need to log in before you can comment on or make changes to this bug.