189052 - remove function pointers from gifdecoder/gif2.cpp

Status: RESOLVED FIXED

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

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

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.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

seems to still pass the tests at http://www.animecity.nu/mozilla/IMGtest/

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

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.

Comment 3

[ArronM (:paper)]

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.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v1.1

ok, unbitrotted and the three paramters removed.

Comment 5

[ArronM (:paper)]

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)

Comment 6

[tor]

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.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v2

ok, make the function static members of nsGIFDecoder2.

also, I removed the aTransparencyChromaKey parameter of BeginImageFrame as it
was not used.

Comment 8

[ArronM (:paper)]

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

Comment 9

[tor]

Comment on attachment 112746 [details] [diff] [review]
patch v2

Do the untabification requested by paper and sr=tor.

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

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