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

[3.x] Fix GLTF light import #53105

Merged
merged 1 commit into from
Sep 27, 2021
Merged

Conversation

aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Sep 26, 2021

This fixes a regression (#53019) from the changes to the GLTF module in #34193 which were then backported to 3.x. I'm making the 3.x PR first since the same numbers in the master branch produce different visuals, so I'll have to tweak those. This PR also adds documentation to the GLTFLight class. master version: #53109

For the record, here's why these values are the way they are. GLTF lights with no range specified behave like physical lights which have infinite range, so we default to INFINITY for the range (but this is later clamped to 4096 if making a node). For the color, default to white, which is the most sensible option, otherwise we'd have black lights by default, which do not lighten things. For the intensity, default to 1.0, since 0.0 means no light. For the spotlight angle, default to an eighth turn (on both sides, so the cone's edges are a quarter turn apart), rather than 0.0, which would mean the spotlight is emitting light in an area of zero size.

@fire
Copy link
Member

fire commented Sep 26, 2021

Test failure. Oh no.

@aaronfranke
Copy link
Member Author

@fire It's just doc changes (which is why I didn't see it until posting the PR). Patience, I'm working on it.

@aaronfranke aaronfranke force-pushed the 3.x-fix-gltf-light branch 3 times, most recently from 3a0f41b to 610f9b5 Compare September 26, 2021 22:57
@fire
Copy link
Member

fire commented Sep 26, 2021

Test strategy:

Open lights in expected Godot Engine version.

Post before and after shot. I expect like a black vs lit scene or something.

Let me know if you can think of others.

Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

image

Lights show as expected.

@akien-mga akien-mga merged commit 29b1d39 into godotengine:3.x Sep 27, 2021
@akien-mga
Copy link
Member

Thanks!

@aaronfranke aaronfranke deleted the 3.x-fix-gltf-light branch September 27, 2021 15:29
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.

3 participants