PixelAccessors
For the 5.0 version of dotImage, I added a new abstraction to the way that images can be manipiulatd - the PixelAccesor class. The intent is to take away the process of addressing memory directly as much as possible. This does a couple of things - it allows images to exist partially in memory, allowing image virtuality and it cleans up the code substantially. The cost is, of course, performance but we provide other access mechanisms as well.
What really struck me yesterday, was that I had to be a client of my own design and it was smooth as silk. I had to write some code that given a 64 bit per pixel image converted it into a 32 bit per pixel image. In this case, the source was CMYK, which we don't support in 64 bits (it turns out that this routine will work for 64 bit BGRA as well):
private static AtalaImage Convert64bppTo32bpp(AtalaImage source, PixelFormat destFormat)
{
if (!(destFormat == PixelFormat.Pixel32bppBgra || destFormat == Pixel32bppCmyk || destFormat == Pixel32bppBgr)
throw new ArgumentOutOfRangeException("destFormat);
AtalaImage dest = new AtalaImage(source.Width, source.Height, destFormat);
using (PixelAccessor sourceAcc = source.PixelMemory.AcquirePixelAccessor(), destAcc = dest.PixelMemory.AcquirePixelAccessor())
{
byte[] sb = null, db = null;
while ((sb = sourceAcc.AcquireNextScanline()) != null && (db = destAcc.AcquireNextScanline()) != null)
{
for (int i=0; i < db.Length; i++)
{
db[i] = sb[2 * i + 1];
}
}
}
return dest;
}
In this code, we're taking the most siginificant byte out of every word and using that for an 8 bit value for each component. One thing you'll note is that this routine never validates the source image pixel format. The reason behind this is that I was converting a pixel format that didn't really exist - so the source PixelFormat is actually invalid. Any assumptions made based on it would lead to disaster. This is clearly an unsafe practice and I'm making sure that the code that calls this doesn't keep the invalid image around any longer than it needs to and the it never goes beyond the boundaries of the method that created it, except for this one routine.
The thing to focus on is that the PixelAccessor code feels very comfortable. The using, while, and for blocks fall out of your vision and your eyes tend to fall on the body of the loop itself which is pure simplicity. In previous versions, you would have to write code with pointer arithmetic and unsafe blocks, so I'm happy to give up some performance for readability.
It turns out that we can pick up some of that performance with a little change to the code. PixelAccessors typically use an Acquire/Release model for scanline access. AcquireNextScanline has an implicit Release built into it - it streamlines the code. The problem is that Acquire and Release have very specific semantics. Acquire copies the scanline into a buffer and informs the underlying memory representation that someone is holding onto it. Release copies the buffer back into memory and relinquishes access. The copy back is clearly not needed - we don't actually change the contents of the scanline, so we'd be happy to just let it go. I could've put a bool into Release (and consequently into AcquireNext) to avoid doing this copy, but I wasn't happy with that. Instead, I put in an explicit method GetReadOnlyScanline. This does two things differently. First, it doesn't necessarily have to do an acquire under the hood. This can eliminate overhead. It also is crystal clear in semantics: I'm doing a Get and it's only for Reading.
Here's the code inside the using rewritten with this method:
byte[] sb = new byte[source.RowStride];
byte[] db = null;
for (int y=0; y < source.Height; y++)
{
sourceAcc.GetReadOnlyScanline(y, sb);
if ((db = destAcc.AcquireNextScanline()) == null)
break;
for (int i=0; i < db.Length; i++)
{
db[i] = sb[2 * i + 1];
}
}
You'll also notice that there is no code to release the final scanline. That's OK - the using block will dispose the PixelAccessor object, which will release the scanline as needed. In addition, if the PixelMemory is disposed, it will release any PixelAccessor objects that have been left behind. The overall model is designed to bbe highly resilient in the face of sloppy code as well as being friendly to a garbage collection environment.
You might also raise your eyebrow at the reassignment of db - does that imply that a new byte array is allocated for every scanline? This is a very important question. The answer is no. Every PixelAccessor has an internal buffer for the most recently acquired scanline. There are several reasons - the first is efficiency. I wanted to avoid allocating and copying memory as much as possible - using one buffers assures this. The second reason is that I don't need to worry about someone trying to release a chunk of memory back into the image that doesn't fit. No range checking needed.
That begs the question of what would happen if I do this:
byte[] one = accessor.AcquireScanline(0);
byte[] two = accessor.AcquireScanline(height-1); // one has just changed!
This violates the Principal of Least Astonishment, but I think it's OK. My take is that PixelAccessors are cheap, so this code should be using two accessors, not one.
The final complication is in this:
byte[] one = accessor1.AcquireScanline(0);
byte[] two = accessor2.AcquireScanline(0); // one has just changed!
two[0] = (byte)0;
accessor2.ReleaseScanline(); // copies data back
accessor1.ReleaseScanline(); // copies data back, undoing the previous release
Here is a limitation of the Acquire/Release model - if I gave back a pointer in AcquireScanline instead of a byte array, the problem still exists, just at the byte granularity. I would argue that you shouldn't be trying to modify the same scanline concurrently from two accessors. Again, a little surprising, but a fair concession.