Bug 16169 - GIF ImageDecoder hasAlpha() return value incorrect
Summary: GIF ImageDecoder hasAlpha() return value incorrect
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Cairo, Gtk
Depends on:
Blocks:
 
Reported: 2007-11-28 03:57 PST by Alp Toker
Modified: 2007-11-28 11:57 PST (History)
1 user (show)

See Also:


Attachments
Test case (5.82 KB, image/gif)
2007-11-28 04:00 PST, Alp Toker
no flags Details
patch v1 (3.85 KB, patch)
2007-11-28 11:26 PST, Peter Kasting
alp: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alp Toker 2007-11-28 03:57:29 PST
Commit r28068 to fix #15974 caused a change in the return value of hasAlpha() for GIF animation frames. Frames that have transparency used to return true but now return false.

Looks like "replace" frames return true as expected but "combine" frames always return false now where one would expect true, which should probably remain an implementation detail internal to the GIF decoder rather than affecting hasAlpha().

Test case attached.
Comment 1 Alp Toker 2007-11-28 04:00:10 PST
Created attachment 17572 [details]
Test case

Before the changes were made:

frame 1 hasAlpha: 1
frame 2 hasAlpha: 1
frame 3 hasAlpha: 1
frame 4 hasAlpha: 1
frame 5 hasAlpha: 1
frame 6 hasAlpha: 1
frame 7 hasAlpha: 1
frame 8 hasAlpha: 1
frame 9 hasAlpha: 1
frame 10 hasAlpha: 1
frame 11 hasAlpha: 1
frame 12 hasAlpha: 1

After the changes:

frame 1 hasAlpha: 1
frame 2 hasAlpha: 0
frame 3 hasAlpha: 0
frame 4 hasAlpha: 0
frame 5 hasAlpha: 0
frame 6 hasAlpha: 1
frame 7 hasAlpha: 0
frame 8 hasAlpha: 0
frame 9 hasAlpha: 0
frame 10 hasAlpha: 0
frame 11 hasAlpha: 0
frame 12 hasAlpha: 0

Frames 1 and 6 are replace/background frames -- the rest are combine frames.
Comment 2 Alp Toker 2007-11-28 04:48:45 PST
The workaround committed in r28109 should be removed when this issue is resolved.
Comment 3 Peter Kasting 2007-11-28 11:26:48 PST
Created attachment 17577 [details]
patch v1

Attempted fix (untested).

This was just oversight -- I meant to copy the alpha state from the previous buffer at the same time I copied its bits.

The FIXME in ImageSourceCairo is based on IRC discussion with alp.
Comment 4 Alp Toker 2007-11-28 11:53:59 PST
Comment on attachment 17577 [details]
patch v1

r=me
Comment 5 Alp Toker 2007-11-28 11:57:48 PST
Landed in r28114.