After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 171562 - Random, discolored are pixels drawn into top rows of some RLE bitmaps
Random, discolored are pixels drawn into top rows of some RLE bitmaps
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Plugins
2.2.x
Other All
: Normal minor
: 2.2
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2005-03-25 08:59 UTC by David Costanzo
Modified: 2008-01-15 12:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
rle8-blank-160x120.bmp -- image mentioned in the repro scenario (1.05 KB, image/bmp)
2005-03-25 09:03 UTC, David Costanzo
  Details
Proposed patch (640 bytes, patch)
2005-03-25 20:44 UTC, David Costanzo
committed Details | Review

Description David Costanzo 2005-03-25 08:59:26 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:
Comment 1 David Costanzo 2005-03-25 09:03:45 UTC
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.
Comment 2 Sven Neumann 2005-03-25 10:03:17 UTC
As usual, a patch would be very much appreciated.
Comment 3 David Costanzo 2005-03-25 19:29:26 UTC
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.
Comment 4 Sven Neumann 2005-03-25 20:25:17 UTC
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.
Comment 5 David Costanzo 2005-03-25 20:44:32 UTC
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.
Comment 6 Sven Neumann 2005-03-25 21:16:02 UTC
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.