Closed Bug 22607 Opened 25 years ago Closed 22 years ago

GIF decoder doesn't support all frame replacement options

Categories

(Core :: Graphics: ImageLib, defect, P2)

Product:
Core
Component:
Graphics: ImageLib
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.0.1

People

(Reporter: 5thShadow, Assigned: saari)

References

(
URL
)

Details

(Keywords: testcase, Whiteboard: [imglib])

Attachments

(3 files)

Animated gif showing bug in frame dispose code in Nav/Moz
55.00 KB, image/gif
Details
Test Case
55.40 KB, image/gif
Details
Test Case
543 bytes, image/gif
Details 
Some animated GIF frames not erased porperly, in M12 Build running on Win98
SE.  The GIF on this page (page at http://www.ptbo.igs.net/~fivekays/GameCenter-
onl/, actual image is at http://www.ptbo.igs.net/~fivekays/GameCenter-
onl/images/LogoAnimSideBar.gif) doess not load properly.  Load this page in
Netscape4.x or IE5, Animation displays fine.  In M12, as the "G" and "C" come
together parts of them appear to be left behind, not erased from one frame to
the next as should occur.  This also occurs when the buttons in the
image "grow".
Hmm...will check to see if this is a duplicate of 8409.
On a surface level, I believe this is an invalid bug, for the reasons explained
by pnunn and myself in 8409.

However, when I replace the control blocks that specify "Use Previous Image" with
"Remove by Background", the image contains a lot of white gunk in the background.

Thus, deferring to pnunn for an investigation to point out what I've missed.
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → DUPLICATE
This is a duplicate of the gif decoder not
supporting all frame replacement options.
The browser has never supported all gif frame
replacement options....but it is planned.

That said, the image supplied in the page given will work
as I think the designer intended if:
1. The 'remove by previous image' is replaced
   with 'remove by background'.
2. Background color in the header is set to black.
3. No transparency is designated. In the example I
  saw it did not look like transparency was needed.

------------
I just noticed someone closed #8409, which is ok.
But this issue is on my personal to-do list.

*** This bug has been marked as a duplicate of 8409 ***
Status: RESOLVED → REOPENED
Summary: Animated GIF frames not erased properly in Win98 → GIF decoder doesn't support all frame replacement options
Resolution: DUPLICATE → ---
Pam, I thought 8409 resulted from the image not following the GIF standards?

Anyway, I don't see a bug that covers the issue of the gif decoder not
supporting all frame replacement options, and would like to re-open this bug so
that there's a tracking bug for it.

If you'd rather not, just close this bug out.
Severity: normal → enhancement
Priority: P3 → P2
Target Milestone: M17
This piece is quoted from 8409, elig@netscape.com said:

>Okay. I've done my due diligence, per the GIF 89a spec, "Restore to Previous"
>tells the user agent that:

>    "The decoder is required to restore the area overwritten by the graphic
>	 with what was there prior to rendering the graphic."

>Thus, we are displaying the image pursuant to the specification, and IE is
>deviating.

Um, no, that actaully says that IE has it right and Netscape is (and always has
deviated from the spec)

I'm going to describe why I believe this to be the case. If you want a test case 
I attached an image (92K) that shows the problem clearly...

Imagine a scene with a beach ball, bouncing on a beach.

We have one large background frame, showing the beach
and one small frame, showing the ball, that we will animate by moving it around
the background.

The beach is set to no dispose, the ball is set to restore to previous.

Here's the sequence, for a couple of bounces:

1. Draw the beach frame
2. Don't dispose it
3. Draw the ball at 10,10
4. Dispose the ball, we now have frame 1 back
5. Draw the ball at 10,100
6. Dispose the ball, we now have fram 4 back, which is the same as frame 1,
	i.e. just the beach.

And repeat....

Navigator does this:

1. Draw the beach frame
2. Don't dispose it
3. Draw the ball at 10,10
4. Doesn't dipose the ball frame, though it should
5. Draw the ball at 10,100, we now have a trailback
6. Doesn't dispose the ball frame :-(

And repeat... trailbacks result


I'l re-iterate the spec:
>    "The decoder is required to restore the area overwritten by the graphic
>	 with what was there prior to rendering the graphic."

So when the ball is disposed the beach should come back. It shouldn't
leave the ball there, nor should it restore to any previous frame that might
have had the ball in a different place. Why? Because the 1st ball frame should be
gone before the second is drawn, and so "what was there prior to rendering
the graphic" was the background frame. (in my example the beach)

I'm afraid IE (4.5 on the Mac at least) has this right and Navigator has always 
had it wrong. Some GIF editing tools handle it others don't. Quicktime has it 
wrong too!

