689962 - Gif images with a 2 colors palette are missing on websites

Status: RESOLVED INCOMPLETE

(Core :: Graphics, defect)

Description

[Pipe Draven]

Attachment: Transparent hole instead a black color line.

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:7.0) Gecko/20100101 Firefox/7.0
Build ID: 20110922153450

Steps to reproduce:

I made a gif with a 2 colors palette (black and white) and I placed it as a background image.

The attached image shows how instead a black line I have this transparent section which holds ghost from everything once above.


Actual results:

The image wasn't there, instead it was a "transparent hole" which hold everything which once was above it as an image glitch. (Image attached).

It can be fixed by adding more colors to the GIF palette of the file.


Expected results:

The gif image rendered as a solid color.

Comment 1

[Tim (fmdeveloper)]

Can you attach the gif image to this bug?

Comment 2

[Pipe Draven]

Attachment: The Original Gif

This image was use as the background-image of an H2, but it can't be seen neither if I open it with Firefox as an image, I can only see it on the favicon, not the window.

Comment 3

[Tim (fmdeveloper)]

I am able to see a 9 px x 2 px image in both 3.6 and the latest nightly.

Does the issue still occur if you start Firefox in Safe Mode? http://support.mozilla.com/en-US/kb/Safe+Mode

How about with a new, empty profile? http://support.mozilla.com/en-US/kb/Basic%20Troubleshooting#w_8-make-a-new-profile

Comment 4

[Pipe Draven]

Restarted Firefox on safe mode and still the gif is Buggy...

I'd been working on that site for a few months.. In each version of firefox it was right, it was until I installed the 7 where it was like that.

When I found the bug, I exported the Gif Image several times without luck, it was until I added a couple of new colors to the image (different unnoticing blacks) when it was visible on my Firefox 7 at my Mac.

Anyway, we can close the issue if you want, still, I think there's something funny on my version of Firefox with the Gifs with a 2 colors palette.

Thanx for replying so fast.

Comment 5

[Matthias Versen [:Matti]]

wfm with Mozilla/5.0 (Windows NT 6.1; rv:9.0a1) Gecko/20110922 Firefox/9.0a1 SeaMonkey/2.6a1

Comment 6

[:aceman]

I do not see it either on Win XP, FF7. Can it be Mac OS specific?

Comment 7

[Boris Zbarsky [:bzbarsky]]

I don't see the problem on Mac either, but maybe I'm looking for the wrong thing?  What exactly are the steps to reproduce using the attached image?

Comment 8

[:aceman]

Attachment: testcase with img as background

maybe it is necessary to have it as background. Try this.

Comment 9

[Boris Zbarsky [:bzbarsky]]

That testcase renders correctly over here as far as I can tell.

Comment 10

[:aceman]

For me too.
Reporter, can you comment if you see the problem on my testcase? If not, can you provide your own one?

Comment 11

[Pipe Draven]

Attachment: screenshot of bug when viewing aceman's Test Case

First I saw a DIV word, changed the tab (tom my yahoo mail tab) and returned leaving the last seen capture (my mail in this case)

Comment 12

[Pipe Draven]

Attachment: Steps to see the bug

I made a different new GIF image (just like aceman) a 300x300px black image.

The I opened as a file with Firefox. The favicon image captures the last image in the area, in this case the last frame from the loading icon (static)-

I changed the tab and the favicon was black ONLY on mouse over, otherwise it was gray.

When I returned to the tab the favicon was ok, all black, but in the area of the gif I saw the last thing in the GIF area, which in this was a part of the yahoo window.

If I add more colors to the gif I can see it properly.. Like, a palette with 000001, 000002, 000003 colors.... instead of the 000000 and FFFFFF which makes by default photoshop when I make a plain black GIF.

So the problem is only with 2 colors in the palette (the minimum in GIF images).

Comment 13

[:aceman]

Simply, the image is not painted at all, it is transparent. But not in the normal way (the page background would be painted instead), the window are is simply not repainted and the old screen content remains.

Could be some subtle problem in Mac OS X drawing libraries. bz, do you have OS X 10.5?

Comment 14

[Boris Zbarsky [:bzbarsky]]

I can if I dig enough.  Probably won't happen till Monday; that computer is pretty well-buried.

I can't reproduce the issue on 10.6, for sure.

Comment 15

[Pipe Draven]

I have Mac OSX 10.5.8

Still, I didn't have this problem in other versions of Firefox, I noticed it until I updated it to 7... in fact I don't have it with different browsers neither... so it has to be something with the program and those images and not the images themself.

Comment 16

[:aceman]

Could you please see if you can see the problem reported in bug 689962?

Comment 17

[Pipe Draven]

(In reply to aceman from comment #16)
> Could you please see if you can see the problem reported in bug 689962?

If the message was for me... that's this very page ^_^U

Comment 18

[:aceman]

Ah, sorry, I meant bug 690710 :)

Comment 19

[Pipe Draven]

Yes, that's my problem... I saw the BG image from that page in Photoshop and it's a 2 colors Gif (black and white).. 

http://www.pentax.de/site/images/IS/contentcol1-bg.gif

That's the BG image from that site.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Pipe Draven, are you willing to use nightly builds to narrow down when the problem appeared?

Comment 21

[:aceman]

Nightly builds between FF6 and 7.

Comment 22

[Pipe Draven]

Sure, just give the instructions.

Comment 23

[:aceman]

First please make sure the bug still exists in latest Nightly 10 (http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/)

Comment 24

[Pipe Draven]

Yes, the problem is still there on Nightly 10.0a1

Comment 25

[Boris Zbarsky [:bzbarsky]]

Pipe Draven, try the instructions at http://harthur.github.com/mozregression/ ?

Comment 26

[Pipe Draven]

Last good nightly: 2011-05-25
First bad nightly: 2011-05-26

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=836aa9658341&tochange=831f8e040f38

I'll try the bisect further... but I leave you this by now.

Comment 27

[Boris Zbarsky [:bzbarsky]]

Thanks!

Almost certainly either the cairo update or the EXTEND_PAD/NONE quartz changes.

Comment 28

[Pipe Draven]

SyntaxError: invalid syntax

I can't bisect further.

Comment 29

[Stefan Waner]

I am experiencing the same bug with gifs I use for mathematical symbols in a mathematics site. This is a quite a serious issue for me as I use such gifs for many mathematical symbols in numerous pages. System: Mac 0s 10.6.8 Mozilla 7.0.1. Sample page:

http://www.zweigmedia.com/SYMB/FRcopy.GIF

It shows the one-pixel wide image as a transparent ghost.

Comment 30

[:aceman]

Pipe & Stefan, can you please also check bug 691699 if you see the problem?

Comment 31

[Stefan Waner]

Yes; bug 691699 shows up in my system.

Comment 32

[Pipe Draven]

Yes, that's the same bug.

Comment 33

[:aceman]

It is similar, but he is using 24bit PNG, so we don't know if it is completely the same. The feature that is the same is that you have 100% black color on one end of the palette, and he does too (as RGB000000). It seems that incidentally you also have the black color as first entry in your palette so it has index 0. So the image is all zeros. Can you confirm that?
Can you then make and image that contains some pattern, i.e. is not solid black but also contains some pixels of the other color? We want to know how the index 1 pixels render.

Comment 34

[Stefan Waner]

I have made a new one (50px square) whose palette contains pure black in entries #1 through 254. (Palette entry #000 is white and #255 is grey). The black is palette entry #254 (I cannot get the graphics program to use the same black with two different palette numbers in different parts of the image).

http://www.zweigmedia.com/SYMB/FRcopy2.png

Same problem as before on Firefox: invisible image.

If I change one or more pixels in the graphic to a non-black color (eg the grey or white) then the image shows perfectly.

Comment 35

[Stefan Waner]

Followup:

An interesting and bizarre feature of the bug:

If you open the image http://www.zweigmedia.com/SYMB/FRcopy2.png in a new tab, then go back to a previously opened tab, and then return to the tab containing the image, what you see instead of the image is the corresponding portion of the window you just left! Thus, if, say, I go to a tab that has "Google" on the top left, then go back to the tab containing the image, what I see is the top left 50x50 pixel piece of the "Google" instead of the graphic. (Reloading the image has no effect..)

Comment 36

[:aceman]

I think that is the original problem. Not only the image is transparent, but also the space taken by it. Previous contents of the screen (e.g. from previously viewed tabs) show through. See attachment 563747 [details] here.

Comment 37

[Jeff Muizelaar [:jrmuizel]]

I can reproduce this on 10.6 by disabling layer acceleration.

Comment 38

[:aceman]

Which disables Quartz/OpenGl and uses plain cairo?

Comment 39

[Joe Drew (not getting mail)]

Layers acceleration on OS X uses OpenGL for layer composition; disabling it means we use Cairo (which internally uses Quartz) for layer composition.

Comment 40

[Jeff Muizelaar [:jrmuizel]]

Attachment: CGLayers need to at least have a content type similar to their parent surfaces

This bug happens as follows:

1. The image gets optimized to a black solid color fill with SOURCE
2. cairo-gstate sees that we're doing a solid color fill of black to a CONTENT_COLOR surface and optimizes this to a CLEAR
3. This gets turned into a CGContextFillRect with operator CLEAR which clears the layer
4. When we draw the layer we end up drawing a transparent surface instead of a black one. Thus we end up drawing nothing.

This makes a layer surface be created with a similar content type to it's parent surface.

Comment 41

[Jeff Muizelaar [:jrmuizel]]

Attachment: The actual version

Comment 42

[Pipe Draven]

Yay, you found it.

Comment 43

[:aceman]

So is it a bug in Gecko that was uncovered by cairo's optimizations in the newest version?

Comment 44

[Jeff Muizelaar [:jrmuizel]]

