GNOME Bugzilla – Bug 171562
Random, discolored are pixels drawn into top rows of some RLE bitmaps
Last modified: 2008-01-15 12:50:52 UTC
Please describe the problem: The GIMP draws random-colored pixels into the top few rows of RLE bitmaps that don't specify the pixels in the top few row. In every row other than the top few rows, the GIMP draws "0" pixels when the bitmap doesn't specify a pixel value. Since RLE bitmaps use a color index, this shows up as the zeroth color in the palette. I see the same discoloration pattern for a given file no matter how many times I load it. RLE bitmaps don't have to specify all pixels--this is part of the compression algorithm. Documentation on this is a bit sketchy, but I *think* that any pixel which an RLE bitmaps doesn't specify is supposed to be transparent, not a 0 pixel. I *think* this is how old Windows systems (pre-Win95) implemented icons and pointers. While it would be nice if the GIMP made the unspecified pixels transparent, that's a separate feature. But clearly, all unspecifed pixels should be treated the same. Steps to reproduce: 1. Load rle8-blank-160x120.bmp 2. Zoom into the top row until individual pixel are easy to distinguish. Actual results: The image shows up entirely black, except for the top few rows, which have sporadically placed discolored pixels. Expected results: The image is one solid color (either black or transparent). Does this happen every time? Yes Other information:
Created attachment 39235 [details] rle8-blank-160x120.bmp -- image mentioned in the repro scenario rle8-blank-160x120.bmp -- This is an "all blank" RLE image. It contains nothing but an "end-of-bitmap" marker. It shows up as all black in MS Paint, which doesn't support transparent pixels. It shows up as a checkerboard pattern in GQView.
As usual, a patch would be very much appreciated.
I still haven't been able to build the CVS, so any patch I create will be against the against the 2.2.4 source. The bug is caused by uninitialized memory. I can either fix it the easy way, or *try* to fix it the hard way. The easy way is to change the "dest = g_malloc()" call to "dest = g_malloc0()". This very obviously initializes all memory to zero, but potentially adds a linear pass over the buffer, even when not processing RLE bitmaps. Most bitmaps are NOT RLE, which means that this strategy slows down the common case. On the other hand, this strategy would help non-RLE bitmaps files that got truncated (as by a botched download) in that the missing part of the picture would show up as black, instead of being uninitialized memory. The hard way is to initialize all skipped pixels to 0 as the RLE processor encounters skip directives. This could be tricky to get right because there are three different ways to skip pixels: an end-of-line record, an end-of-bitmap record, or a delta record. The advantage of this strategy is that it doesn't slow down the common-case when the image is uncompressed and well-formed.
I doubt that using g_malloc0() will introduce a noticeable slowdon at all. I definitely prefer the fix that keeps the code simple even if it slows down things a little bit.
Created attachment 39254 [details] [review] Proposed patch This patch was generated against the gimp-2.2.4 source. PATCH SUMMARY: Allocate the pixel buffer with g_malloc0 (instead of g_malloc) to ensure that all pixel values are initialized to a good value. Add a comment that makes it obvious that we're calling g_malloc0 and explains why.
Applied to both branches. Thanks for the patch. 2005-03-25 Sven Neumann <sven@gimp.org> * plug-ins/bmp/bmpread.c: applied patch from David Costanzo that initializes unspecified pixels in RLE bitmaps. Fixes bug #171562.