The default bug view has changed. See this FAQ.

combine commonly used image decoders into imglib2

RESOLVED FIXED in mozilla1.2beta

Status

()

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

People

(Reporter: Alec Flett, Assigned: Alec Flett)

Tracking

Trunk
mozilla1.2beta
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

15 years ago
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.
(Assignee)

Comment 1

15 years ago
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
(Assignee)

Comment 2

15 years ago
Created attachment 98767 [details] [diff] [review]
combine imglib2, gif, jpeg, png, and bmp

..plus mac project changes of course...
(Assignee)

Comment 3

15 years ago
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.
(Assignee)

Comment 4

15 years ago
Created attachment 98774 [details] [diff] [review]
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.
Attachment #98767 - Attachment is obsolete: true
(Assignee)

Comment 5

15 years ago
Created attachment 98890 [details] [diff] [review]
combine decoders, plus packaging

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?
(Assignee)

Comment 7

15 years ago
oops, good spot. updated in my tree.
(Assignee)

Comment 8

15 years ago
Created attachment 98963 [details] [diff] [review]
combine decoders, plus packaging, all platforms

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

Reviews?
Attachment #98890 - Attachment is obsolete: true
(Assignee)

Comment 9

15 years ago
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?

Comment 11

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

Comment 12

15 years ago
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
(Assignee)

Comment 14

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

Thanks

Comment 15

15 years ago
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 16

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

r=pavlov
Attachment #98963 - Flags: review+
(Assignee)

Comment 17

15 years ago
ok, fix is in. we probably need a new bug about libmng and libicon
Status: ASSIGNED → RESOLVED
Last Resolved: 15 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?)
(Assignee)

Comment 19

15 years ago
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?

Comment 21

15 years ago
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.