Bear in mind the erase to previous version of the image is 92K whereas the 
version I have to use to get it to work in Nav/Moz is 284K. Fix this and you 
could make a *LOT* of modem users lives better...
OS: Windows NT → All
Hardware: PC → All
I love it when people point out how little I really know about this stuff. ;)

Bumping Severity to 'Normal', per lordpixel@mac.com's comments.
Severity: enhancement → normal
Changing to M20
Target Milestone: M17 → M20
Been looking at the source for this... it needs to be handled in Gif.cpp 
(surprisingly enough :-)

Essentially there are 4 types of update thus:

/* "Disposal" method indicates how the image should be handled in the
   framebuffer before the subsequent image is displayed. */
typedef enum 
{
    DISPOSE_NOT_SPECIFIED      = 0,
    DISPOSE_KEEP               = 1, /* Leave it in the framebuffer */
    DISPOSE_OVERWRITE_BGCOLOR  = 2, /* Overwrite with background color */
    DISPOSE_OVERWRITE_PREVIOUS = 4  /* Save-under */
} gdispose;

DISPOSE_OVERWRITE_BGCOLOR is explicitly implemented in the method

gif_clear_screen

which implicitly implements DISPOSE_KEEP by doing absolutely nothing as you might 
expect, and DISPOSE_NOT_SPECIFIED by doing nothing also, which is within spec and 
should be left alone for compatibility reasons if nothing else.

    DISPOSE_OVERWRITE_PREVIOUS is the mode that's not implemented. I've been 
looking at how to achieve this. Funamentally one needs to restore the pixels to 
the state they were in in the previous frame.

I depends if the libimg architecture would give you access to the previous frame, 
but I'm not really qualified to say - I followed the code through a few structs 
and files but it gets very hairy in the generic image handling stuff and I can't 
tell.

This comment in gif.cpp:

            /* Overlaying interlaced, transparent GIFs over
                   existing image data using the Haeberli display hack
                   requires saving the underlying image in order to
                   avoid jaggies at the transparency edges.  We are
                   unprepared to deal with that, so don't display such
                   images progressively */

Leads me to believe this may be more difficult than we'd like. It depends whether 
the perceived difficulty of rendering successive frames in an animated Gif 
progressively was because of lack of access to te previous frame, or some other 
complication not directly alluded to.

Obviously some generic mechanism for having access to the previous frame in an 
animation is the best solution, as the above comment would seem to indicate that 
it would allow other optimisations to be employed.

Also, one might wish to add this functionality to the generic framework, and 
simply have the gif decoder use a callback to signal that this should be done, so 
other image formats could take advantage of the functionality.
One can animate PNG graphics no?


*** Bug 45123 has been marked as a duplicate of this bug. ***
Target Milestone: M20 → Future
Some comments I got from Tim Rowley tor@cs.brown.edu (the guy who implemented MNG 
for Mozilla)

Here's a few quick thoughts about this problem.  As I understand
it, the gif block mode that's experiencing problems is one in
which a subimage is laid down, but is supposed to be restored to
the original data at the end of the frame.  The problem mozilla
has with this is twofold - 1) the gif decoder only operates a
line at a time and 2) libimg stores the images scaled to the
target resolution.  The image decoder tells libimg about image
data via a method that reduces to il_emit_row(), which if you
look at it is responsible for doing scaling, creating the alpha
mask, etc.  To save a block to restore it later, you would need
to add accessors so the gif decoder can pull data out and to
place it back in without rescaling again.  Not too hard, but
these are non-trivial changes.

The MNG decoder gets around this by keeping around a fullscale
version of the image.

-tor



*** Bug 2805 has been marked as a duplicate of this bug. ***
Blocks: 61481
QA Contact: elig → tpreston
Chris Saari,

Can you look at/prioritise this as a part of the new libimg2 gif decoder work
please?

Saari and Pavlov are doing GIF transitions this week / next week for libPr0n.

I think this should be assgined to one of them...
Yeah, all the GIF frame replacement options can now be properly supported in the
new design.
Assignee: pnunn → saari
Status: REOPENED → NEW
Target Milestone: Future → mozilla1.0
->moz0.9
Target Milestone: mozilla1.0 → mozilla0.9
not necessary for 0.9
Target Milestone: mozilla0.9 → mozilla0.9.1
libpr0n has landed and this is still not fixed in 2001041104 bits. 
(see attachment for example)
Yeah, I'm still working on the GIF decoder, expect to see more progress on this
shortly
Status: NEW → ASSIGNED
changing status to [imglib]
Whiteboard: [imglib]
Target Milestone: mozilla0.9.1 → mozilla1.0
*** Bug 85177 has been marked as a duplicate of this bug. ***
Saari,