(In reply to aceman from comment #43)
> So is it a bug in Gecko that was uncovered by cairo's optimizations in the
> newest version?

The bug was in some of the code we've added to cairo. It was uncovered by cairo's optimizations.

Comment 45

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 566664 [details] [diff] [review]
The actual version

Review of attachment 566664 [details] [diff] [review]:
-----------------------------------------------------------------

Let's have a reftest here.

Comment 46

[Joe Drew (not getting mail)]

roc, isn't it really weird that we're semi-ignoring the content type we pass to cairo_quartz_surface_create_cg_layer?

Comment 47

[Jeff Muizelaar [:jrmuizel]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #45)
> Comment on attachment 566664 [details] [diff] [review] [diff] [details] [review]
> The actual version
> 
> Review of attachment 566664 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> Let's have a reftest here.

I haven't been able to think of how to reftest this yet. Suggestions?

Comment 48

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can't we use one of the testcases in this bug?

Actually I wonder if this patch is right. Seems to me the underlying issue here is that CGLayers always have transparency, so claiming the CGLayer surface is CONTENT_COLOR is never the right thing to do. We should always treat them as CONTENT_COLOR_ALPHA. So instead of passing surface->content, we should just pass CONTENT_COLOR_ALPHA.

(In reply to Joe Drew (:JOEDREW!) (Away October 18–November 2) from comment #46)
> roc, isn't it really weird that we're semi-ignoring the content type we pass
> to cairo_quartz_surface_create_cg_layer?

Yes, that'd be because CGLayers only support CONTENT_COLOR_ALPHA really.

Comment 49

[Jeff Muizelaar [:jrmuizel]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #48)
> Can't we use one of the testcases in this bug?

Not easily, because the transparent layer ends up as black (the correct color) in the reftest.

> Actually I wonder if this patch is right. Seems to me the underlying issue
> here is that CGLayers always have transparency, so claiming the CGLayer
> surface is CONTENT_COLOR is never the right thing to do. We should always
> treat them as CONTENT_COLOR_ALPHA. So instead of passing surface->content,
> we should just pass CONTENT_COLOR_ALPHA.

I'm fine with that change too.

Comment 51

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #49)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #48)
> > Can't we use one of the testcases in this bug?
> 
> Not easily, because the transparent layer ends up as black (the correct
> color) in the reftest.
> 
> > Actually I wonder if this patch is right. Seems to me the underlying issue
> > here is that CGLayers always have transparency, so claiming the CGLayer
> > surface is CONTENT_COLOR is never the right thing to do. We should always
> > treat them as CONTENT_COLOR_ALPHA. So instead of passing surface->content,
> > we should just pass CONTENT_COLOR_ALPHA.
> 
> I'm fine with that change too.

But I wonder what impact will have on bug 579985 where we added this content stuff in?

Comment 52

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes, that is a good point :-(.

Inside cairo we need to be able to distinguish "this surface has all opaque pixels" from "this surface needs its alpha channel values set correctly". Step 2 of comment #40 needs to check the latter. Optimizations that replace OVER with SOURCE need to check the former.

One of those can be represented by CONTENT_COLOR, but I'm not sure which one.

Comment 53

[Jeff Muizelaar [:jrmuizel]]

Attachment: Another attempt at this

Comment 54

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 573012 [details] [diff] [review]
Another attempt at this

Review of attachment 573012 [details] [diff] [review]:
-----------------------------------------------------------------

This seems reasonable

Comment 58

[Cameron Kaiser [:spectre]]

Just as a note, I whipped up a build with this patch and while it does fix the issue it makes screen paints require significantly more CPU.

Comment 61

[Cameron Kaiser [:spectre]]

FTR, we're shipping a bandaid in TenFourFox that doesn't optimize the image further if the first pixel is black (either it's all black, in which case it trips this bug, or it's not all black and couldn't be optimized anyway). This doesn't seem to regress anything than not optimizing such images, but it might not be a generally acceptable solution, of course.

Comment 63

[Yun Guo]

(In reply to Cameron Kaiser [:spectre] from comment #61)
> FTR, we're shipping a bandaid in TenFourFox that doesn't optimize the image
> further if the first pixel is black (either it's all black, in which case it
> trips this bug, or it's not all black and couldn't be optimized anyway).
> This doesn't seem to regress anything than not optimizing such images, but
> it might not be a generally acceptable solution, of course.

Cameron, could you share your changes for this issue? We meet the same issue, after applying the patch, high CPU used for some pages painting.

Comment 64

[Cameron Kaiser [:spectre]]

Sure. Please note that we're big endian, so you might need to tweak the bitmask; I haven't tested this on anything else other than PowerPC. It looks like this:

diff --git a/image/src/imgFrame.cpp b/image/src/imgFrame.cpp
--- a/image/src/imgFrame.cpp
+++ b/image/src/imgFrame.cpp
@@ -244,16 +244,24 @@ nsresult imgFrame::Optimize()
   /* Figure out if the entire image is a constant color */
 
   // this should always be true
   if (mImageSurface->Stride() == mSize.width * 4) {
     uint32_t *imgData = (uint32_t*) mImageSurface->Data();
     uint32_t firstPixel = * (uint32_t*) imgData;
     uint32_t pixelCount = mSize.width * mSize.height + 1;
 
+// TenFourFox kludge:
+// If the first pixel is black, then don't optimize (either it is all
+// black, and will hit issue 132, or it isn't, and we don't optimize
+// anyway).
+   if (!(firstPixel & 0x00ffffff)) { // assume ARGB or XRGB
+       return NS_OK;
+   }
+
     while (--pixelCount && *imgData++ == firstPixel)
       ;
 
     if (pixelCount == 0) {
       // all pixels were the same
       if (mFormat == gfxASurface::ImageFormatARGB32 ||
           mFormat == gfxASurface::ImageFormatRGB24)
       {

Comment 65

[Yun Guo]

Cameron, thanks very much for the help. I'll try to update and do some tests with it. :)

Comment 66

[u279076]

Is this bug still relevant with current Firefox builds?

Comment 67

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 7 duplicates.
:bhood, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 68

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Comment 69

[BugBot [:suhaib / :marco/ :calixte]]

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit auto_nag documentation.