ColorPicker icon indicating copy to clipboard operation
ColorPicker copied to clipboard

`Bitmap` and `Graphics` objects not disposed in `GetPixelColor`

Open daiplusplus opened this issue 3 years ago • 0 comments

I was just browsing the source-code and looking to see how it worked and saw this:

https://github.com/martinchrzan/ColorPicker/blob/ea493022f9043c91761d3d6317810976a3e17e8a/ColorPicker/Mouse/MouseInfoProvider.cs#L80-L88

This code should be this:

        private static Color GetPixelColor(System.Windows.Point mousePosition)
        {
            var rect = new Rectangle((int)mousePosition.X, (int)mousePosition.Y, 1, 1);
            using(var bmp = new Bitmap(rect.Width, rect.Height, PixelFormat.Format32bppArgb))
            using(var g = Graphics.FromImage(bmp))
            {
                g.CopyFromScreen(rect.Left, rect.Top, 0, 0, bmp.Size, CopyPixelOperation.SourceCopy);
                return bmp.GetPixel(0, 0);
            }
        }

That said, performance could be massively improved by caching the Bitmap in a field and only recreating it if the user's desktop resolution changes. GDI objects are expensive.

...also, this code won't work to accurately capture people in 30-bit color mode (I am) due to GDI's built-in color-depth limitation of 8bpp. That said, I'm unsure how one is able to capture 30-bit color from the framebuffer.

daiplusplus avatar Aug 12 '21 20:08 daiplusplus