Fix multiple issues with the exporter

Adds a test which goes through every single glTF in a specific folder and tests if the re-export from fastgltf is valid
spnda · Jan 28, 2024 · b174a16 · b174a16
1 parent 60cf292
commit b174a16
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 16 deletions.
diff --git a/include/fastgltf/types.hpp b/include/fastgltf/types.hpp
@@ -393,6 +393,32 @@ namespace fastgltf {
 		auto idx = to_underlying(type) & 0xFF;
 		return accessorTypeNames[idx - 1];
 	}
+
+	constexpr std::string_view mimeTypeJpeg = "image/jpeg";
+	constexpr std::string_view mimeTypePng = "image/png";
+	constexpr std::string_view mimeTypeKtx = "image/ktx2";
+	constexpr std::string_view mimeTypeDds = "image/vnd-ms.dds";
+	constexpr std::string_view mimeTypeGltfBuffer = "application/gltf-buffer";
+	constexpr std::string_view mimeTypeOctetStream = "application/octet-stream";
+
+	constexpr std::string_view getMimeTypeString(MimeType mimeType) noexcept {
+		switch (mimeType) {
+			case MimeType::JPEG:
+				return mimeTypeJpeg;
+			case MimeType::PNG:
+				return mimeTypePng;
+			case MimeType::KTX2:
+				return mimeTypeKtx;
+			case MimeType::DDS:
+				return mimeTypeDds;
+			case MimeType::GltfBuffer:
+				return mimeTypeGltfBuffer;
+			case MimeType::OctetStream:
+				return mimeTypeOctetStream;
+			default:
+				return "";
+		}
+	}
 #pragma endregion
 
 #pragma region Containers

diff --git a/src/fastgltf.cpp b/src/fastgltf.cpp
@@ -63,13 +63,6 @@ namespace fg = fastgltf;
 namespace fs = std::filesystem;
 
 namespace fastgltf {
-    constexpr std::string_view mimeTypeJpeg = "image/jpeg";
-    constexpr std::string_view mimeTypePng = "image/png";
-    constexpr std::string_view mimeTypeKtx = "image/ktx2";
-    constexpr std::string_view mimeTypeDds = "image/vnd-ms.dds";
-    constexpr std::string_view mimeTypeGltfBuffer = "application/gltf-buffer";
-    constexpr std::string_view mimeTypeOctetStream = "application/octet-stream";
-
     constexpr std::uint32_t binaryGltfHeaderMagic = 0x46546C67; // ASCII for "glTF".
     constexpr std::uint32_t binaryGltfJsonChunkMagic = 0x4E4F534A;
     constexpr std::uint32_t binaryGltfDataChunkMagic = 0x004E4942;
@@ -793,6 +786,11 @@ fg::Error fg::validate(const fastgltf::Asset& asset) {
 		return false;
 	};
 
+	// From the spec: extensionsRequired is a subset of extensionsUsed. All values in extensionsRequired MUST also exist in extensionsUsed.
+	if (asset.extensionsRequired.size() > asset.extensionsUsed.size()) {
+		return Error::InvalidGltf;
+	}
+
 	for (const auto& accessor : asset.accessors) {
 		if (accessor.type == AccessorType::Invalid)
 			return Error::InvalidGltf;
@@ -3416,9 +3414,12 @@ fg::Expected<fg::Asset> fg::Parser::loadGltfBinary(GltfDataBuffer* buffer, fs::p
 
     BinaryGltfHeader header = {};
     read(&header, sizeof header);
-    if (header.magic != binaryGltfHeaderMagic || header.version != 2) {
+    if (header.magic != binaryGltfHeaderMagic) {
 	    return Expected<Asset>(Error::InvalidGLB);
     }
+	if (header.version != 2) {
+		return Expected<Asset>(Error::UnsupportedVersion);
+	}
     if (header.length >= buffer->allocatedSize) {
 	    return Expected<Asset>(Error::InvalidGLB);
     }
@@ -3685,7 +3686,11 @@ void fg::Exporter::writeBuffers(const Asset& asset, std::string& json) {
 			[&](const sources::URI& uri) {
 				json += std::string(R"("uri":")") + fg::escapeString(uri.uri.string()) + '"' + ',';
                 bufferPaths.emplace_back(std::nullopt);
-			}
+			},
+			[&](const sources::Fallback& fallback) {
+				json += R"("extensions":{"EXT_meshopt_compression":{"fallback":true}},)";
+				bufferPaths.emplace_back(std::nullopt);
+			},
 		}, it->data);
 
 		json += "\"byteLength\":" + std::to_string(it->byteLength);
@@ -3800,7 +3805,7 @@ void fg::Exporter::writeCameras(const Asset& asset, std::string& json) {
 				json += "\"xmag\":" + std::to_string(orthographic.xmag) + ',';
 				json += "\"ymag\":" + std::to_string(orthographic.ymag) + ',';
 				json += "\"zfar\":" + std::to_string(orthographic.zfar) + ',';
-				json += "\"znear\":" + std::to_string(orthographic.znear) + ',';
+				json += "\"znear\":" + std::to_string(orthographic.znear);
 				json += R"(},"type":"orthographic")";
 			}
 		}, it->camera);
