4

I set up a code to randomly cover a bitmap 2 different colors, 7 out of 10 times the color would be blue, and 3 out of 10 times, the color would be green. However, when it's done it looks very un-random, like it decided to put 7 blue pixels a few times, then 3 green pixels a few times and so on.
Example:
alt text My code is: 

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;

namespace FourEx
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
        }

        private void Form1_Load(object sender, EventArgs e)
        {
            Bitmap bmp = new Bitmap(canvas.Image);
            System.Drawing.Imaging.BitmapData bmpdata = bmp.LockBits(new Rectangle(0, 0, 800, 600), System.Drawing.Imaging.ImageLockMode.ReadWrite, System.Drawing.Imaging.PixelFormat.Format32bppArgb);
            unsafe
            {
                int tempy = 0;
                while (tempy < 600)
                {
                    byte* row = (byte*)bmpdata.Scan0 + (tempy * bmpdata.Stride);
                    for (int x = 0; x <= 800; x++)
                    {
                        Random rand = new Random();
                        if (rand.Next(1,10) <= 7)
                        {
                            row[x * 4] = 255;
                        }
                        else
                        {
                            row[(x * 4) + 1] = 255;
                        }
                    }
                    tempy++;
                }
            }
            bmp.UnlockBits(bmpdata);
            canvas.Image = bmp;
        }
    }
}

If you need an additional information, let me know.

Cody Gray - on strike
  • 239,200
  • 50
  • 490
  • 574
GunnarJ
  • 440
  • 4
  • 12
  • 1
    Is using `LockBits` and `unsafe` a performance optimization? It seems like you could do this much more easily using [`Bitmap.SetPixel`](http://msdn.microsoft.com/en-us/library/system.drawing.bitmap.setpixel.aspx). – Cody Gray - on strike Dec 12 '10 at 05:14
  • @Code Gray: The performance of Get/SetPixel is really poor for doing image manipulation, this is a pretty common method to get acceptable performance for large scale image generation where pixel by pixel color matters. – Chris Pitman Dec 12 '10 at 05:19
  • 1
    @Chris: I'm aware. I asked if it was done for performance reasons, or because he was unaware of the simpler alternative. I've done both in applications, depending on how, as you say, "large scale" that image manipulation that I have to do is. If you're just creating a random image one time (like in your `Form.Load` event), it's probably not necessary that you squeeze every ounce of performance possible out of it. – Cody Gray - on strike Dec 12 '10 at 05:21
  • 1
    Mitch already mention why your randomness is coming back so un-random. But also as a rule of thumb for optimization: creating new objects in a loop is usually bad news. Allocation of new objects is a significant performance hit when done hundreds/thousands of times in a loop that needs to be performant. – Paul Sasik Dec 12 '10 at 06:42

1 Answers1

11

move this line:

Random rand = new Random();

to the outermost scope. If you create these quickly many will get the same time seed (due to precision of the clock) and will generate the same 'random' sequence. Plus, you only really need one Random instance...

private void Form1_Load(object sender, EventArgs e) 
{ 
    Bitmap bmp = new Bitmap(canvas.Image); 
    System.Drawing.Imaging.BitmapData bmpdata = bmp.LockBits(new Rectangle(0, 0, 800, 600), System.Drawing.Imaging.ImageLockMode.ReadWrite, System.Drawing.Imaging.PixelFormat.Format32bppArgb); 

    Random rand = new Random(); 

    unsafe 
    { 
    // ....
Mitch Wheat
  • 295,962
  • 43
  • 465
  • 541
  • Why are we (application developers) are meant to create random seed instances at all then? Why not just move that to the ever-outermost scope of the standard library itself or something like that? I have been wondering about this for about 20 years already, since I've first met this pattern (in QuickBasic for DOS as far as I can remember). There actually is system-global `/dev/random` in Unix-like OSes, why aren't there cross-platform alternatives in C# and other languages? – Ivan Jun 27 '17 at 20:48