Given the following code I am able to access the pixels of an image and map them to a Color
struct.
To my eye though this looks inefficient since every time I access the indexer I am reading from the array four times. It takes about 9 seconds on my computer to run.
What I am looking for is an example of how someone would optimise the code. I'm guessing that I can get some sort of reference to the array using unsafe code and read in one move but I have yet to find a decent guide to explain how you would do that and how you would apply the approach to other situations.
While a code example on its own would aid me, an explanation of exactly what is going on and why this I would improve performance would be more beneficial to myself and anyone reading.
public class Program
{
public static void Main(string[] args)
{
Image img = new Image(10000,10000);
Stopwatch stopwatch = new Stopwatch();
stopwatch.Start();
for (int y = 0; y < img.Height; y++)
{
for (int x = 0; x < img.Width; x++)
{
Color color = img[x,y];
}
}
stopwatch.Stop();
Console.WriteLine(
string.Format("Time: {0} ms", stopwatch.ElapsedMilliseconds));
Console.ReadKey();
}
}
// Define other methods and classes here
public class Image {
private float[] pixels;
public Image (int width, int height){
pixels = new float[width * height * 4];
this.Width = width;
this.Height = height;
}
public int Width { get; private set; }
public int Height { get; private set; }
public Color this[int x, int y]
{
get
{
int start = ((y * this.Width) + x) * 4;
return new Color(this.pixels[start],
this.pixels[start + 1],
this.pixels[start + 2],
this.pixels[start + 3]);
}
set
{
int start = ((y * this.Width) + x) * 4;
this.pixels[start] = value.R;
this.pixels[start + 1] = value.G;
this.pixels[start + 2] = value.B;
this.pixels[start + 3] = value.A;
}
}
}
public struct Color {
public Color (float r, float g, float b, float a):this(){
this.R = r;
this.G = g;
this.B = b;
this.A = a;
}
public float R {get;set;}
public float G {get;set;}
public float B {get;set;}
public float A {get;set;}
}
UPDATE
So with some investigation I've been able to perform some improvements using unsafe code.
Here's one approach to my indexer:
public unsafe class Image {
private byte* pixels;
public Image (int width, int height){
fixed(byte* p = &(new byte[width * height * 4])[0]){
this.pixels = p;
}
this.Width = width;
this.Height = height;
}
public int Width { get; private set; }
public int Height { get; private set; }
public Color this[int x, int y]
{
get { return *((Color*)(this.pixels + ((y * this.Width) + x) * 4)); }
set {
Color* c = (Color*)(this.pixels + (((y * this.Width) + x) * 4));
c->R = value.R;
c->G = value.G;
c->B = value.B;
c->A = value.A;
}
}
}
This is around 50% faster but I am concerned about what happens to the pixels
property. Does that ever get cleaned up by the garbage collector? Should Image be implementing IDisposable
so that I can set the value to null?
Here's a second approach:
public unsafe class Image {
private byte[] pixels;
public Image (int width, int height){
pixels = new byte[width * height * 4];
this.Width = width;
this.Height = height;
}
public int Width { get; private set; }
public int Height { get; private set; }
public Color this[int x, int y]
{
get
{
fixed(byte* p = &this.pixels[0]){
return *((Color*)(p + ((y * this.Width) + x) * 4));
}
}
set
{
fixed(byte* p = &this.pixels[0]){
Color* c = (Color*)(p + (((y * this.Width) + x) * 4));
c->R = value.R;
c->G = value.G;
c->B = value.B;
c->A = value.A;
}
}
}
}
This is about 28% faster and as far as I can determine it means I do not have to do anything to further manage memory.
Are there any obvious flaws in either approach?