raysan5 / raygui

A simple and easy-to-use immediate-mode gui library
zlib License
3.26k stars 280 forks source link

Fixed dragging GuiColorPanel outside of bounds. #366

Closed kolunmi closed 6 months ago

kolunmi commented 6 months ago

Hello! This pr fixes an issue with GuiColorPanel() where, unlike sliders, the element would not properly track a drag once the mouse exited the bounds. No new global variables are added.

Before

https://github.com/raysan5/raygui/assets/113054217/a2830c29-9354-4491-926d-0d214eee17de

After

https://github.com/raysan5/raygui/assets/113054217/c43d9b88-3801-477a-9540-f7a4a90a8ef7

Thank you very much!

LievenPetersen commented 6 months ago

I like this change. However before merging, this new behavior would also need to be transferred to GuiColorPanelHSV(). But before spending more time on it, you should probably wait to hear what raysan5 has to say about it.

LievenPetersen commented 6 months ago

Maybe it is a good idea to call GuiColorPanelHSV inside GuiColorPanel? The logic should essentially be a code duplication.

raysan5 commented 6 months ago

Maybe it is a good idea to call GuiColorPanelHSV inside GuiColorPanel? The logic should essentially be a code duplication.

@LievenPetersen That's probably a good idea, feel free to send a PR.

raysan5 commented 6 months ago

@kolunmi Thank you very much for the review!

I think it could be reviewed a bit further to also consider other controls that could require it or at least rename the globals to a more generic and comprehensive naming:

LievenPetersen commented 6 months ago

Maybe it is a good idea to call GuiColorPanelHSV inside GuiColorPanel? The logic should essentially be a code duplication.

@LievenPetersen That's probably a good idea, feel free to send a PR.

@raysan5 Looking into it, it would be convenient to use the result of GuiColorPanel (so far unused and always 0) to indicate, if the color was changed. Can I do that? Setting the return value to 1 on modification seems handy in general and could be propagated to other color-components (or even slider-like components).

longer explanation:

Currently the color is only changed, if the mouse was dragged in the color panel. If one panel calls the other, it would have to know if that change occurred, so that it only then updates it's own color parameter.

Alternatively the RGBA panel would modify it's color every frame, which would alter the behavior, because the RGB->HSV->RGB conversion is not always accurate. This would mean, that if a color is set from a different source, it would often instantly change it's color code to something slightly different. Which is undesirable IMO.