HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

C# GDI+ rendering method performance

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
renderingperformancemethodgdi

Problem

I am looking for any performance insights or better coding style for my rendering method.

public Bitmap bmp;
public void render()
{
    Brush[] brushes = new SolidBrush[NumOfShapes];
    PointF[][] points = new PointF[NumOfShapes][];
    float[] tensions = new float[NumOfShapes];

    // We have multiply shapes, every shape has parameters for brush,points,tension
    for (int i = 0; i < NumOfShapes; i++)
    {
        int curT = i * ParameterNumber;
        brushes[i] = getBrush(curT); // get value and clip it if need
        points[i] = getPoints(curT); 
        tensions[i] = getTension(curT);
    }

    using (Graphics graphics = Graphics.FromImage(bmp))
    {
        graphics.Clear(Color.Black);
        graphics.SmoothingMode = Globals.smoothingMode;
        for (int i = 0; i < NumOfShapes; i++)
        {
            graphics.FillPolygon(brushes[i], points[i]);
            // I also use shapes bellow
            //graphics.FillClosedCurve(brushes[i], points[i]);
            //graphics.DrawPolygon(new Pen(brushes[i],tensions[i]), points[i]);
            //graphics.FillClosedCurve(brushes[i], points[i], FillMode.Alternate, tensions[i]);
            //graphics.DrawCurve(new Pen(brushes[i], tensions[i]), points[i]);
        }

        graphics.Flush();
    }
}


With this, I am currently getting ~2000 bitmaps per second.

Generated bitmap is not displayed, I just get pixels with Marshal.Copy() and dispose bitmap. So is there a quicker way to render bitmap from shapes and get pixels.

```
private SolidBrush getBrush(int curT)
{
parameters[curT] = clippingColor(parameters[curT]);
parameters[curT + 1] = clippingColor(parameters[curT + 1]);
parameters[curT + 2] = clippingColor(parameters[curT + 2]);
parameters[curT + 3] = clippingColor(parameters[curT + 3]);
Color color = Color.FromArgb(Convert.ToInt32(parameters[curT]), Convert.ToInt32(parameters[curT + 1]), Convert.ToInt32(parameters[curT + 2]), Convert.ToInt32(parameters[curT + 3]));

Solution

It looks like you may come from a Java or scripting background. For .NET, the standard recommends PascalCasing for method names. This would turn your methods into:

Render()

GetBrush(...)

GetPoints(...)

GetTension(...)

Variable names should be camelCasing, where the first letter is lower case and the remaining words are capitalized. I'm not sure if NumOfShapes or ParameterNumber are class properties (Pascal Cased) or class-level fields (camel Cased), but you should follow the standard.

As far as your method names, you should avoid GetXXX which suggests that they should be properties. I would recommend the following changes:

GetBrush(...) -> CreateBrush(...)

GetPoints(...) -> GeneratePoints(...)

GetTension(...) -> CalculateTension(...)

For your variable names, curT doesn't help me understand what the parameter is for. :

for (int i = 0; i < NumOfShapes; i++)
{
    int curT = i * ParameterNumber;
    brushes[i] = getBrush(curT); // get value and clip it if need
    points[i] = getPoints(curT); 
    tensions[i] = getTension(curT);
}


The definition and modification must be elsewhere. I'd have to go searching for it. It would be much simpler if you named it something meaningful, even if it is long, like currentImageIteration.

As far as coding goes, always, always dispose of GDI objects when you are done using them. You can get some pretty strange errors (OutOfMemoryException for example) when hitting the GDI object limit. Because GDI uses unmanaged objects, you should always dispose of them when you are done, see this question, and this question. The object will be disposed at some time in the future, but because the GC doesn't clean up managed objects, this will cause a memory leak.

Other than that, everything seems OK.

Code Snippets

for (int i = 0; i < NumOfShapes; i++)
{
    int curT = i * ParameterNumber;
    brushes[i] = getBrush(curT); // get value and clip it if need
    points[i] = getPoints(curT); 
    tensions[i] = getTension(curT);
}

Context

StackExchange Code Review Q#148221, answer score: 2

Revisions (0)

No revisions yet.