cyotek / Cyotek.Windows.Forms.ColorPicker

Color picker control suite for Windows Forms applications.
http://cyotek.com/blog/tag/colorpicker
MIT License
149 stars 41 forks source link

Color Picker returns wrong color #15

Closed foreachthing closed 6 years ago

foreachthing commented 6 years ago

Picking the main color (in Inkscape rgb(87, 106, 121)) returns rgb(87, 106, 120) . And that is only ONE example.

Using Cyotek.Windows.Forms.ColorPicker v 1.7.2.

Inkscape: image

Colorpicker image

Another wrong color: Inkscape rgb(226,230,238) will be picked up as rgb(224, 229, 237). Confirmed with screenshots and color picker from paint.NET.

cyotek commented 6 years ago

Hello,

Thank you for the issue report, although I'm not sure if you're being condescending or not with your "ONE" example, but hopefully not.

I've given this a test and I'm not able to reproduce - when using the source image above, you can see from my example below that it is picked up correctly.

norepro

The ScreenColorPicker isn't doing any magic conversions from HSL to RGB (which could cause a rounding error that mimics your example) and is actually just pure BCL without the need of interop or custom code so I'm not exactly sure why this is happening for you. I'll do some testing in a VM as well and see if I get different results there.

Can you confirm what operating system you saw this behaviour on?

Does the same behaviour happen with the demonstration program supplied with the software? I was recently adding (completely different) HSL conversion code to a different library and dealing with a variety of rounding errors, this feels similar so it would be good to know if it's present in the demo application as well or just your own code. (For the recorded, I tested the demo program myself and couldn't reproduce with that either).

Thanks; Richard Moss

foreachthing commented 6 years ago

Ok .... !?

Condescending? I'd not! My apologies if it sounded like that. With ONE I meant, that there are other colors that get picked up wrongly. Funny enough, the bright greenish circles are correct!

OS: I am on Windwoes 7 x64.

Long story short: Your application-demo does it right. If used in my application, something's going wrong....

Please let me know if I can try something to track that issue down. Gilbert

foreachthing commented 6 years ago

I've found the issue!!! It was before I found out about the CEM, I had an event when changing the Color Picker color:

private void screenColorPicker1_ColorChanged(object sender, EventArgs e)
    {
        colorEditor1.Color = screenColorPicker1.Color;
        colorWheel1.Color = screenColorPicker1.Color;
    }

So, I comment these two lines and it works as it should! Somehow, somewhere something's lost...!

Thanks though, for your quick responds.

cyotek commented 6 years ago

Hello,

Thanks for the follow ups, apologies for reading something in your initial message that wasn't there.

Based on your comment, I think the bug was in the ColorWheel - setting the Color property would also set an internal HslColor and I wonder if that would trigger another color change if there was a rounding error. I think I may have already fixed this - I did some refactoring of the ColorWheel control in April for v1.8, but as I'm still using 1.7 myself, evidently I felt there was more to do and so didn't create a new package. I'll see if I can reproduce your scenario with that build and make sure that the bug is squashed.

Thanks again for the bug report!

Regards; Richard Moss