remove function pointers from gifdecoder/gif2.cpp

RESOLVED FIXED in mozilla1.4alpha

Status

()

Core
ImageLib
P2
normal
RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: Biesinger, Assigned: Biesinger)

Tracking

Trunk
mozilla1.4alpha
x86
Windows 98
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
Priority: -- → P2
Target Milestone: --- → mozilla1.4alpha
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)
Status: NEW → ASSIGNED

Comment 3

15 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-
Created attachment 112274 [details] [diff] [review]
patch v1.1

ok, unbitrotted and the three paramters removed.
Attachment #111979 - Attachment is obsolete: true
Attachment #112274 - Flags: review?(paper)

Comment 5

15 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 6

15 years ago
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.
Created attachment 112746 [details] [diff] [review]
patch v2

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
Attachment #112746 - Flags: review?(paper)

Comment 8

15 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 9

15 years ago
Comment on attachment 112746 [details] [diff] [review]
patch v2

Do the untabification requested by paper and sr=tor.
Attachment #112746 - Flags: superreview+
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
Last Resolved: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.