martinchrzan / ColorPicker

Windows system-wide color picker
MIT License
339 stars 29 forks source link

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

Closed daiplusplus closed 1 year ago

daiplusplus commented 3 years ago

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.

martinchrzan commented 1 year ago

Thanks, I have fixed this!