@@ -3831,7 +3836,8 @@ void fg::Exporter::writeImages(const Asset& asset, std::string& json) {
 				errorCode = Error::InvalidGltf;
 			},
             [&](const sources::BufferView& bufferView) {
-                json += std::string(R"("bufferView":)") + std::to_string(bufferView.bufferViewIndex) + '"';
+                json += std::string(R"("bufferView":)") + std::to_string(bufferView.bufferViewIndex) + ',';
+				json += std::string(R"("mimeType":")") + std::string(getMimeTypeString(bufferView.mimeType)) + '"';
                 imagePaths.emplace_back(std::nullopt);
             },
             [&](const sources::Vector& vector) {
@@ -3860,7 +3866,7 @@ void fg::Exporter::writeLights(const Asset& asset, std::string& json) {
 	if (json.back() == ']' || json.back() == '}')
 		json += ',';
 
-	json += "\"KHR_lights_punctual\":{\"lights\":[";
+	json += R"("KHR_lights_punctual":{"lights":[)";
 	for (auto it = asset.lights.begin(); it != asset.lights.end(); ++it) {
 		json += '{';
 
@@ -3926,7 +3932,7 @@ void fg::Exporter::writeMaterials(const Asset& asset, std::string& json) {
 			json += R"("baseColorFactor":[)";
 			json += std::to_string(it->pbrData.baseColorFactor[0]) + ',' + std::to_string(it->pbrData.baseColorFactor[1]) + ',' +
 				std::to_string(it->pbrData.baseColorFactor[2]) + ',' + std::to_string(it->pbrData.baseColorFactor[3]);
-			json += "],";
+			json += "]";
 		}
 
 		if (it->pbrData.baseColorTexture.has_value()) {
@@ -4386,6 +4392,7 @@ void fg::Exporter::writeNodes(const Asset& asset, std::string& json) {
 		}, it->transform);
 
         if (!it->instancingAttributes.empty()) {
+			if (json.back() != '{') json += ',';
             json += R"("extensions":{"EXT_mesh_gpu_instancing":{"attributes":{)";
             for (auto ait = it->instancingAttributes.begin(); ait != it->instancingAttributes.end(); ++ait) {
                 json += '"' + std::string(ait->first) + "\":" + std::to_string(ait->second);
@@ -4618,22 +4625,32 @@ std::string fg::Exporter::writeJson(const fastgltf::Asset &asset) {
     outputString += '}';
 
 	// Write extension usage info
+	if (!asset.extensionsUsed.empty()) {
+		if (outputString.back() != '{') outputString += ',';
+		outputString += "\"extensionsUsed\":[";
+		for (auto it = asset.extensionsUsed.begin(); it != asset.extensionsUsed.end(); ++it) {
+			outputString += '\"' + *it + '\"';
+			if (std::distance(asset.extensionsUsed.begin(), it) + 1 < asset.extensionsUsed.size())
+				outputString += ',';
+		}
+		outputString += ']';
+	}
 	if (!asset.extensionsRequired.empty()) {
-		outputString += "\"extensionsRequired\":{";
+		if (outputString.back() != '{') outputString += ',';
+		outputString += "\"extensionsRequired\":[";
 		for (auto it = asset.extensionsRequired.begin(); it != asset.extensionsRequired.end(); ++it) {
 			outputString += '\"' + *it + '\"';
 			if (std::distance(asset.extensionsRequired.begin(), it) + 1 < asset.extensionsRequired.size())
 				outputString += ',';
 		}
-		outputString += '}';
+		outputString += ']';
 	}
 
     writeAccessors(asset, outputString);
     writeBuffers(asset, outputString);
     writeBufferViews(asset, outputString);
     writeCameras(asset, outputString);
     writeImages(asset, outputString);
-    writeLights(asset, outputString);
     writeMaterials(asset, outputString);
     writeMeshes(asset, outputString);
     writeNodes(asset, outputString);

diff --git a/tests/write_tests.cpp b/tests/write_tests.cpp
@@ -128,3 +128,53 @@ TEST_CASE("Test pretty-print", "[write-tests]") {
     fastgltf::prettyPrintJson(json);
 	REQUIRE(json == "{\n\t\"value\":5,\n\t\"thing\":{\n\t\t\n\t}\n}");
 }
+
+TEST_CASE("Test all local models and re-export them", "[write-tests]") {
+	// Enable all extensions
+	static constexpr auto requiredExtensions = static_cast<fastgltf::Extensions>(~0U);
+	fastgltf::Parser parser(requiredExtensions);
+
+	static std::filesystem::path folderPath = "";
+	if (folderPath.empty()) {
+		SKIP();
+	}
+
+	std::uint64_t testedAssets = 0;
+	for (const auto& entry : std::filesystem::recursive_directory_iterator(folderPath)) {
+		if (!entry.is_regular_file())
+			continue;
+		const auto& epath = entry.path();
+		if (!epath.has_extension())
+			continue;
+		if (epath.extension() != ".gltf" && epath.extension() != ".glb")
+			continue;
+
+		// Parse the glTF
+		fastgltf::GltfDataBuffer gltfDataBuffer;
+		gltfDataBuffer.loadFromFile(epath);
+		auto model = parser.loadGltf(&gltfDataBuffer, epath.parent_path());
+		if (model.error() == fastgltf::Error::UnsupportedVersion || model.error() == fastgltf::Error::UnknownRequiredExtension)
+			continue; // Skip any glTF 1.0 or 0.x files or glTFs with unsupported extensions.
+
+		REQUIRE(model.error() == fastgltf::Error::None);
+
+		REQUIRE(fastgltf::validate(model.get()) == fastgltf::Error::None);
+
+		// Re-export the glTF as an in-memory JSON
+		fastgltf::Exporter exporter;
+		auto exported = exporter.writeGltfJson(model.get());
+		REQUIRE(exported.error() == fastgltf::Error::None);
+
+		// Parse the re-generated glTF and validate
+		auto& exportedJson = exported.get().output;
+		fastgltf::GltfDataBuffer regeneratedJson;
+		regeneratedJson.copyBytes(reinterpret_cast<const uint8_t*>(exportedJson.data()), exportedJson.size());
+		auto regeneratedModel = parser.loadGltf(&regeneratedJson, epath.parent_path());
+		REQUIRE(regeneratedModel.error() == fastgltf::Error::None);
+
+		REQUIRE(fastgltf::validate(regeneratedModel.get()) == fastgltf::Error::None);
+
+		++testedAssets;
+	}
+	std::printf("Successfully tested fastgltf exporter on %llu assets.", testedAssets);
+}