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

If available, use the vertex colors in gltf #41149

Closed
wants to merge 1 commit into from
Closed

If available, use the vertex colors in gltf #41149

wants to merge 1 commit into from

Conversation

capnm
Copy link
Contributor

@capnm capnm commented Aug 9, 2020

PR: #41007
Fixes #21087

This is #41007 for the 3.2 branch. Tested, works as intended.

@fire
Copy link
Member

fire commented Aug 10, 2020

I would reconsider merging this for 3.2. If content has a vertex color the material vertex color would now be on instead of off by default. This is what other gltf importers do. However, this behavior would a large noticeable change.

@capnm
Copy link
Contributor Author

capnm commented Aug 11, 2020

I would reconsider merging this for 3.2. If content has a vertex color the material vertex color would now be on instead of off by default. This is what other gltf importers do. However, this behavior would a large noticeable change.

I think correctly viewed, this is not an enhancement but a fatal error ;) When you export vertex colors to your glTF file, you probably expect them to be displayed without further ado. Think you have 100 objects where you have to enable an option by hand so that they are displayed correctly. Is a nightmare at least for me.

@akien-mga
Copy link
Member

Thanks for the PR, but in such cases it's preferred to use git cherry-pick -x <hash> instead of redoing the commit manually. The former preserves the author information and commit log, and adds a reference to the master commit that is being cherry-picked. This is useful to be able to go back from the cherry-picked commit to the original commit/PR that implemented it, and relevant discussion.

It then looks like this:

commit 13f6efdcc76ce50071e62416049c4673e111e06d (HEAD -> 3.2)
Author:     K. S. Ernest (iFire) Lee <[email protected]>
AuthorDate: Mon Aug 3 11:42:34 2020 -0700
Commit:     Rémi Verschelde <[email protected]>
CommitDate: Tue Sep 29 09:23:10 2020 +0200

    Use the vertex colors by default in gltf.
    
    (cherry picked from commit 43424e132173553201cd26f5aca064ad30e96bda)

@akien-mga akien-mga closed this Sep 29, 2020
@capnm capnm deleted the capnm-32x-pr41007 branch September 29, 2020 09:25
@akien-mga akien-mga modified the milestones: 3.2, 3.3 Apr 21, 2021
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