Skip to content

Conversation

@KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Nov 21, 2024

Closes godotengine/godot-proposals#4353
Basically #62075, but for picker shapes.

Adds a new class ColorPickerShape, which is an abstraction of picker shapes (similar to ColorMode). This allows to split some behemoth methods into a more manageable pieces. For now I removed the huge _hsv_draw() method, which had entangled code for drawing every piece of ColorPicker. Now each part has its own method. The shapes should appear as before.

@KoBeWi KoBeWi force-pushed the shapez.godot branch 2 times, most recently from 029beba to 04e059f Compare December 1, 2024 22:47
@KoBeWi
Copy link
Member Author

KoBeWi commented Dec 1, 2024

I reimplemented the input. The shapes should behave the same as before.
Feel free to test, I'm not sure if deferred mode is done correctly.

I'll be probably changing the shapes to be Object, because the current code is far from ideal and the little saved memory is not worth it.

@KoBeWi KoBeWi force-pushed the shapez.godot branch 3 times, most recently from 42c56a6 to dd5783d Compare December 6, 2024 15:02
@KoBeWi KoBeWi marked this pull request as ready for review December 6, 2024 15:03
@KoBeWi KoBeWi requested a review from a team as a code owner December 6, 2024 15:03
@KoBeWi
Copy link
Member Author

KoBeWi commented Mar 4, 2025

Rebased. There were many changes to ColorPicker in the meantime, hopefully I didn't accidentally revert anything.

@KoBeWi KoBeWi force-pushed the shapez.godot branch 2 times, most recently from 1c17182 to bcafac0 Compare March 4, 2025 00:24
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 (in the editor and a minimal project), it mostly works as expected.

I spotted one functional regression though. ColorPickerButton with the OKHSL slider mode (not the color wheel mode) will have the color wheel disappear entirely, while a standalone ColorPicker with the same setting is fine.

image

Before After
image image

@KoBeWi
Copy link
Member Author

KoBeWi commented Mar 4, 2025

Good catch, fixed.

@KoBeWi KoBeWi modified the milestones: 4.x, 4.5 Mar 13, 2025
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.

Works great now 🙂

Code looks good to me.

@akien-mga akien-mga merged commit 5045d54 into godotengine:master Mar 19, 2025
20 checks passed
@KoBeWi KoBeWi deleted the shapez.godot branch March 19, 2025 14:04
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor ColorPicker

3 participants