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] GLTF imports & exports material texture filters #66856

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

LunaticInAHat
Copy link
Contributor

This PR backports improvements to the GLTF import code from PR #59481 into the 3.x branch. Specifically, the importer is updated such that imported materials have the correct texture filter modes set (with respect to the sampling specified in the imported file).

Note that the logic and implementation of this PR aren't exactly the same as in the 4.x branch, due to some shifts in responsibility for filtering between the Texture and Material classes, however it targets the same behavior, and has the same general structure.

@LunaticInAHat LunaticInAHat requested a review from a team as a code owner October 4, 2022 01:06
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.

Please do extensive testing on this for godot 3. I haven't checked the godot 3 code in like a year.

@Sirus97
Copy link

Sirus97 commented Oct 4, 2022

It works on the Windows build.

image
image

@akien-mga akien-mga modified the milestones: 3.x, 3.6 Oct 4, 2022
@akien-mga akien-mga merged commit 9bf589d into godotengine:3.x Oct 4, 2022
@akien-mga
Copy link
Member

Thanks!

@LunaticInAHat
Copy link
Contributor Author

I have unfortunately just found an asset that this code does not work properly for. I will be opening a PR with a fix shortly, once I've convinced myself that I've finally got things right. My apologies for not discovering this sooner.

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.

6 participants