search

jsummers / bmpsuite

A set of Windows BMP image files, for testing purposes
https://entropymine.com/jason/bmpsuite/
GNU General Public License v3.0
55 stars 16 forks source link

Suggestion: Bad color palettes with too few bytes #22

Open squeek502 opened 3 weeks ago

squeek502 commented 3 weeks ago

(for context, g/pal4.bmp is a "Paletted image with 12 palette colors, and 4 bits/pixel.")

From what I've seen, both Windows and Firefox reject both of the above as invalid.

These would nicely complement q/pal8offs.bmp, which has more bytes in the palette than expected / extra bytes between the palette and the pixel data.

Electron-x commented 3 weeks ago

Test bitmaps where the size of the color table differs from the value specified in the header are very helpful, as it is no longer possible to determine the start of the bitmap bits for packed memory DIBs (e.g. bitmaps pasted from the clipboard).

The BMP Suite already contains two bitmaps in which the color table and the bitmap bits overlap: q/pal8os2sp.bmp b/badpalettesize.bmp

The variants you suggest could be counterproductive. A reader could easily correct the mismatch by adjusting biClrUsed. However, the test files would then lull you into a false sense of security. This is because the special case - a bitmap completely without the required color table - would not be fixed in this way. This case can only be corrected by adding the missing color table entries.

Furthermore, I cannot think of any reason why a bitmap writer should write a different number of color entries. However, a value of zero with a completely missing color table is not unrealistic. This is also the case if the color table exists but is located behind the bitmap bits. This case is not so far-fetched when you consider that you have to move a color profile in front of the bitmap bits before copying a bitmap to the clipboard and then move it behind the bitmap bits again after pasting it from the clipboard.

I had generated a test bitmap without a color table for myself at the time and also created a commit for it three weeks ago. As a supplement to the existing files mentioned above, I therefore suggest including this variant as a substitute for your two proposals. Alternatively, I recommend including a warning about the special case in the HTML description.

squeek502 commented 3 weeks ago

The variants you suggest could be counterproductive.

I'm not sure I follow. To be clear, I'm suggesting these as 'bad' variants (see the PR). I like your nopalette variant as well, but I don't see why including all 3 wouldn't be even better.

To give some context, the Windows resource compiler (rc.exe) writes BITMAP resources by (1) stripping the BITMAPFILEHEADER and (2) forcing the number of bytes in the color table entries to match the expected number of bytes, even if there are missing/extra bytes (i.e. it will use the bytes from the pixel data and/or pad with zeroes if it runs past the end-of-file). This means that the variants I suggest and your nopalette variant would become 'valid' (but very likely wrong) when run through rc.exe. This is just one strange quirk of one particular tool, but I think it shows that these are all potentially relevant edge cases when dealing with bitmaps.

Electron-x commented 3 weeks ago

Yes, my comment was a little too dramatic. Sorry about that. That's partly because I fell for it myself and my own code was inadequate. It was only with a bitmap from another test suite that I was able to find the bug. On the other hand, too many similar variants might not be good either (but that's not for me to decide). We are already talking about three additional bitmaps. If you load them into the resource editor of Visual Studio, for example, the program tries to correct the files by changing OffBits in the file header (OffBits is increased by the missing palette entries). And we already have six variants...