Closed
Bug 189052
Opened 22 years ago
Closed 22 years ago
remove function pointers from gifdecoder/gif2.cpp
Categories
(Core :: Graphics: ImageLib, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla1.4alpha
People
(Reporter: Biesinger, Assigned: Biesinger)
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
paper
:
review+
tor
:
superreview+
|
Details | Diff | Splinter Review |
currently, GIF2.cpp is init'ed with a lot of function pointers, which need ram.
however, always the same set of pointers is used. it would make sense to
directly call the functions.
This will save some memory; perfomance effect will be neglegible though, I suspect.
Assignee | ||
Updated•22 years ago
|
Priority: -- → P2
Target Milestone: --- → mozilla1.4alpha
Assignee | ||
Comment 1•22 years ago
|
||
seems to still pass the tests at http://www.animecity.nu/mozilla/IMGtest/
Assignee | ||
Comment 2•22 years ago
|
||
Comment on attachment 111979 [details] [diff] [review]
patch
note, I have two unrelated changes in here:
firstly, removal of two commented out lines (the hist_count lines in GIF2.h)
secondly, an if (!created) check in nsGIFDecoder2.cpp, which also got rid of a
warning about uninitialized variable.
Also, I removed some of the callbacks, because they were never called.
Attachment #111979 -
Flags: review?(paper)
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Comment 3•22 years ago
|
||
Comment on attachment 111979 [details] [diff] [review]
patch
looks good, but I'd like to compile it. Could you unbitrot and attach a new
one?
One thing to note, which does not have to be done for this bug, is that the
parameters aXOffset, aLength, and aDrawMode from HaveDecodedRow do not appear
to be used at all.
Attachment #111979 -
Flags: review?(paper) → review-
Assignee | ||
Comment 4•22 years ago
|
||
ok, unbitrotted and the three paramters removed.
Attachment #111979 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #112274 -
Flags: review?(paper)
Comment 5•22 years ago
|
||
Comment on attachment 112274 [details] [diff] [review]
patch v1.1
Sorry for the delay.. busy day! Compiled and running.
In nsGIFDecoder2::Init
+ if (!created)
That's a tab!
The rest looks good. No need to attach a new patch (at least for me)
Attachment #112274 -
Flags: review?(paper) → review+
Comment on attachment 112274 [details] [diff] [review]
patch v1.1
Reading through the patch, the thing that jumps out at me is that the
callback functions are now stuck in the global namespace. One possible
way of avoid that would be to make them static methods of nsGIFDecoder.
If it is decided to leave them as globals, they should have some
consistent naming to prevent collisions and to make identification
in stack traces easy.
Assignee | ||
Comment 7•22 years ago
|
||
ok, make the function static members of nsGIFDecoder2.
also, I removed the aTransparencyChromaKey parameter of BeginImageFrame as it
was not used.
Attachment #112274 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #112746 -
Flags: review?(paper)
Comment 8•22 years ago
|
||
Comment on attachment 112746 [details] [diff] [review]
patch v2
Please do a replace of all tabs with 2-spaces before commiting. There's a lot
more in this patch compared to the last.
r+ with that
Attachment #112746 -
Flags: review?(paper) → review+
Comment on attachment 112746 [details] [diff] [review]
patch v2
Do the untabification requested by paper and sr=tor.
Attachment #112746 -
Flags: superreview+
Assignee | ||
Comment 10•22 years ago
|
||
Checking in GIF2.cpp;
/cvsroot/mozilla/modules/libpr0n/decoders/gif/GIF2.cpp,v <-- GIF2.cpp
new revision: 1.38; previous revision: 1.37
done
Checking in GIF2.h;
/cvsroot/mozilla/modules/libpr0n/decoders/gif/GIF2.h,v <-- GIF2.h
new revision: 1.15; previous revision: 1.14
done
Checking in nsGIFDecoder2.cpp;
/cvsroot/mozilla/modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp,v <--
nsGIFDecoder2.cpp
new revision: 1.46; previous revision: 1.45
done
Checking in nsGIFDecoder2.h;
/cvsroot/mozilla/modules/libpr0n/decoders/gif/nsGIFDecoder2.h,v <--
nsGIFDecoder2.h
new revision: 1.22; previous revision: 1.21
done
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•