Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ColorPicker: Fix OKHSL circle in HSV mode #99461

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

beicause
Copy link
Contributor

Fixes #68286.
OKHSL and HSV are different things, they should be stored separately in different fields.

@beicause beicause requested a review from a team as a code owner November 20, 2024 11:31
@beicause beicause force-pushed the color-picker-fix-68286 branch from 2f2bbb9 to e101311 Compare November 20, 2024 11:34
@Calinou Calinou added bug topic:gui cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Nov 20, 2024
@Calinou Calinou added this to the 4.4 milestone Nov 20, 2024
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, it works as expected.

image

@fire fire added the confirmed label Nov 20, 2024
@beicause
Copy link
Contributor Author

In addition to fixing the issue, I think it should be allowed to use other color wheels in okhsl mode, not just okhsl circle. If possible I would like to update this PR, or I will open a new PR after this merges.

@fire
Copy link
Member

fire commented Nov 21, 2024

Please add the modes in a separate pr! 👍

@beicause beicause force-pushed the color-picker-fix-68286 branch from e101311 to d6170f3 Compare November 21, 2024 05:25
@beicause beicause force-pushed the color-picker-fix-68286 branch from d6170f3 to f6751d8 Compare November 21, 2024 05:26
@KoBeWi
Copy link
Member

KoBeWi commented Nov 21, 2024

#99515 should make it easier to add new shapes.

@beicause
Copy link
Contributor Author

beicause commented Nov 22, 2024

Actually in my test, after this pr, we just need to remove the get_shape_override and _get_actual_shape in okhsl mode to let it work in other color wheels. IMO the shape override is unnecessary and this why I want to also do it in this pr.

Of course, the refactoring is great and necessary.

@Repiteo Repiteo merged commit d58d891 into godotengine:master Nov 22, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 22, 2024

Thanks! Congratulations on your first merged contribution! 🎉

@akien-mga akien-mga changed the title ColorPicker: fix OKHSL circle in HSV mode ColorPicker: Fix OKHSL circle in HSV mode Dec 5, 2024
@beicause beicause deleted the color-picker-fix-68286 branch January 9, 2025 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release confirmed topic:gui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editor ColorPicker color wheel preview incorrect in OKHSL / HSV mode
5 participants