Skip to content

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