Closed
Bug 429915
Opened 17 years ago
Closed 17 years ago
with gfx.color_management.enabled [true], severe colour mismatch between html colours and gif/png images
Categories
(Core :: Graphics, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: phiw2, Assigned: vlad)
References
()
Details
(Keywords: regression)
Attachments
(3 files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
pavlov
:
review+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
What the summary says. Png or gif images don't match the html/css colours.
regressed:
works: Gecko/2008020604 Minefield/3.0b4pre
fails: Gecko/2008020615 Minefield/3.0b4pre
Mac only. (b5 Win shows the page correctly)
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-02-06+04%3A00%3A00&maxdate=2008-02-06+15%3A00%3A00&cvsroot=%2Fcvsroot
--> bug 371867
(asking for blocking to get at least wanted soonish :-)
Flags: blocking1.9?
Reporter | ||
Updated•17 years ago
|
Summary: with gfx.color_management.enabled [true], sever colour mismatch between html colours and gif/png images → with gfx.color_management.enabled [true], severe colour mismatch between html colours and gif/png images
Reporter | ||
Comment 1•17 years ago
|
||
Reporter | ||
Updated•17 years ago
|
Attachment #316682 -
Attachment mime type: text/html → image/png
Comment 2•17 years ago
|
||
Are you sure you have color management support for your monitor set up correctly? What did the page look like before the patch for bug 371867 landed?
Looking at the code, it appears that we weren't calling any CMS code for these images prior to the patch in bug 371867 landing. With the patch, we call gfxContext::SetColor for these optimized images, whereas before we didn't. So it seems like this just exposed something in the underlying CMS code, such as you not having a color profile enabled or something.
Keywords: qawanted
Reporter | ||
Comment 3•17 years ago
|
||
Page looks like this, prior to this bug.
Exactly same settings.
Updated•17 years ago
|
Flags: blocking1.9? → blocking1.9+
Updated•17 years ago
|
Assignee: nobody → vladimir
Assignee | ||
Comment 4•17 years ago
|
||
Grr. SetColor really shouldn't have changed to take sRGB colors, given that none of the rest of gfxContext works that way... but that ship has sailed. So, add SetDeviceColor and GetDeviceColor and use them where needed instead.
Attachment #317779 -
Flags: review?(pavlov)
Assignee | ||
Comment 5•17 years ago
|
||
Builds with this patch available here:
https://build.mozilla.org/tryserver-builds/2008-04-25_19:01-vladimir@mozilla.com-devicecolor/
Reporter | ||
Comment 6•17 years ago
|
||
(In reply to comment #4)
> Created an attachment (id=317779) [details]
> add SetDeviceColor, rename GetColor to GetDeviceColor
>
The world looks much better now !
The patch works fine so far. The test pages on the libpng site display mostly correctly now, as do my own test cases.
Some colours are still a bit off, with Gamma-labelled png's, e.g. in this range:
0033ff 0033cc 003399 003366 003333 003300
at the bottom of
http://www.libpng.org/pub/png/colorcube/colorcube-pngs-gamma16.html
(but that is probably outside of the scope of this bug, and I'm not sure those ever displayed correctly.)
Unlabelled PNGs and GIFs display perfectly fine.
Updated•17 years ago
|
Attachment #317779 -
Flags: review?(pavlov) → review+
Assignee | ||
Updated•17 years ago
|
Attachment #317779 -
Flags: approval1.9?
Comment 7•17 years ago
|
||
Comment on attachment 317779 [details] [diff] [review]
add SetDeviceColor, rename GetColor to GetDeviceColor
a1.9+=damons
Attachment #317779 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 8•17 years ago
|
||
Revision 409...
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
Checking in gfx/src/thebes/nsThebesImage.cpp;
/cvsroot/mozilla/gfx/src/thebes/nsThebesImage.cpp,v <-- nsThebesImage.cpp
new revision: 1.86; previous revision: 1.85
done
Checking in gfx/src/thebes/nsThebesRenderingContext.cpp;
/cvsroot/mozilla/gfx/src/thebes/nsThebesRenderingContext.cpp,v <-- nsThebesRenderingContext.cpp
new revision: 1.54; previous revision: 1.53
done
Checking in gfx/thebes/public/gfxContext.h;
/cvsroot/mozilla/gfx/thebes/public/gfxContext.h,v <-- gfxContext.h
new revision: 1.59; previous revision: 1.58
done
Checking in gfx/thebes/src/gfxContext.cpp;
/cvsroot/mozilla/gfx/thebes/src/gfxContext.cpp,v <-- gfxContext.cpp
new revision: 1.60; previous revision: 1.59
done
Checking in gfx/thebes/src/gfxFont.cpp;
/cvsroot/mozilla/gfx/thebes/src/gfxFont.cpp,v <-- gfxFont.cpp
new revision: 1.95; previous revision: 1.94
done
Checking in gfx/thebes/src/gfxFontMissingGlyphs.cpp;
/cvsroot/mozilla/gfx/thebes/src/gfxFontMissingGlyphs.cpp,v <-- gfxFontMissingGlyphs.cpp
new revision: 1.4; previous revision: 1.3
done
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 9•17 years ago
|
||
verified fixed with
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008042904 Minefield/3.0pre
Status: RESOLVED → VERIFIED
Comment 10•10 years ago
|
||
Issue is Resolved - removing QA-Wanted Keywords - QA-Wanted query clean-up task
Keywords: qawanted
You need to log in
before you can comment on or make changes to this bug.
Description
•