[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.


--- 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   
 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;
+  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]);
+  }
+  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-
+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:
+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:
+  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 
+    - 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...
+  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.
+  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.
+  Bitmap1bitData() and Bitmap4bitData():
+    - These tests suffer the same stride assumption as the NotWorking 
+tests in TestBmpCodec.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.
+  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:
+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