168048 - combine commonly used image decoders into imglib2

Status: RESOLVED FIXED

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

Description

[Alec Flett]

right now we have a bunch of image decoders that each reside in their own DLL.
This is all well and good for high granularity of features, but its pretty
rediculous practically speaking. 99% of consumers (including mozilla the browser
and most embeddors) want an image library, and decoders included for a common
image formats. 

In a windows build, we currently have this:
28      imgbmp.dll
28      imggif.dll
28      imgicon.dll
68      imgjpeg.dll
44      imglib2.dll
176     imgmng.dll
64      imgpng.dll
24      imgppm.dll
24      imgxbm.dll
484     total

these little 28k dlls are pretty stupid.

I've been combining some of them them into one DLL and gotten some significant
space savings, and probably a memory and performance improvement as everything
can be loaded into the same segment in memory, and so forth.

Comment 1

[Alec Flett]

ok, so my initial cut at this combines gif, jpeg, bmp, and png. I got a space
savings of 88k and 4 less DLLs. 

some of the other small DLLs (like ppm, xbm) are so small (like 4-8k) I'm going
to see if the incremental cost of including the DLLs is small enough to justify
just rolling them into the main DLL. If the incremental cost is large (like more
than 12k) then I'll probably roll them all (including MNG, and icon) into a
secondary image dll.

Thus far it looks like we saved an average of 22k per additional DLL, and the
old dlls were around 28k each, so I'm guessing that this will be more like 6k,
and it will be worth it.

Comment 2

[Alec Flett]

Attachment: combine imglib2, gif, jpeg, png, and bmp

..plus mac project changes of course...

Comment 3

[Alec Flett]

ok, so for example I combined xbm into the main library, and I got:
- imglib2.dll increased by 4k
- got a total savings of 20k, over the previous one.

(and later..)

ok this is awesome. ppm is so small that it didn't even make imglib2 grow once I
had added xbm.. so I got
- imglib2.dll: no change
- got a total savings of 24k, over the previous one (with xbm)

that means combining stuff down to 3 dlls (imglib2, imgicon, and imgmng) is down
to 352k, down from 484k, a total savings of 132k, with 7 fewer DLLs than we had
before!

new patch forthcoming. I'm not sure if I should try going further and add
imgicon, or leave imgicon and mng to be combined into their own DLL.

Comment 4

[Alec Flett]

Attachment: combine imglib2, gif, jpeg, png, bmp, xbm, and ppm

ok, here's the updated patch... now to go off and do the mac and packaging
work.

Comment 5

[Alec Flett]

Attachment: combine decoders, plus packaging

attaching this here so I can apply it on my mac

Comment 6

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

+  deleteThisFile("Components", "libxbm.so");

hm, iirc these files are called libimgxbm.so, libimgbmp.so etc. are you sure the
files have this name?

Comment 7

[Alec Flett]

oops, good spot. updated in my tree.

Comment 8

[Alec Flett]

Attachment: combine decoders, plus packaging, all platforms

Ok, here's the final patch. Gotta love that mac.

Reviews?

Comment 9

[Alec Flett]

oh, and the reason for all the excess search paths in libimg2.xml is that there
is this directory libpr0n/decoders/ijpeg (note the extra "i") which mac searches
first to find nsJPEGDecoder.h. So instead of adding :: to the search path, I add
each one individually, which is probably the right thing anyway.

Comment 10

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

Comment on attachment 98963 [details] [diff] [review]
combine decoders, plus packaging, all platforms

I know, you didn't touch this in this patch, but anyway:
in a REQUIRES line:
		  gfx2 \

haven't you removed gfx2 recently? ie., should these lines be removed?

Comment 11

[tor]

If the ijpeg directory is causing difficulties I'd say remove it.

Comment 12

[tor]

The patch is missing the files in the new decoders/build directory - could
you attach a tarball of those?

Comment 13

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

tor, I'm pretty sure that they are already checked in

Comment 14

[Alec Flett]

tor: see http://lxr.mozilla.org/seamonkey/source/modules/libpr0n/build/

Thanks

Comment 15

[tor]

Comment on attachment 98963 [details] [diff] [review]
combine decoders, plus packaging, all platforms

There's nothing like mac build changes for making a patch seem excessively
large, is there?

sr=tor

(btw, a bug should be filed about getting rid of the final gfx2 traces)

Comment 16

[Stuart Parmenter]

Comment on attachment 98963 [details] [diff] [review]
combine decoders, plus packaging, all platforms

r=pavlov

Comment 17

[Alec Flett]

ok, fix is in. we probably need a new bug about libmng and libicon

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It looks like there are some Mac problems.

(Also, are some of those old project files unneeded?  Should they be cvs-removed?)

Comment 19

[Alec Flett]

I just sent mail to the hook about the mac project problem. I think debug builds
will work fine, but release builds are horked. Not sure though. I need a mac
guru to tell me what I did wrong.

(and as for those old .xml files, yeah I'll cvs remove them, but after we fix
the current problem, we might need those .xml files as reference)

Comment 20

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

shouldn't the ns*Decoder.cpp / ns*Factory.cpp files be cvs removed now?

Comment 21

[marni894]

Isn't favicon.ico decoded more often than bmp-images thus implying that the icon
decoder should be with the gif, jpeg and png decoders?

Comment 22

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

>Isn't favicon.ico decoded more often than bmp-images

The "icon" decoder is not the one used for .ico images.
The .ico decoder is part of the .bmp decoder and therefore part of the "imglib2"
library together with the jpeg, png and gif decoders.