From cb76a49b824c868931e5f4b505bf175d1c207c12 Mon Sep 17 00:00:00 2001 From: Par Winzell Date: Sun, 8 Apr 2018 15:47:27 -0700 Subject: [PATCH 1/9] List all sources in CMakeLists.txt. If nothing else, it helps IDEs do the right thing. --- CMakeLists.txt | 48 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9362264..1d3bb90 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -130,28 +130,48 @@ if (APPLE) endif() set(SOURCE_FILES - src/utils/File_Utils.cpp - src/utils/Image_Utils.cpp - src/utils/String_Utils.cpp - src/main.cpp + src/FBX2glTF.h src/Fbx2Raw.cpp + src/Fbx2Raw.h src/Raw2Gltf.cpp + src/Raw2Gltf.h src/RawModel.cpp - src/glTF/BufferData.cpp - src/glTF/MaterialData.cpp - src/glTF/MeshData.cpp - src/glTF/NodeData.cpp - src/glTF/PrimitiveData.cpp - src/glTF/BufferViewData.cpp - src/glTF/BufferViewData.h + src/RawModel.h src/glTF/AccessorData.cpp src/glTF/AccessorData.h - src/glTF/ImageData.cpp - src/glTF/TextureData.cpp - src/glTF/SkinData.cpp src/glTF/AnimationData.cpp + src/glTF/AnimationData.h + src/glTF/BufferData.cpp + src/glTF/BufferData.h + src/glTF/BufferViewData.cpp + src/glTF/BufferViewData.h src/glTF/CameraData.cpp + src/glTF/CameraData.h + src/glTF/ImageData.cpp + src/glTF/ImageData.h + src/glTF/MaterialData.cpp + src/glTF/MaterialData.h + src/glTF/MeshData.cpp + src/glTF/MeshData.h + src/glTF/NodeData.cpp + src/glTF/NodeData.h + src/glTF/PrimitiveData.cpp + src/glTF/PrimitiveData.h + src/glTF/SamplerData.h src/glTF/SceneData.cpp + src/glTF/SceneData.h + src/glTF/SkinData.cpp + src/glTF/SkinData.h + src/glTF/TextureData.cpp + src/glTF/TextureData.h + src/main.cpp + src/mathfu.h + src/utils/File_Utils.cpp + src/utils/File_Utils.h + src/utils/Image_Utils.cpp + src/utils/Image_Utils.h + src/utils/String_Utils.cpp + src/utils/String_Utils.h ) add_executable(FBX2glTF ${SOURCE_FILES}) From 68983ad0d02d5ce4ea7b63f7964cb63743234ad7 Mon Sep 17 00:00:00 2001 From: Par Winzell Date: Sun, 8 Apr 2018 17:54:42 -0700 Subject: [PATCH 2/9] Fix skinning edge case. A mesh with a single (skinning) deformer which had zero clusters would erroneously register as skinned, leading GetRoodNode() to an assertion failure. Fixed. --- src/Fbx2Raw.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Fbx2Raw.cpp b/src/Fbx2Raw.cpp index 671db6c..2b072d0 100644 --- a/src/Fbx2Raw.cpp +++ b/src/Fbx2Raw.cpp @@ -401,12 +401,14 @@ public: for (int deformerIndex = 0; deformerIndex < pMesh->GetDeformerCount(); deformerIndex++) { FbxSkin *skin = reinterpret_cast< FbxSkin * >( pMesh->GetDeformer(deformerIndex, FbxDeformer::eSkin)); if (skin != nullptr) { + const int clusterCount = skin->GetClusterCount(); + if (clusterCount == 0) { + continue; + } int controlPointCount = pMesh->GetControlPointsCount(); - vertexJointIndices.resize(controlPointCount, Vec4i(0, 0, 0, 0)); vertexJointWeights.resize(controlPointCount, Vec4f(0.0f, 0.0f, 0.0f, 0.0f)); - const int clusterCount = skin->GetClusterCount(); for (int clusterIndex = 0; clusterIndex < clusterCount; clusterIndex++) { FbxCluster *cluster = skin->GetCluster(clusterIndex); const int indexCount = cluster->GetControlPointIndicesCount(); From 447333a16a8a5518d2c760576191d660b7857ee1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dar=C3=ADo=20Here=C3=B1=C3=BA?= Date: Thu, 26 Apr 2018 07:29:01 -0300 Subject: [PATCH 3/9] Typo on string #239, #241, #244 --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index d317cb6..affb2d0 100644 --- a/README.md +++ b/README.md @@ -236,12 +236,12 @@ ratification process** Given the command line flag --pbr-metallic-roughness, we throw ourselves into the warm embrace of glTF 2.0's PBR preference. -As mentioned above, there is lilttle consensus in the world on how PBR should be +As mentioned above, there is little consensus in the world on how PBR should be represented in FBX. At present, we support only one format: Stingray PBS. This -is a featue that comes bundled with Maya, and any PBR model exported through +is a feature that comes bundled with Maya, and any PBR model exported through that route should be digested propertly by FBX2glTF. -(A happy note: Allegorithmic's Susbstance Painter also exports Stingray PBS, +(A happy note: Allegorithmic's Substance Painter also exports Stingray PBS, when hooked up to Maya.) ## Draco Compression From f530af8bf4f82c1816a406b4230c1852f4e87913 Mon Sep 17 00:00:00 2001 From: Par Winzell Date: Thu, 3 May 2018 08:21:37 -0700 Subject: [PATCH 4/9] Use std:: whenever possible. --- src/Fbx2Raw.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Fbx2Raw.cpp b/src/Fbx2Raw.cpp index 2b072d0..3f041e2 100644 --- a/src/Fbx2Raw.cpp +++ b/src/Fbx2Raw.cpp @@ -1263,7 +1263,7 @@ static void ReadAnimations(RawModel &raw, FbxScene *pScene) for (int targetIx = 0; targetIx < targetCount; targetIx++) { if (curve) { float result = findInInterval(influence, targetIx-1); - if (!isnan(result)) { + if (!std::isnan(result)) { // we're transitioning into targetIx channel.weights.push_back(result); hasMorphs = true; @@ -1271,7 +1271,7 @@ static void ReadAnimations(RawModel &raw, FbxScene *pScene) } if (targetIx != targetCount-1) { result = findInInterval(influence, targetIx); - if (!isnan(result)) { + if (!std::isnan(result)) { // we're transitioning AWAY from targetIx channel.weights.push_back(1.0f - result); hasMorphs = true; From 3c8cad47302baf00feadc3f1c9fba6a9c3aaacef Mon Sep 17 00:00:00 2001 From: Par Winzell Date: Thu, 3 May 2018 14:44:43 -0700 Subject: [PATCH 5/9] Fix crash from dangling surface reference. The condense operation recreates the vectors of surfaces, materials, textures and vertices so as to exclude anything that isn't referenced explicitly by a triangle. In the process, we must take care that references from other properties are cleared out. This fixes the case when a node references a mesh by id, and then the mesh is deleted because no triangle references. TODO: go through other properties and make sure the same problem doesn't exist there. It is also possible that these vectors should be replaced by maps, at least for the elements that (now) have unique IDs. --- src/RawModel.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/RawModel.cpp b/src/RawModel.cpp index 230b180..1353743 100644 --- a/src/RawModel.cpp +++ b/src/RawModel.cpp @@ -269,11 +269,20 @@ void RawModel::Condense() surfaces.clear(); + std::set survivingSurfaceIds; for (auto &triangle : triangles) { + int oldSurfaceIndex = triangle.surfaceIndex; const RawSurface &surface = oldSurfaces[triangle.surfaceIndex]; const int surfaceIndex = AddSurface(surface.name.c_str(), surface.id); surfaces[surfaceIndex] = surface; triangle.surfaceIndex = surfaceIndex; + survivingSurfaceIds.emplace(surface.id); + } + // clear out references to meshes that no longer exist + for (auto &node : nodes) { + if (node.surfaceId != 0 && survivingSurfaceIds.find(node.surfaceId) == survivingSurfaceIds.end()) { + node.surfaceId = 0; + } } } From e178a75be3b9727effbb41995a1888969cb25bf0 Mon Sep 17 00:00:00 2001 From: Par Winzell Date: Thu, 3 May 2018 18:51:35 -0700 Subject: [PATCH 6/9] Start tracking and using FBX2glTF version. --- src/FBX2glTF.h | 2 ++ src/Raw2Gltf.cpp | 2 +- src/main.cpp | 9 +++------ 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/FBX2glTF.h b/src/FBX2glTF.h index 27278eb..ebdfe77 100644 --- a/src/FBX2glTF.h +++ b/src/FBX2glTF.h @@ -18,6 +18,8 @@ #include #endif +const std::string FBX2GLTF_VERSION = "0.9.5"; + #include #include diff --git a/src/Raw2Gltf.cpp b/src/Raw2Gltf.cpp index b27ec90..708ca1b 100644 --- a/src/Raw2Gltf.cpp +++ b/src/Raw2Gltf.cpp @@ -1077,7 +1077,7 @@ ModelData *Raw2Gltf( json glTFJson { { "asset", { - { "generator", "FBX2glTF" }, + { "generator", "FBX2glTF v" + FBX2GLTF_VERSION }, { "version", "2.0" }}}, { "scene", rootScene.ix } }; diff --git a/src/main.cpp b/src/main.cpp index 595b184..021d9b7 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -34,7 +34,8 @@ int main(int argc, char *argv[]) { cxxopts::Options options( "FBX2glTF", - "FBX2glTF 2.0: Generate a glTF 2.0 representation of an FBX model."); + fmt::sprintf("FBX2glTF %s: Generate a glTF 2.0 representation of an FBX model.", FBX2GLTF_VERSION) + ); std::string inputPath; std::string outputPath; @@ -98,11 +99,7 @@ int main(int argc, char *argv[]) } if (options.count("version")) { - fmt::printf( - R"( -FBX2glTF version 2.0 -Copyright (c) 2016-2017 Oculus VR, LLC. -)"); + fmt::printf("FBX2glTF version %s\nCopyright (c) 2016-2017 Oculus VR, LLC.\n", FBX2GLTF_VERSION); return 0; } From 5788eea8ade2ebe03c30a12ec3e03b927555e313 Mon Sep 17 00:00:00 2001 From: Par Winzell Date: Thu, 3 May 2018 18:58:58 -0700 Subject: [PATCH 7/9] Don't tell me you did nothing. --- src/RawModel.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/RawModel.cpp b/src/RawModel.cpp index 1353743..148776d 100644 --- a/src/RawModel.cpp +++ b/src/RawModel.cpp @@ -350,7 +350,9 @@ void RawModel::TransformGeometry(ComputeNormalsOption normals) if (verboseOutput) { if (normals == ComputeNormalsOption::BROKEN) { - fmt::printf("Repaired %lu empty normals.\n", computedNormalsCount); + if (computedNormalsCount > 0) { + fmt::printf("Repaired %lu empty normals.\n", computedNormalsCount); + } } else { fmt::printf("Computed %lu normals.\n", computedNormalsCount); } From d3f9a269ba90659ea0a0e61521128f2b806e01db Mon Sep 17 00:00:00 2001 From: Par Winzell Date: Tue, 8 May 2018 09:01:54 -0700 Subject: [PATCH 8/9] Customisable Draco encoding options. --- CMakeLists.txt | 2 +- src/Raw2Gltf.cpp | 32 ++++++++++++++++++++++---------- src/RawModel.h | 14 ++++++++++++-- src/main.cpp | 26 +++++++++++++++++++++++++- 4 files changed, 60 insertions(+), 14 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1d3bb90..9c1cbf8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -27,7 +27,7 @@ endif() # DRACO ExternalProject_Add(Draco GIT_REPOSITORY https://github.com/google/draco - GIT_TAG 1.2.5 + GIT_TAG 1.3.1 PREFIX draco INSTALL_DIR CMAKE_ARGS diff --git a/src/Raw2Gltf.cpp b/src/Raw2Gltf.cpp index 708ca1b..6931ac4 100644 --- a/src/Raw2Gltf.cpp +++ b/src/Raw2Gltf.cpp @@ -827,7 +827,7 @@ ModelData *Raw2Gltf( && surfaceModel.GetVertexCount() > 65535); std::shared_ptr primitive; - if (options.useDraco) { + if (options.draco.enabled) { int triangleCount = surfaceModel.GetTriangleCount(); // initialize Draco mesh with vertex index information @@ -943,17 +943,29 @@ ModelData *Raw2Gltf( primitive->AddTarget(pAcc.get(), nAcc.get(), tAcc.get()); } } - if (options.useDraco) { + if (options.draco.enabled) { // Set up the encoder. draco::Encoder encoder; - // TODO: generalize / allow configuration - encoder.SetSpeedOptions(5, 5); - encoder.SetAttributeQuantization(draco::GeometryAttribute::POSITION, 14); - encoder.SetAttributeQuantization(draco::GeometryAttribute::TEX_COORD, 10); - encoder.SetAttributeQuantization(draco::GeometryAttribute::NORMAL, 10); - encoder.SetAttributeQuantization(draco::GeometryAttribute::COLOR, 8); - encoder.SetAttributeQuantization(draco::GeometryAttribute::GENERIC, 8); + if (options.draco.compressionLevel != -1) { + int dracoSpeed = 10 - options.draco.compressionLevel; + encoder.SetSpeedOptions(dracoSpeed, dracoSpeed); + } + if (options.draco.quantBitsPosition != -1) { + encoder.SetAttributeQuantization(draco::GeometryAttribute::POSITION, options.draco.quantBitsPosition); + } + if (options.draco.quantBitsTexCoord != -1) { + encoder.SetAttributeQuantization(draco::GeometryAttribute::TEX_COORD, options.draco.quantBitsTexCoord); + } + if (options.draco.quantBitsNormal != -1) { + encoder.SetAttributeQuantization(draco::GeometryAttribute::NORMAL, options.draco.quantBitsNormal); + } + if (options.draco.quantBitsColor != -1) { + encoder.SetAttributeQuantization(draco::GeometryAttribute::COLOR, options.draco.quantBitsColor); + } + if (options.draco.quantBitsGeneric != -1) { + encoder.SetAttributeQuantization(draco::GeometryAttribute::GENERIC, options.draco.quantBitsGeneric); + } draco::EncoderBuffer dracoBuffer; draco::Status status = encoder.EncodeMeshToBuffer(*primitive->dracoMesh, &dracoBuffer); @@ -1070,7 +1082,7 @@ ModelData *Raw2Gltf( if (options.useKHRMatUnlit) { extensionsUsed.push_back(KHR_MATERIALS_CMN_UNLIT); } - if (options.useDraco) { + if (options.draco.enabled) { extensionsUsed.push_back(KHR_DRACO_MESH_COMPRESSION); extensionsRequired.push_back(KHR_DRACO_MESH_COMPRESSION); } diff --git a/src/RawModel.h b/src/RawModel.h index ce3ab67..4fbf4d1 100644 --- a/src/RawModel.h +++ b/src/RawModel.h @@ -46,8 +46,18 @@ struct GltfOptions bool outputBinary { false }; /** If non-binary, whether to inline all resources, for a single (large) .glTF file. */ bool embedResources { false }; - /** Whether to use KHR_draco_mesh_compression to minimize static geometry size. */ - bool useDraco { false }; + + /** Whether and how to use KHR_draco_mesh_compression to minimize static geometry size. */ + struct { + bool enabled = false; + int compressionLevel = -1; + int quantBitsPosition = -1; + int quantBitsTexCoord = -1; + int quantBitsNormal = -1; + int quantBitsColor = -1; + int quantBitsGeneric = -1; + } draco; + /** Whether to use KHR_materials_unlit to extend materials definitions. */ bool useKHRMatUnlit { false }; /** Whether to populate the pbrMetallicRoughness substruct in materials. */ diff --git a/src/main.cpp b/src/main.cpp index 021d9b7..d608dd7 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -63,7 +63,25 @@ int main(int argc, char *argv[]) cxxopts::value>()) ( "d,draco", "Apply Draco mesh compression to geometries.", - cxxopts::value(gltfOptions.useDraco)) + cxxopts::value(gltfOptions.draco.enabled)) + ( + "draco-compression-level", "The compression level to tune Draco to, from 0 to 10. (default: 7)", + cxxopts::value(gltfOptions.draco.compressionLevel)) + ( + "draco-bits-for-positions", "How many bits to quantize position to. (default: 14)", + cxxopts::value(gltfOptions.draco.quantBitsPosition)) + ( + "draco-bits-for-uv", "How many bits to quantize UV coordinates to. (default: 10)", + cxxopts::value(gltfOptions.draco.quantBitsTexCoord)) + ( + "draco-bits-for-normals", "How many bits to quantize normals to. (default: 10)", + cxxopts::value(gltfOptions.draco.quantBitsNormal)) + ( + "draco-bits-for-colors", "How many bits to quantize color to. (default: 8)", + cxxopts::value(gltfOptions.draco.quantBitsColor)) + ( + "draco-bits-for-other", "How many bits to quantize other vertex attributes to to. (default: 8)", + cxxopts::value(gltfOptions.draco.quantBitsGeneric)) ( "compute-normals", "When to compute normals for vertices (never|broken|missing|always).", cxxopts::value>()) @@ -125,6 +143,12 @@ int main(int argc, char *argv[]) gltfOptions.usePBRMetRough = true; } + if (gltfOptions.draco.compressionLevel != -1 && + (gltfOptions.draco.compressionLevel < 1 || gltfOptions.draco.compressionLevel > 10)) { + fmt::printf("Draco compression level must lie in [1, 10].\n"); + return 0; + } + if (options.count("flip-u") > 0) { texturesTransforms.emplace_back([](Vec2f uv) { return Vec2f(1.0f - uv[0], uv[1]); }); } From 7995f7b341579e6a2866f323391061b30556a210 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A4r=20Winzell?= Date: Sun, 27 May 2018 13:28:49 -0700 Subject: [PATCH 9/9] Resolve _WIN32 isnan() fiasco completely. --- src/FBX2glTF.h | 7 +++++-- src/Fbx2Raw.cpp | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/FBX2glTF.h b/src/FBX2glTF.h index ebdfe77..53b57ce 100644 --- a/src/FBX2glTF.h +++ b/src/FBX2glTF.h @@ -11,8 +11,6 @@ #define __FBX2GLTF_H__ #if defined ( _WIN32 ) -// This can be a macro under Windows, confusing FMT -#undef isnan // Tell Windows not to define min() and max() macros #define NOMINMAX #include @@ -23,6 +21,11 @@ const std::string FBX2GLTF_VERSION = "0.9.5"; #include #include +#if defined ( _WIN32 ) +// this is defined in fbxmath.h +#undef isnan +#endif + #include "mathfu.h" #endif // !__FBX2GLTF_H__ diff --git a/src/Fbx2Raw.cpp b/src/Fbx2Raw.cpp index 3f041e2..a192d8e 100644 --- a/src/Fbx2Raw.cpp +++ b/src/Fbx2Raw.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include "FBX2glTF.h" #include "utils/File_Utils.h"