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

BUGZ-163: Empty Weights FBX fix #15545

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@SaracenOne
Copy link
Contributor

commented May 12, 2019

https://highfidelity.atlassian.net/browse/BUGZ-163

Forces unweighted vertices to be weighted to the root bone in FBX loader rather than the root of the model itself. This mimics the behavior found in Unity's FBX loader and can account for certain scuffed models which may have errors resulting in bones below the hip bone being deleted. Will provide an example model later which will demonstrate the differences in Unity's handling of the FBX model (which I believe uses the official Autodesk SDK) vs. HiFi's

@hifi-gustavo

This comment has been minimized.

@hifi-gustavo

This comment has been minimized.

@hifi-gustavo

This comment has been minimized.

Copy link
Contributor

commented May 12, 2019

Android build is available here. Quest build is available here

@hifi-gustavo

This comment has been minimized.

@hifi-gustavo

This comment has been minimized.

@SamGondelman SamGondelman changed the title Empty Weights FBX fix BUGZ-163: Empty Weights FBX fix May 13, 2019

@shanzzam shanzzam modified the milestones: v0.82.1, v0.83.0 May 13, 2019

@SaracenOne

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2019

Okay, the latest commit fixes the same bug but is much simpler.

@@ -1523,7 +1523,7 @@ HFMModel* FBXSerializer::extractHFMModel(const hifi::VariantHash& mapping, const
// this is a multi-mesh joint
const int WEIGHTS_PER_VERTEX = 4;
int numClusterIndices = extracted.mesh.vertices.size() * WEIGHTS_PER_VERTEX;
extracted.mesh.clusterIndices.fill(extracted.mesh.clusters.size() - 1, numClusterIndices);
extracted.mesh.clusterIndices.fill(clusterIDs.size() - 1, numClusterIndices);

This comment has been minimized.

Copy link
@SamGondelman

SamGondelman May 13, 2019

Contributor

Hm...why does this fix it? How is the last clusterID always the root bone?

This comment has been minimized.

Copy link
@SaracenOne

SaracenOne May 13, 2019

Author Contributor

I'm not 100% sure why, but I assume that clusterID only consists of the list of skeleton joints, hence why writing the top-level one seems to fix it. My assumption is that extracted.mesh.clusters also includes non-skeleton joints, hence why the top-level one seems to weight vertices to the mesh body object itself, at least that's what I was able to discern from my debugging of the system. I can't say entirely why it works, just that this DOES indeed fix the specific example I have with an avatar which has some unweighted vertices, and which gets imported into Unity correctly accounting for this discrepancy.

This comment has been minimized.

Copy link
@SamGondelman

SamGondelman May 14, 2019

Contributor

Hm...ok...I think I'd still prefer if we could detect the root index higher up and then make sure the last cluster points to that jointIndex. That seems safer, long term.

@hifi-gustavo

This comment has been minimized.

@hifi-gustavo

This comment has been minimized.

@hifi-gustavo

This comment has been minimized.

@hifi-gustavo

This comment has been minimized.

@ZappoMan ZappoMan modified the milestones: v0.83.0, v0.84.0 May 13, 2019

@ZappoMan

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

This isn't compiling on android, needs to be fixed before we can proceed.

@hifi-gustavo

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

Android build is available here. Quest build is available here

@hifi-gustavo

This comment has been minimized.

@hifi-gustavo

This comment has been minimized.

@hifi-gustavo

This comment has been minimized.

@hifi-gustavo

This comment has been minimized.

const int WEIGHTS_PER_VERTEX = 4;
int numClusterIndices = extracted.mesh.vertices.size() * WEIGHTS_PER_VERTEX;
extracted.mesh.clusterIndices.fill(extracted.mesh.clusters.size() - 1, numClusterIndices);
extracted.mesh.clusterIndices.fill(rootSkeletonJoint, numClusterIndices);

This comment has been minimized.

Copy link
@SamGondelman

SamGondelman May 14, 2019

Contributor

Sorry, I meant up on lines 1510 - 1518

@hifi-gustavo

This comment has been minimized.

Copy link
Contributor

commented May 14, 2019

Android build is available here. Quest build is available here

@hifi-gustavo

This comment has been minimized.

@hifi-gustavo

This comment has been minimized.

@ZappoMan

This comment has been minimized.

Copy link
Contributor

commented May 14, 2019

FBXSerializer.cpp:1529, GNU C Compiler 4 (gcc), Priority: Normal

unused variable ‘cluster’ [-Wunused-variable]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.