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

Normalize the glTF2 weights on export. #48360

Closed

Conversation

fire
Copy link
Member

@fire fire commented May 1, 2021

Resolve validator bugs on Mira model.

https://sketchfab.com/3d-models/mira-573a125189704a7685389ca6a13d7350

Take this model. Open it in Godot and then export.

Original report.

        "numErrors": 3,
        "numWarnings": 191,
        "numInfos": 6,
        "numHints": 0,

Node3D_report.json.txt

Final report.

        "numErrors": 0,
        "numWarnings": 1,
        "numInfos": 4,
        "numHints": 0,

Node3D_report.json.txt

See commit message for details.

@fire fire requested a review from a team as a code owner May 1, 2021 21:19
@fire fire force-pushed the gltf-bone-skinning-normalization branch 3 times, most recently from 0626ab5 to b0220c9 Compare May 1, 2021 21:48
@Calinou Calinou added bug cherrypick:3.x Considered for cherry-picking into a future 3.x release topic:3d topic:import labels May 1, 2021
@Calinou Calinou added this to the 4.0 milestone May 1, 2021
@Calinou Calinou added topic:export and removed cherrypick:3.x Considered for cherry-picking into a future 3.x release topic:import labels May 1, 2021
@fire
Copy link
Member Author

fire commented May 1, 2021

Asked SJ to review.

@fire fire force-pushed the gltf-bone-skinning-normalization branch from b0220c9 to b2dbd4b Compare May 1, 2021 22:06
Copy link
Contributor

@kalysti kalysti left a comment

Choose a reason for hiding this comment

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

Asked SJ to review.

your own fault xD

modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
@fire fire force-pushed the gltf-bone-skinning-normalization branch 2 times, most recently from e82d521 to 7969990 Compare May 2, 2021 20:12
@fire
Copy link
Member Author

fire commented May 2, 2021

@sboron Can you review again?

modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved
@fire fire force-pushed the gltf-bone-skinning-normalization branch 5 times, most recently from cf6385e to 8160afc Compare May 3, 2021 13:13
Remove max and min on weights because it was difficult and it was allowed.

Handle case of weight normalization.

Joints [2, 2, 2, 2] Weights [0.7, 0.3, 0.0, 0.0]

becomes

Joints [2, 0, 0, 0] Weights [1.0, 0.0, 0.0, 0.0]
@fire fire force-pushed the gltf-bone-skinning-normalization branch from 8160afc to 09d4411 Compare May 3, 2021 13:17
@fire
Copy link
Member Author

fire commented May 3, 2021

@sboron Can you review with <3?

@fire fire requested review from a team as code owners June 20, 2021 11:39
@madmiraal madmiraal removed request for a team June 21, 2021 10:01
@fire fire marked this pull request as draft August 9, 2021 08:19
@fire
Copy link
Member Author

fire commented Aug 9, 2021

Superseded by: #50019

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