Closed Bug 168048 Opened 22 years ago Closed 22 years ago

combine commonly used image decoders into imglib2

Categories

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

x86
Windows 2000
defect

Tracking

()

RESOLVED FIXED
mozilla1.2beta

People

(Reporter: alecf, Assigned: alecf)

References

Details

Attachments

(1 file, 3 obsolete files)

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.
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.
Blocks: 163737
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: fix in hand
Target Milestone: --- → mozilla1.2beta
Attached patch combine imglib2, gif, jpeg, png, and bmp (obsolete) (deleted) — Splinter Review
..plus mac project changes of course...
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.
ok, here's the updated patch... now to go off and do the mac and packaging work.
Attachment #98767 - Attachment is obsolete: true
Attached patch combine decoders, plus packaging (obsolete) (deleted) — Splinter Review
attaching this here so I can apply it on my mac
Attachment #98774 - Attachment is obsolete: true
+ deleteThisFile("Components", "libxbm.so"); hm, iirc these files are called libimgxbm.so, libimgbmp.so etc. are you sure the files have this name?
oops, good spot. updated in my tree.
Ok, here's the final patch. Gotta love that mac. Reviews?
Attachment #98890 - Attachment is obsolete: true
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 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?
If the ijpeg directory is causing difficulties I'd say remove it.
The patch is missing the files in the new decoders/build directory - could you attach a tarball of those?
tor, I'm pretty sure that they are already checked in
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)
Attachment #98963 - Flags: superreview+
Comment on attachment 98963 [details] [diff] [review] combine decoders, plus packaging, all platforms r=pavlov
Attachment #98963 - Flags: review+
ok, fix is in. we probably need a new bug about libmng and libicon
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
It looks like there are some Mac problems. (Also, are some of those old project files unneeded? Should they be cvs-removed?)
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)
shouldn't the ns*Decoder.cpp / ns*Factory.cpp files be cvs removed now?
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?
>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.
Whiteboard: fix in hand
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: