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 misleading light occlusion preview #102198

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

huwpascoe
Copy link
Contributor

Deletes the preview shape raised in #102160

The preview shape was not fit for purpose

  • The fill/line color was both misleading and hard to see
  • Couldn't preview how the result is supposed to look
  • Designers work with the actual lighting visible anyway
  • The selected outline already covers that functionality:
    image

@huwpascoe huwpascoe requested a review from a team as a code owner January 30, 2025 17:38
@Calinou Calinou added this to the 4.x milestone Jan 30, 2025
@AThousandShips AThousandShips changed the title fix misleading light occlusion preview Fix misleading light occlusion preview Jan 31, 2025
@KoBeWi
Copy link
Member

KoBeWi commented Feb 1, 2025

Some people might be unhappy with their occluders suddenly becoming invisible. Maybe we should have a property for whether the preview should draw or not.

@huwpascoe
Copy link
Contributor Author

Some people might be unhappy with their occluders suddenly becoming invisible. Maybe we should have a property for whether the preview should draw or not.

If that's the case then it should be promoted to a viewport overlay, with configurable color in editor settings.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 2, 2025

It can be property of the node. We have a couple of editor-only properties, so it's not really a problem.

@clayjohn
Copy link
Member

clayjohn commented Feb 2, 2025

I agree with KoBeWi, we need an indicator to show where occluders are. We just need to make it not look like a shadow.

@Calinou
Copy link
Member

Calinou commented Feb 3, 2025

Occluders can be displayed as an outline, but at the same time, if we want to only display gizmos for selected nodes, the orange outline already serves that purpose.

In general, following godotengine/godot-proposals#6564, there's been a push to only show gizmos of visual-oriented nodes when the node in question is selected to reduce visual clutter.

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.

4 participants