Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Goal: Using P/Invoke to capture specified portion of screen to byte[]

Hello, this is my first Code Review post and also first project.

After a week of searching and testing each approach via Stopwatch, I came to this method using the fastest way possible to capture screen into a bitmap and then to a byte[].

  1. Is it possible to make it any faster using parallel features or any idea I have not taken into account? (As I am a newbie, 4 months of self learning.)

  2. I mixed two or three versions of the copying function (portion of screen to memory then convert captured/crop into byte[]). I might have left unnecessary lines of code, I would like to refine it (if and where needed).

If something is not clear, please let me know.

Thanks a lot, here is the code:

unsafe public static Bitmap NatUnsfBtmp(IntPtr hWnd, Size Ms)
{
    Stopwatch swCap2Byte = new Stopwatch();
    swCap2Byte.Start();
    WINDOWINFO winInfo = new WINDOWINFO();
    bool ret = GetWindowInfo(hWnd, ref winInfo);
    if (!ret)
    {
        return null;
    }

    int height = Ms.Height;
    int width = Ms.Width;
    if (height == 0 || width == 0) return null;

    Graphics frmGraphics = Graphics.FromHwnd(hWnd);
    IntPtr hDC = GetWindowDC(hWnd); //gets the entire window
    //IntPtr hDC = frmGraphics.GetHdc(); -- gets the client area, no menu bars, etc..

    System.Drawing.Bitmap tmpBitmap = new System.Drawing.Bitmap(width, height, frmGraphics);
    Bitmap bitmap = (Bitmap)Clipboard.GetDataObject().GetData(DataFormats.Bitmap);
    Graphics bmGraphics = Graphics.FromImage(tmpBitmap);
    IntPtr bmHdc = bmGraphics.GetHdc();
    BitBlt(bmHdc, 0, 0, width, height, hDC, 0, 0, TernaryRasterOperations.SRCCOPY);


    swCap2Byte.Stop();
    string swCopiedFF = swCap2Byte.Elapsed.ToString().Remove(0, 5);
    swCap2Byte.Restart();
    #region <<=========== CopytoMem->ByteArr ============>>

    BitmapData bData = tmpBitmap.LockBits(new Rectangle(new Point(), Ms),
    ImageLockMode.ReadOnly,
    PixelFormat.Format24bppRgb);
    MyForm1.MyT.Cap.TestBigCapturedBtmp = tmpBitmap;
    // number of bytes in the bitmap
    int byteCount = bData.Stride * tmpBitmap.Height;
    byte[] bmpBytes = new byte[byteCount];

    // Copy the locked bytes from memory
    Marshal.Copy(bData.Scan0, bmpBytes, 0, byteCount);
    byte[] OrgArr = bmpBytes;//File.ReadAllBytes("testFcompScr.bmp");
    // don't forget to unlock the bitmap!!
    swCap2Byte.Stop();
    string SwFCFscr = swCap2Byte.Elapsed.ToString().Remove(0, 5);
    System.IO.File.WriteAllBytes(MyForm1.AHItemsInitialDir + "testBig4BenchViaChaos.bar", OrgArr);
    System.Windows.Forms.MessageBox.Show("   Copied @  " +swCopiedFF + Environment.NewLine+"Converted @ " + SwFCFscr);
    btmp.UnlockBits(bData);

    if(System.IO.File.ReadAllBytes(MyForm1.AHItemsInitialDir + "testBig4BenchViaChaos.bar")== OrgArr)
        System.Windows.Forms.MessageBox.Show("OK");
    else System.Windows.Forms.MessageBox.Show("Not same");

    if (BigOrsmall == "Big")
    {

        MyT.Cap.TestBigCapturedBtmp = btmp;
        MyT.CapSave.TestBigCaptSavedAsBar = OrgArr;
        File.WriteAllBytes(AHItemsInitialDir + "testBig4BenchViaChaos.bar", OrgArr);
    }
    else if (BigOrsmall == "Small")
    {
        MyT.Cap.TestSmallCapturedBtmp = btmp;
        File.WriteAllBytes(AHItemsInitialDir + "testSmall4BenchViaChaos.bar", OrgArr);
        MyT.CapSave.TestSmallCaptSavedAsBar = OrgArr;
    }
    TestedCap_DoPutInPicBox(PicBox_CopiedFromScreen);


    #endregion





    bmGraphics.ReleaseHdc(bmHdc);
    ReleaseDC(hWnd, hDC);


    return tmpBitmap;
}
share|improve this question
Since your code sample is somewhat long, could you start incorporating some of the feedback and post a second, revised code sample below the first one? – Leonid Sep 5 '12 at 19:59
@Leonid i will, i don't understand why is: when i built it first i was only mesuring the Stopwatch not seeing that it actually yield nothing inside the array. later on i have progressed with dibuging and testing results seing that the array is actually empty. what ever sizefor a given crop it return the magazin that suppose to contain this size data e.g. array.Length but full of zeros-empty data... i'm far from looking 4 what 2 improve, and it comes from somewhare in the GetWindowInfo() to the bitmap converted from pixels on screen. i gonna resarch on this let me know if you have a clue Y. – Robbie banay Sep 5 '12 at 22:51

1 Answer

There are a few things I see at a glance:

  • If you have to enclose a section of code within a method in region tags, you should be pulling that code out into a helper method. You could probably split this method into several. Don't worry about overhead of extra method calls -- the compiler will likely re-inline them anyways.
  • Split the body of your if/else on "Big" and "Small" into a helper method that takes in a file name. That's the only difference I see in each block.
  • There are a lot of lines where you are declaring and assigning variables. When you do that, it is likely more readable to declare as var, because you indicate what type it is immediately to the right of the assignment operator.
  • Rather than converting your elapsed times with ToString ().Remove () calls, I would instead suggest storing them as TimeSpan types and using a string format in your message boxes.
  • Move "Big", "Small", "testBig4BenchViaChaos.bar", and "testSmall4BenchViaChaos.bar" into constants.
  • If this is just a utility that is not client-facing, move your other strings to constants. Otherwise, move them into a resx so they can be localized.
  • Naming - you should use longer, more descriptive names. You are not saving much by using a name like OrgArr instead of the longer OriginalArray. You should also be consistent in capitalization. Some of your local variables are camel case, and some are pascal case. The standard for the framework is camel case for local variables.
  • You are overwriting testBig4BenchViaChaos.bar with OrgArr, but then comparing the bytes in testBig4BenchViaChaos.bar with OrgArr. Am I missing something, or should this always return a match because the data is the same? Though, on that note, you are doing == with byte arrays, so you should really always mis-match due to object reference comparison.
  • Your quickest way to compare the two byte arrays is probably going to be using a for loop (single-threaded) or possibly PLINQ using SequenceEqual. As always, benchmark to be sure because test results are worth a thousand expert opinions.
share|improve this answer
yeah i was thinking of namings, name it clearer . it's a thought about using to much long names so maybe the compiler will get tired reading long names . but as i mtured a little i could have a clue here and there to the theory that eventually it's translated into from myLongNameFunctionName() into somthing like ^$5 – Robbie banay Sep 5 '12 at 22:32
data suppose to be same ...it all started like an analogy to human finding meat to eat, there was a fire then they discoverd rosted meat, i did try save as bmp. then when loaded from file was not the same as the one that is just captured to memory. no matter what i chose as a file format. so then came bitmap-byteArray brotherhood, when i saved as byte[] it did match a second crop of same area in byte[] against byte[] data comparison then i thought i could split them to four slices an load it via paralel class to compute on all I5 4Cores – Robbie banay Sep 5 '12 at 23:01

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.