single pixel background image is incorrect color (third line of acid2)

RESOLVED FIXED

Status

()

Core
Graphics
--
major
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: dbaron, Assigned: vlad)

Tracking

(Blocks: 1 bug, {regression})

Trunk
x86
Linux
regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments)

(Reporter)

Description

11 years ago
The third line of the acid2 test regressed between Linux nightlies 2006-10-26-04-trunk and 2006-10-27-04-trunk.  It doesn't look particularly easy to simplify a testcase, so it may be easier to figure out by isolating the offending checkin.

Steps to reproduce:
1. load http://www.webstandards.org/files/acid2/test.html#top

Expected results:  No aqua.  Other than the eyes, the test should be just yellow and black.

Actual results:  The 12px high row just above the eyes is aqua instead of yellow.

Possible checkins are:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-10-26+04%3A02&maxdate=2006-10-27+04%3A09&cvsroot=%2Fcvsroot
I don't see any layout changes other than SVG, a focus bug, and some regression tests being added in that range.

I do see a bunch of cairo changes, though none look obviously relevant.

Interestingly, I cannot reproduce the bug over here with Seamonkey builds pulled at MOZ_CO_DATE="Sat Oct 28 00:03:30 CDT 2006" (both cairo and gtk2 GFX)...  I _can_ reproduce with a Firefox build pulled at the same time (!).

No clue what's up with that.

Comment 2

11 years ago
looks ok on mac.
That row has a 1x1 pixel png background, which I handle specially in 357761; it's possible that that part's broken somehow.  I'll take a look tomorrow.

Comment 4

11 years ago
(In reply to comment #3)
> That row has a 1x1 pixel png background, which I handle specially in 357761;
> it's possible that that part's broken somehow.  I'll take a look tomorrow.
> 
Looks that way, vlad. Removing the background png url from the css makes it go to red (as expected) instead of aqua.
(Reporter)

Comment 5

11 years ago
Created attachment 244288 [details]
simplified testcase
(Reporter)

Updated

11 years ago
Component: General → GFX: Thebes
QA Contact: general → thebes
(Reporter)

Updated

11 years ago
Assignee: nobody → vladimir
Keywords: regression
Summary: third line of acid2 regressed 2006-10-27 → single pixel background image is incorrect color (third line of acid2)
(Reporter)

Updated

11 years ago
Blocks: 357761
(Reporter)

Comment 6

11 years ago
I put a printf in ThebesDrawTile, and mSinglePixelColor is wrong there -- the r component is 0 instead of the b.
(Reporter)

Comment 7

11 years ago
gfxImageSurface is (maybe) documented as ARGB.  gfxRGBA's constructor is documented as taking ABGR.  (Should they really be different?)  Yet nsThebesImage::Optimize goes from one to the other without conversion.
(Reporter)

Comment 8

11 years ago
(And I did confirm by local backout that bug 357761 was the cause.)
For what it's worth, it looks like I had somehow failed to build my thebes seamonkey tree, which is why it wasn't showing the bug...
Blocks: 334730
Flags: blocking1.9-
Created attachment 244355 [details] [diff] [review]
fix single-pixel color conversion

dbaron's analysis was correct; I was passing in ARGB to a constructor that expected ABGR.  I tried to make it a little more obvious as to what was going on, to avoid creating this problem in the future.
Attachment #244355 - Flags: review?(pavlov)

Updated

11 years ago
Attachment #244355 - Flags: review?(pavlov) → review+
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Created attachment 244935 [details] [diff] [review]
Correctly read in colors for premultiplied source colors, too

Whoops, forgot to read in r/g/b/a for the premultiplied case.
Attachment #244935 - Flags: review+
That last checkin isn't likely to be causing the Firefox Mac orange, right?  ;)
I sure hope not!
You need to log in before you can comment on or make changes to this bug.