Bugzilla – Full Text Bug Listing
| Summary: | Check that the ico file contains all the expected images | ||
|---|---|---|---|
| Product: | [Eclipse Project] PDE | Reporter: | Pascal Rapicault <pascal> |
| Component: | Build | Assignee: | pde-build-inbox <pde-build-inbox> |
| Status: | RESOLVED WONTFIX | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | aniefer, ed.burnette, Erik.Vanherck, evans, janek.lb, litrik_de_roy, n.a.edgar |
| Version: | 3.2 | Keywords: | helpwanted |
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | |||
|
Description
Pascal Rapicault
Given the three valid ICO files attached by Pascal in bug 102028 comment 20, it is unclear to me what depth we should be checking for as each of the ICO files contains a different subset. It seems that anything goes...ie. depth 4, depth8 and depth 32 all seem to work. It also appears that there is each ico file contains a different number of images, eg. 7, 9, 6 What exact guidelines are we enforcing/validating here? Pascal is silent, so there is no right answer and anything goes. The most we can do for the ICO is check the number of images and flag a warning if the number of images is < 6 Since we are itching to use the new form validation in editors, we also validate the paths and size and depth of images, but not the ICO. pretty nice. Looks like Vista added a 256x256 option but if you're checking for <6 that should be ok, right? correct. the only check we do is the number of images (with 6 being the minimum magic number) for an ICO file to avoid situations as in bug 102028. As for depth of the image, it seems as though anything goes, so PDE validation is staying out of it for now. I'm taking the liberty of reopening this bug because I now I have things to say :-). Not everything fits to brand the exe. 7 images should be present in the ico file to completly brand eclipse.exe. Their size and resolution should be: 48 x 48 (256 colors) 32 x 32 (256 colors) 24 x 24 (256 colors) 16 x 16 (256 colors) 48 x 48 (16.8mil colors) 32 x 32 (16.8mil colors) 16 x 16 (16.8mil colors) BTW, this is great info to put somewhere accessible (like in the editor itself). Otherwise the actual requirements here are a bit opaque. The requirements are captured in the initial comment: we want the product editor to give a warning when the ico file does not contain the proper images for a succesfull branding. First, there is no reason to require anything but a valid Windows .ico file. I have been making these for years and usually just supply 16x16 and 32x32. This works, except perhaps for 16-color machines, which few people have. If a 48x48 image is needed, Windows expands the 32x32 one. The XP icons are for people who have graphics designers that use Photoshop, etc. They are pretty, but time and resource consuming. See http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnwxp/html/winxpicons.asp. This Microsoft document says there should be 9 icons, including 24x24 ones, somewhat orthogonal to the discussion here. Second, there seems to be no correlation between the BMP files that can supposedly be used and what are stated here as the possible number of required types in the .ico file. Third, I strongly support the comment: "we want the product editor to give a warning when the ico file does not contain the proper images for a succesfull branding." Ditto for BMP. And I would add that the documentation needs to improved, actually to exist in the first place. Finally, I have been unable to make it work in *any* way, either with a .ico file or .bmp files. See the thread "How to Supply Launcher Icons for Product" on eclipse.platform.rcp and the following: I captured the Eclipse .ico in the exported .exe file with IconJack32 (PC Magazine Utility) then opened it in Visual Studio. It has 7 images (16x16-256, 16x16-XP, 24x24-256, 32x32-256, 32x32-XP, 48x48-256, and 48x48-XP) for me. Visual Studio says the XP type is 32-bit and cannot be edited in Visual Studio. I changed pixels in the ones I could and saved it with a new name, Eclipse.rcp.new.ico. On export I did not see my changes in Explorer. I also assumed the one it was using in Windows Explorer might not be one of the ones I changed, so I captured it again and it is the original one, not the one I specified. It looks to me as if Eclipse just copies the exported .exe file. (Mine are all alike, apart from the name.) It would be a lot more work to rebuild it, as would be necessary to change the icon. I am skeptical they do that. The slightly-changed Eclipse icon I used should have the right number and kind of types, as it is essentially the same as the default one, I captured. I am using 3.1.1. This is from the text version of my .product file: <launcher name="JProbe SWT RCP"> <solaris/> <win useIco="true"> <ico path="/JProbe SWT RCP/icons/Eclipse.rcp.new.ico"/> <bmp winSmallHigh="/JProbe SWT RCP/icons/xrays16-256.bmp" winSmallLow="/JProbe SWT RCP/icons/xrays16-16.bmp" winMediumHigh="/JProbe SWT RCP/icons/xrays32-256.bmp" winMediumLow="/JProbe SWT RCP/icons/xrays32-16.bmp" winLargeHigh="/JProbe SWT RCP/icons/xrays48-256.bmp" winLargeLow="/JProbe SWT RCP/icons/xrays48-16.bmp"/> </win> </launcher> As you can see, I also made the required .bmp files, and they did not appear either, (when useIco was false). Note that there are 6 (not 7 or 9) of them and none is supposed to be 32-bit (or 24x24, either). There is a solaris markup above though I do not have Eclipse installed on Solaris, yet. I *do* change the launcher name. Yes, I have synchronized, cleaned, and done everything else that should be necessary to insure the changes have been made. It has been pretty time consuming. I am new to submitting bugs and also previously added comments in bug 102259 on this same issue. Sorry to be critical and hope this is perceived to be constructive. Okay, I found the cause of this problem and how to solve it. Actually there are several unrelated problems, which combined make this feature unusable to just about anyone except the most lucky ones... This goes for Eclipse 3.1.2, maybe some of the problems were already fixed. 1.) For icon branding to work, your project MUST be named the same as your plugin ID. This is needed because the branding mechanism (AssembleConfigScriptGenerator class) looks for the .ico file by BUNDLE NAME (which is the same as plugin ID), while the Branding property editor begins the icon path with PROJECT NAME when selecting the file with Browse. 2.) The class that replaces the icons (IconExe) works only when EVERY icon in the template launcher executable has its counterpart (with the same size and color depth) in the branding .ico file. So the best thing to do is to take the original .ico from eclipse/plugins/org.eclipse.platform.source_3.x.x/src/org.eclipse.platform_3.x.x/launchersrc.zip and just replace the icons. There is a free icon editor that works pretty well: Junior Icon Editor. Other editors work well too, as these branding difficulties never had anything to do with icon editors. Currently, there are 7 icons in the original .ico (and launcher .exe) file: 48x32, 48x8, 32x32, 32x8, 24x8, 16x32, 16x8. But don't rely on this info, as it might change in future versions. You may put more than this number of icons in the .ico, but not less. 3.) Looking at the source of IconExe class, you can see comments about the kind of icons the .ico file should contain (48x8, 48x4, 32x8, 32x4, 16x8, 16x4). THESE COMMENTS ARE WRONG, they are probably from some ancient version of PDE. 4.) Branding by using BMP files can NEVER work in current PDE versions! Like I said before, the icons are only replaced when every one of the original ones is matched by a branding replacement icon. So, since there are 7 icons in the original .exe, they can never be replaced by the 6 provided BMPs. 5.) The BMP image parameters in the Branding property editor are totally wrong. However this probably does not have any other effect - if there were enough slots to cover the 7 original icons, things would probably work. The previous analysis of the problem seems to be the most accurate, yet. Thanks. However, the whole situation needs to be fixed up. 1. The project name should not have to be the same as the plug-in id. (Apparently that is why mine personally doesn't work.) 2. You should be able to use any valid .ico file. There is no need to require those specific 7 types, especially the hi-res ones, which are hard to create. 3. The bitmap images should work or be eliminated from the interface. It would be nice if they worked, though. I personally have better image editors than icon editors. Even Visual Studio will not edit the Eclipse .ico file. (Perhaps Eclipse should have an icon editor.) 4. There should be a warning if the requirements are not matched. 5. It should all be documented. After all, we are dealing with computers. They are supposed to be our servants, not masters. (I think ;-) The downside is that they are stupid (but fast) and need to be told what to do. That is why we have developers. It's cool that you have fixed what you have in Bug #121735. However, what one really wants is to be able to replace the icons with any .ico file. The hi-res images are especially hard to deal with. (See Comment #3 in Bug #121735.) I did a fast Google search and came up with several programs that purport to change the icon in an existing .exe file. I tried the first one: http://www.softboy.net/exeico/ and it worked. (It replaces each image separately.) There are other programs that do this, as well. If they can do it, then Eclipse should be able to do it. As long as you are fixing it up, why not do it right. Sorry if I am becoming annoying. Now that we actually know the requirements, the language in the product editor is now clear as to what the build expects. As far as the rest of the ICO conversation is going and how to make it easier, this is strictly a pde/build issue and I will move the bug back to that component. Should the ICO specs be made simpler, we would be happy to update the text in the editor to reflect the new requirements. Kenneth, since others can do icon replacements, why can't you provide us a patch? :-) The difficulty here is that the programs you are talking about actually remove data from the exe which is a more heavy duty task than just changing the icons in the exe like we do today. Another solution would consist for the brander that we have to interpolate the missing images from the provided one. This will give a result as good as what the os would do. The bottom line is: if you don't provide all the images, at one point someone will have to interpolate the missing images from the provided one and there are chances that it looks bad. >> Kenneth, since others can do icon replacements, why can't you provide us a patch? :-) No promises, but first, where would I find out what you are doing now. >> Another solution would consist for the brander that we have to interpolate the missing images from the provided one. Based on what the program I tried is doing, that would appear to be a reasonable and possible solution. I assume that just means finding the offsets and replacing the bits. (But, as stated, I don't know what Eclipse is doing, or what, in fact, is possible.) Given an icon of a given size, it can be expanded to a greater bit depth without loss of resolution. It can be reduced, also, by stripping the extra bits, with possible, usually small, color changes. It can be made larger at the expense of pixelation, and can be made smaller, with loss of information and usually unattractive. Windows does that for you in many cases, if you don't supply enough images. I'm sure an algorithm for mapping what is included to the other sizes, can be readily designed. If the programmer doesn't supply enough images, that is the best that can be done. However, that is better than what happens now. It would certainly get around the hi-res problem, the one that requires special programs. It also seems that the Eclipse .ico doesn't contain the most flexible set of icons. Given that people are going to be using it for RCP branding, it might be a good idea to rethink the basic Eclipse .ico. Andrew, ICO files that only have six images (ie. missing the 24x24 one) work fine. So what are we validating for exactly? I think we want to validate that the 7 sizes we want exist in the ico file. Missing one of the sizes is not really an error, you just end up with a partially branded exe that will have the eclipse icons for the sizes you were missing. Other images in the ico file that have sizes that don't match will simply be ignored. RE: Comment #14, the current icon replacement is done by org.eclipse.pde.build/org/eclipse/swt/tools/internal/IconExe It takes an executable and either an ico file or a list of bmps, for each image in the executable, it replaces it with the matching size from the provided images if one exists. I think it would be worth investigating using swt to interpolate/generate the missing sizes based on the provided images. Kenneth I have opened the bug #130210 in order to get the product editor to generate the missing files. You may want to make your contribution there :-) The product editor has been updated to validate the .ico field. It now checks that the supplied .ico file contains at least one of each of the seven formats. If any formats are missing a warning, listing the missing ones, is displayed. Closing as wontfix as per the last comment. I am sorry to see that. I think requiring the user to provide all the formats is a significant hardship and quite unnecessary. The format set you require is not even standard. It takes sophisticated graphics programs to produce the higher-resolution formats, and they are not needed. It is an unnecessary penalty on developers. And considering the effort that goes into almost every aspect of Eclipse, the effort to provide automatic generation of the missing formats would be small. Evan, you should open an enhancement request against PDE UI. For it to be processed promptly I would attach the code that generates the new image. Then they can do the hooking in the product editor (which is still their theme of the week). OK. It's Bug #180384. Unfortunately, I have no source code, and I assume they know where theirs is. -Ken |