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

Improve color picking #74962

Merged
merged 1 commit into from
May 22, 2023
Merged

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Mar 15, 2023

This PR makes use of the new screen_get_pixel() method for ColorPicker. It allows to pick the color anywhere on the screen(s), with a caveat that it only works on supported platforms (unsupported have the picking button disabled).

I used a little hack suggested by @Sauermann to detect clicks outside the main window. I spawn a tiny 1x1 popup and wait until it disappears (which happens when you click anywhere). The color reading is done inside internal process. Pressing Escape will cancel picking and restore previous color.

Fixes #74048

EDIT:
After some feedback, I left the old picking code for platforms that don't support screen capture. It works e.g. on Android, so no reason to remove it I guess.

@KoBeWi KoBeWi added this to the 4.1 milestone Mar 15, 2023
@KoBeWi KoBeWi requested a review from a team as a code owner March 15, 2023 22:34
@Sauermann
Copy link
Contributor

I noticed, that this PR will remove picking for non-desktop-platforms like Android. Perhaps the previous method should be kept for these platforms?

@KoBeWi
Copy link
Member Author

KoBeWi commented Mar 15, 2023

I tested on Android and it seems to be working 🤔
I can restore it, I'm a bit worried about code bloat tho.

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

I used a little hack suggested by @Sauermann to detect clicks outside the main window. I spawn a tiny 1x1 popup and wait until it disappears (which happens when you click anywhere).

There's an issue with this approach, other apps can receive click and take focus when picking color, but I'm not sure how to solve it. On Windows, the global input hook used to close popups will consume the click event, but on other platforms it's only works with other windows of the same app.

scene/gui/color_picker.cpp Outdated Show resolved Hide resolved
@KoBeWi
Copy link
Member Author

KoBeWi commented Mar 16, 2023

Ok I restored the old method of color picking. It replaces the new one on unsupported platforms.

There's an issue with this approach, other apps can receive click and take focus when picking color, but I'm not sure how to solve it.

I wouldn't worry about that too much. GIMP for example has the same issue.

@KoBeWi KoBeWi force-pushed the pick_outside_the_box branch 2 times, most recently from bbd5d50 to 02e8537 Compare March 16, 2023 22:01
@akien-mga akien-mga merged commit 5ac6e92 into godotengine:master May 22, 2023
@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.

Color picker eyedropper creates a black rectangle on the editor viewport
4 participants