Make blend shape normals/tangents opt-in. (#40)

The FBX SDK absolutely claims that there is a normal layer to each
FbxShape, with non-trivial data, even when the corresponding FBX file,
upon visual inspection, explicitly contains nothing but zeroes. The only
conclusion I can draw is that the SDK is computing normals from
geometry, without being asked to, which seems kind of sketchy.

These computed normals are often not at all what the artist wanted, they
take up a lot of space -- often pointlessly, since if they're computed,
we could just as well compute them on the client -- and at least in the
case of three.js their inclusion uses up many of the precious 8 morph
target slots in the shader.

So, they are now opt-in, at least until we can solve the mystery of just
what goes on under the hood in the SDK.
This commit is contained in:
Pär Winzell 2017-11-14 14:36:44 -08:00 committed by GitHub
parent e73c2f46e2
commit 5eb36702b5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 33 additions and 10 deletions

View File

@ -48,6 +48,10 @@ Usage:
--pbr-specular-glossiness --pbr-specular-glossiness
(WIP) Experimentally fill in the (WIP) Experimentally fill in the
KHR_materials_pbrSpecularGlossiness extension. KHR_materials_pbrSpecularGlossiness extension.
--blend-shape-normals Include blend shape normals, if reported
present by the FBX SDK.
--blend-shape-tangents Include blend shape tangents, if reported
present by the FBX SDK.
-k, --keep-attribute arg Used repeatedly to build a limiting set of -k, --keep-attribute arg Used repeatedly to build a limiting set of
vertex attributes to keep. vertex attributes to keep.
-v, --verbose Enable verbose output. -v, --verbose Enable verbose output.
@ -81,6 +85,13 @@ Some of these switches are not obvious:
the conversion process. This is a way to trim the size of the resulting glTF the conversion process. This is a way to trim the size of the resulting glTF
if you know the FBX contains superfluous attributes. The supported arguments if you know the FBX contains superfluous attributes. The supported arguments
are `position`, `normal`, `tangent`, `color`, `uv0`, and `uv1`. are `position`, `normal`, `tangent`, `color`, `uv0`, and `uv1`.
- When **blend shapes** are present, you may use `--blend-shape-normals` and
`--blend-shape-tangents` to include normal and tangent attributes in the glTF
morph targets. They are not included by default because they rarely or never
seem to be correctly present in the actual FBX source, which means the SDK
must be computing them from geometry, unasked? In any case, they are beyond
the control of the artist, and can yield strange crinkly behaviour. Since
they also take up significant space in the output file, we made them opt-in.
## Building it on your own ## Building it on your own

View File

@ -619,10 +619,10 @@ ModelData *Raw2Gltf(
auto blendVertex = surfaceModel.GetVertex(jj).blends[channelIx]; auto blendVertex = surfaceModel.GetVertex(jj).blends[channelIx];
shapeBounds.AddPoint(blendVertex.position); shapeBounds.AddPoint(blendVertex.position);
positions.push_back(blendVertex.position); positions.push_back(blendVertex.position);
if (channel.hasNormals) { if (options.useBlendShapeTangents && channel.hasNormals) {
normals.push_back(blendVertex.normal); normals.push_back(blendVertex.normal);
} }
if (channel.hasTangents) { if (options.useBlendShapeTangents && channel.hasTangents) {
tangents.push_back(blendVertex.tangent); tangents.push_back(blendVertex.tangent);
} }
} }
@ -633,14 +633,14 @@ ModelData *Raw2Gltf(
pAcc->max = toStdVec(shapeBounds.max); pAcc->max = toStdVec(shapeBounds.max);
std::shared_ptr<AccessorData> nAcc; std::shared_ptr<AccessorData> nAcc;
if (channel.hasNormals) { if (!normals.empty()) {
nAcc = gltf->AddAccessorWithView( nAcc = gltf->AddAccessorWithView(
*gltf->GetAlignedBufferView(buffer, BufferViewData::GL_ARRAY_BUFFER), *gltf->GetAlignedBufferView(buffer, BufferViewData::GL_ARRAY_BUFFER),
GLT_VEC3F, normals); GLT_VEC3F, normals);
} }
std::shared_ptr<AccessorData> tAcc; std::shared_ptr<AccessorData> tAcc;
if (channel.hasTangents) { if (!tangents.empty()) {
nAcc = gltf->AddAccessorWithView( nAcc = gltf->AddAccessorWithView(
*gltf->GetAlignedBufferView(buffer, BufferViewData::GL_ARRAY_BUFFER), *gltf->GetAlignedBufferView(buffer, BufferViewData::GL_ARRAY_BUFFER),
GLT_VEC4F, tangents); GLT_VEC4F, tangents);

View File

@ -53,6 +53,10 @@ struct GltfOptions
bool usePBRMetRough; bool usePBRMetRough;
/** Whether to use KHR_materials_pbrSpecularGlossiness to extend material definitions. */ /** Whether to use KHR_materials_pbrSpecularGlossiness to extend material definitions. */
bool usePBRSpecGloss; bool usePBRSpecGloss;
/** Whether to include blend shape normals, if present according to the SDK. */
bool useBlendShapeNormals;
/** Whether to include blend shape tangents, if present according to the SDK. */
bool useBlendShapeTangents;
}; };
struct ComponentType { struct ComponentType {

View File

@ -49,7 +49,9 @@ int main(int argc, char *argv[])
false, // useDraco false, // useDraco
false, // useKHRMatCom false, // useKHRMatCom
false, // usePBRMetRough false, // usePBRMetRough
false // usePBRSpecGloss false, // usePBRSpecGloss
false, // useBlendShapeNormals
false, // useBlendShapeTangents
}; };
options.positional_help("[<FBX File>]"); options.positional_help("[<FBX File>]");
@ -81,6 +83,12 @@ int main(int argc, char *argv[])
( (
"pbr-specular-glossiness", "(WIP) Experimentally fill in the KHR_materials_pbrSpecularGlossiness extension.", "pbr-specular-glossiness", "(WIP) Experimentally fill in the KHR_materials_pbrSpecularGlossiness extension.",
cxxopts::value<bool>(gltfOptions.usePBRSpecGloss)) cxxopts::value<bool>(gltfOptions.usePBRSpecGloss))
(
"blend-shape-normals", "Include blend shape normals, if reported present by the FBX SDK.",
cxxopts::value<bool>(gltfOptions.useBlendShapeNormals))
(
"blend-shape-tangents", "Include blend shape tangents, if reported present by the FBX SDK.",
cxxopts::value<bool>(gltfOptions.useBlendShapeTangents))
( (
"k,keep-attribute", "Used repeatedly to build a limiting set of vertex attributes to keep.", "k,keep-attribute", "Used repeatedly to build a limiting set of vertex attributes to keep.",
cxxopts::value<std::vector<std::string>>()) cxxopts::value<std::vector<std::string>>())