Closed
Bug 366465
Opened 18 years ago
Closed 17 years ago
GIF Decoding in Cairo can skip all row buffers
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
VERIFIED
FIXED
People
(Reporter: alfredkayser, Assigned: alfredkayser)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
pavlov
:
review-
|
Details | Diff | Splinter Review |
(deleted),
patch
|
pavlov
:
review+
tor
:
superreview+
pavlov
:
approval1.9+
|
Details | Diff | Splinter Review |
Currently the GIF decoder first decodes the lzw data into a rowbuffer with the color index as a byte value, and then it is colormapped to mRGBLine in Cairo format, and then that line is copied to the Cairo image buffer.
However, the 'do_lzw' function can also directly colormap the pixel into the Cairo image data, skipping the two row buffers completely.
Note, this is similar to what BMP/ICO decoders also do (see bug 360000 for an example of it is fixed over there).
Note 2: It is probably best to do bug 196295 first, so that the decoder bits are all in one file making it easier to rearrange.
Note 3: When this and bug 196295 are done, the only dynamically allocated data in the gif decoder is the 'local_colormap' which is for animated frames only.
(besides all the image frames themselves...)
Assignee | ||
Comment 1•18 years ago
|
||
Another item to look for in optimizing GIF decoding:
Found on vector64.com:
http://www.vector64.com/todo.html
# When writing out GIF image data in nsGIFDecoder2.cpp, and in some types of nsPNGDecoder.cpp, we currently write out three bytes at a time. We actually write out a doubleword but the last character gets overwritten the next time around. For Win64, we have __int64 so we could theoretically store the data in an integer and then write out the 8-byte integer instead of two four-byte integers reducing the number of writes by a factor of two.
# The colormaps used in GIF are three characters and perhaps we could improve GIF rendering performance if we made them four so that they would be aligned.
Note, this can also apply to palette based decoding of BMP, ICO and PNG images.
Assignee | ||
Comment 2•17 years ago
|
||
This patch makes the image decoder directly store the pixels into the Cairo image buffer. Previously each row was first decoded to rowbuf (byte per pixel), and then into a RGB row buffer (mRGBLine), and then copied into the image buffer.
By converting the colormaps from the GIF data directly (after optional CMS transformation) into a Cairo compatible colormap, we can directly decode from the LZW decoding into the image data.
This saves two mallocs (width bytes and width*4 bytes), as well as two copies.
Also because the pixel conversion is done in the colormap instead of for each pixel this also saves CPU cycles.
Attachment #275396 -
Flags: review?(pavlov)
Assignee | ||
Comment 3•17 years ago
|
||
Attachment #275396 -
Attachment is obsolete: true
Attachment #276010 -
Flags: review?(pavlov)
Attachment #275396 -
Flags: review?(pavlov)
Comment 4•17 years ago
|
||
Comment on attachment 276010 [details] [diff] [review]
V2: Updated to current trunk
can you please change all the places you're using 'foo* bar' to 'foo *bar'?
Comment 5•17 years ago
|
||
Comment on attachment 276010 [details] [diff] [review]
V2: Updated to current trunk
waiting on new patch then r+
Attachment #276010 -
Flags: review?(pavlov) → review-
(there might be a few small conflicts in this with my work in bug 391583 but they should be trivial to merge)
Assignee | ||
Comment 7•17 years ago
|
||
Attachment #277430 -
Flags: review?(pavlov)
Updated•17 years ago
|
Attachment #277430 -
Flags: review?(pavlov)
Attachment #277430 -
Flags: review+
Attachment #277430 -
Flags: approval1.9+
Assignee | ||
Updated•17 years ago
|
Attachment #277430 -
Flags: superreview?(tor)
Attachment #277430 -
Flags: superreview?(tor) → superreview+
Assignee | ||
Updated•17 years ago
|
Keywords: checkin-needed
Comment 8•17 years ago
|
||
modules/libpr0n/decoders/gif/GIF2.h 1.25
modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp 1.82
modules/libpr0n/decoders/gif/nsGIFDecoder2.h 1.30
Assignee | ||
Comment 9•17 years ago
|
||
Green tree's, and current nightly (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007082618 Minefield/3.0a8pre) working fine.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•