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

Fix Color Picking #72764

Merged
merged 1 commit into from
Feb 16, 2023
Merged

Fix Color Picking #72764

merged 1 commit into from
Feb 16, 2023

Conversation

Sauermann
Copy link
Contributor

@Sauermann Sauermann commented Feb 5, 2023

With the 4.x-introduction of Windows the previous method for color picking was no longer working.

Resolve #68979

This PR uses the following approach to reimplement color-picking:

  1. When the Color-Picking-Button is pressed, a screenshot of the Window-content is created and displayed in a new Popup-Window. (Areas outside of application Windows are interpreted as black color)
  2. This new Window allows selecting colors by Mouse-Click. A Preview of the targeted Color is also displayed.

Other failed attempts to solve this include:

  • Simple replacement of Control *screen by Popup *screen: failed because Popup is not reliably transparent (Platform DisplayServer limitation).
  • Use Drag&Drop-mechanic: might be possible, but the Input-mechanic for this becomes very complex and in effect error-prone.

Alternative ideas for solving this issue are mentioned in #68979 (comment)

This PR currently has the following shortcomings, but they could be solved in later PRs:

  • Windows need to be sorted by visibility.
  • Non-default Transform2D of the Window can lead to wrong image-coordinates.
  • The created image-texture could be reused in later Color-Picking operations.
  • The created Popup-Nodes and Images linger in memory and could be removed after the Color-Picking operation.
  • The created Popup and its content could be visually improved.
  • Multiple Display Screens would likely need to be supported.

Updated: 2023-02-06: Fix also the embedded-use-case after #72409 got fixed
Updated: 2023-02-09: Fix merge conflict

@Sauermann Sauermann requested a review from a team as a code owner February 5, 2023 16:53
@Chaosus Chaosus added this to the 4.0 milestone Feb 6, 2023
@Sauermann Sauermann requested a review from a team as a code owner February 6, 2023 12:08
@Sauermann Sauermann changed the title Fix Color Picking for non-embedded Window Fix Color Picking Feb 6, 2023
@akien-mga akien-mga requested a review from bruvzg February 6, 2023 12:26
@Sauermann Sauermann force-pushed the fix-color-picking branch 2 times, most recently from ecd3a0e to 6b70579 Compare February 6, 2023 19:58
With the 4.x-introduction of Windows the previous method for
color picking was no longer working.

This PR uses the following approach to reintroduce color-picking.
When the Color-Picking-Button is pressed, a quasi-screenshot of the
Window-content is created and displayed in a new Popup-Window.
This new Window allows selecting colors by Mouse-Click.
A Preview of the targeted Color is also displayed.
@bruvzg
Copy link
Member

bruvzg commented Feb 13, 2023

Color picker probably needs a native API to get screen pixel added to the DisplayServer. This way it will be able to pick from other apps and avoid overcomplicated window sorting.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 13, 2023

I think it would be nice to have this PR merged, because I can confirm it fixes the issue (and picking is completely broken right now).

@akien-mga akien-mga merged commit b90d70d into godotengine:master Feb 16, 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 picking not working on Windows
5 participants