The frame replacement DISPOSE_OVERWRITE_BGCOLOR should only overwrite the
portion of the frame. Not the whole image. Some testcases would be fixed with
this change.

Se attachment 37874 [details]

(Sorry if I'm saying something known or obvious)
You are exactly right, and I actually had a fix in my tree (which regressed
other things, so it wasn't soup yet). I'll pull this back to 0.9.3
Target Milestone: mozilla1.0 → mozilla0.9.3
I originally filed this under Bug 77914, but was told that this bug (22607) was
the more appropriate place to file this.  I am using build 2001062218 on a
Windows 2000 machine, and I see some problems with certain animated GIFs.
 For example, the image located at
"http://cgi.resourceindex.com/ads/images/wwm/wwm7.gif" has problems with the
black text when it first displays, in that some of the text has white blotches
in it.  After the first animation loop, the blotches disappear and the image
appears normal.  Other browsers do not have this problem with this image.  I
believe that this report should go here.  If not, please let me know, and I will
file a new bug report for this.  Thanks.
I have to admit I'm not sure this apparant bug fits under this catagory, but
it's certainly related.  If you understand this, please feel free to take care
of it! :)  with Build ID: 2001071803 on Win2000 I'm finding that some gifs just
plan don't load.  For example on the following site
http://www.crosswinds.net/~bobgray66/index.html there are two gifs at the top of
the page which are copies of each other, with left-right alignment switched. 
The left most one doesn't load at all.  This page has browser detection but
doesn't know the difference between communicator NET6 and Mozilla.  When viewed
in IE, communicator, or Net6 the gif appears.  In past builds of Mozilla the gif
appeared but had funky problems (a black frame appeared when the gif repeated.)
this is why I wondered if it might be related to this bug.

Thanks
Target Milestone: mozilla0.9.3 → mozilla0.9.4
*** Bug 86342 has been marked as a duplicate of this bug. ***
Keywords: 4xp
*** Bug 94139 has been marked as a duplicate of this bug. ***
i think bug 84080 is a dup, but it seems slightly different from the others
Blocks: 84080
I'm linking bug 84080 to here.  I'm not sure that the bugs are similar or
duplicated, but I belive they are related (fixing this one will fix everything
else).

Does Mozilla "flatten" or unoptimizes each frame before it shows the animation?
No longer blocks: 84080
Blocks: 84080
->0.9.5
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Is bug 87823 a dupe of this?
*** Bug 96775 has been marked as a duplicate of this bug. ***
*** Bug 91435 has been marked as a duplicate of this bug. ***
*** Bug 101681 has been marked as a duplicate of this bug. ***
*** Bug 87823 has been marked as a duplicate of this bug. ***
saari, What's the status on this bug? three months ago you said you had a fix in
your tree. We are quickly reaching mostfreq status, and it would be good to fix
at least a part of this bug so we can figure out about all the others
mentioned/duped here.
what I had was a step in the right direction, but had other side effects and
needed more work, which hasn't been my priority and thus hasn't gotten done yet
Target Milestone: mozilla0.9.5 → mozilla0.9.7
*** Bug 105180 has been marked as a duplicate of this bug. ***
*** Bug 108562 has been marked as a duplicate of this bug. ***
*** Bug 98013 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla0.9.7 → mozilla1.0.1
Nominating 1.0 as there are many duplicates.
Keywords: mozilla1.0
Blocks: 119597
Blocks: advocacybugs
Keywords: mozilla0.9.9
saari: what happened to the fix? bitrotten till it can't be reconized anymore? ;-)
fix wasn't right if I remember correctly
I have a patch in Bug 85595 which adds support for the Previous Image frame
replacement/disposal method. It's awaiting testers, reviewers, etc.  (consider
this a request to try it :P)
*** Bug 137870 has been marked as a duplicate of this bug. ***
*** Bug 137926 has been marked as a duplicate of this bug. ***
*** Bug 139324 has been marked as a duplicate of this bug. ***
*** Bug 142392 has been marked as a duplicate of this bug. ***
*** Bug 147648 has been marked as a duplicate of this bug. ***
*** Bug 131356 has been marked as a duplicate of this bug. ***
*** Bug 131660 has been marked as a duplicate of this bug. ***
Keywords: mozilla0.9.9 → testcase
the fix for bug 85595 fixed this.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago → 22 years ago
Depends on: 85595
Resolution: --- → FIXED
Verified fixed with my test images.

This is the first bug I ever commented on in Mozilla. Yay for progress!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: