[Mono-bugs] [Bug 79246][Nor] Changed - Bitmap.LockBits doesn't behave like MS
bugzilla-daemon at bugzilla.ximian.com
bugzilla-daemon at bugzilla.ximian.com
Fri Sep 8 00:47:06 EDT 2006
Please do not reply to this email- if you want to comment on the bug, go to the
URL shown below and enter your comments there.
Changed by 4lw0e0402 at sneakemail.com.
http://bugzilla.ximian.com/show_bug.cgi?id=79246
--- shadow/79246 2006-09-05 09:10:00.000000000 -0400
+++ shadow/79246.tmp.27323 2006-09-08 00:47:06.000000000 -0400
@@ -1,12 +1,12 @@
Bug#: 79246
Product: Mono: Class Libraries
Version: 1.1
OS: All
OS Details:
-Status: NEW
+Status: ASSIGNED
Resolution:
Severity: Unknown
Priority: Normal
Component: libgdiplus
AssignedTo: 4lw0e0402 at sneakemail.com
ReportedBy: sebastien at ximian.com
@@ -87,6 +87,220 @@
moving by Z bytes would results in a different X, Y pixel position (as
this seems to be the main motivation of BitmapData).
[3] Unless we're 100% sure about this being a bug we should strive for
compatibility with MS implementation. People won't like having Mono/MS
dependant code when using System.Drawing.
+
+------- Additional Comments From 4lw0e0402 at sneakemail.com 2006-09-08 00:47 -------
+In reply to [1]: There seem to be a number of confusions in the
+namings of bitmap files, but the ones I put there are definitely of
+the bit depth they claim to be -- 1bit.png and 4bit.png. Of course,
+that doesn't help when you need to test the BMP codec ;-)
+--------------------
+In reply to [2]: The design of BitmapData has one main motivation: to
+be able to guarantee word-alignment of scan lines. With 32bpp and
+64bpp pixel formats, this is trivial, because the exact number of
+bytes needed will just happen to be an exact multiple of the word
+size, but with smaller colour depths, any byte offset is possible.
+With very low depths (anything less than 8), there isn't even a
+guarantee that the end of a scan line will be on a byte boundary.
+
+The reason BitmapData wants to align each scan with a word boundary
+is that the Windows GDI has that requirement (and remember, GDI+ and
+System.Drawing were designed by Windows-centric programmers). So,
+padding bytes will be used to separate the end of the scan line. So,
+without documenting a strict method of computing the amount of
+padding that will be used (and for sure, this wouldn't be impossible,
+but it hasn't been done), how, then, do programs find the start of
+each successive scan line?
+
+That's where the "Stride" member comes into play. No matter what the
+codec actually does, as long as it places enough bytes for a scan at
+each multiple of "Stride" within the range specified by the "Height"
+member, a consumer can find the scans & process them. Assuming that
+the size of the pixels is known (say, 8 bpp), the following code
+snippet is a correct loop for processing a bitmap directly:
+
+Rectangle lock_rectangle = ...;
+
+BitmapData data = null;
+
+try
+{
+ data = bmp.LockBits(
+ lock_rectangle,
+ ImageLockMode.ReadWrite,
+ PixelFormat.Format8bppIndexed);
+
+ byte *data_ptr = (byte *)data.Scan0.ToPointer();
+
+ for (int y=0; y < data.Height; y++)
+ {
+ byte *scan_ptr = data_ptr + y * data.Stride; // <-- KEY IDIOM
+
+ for (int x=0; x < data.Width; x++)
+ scan_ptr[x] = process_pixel(x, y, scan_ptr[x]);
+ }
+}
+finally
+{
+ if (data != null)
+ bmp.UnlockBits(data);
+}
+
+So, if you remove any assumption about how the codec is going to
+compute the stride (maybe the author was lazy and chose to always
+allocate the maximum required stride for the pixel formats, even if
+the file in question used less -- not good code, but certainly not
+incorrect as far as the BitmapData structure is concerned), you
+really have no idea, short of doing complex calculations, where in
+the bitmap an offset farther than the end of the current scan will
+end up. In fact, the offset might not even be inside of the bitmap!
+Almost certainly, the code which advances by 1009 bytes every
+iteration will eventually hit unused bytes between scans.
+
+Anyway, to give a specific example, that code which
+reads "almogaver24bits.bmp" gets a stride of 520 when run on MS's
+runtime (the next multiple of 4 after 519, the number of bytes in
+each scan). Therefore, the scan y=1 starts at offset 520 and runs up
+to offset 1039. Offset 1009 into the bitmap data will be offset 489
+into the scan, which corresponds to the blue component of the pixel
+at (163, 1) (assuming BGR order in-memory).
+
+When the same code is run on mono, a quirk of the bitmap codec
+(actually quite possibly code I wrote :-) gives the data a stride of
+692 bytes. This means that 173 bytes per row are unused. Yes, this is
+probably a bug and probably should be fixed, and fixing it will
+probably make the test case start working, but that doesn't make the
+test case correct! Anyway, with a stride of 692, offset 1009 will be
+317 bytes into the second scan line, which corresponds to the red
+component of the pixel at (105, 1) (again, assuming BGR order in-
+memory).
+
+To summarize:
+
+ When the stride is 520, offset 1009 is in pixel (163, 1)
+ When the stride is 692, offset 1009 is in pixel (105, 1)
+--------------------
+In reply to [3]: Okay. I will adjust the BMP codec so that 32bpp
+images get loaded as PixelFormat.Format32bppRgb. I found
+justification for this at:
+
+http://www.codeproject.com/cs/miscctrl/TransButtonNetDemo.asp
+
+The advantage of this approach is that it is compatible with the work-
+around on that page: Though the pixel format is set to not use alpha,
+the bytes are still loaded, and manually transferring them to a
+bitmap with the correct pixel format will give people the
+transparency they want. Default drawing functions will have the alpha
+bytes forced to 255 when painting the bitmap.
+--------------------
+Further notes: I check out some of the other [NotWorking] tests in
+the ./System.Drawing/Test/System.Drawing.Imaging directory, and here
+are my analyses:
+
+GifCodecTest.cs:
+ Bitmap8bitsData():
+ - It is loading a file called "bitmaps/nature24bits.gif". One
+thing I'm pretty sure of is that GIFs cannot be 24-bit. :-) Since I
+added native support for indexed pixel formats, the bitmap will
+actually be loaded at 8bpp.
+ - The Bitmap object is then locked at 24bpp. The pixel stream
+coded I added will convert the data from 8bpp to 24bpp without
+problems. Therefore, the test really isn't doing much of any 8bpp
+testing.
+ - In addition to these issues, this test suffers the same stride
+assumption as the NotWorking tests in TestBmpCodec.cs.
+ Save():
+ - The test is constructing a 32-bpp RGB image and then saving it
+using an 8bpp codec -- this will be troublesome at best, since we
+have little or no control over the quantization to 8-bit data
+(especially how the palette is generated). The test was probably
+written before my indexed pixel formats patch, so it could be
+rewritten now to create a Bitmap using PixelFormat.Format8bppIndexed
+and then directly draw some things onto it by locking the bits and
+setting a palette. The resulting data should round-trip perfectly.
+ - It saves the test image to an actual disk file -- why not save
+it to a MemoryStream instead? I suppose one could argue that if the
+test bombed catastrophically, it would leave a file around which
+might contain evidence...
+
+IconCodecTest.cs:
+ The tests in this file are all marked as [Ignore] with the
+message "NotWorking". This could be fixed by adding an icocodec.c to
+libgdiplus :-) The following URL contains details about the layout
+of .ICO files; it looks like it shouldn't be too hard to adapt
+existing codecs, though some testing would be required to find out
+exactly what MS's implementation does with regards to masks &
+multiple icon sizes.
+
+http://msdn.microsoft.com/library/default.asp?url=/library/en-
+us/dnwui/html/msdn_icons.asp
+
+ After a codec has been created, these tests will still need to be
+changed, because they suffer the same stride assumption as the
+NotWorking tests in TestBmpCodec.cs.
+
+PngCodecTest.cs:
+ Bitmap1bitData() and Bitmap4bitData():
+ - These tests suffer the same stride assumption as the NotWorking
+tests in TestBmpCodec.cs.
+
+JpegCodecTest.cs:
+ Bitmap24bitData();
+ - This test loads a BMP file, not a JPEG file!
+ - This test suffers the same stride assumption as the NotWorking
+tests in TestBmpCodec.cs.
+
+TiffCodecTest.cs:
+ Bitmap32bitsData():
+ - This test suffers the same stride assumption as the NotWorking
+tests in TestBmpCodec.cs.
+
+There is clearly a common thread among the [NotWorking] tests :-) I
+think a reasonable fix would be to alter the tests to read pixels by
+(x, y) instead of locking the bits and randomly peeking at them. The
+only question is how to pick the pixels. One way to avoid patterns
+would be to use a pseudo-random number generator. Obviously,
+System.Random is out of the question, because its implementation
+differs between Microsoft's corlib and ours, but there are freely-
+available alternatives to System.Random, and if those aren't
+suitable, we could always roll our own -- after all, we don't need
+cryptographically-sound numbers, but rather just a bit of
+stochasticism to avoid possible patterns.
+
+One possible alternative is at:
+
+http://www.codeproject.com/csharp/FastRandom.asp
+
+One way to integrate this would be:
+
+FastRandom r = new FastRandom(0xCAFEBABE);
+
+int x=0, y=0;
+
+Color[] expected_pixels = new Color[] { <insert manually-obtained
+values here> };
+
+for (int i=0; i < expected_pixels.Length; i++)
+{
+ if (y >= bmp.Height)
+ Assert.Fail("Fell off the end of the bitmap; the data and test
+code are probably out of sync");
+
+ Assert.AreEqual(bmp.GetPixel(x, y), expected_pixels[i],
+string.Format("[{0}, {1}]", x, y));
+
+ x += r.Next(bmp.Width);
+ if (x >= bmp.Width)
+ {
+ // Wrap around to the next row
+ x -= bmp.Width;
+ y++;
+ }
+}
+
+This particular approach would end up comparing roughly (Height * 2)
+of the pixels in the bitmap. Adjust the upper bound of the r.Next()
+call in the middle of the loop to compare greater or fewer pixels.
+
More information about the mono-bugs
mailing list