Fix existing "extensions" were overwritten in serialization when the scene contains lights(serialized as "KHR_light_punctual")

Append "KHR_light_punctual" to `extensionsUsed` if not exist in serialization.
Apply clang-format.
Fixes #261

models/Extensions-overwrite-issue261/issue-261.gltf

@@ -0,0 +1,376 @@
+{
+  "accessors": [
+    {
+      "bufferView": 0,
+      "componentType": 5126,
+      "count": 8,
+      "max": [
+        0.5,
+        0.5,
+        0.5
+      ],
+      "min": [
+        -0.5,
+        -0.5,
+        -0.5
+      ],
+      "type": "VEC3"
+    },
+    {
+      "bufferView": 1,
+      "componentType": 5125,
+      "count": 36,
+      "type": "SCALAR"
+    }
+  ],
+  "asset": {
+    "copyright": "NVIDIA Corporation",
+    "generator": "Iray glTF plugin",
+    "version": "2.0"
+  },
+  "bufferViews": [
+    {
+      "buffer": 0,
+      "byteLength": 96,
+      "byteStride": 12
+    },
+    {
+      "buffer": 0,
+      "byteLength": 144,
+      "byteOffset": 96
+    }
+  ],
+  "buffers": [
+    {
+      "byteLength": 240,
+      "uri": "issue-261.bin"
+    }
+  ],
+  "cameras": [
+    {
+      "extensions": {
+        "NV_Iray": {
+          "mip_burn_highlights": 0.699999988079071,
+          "mip_burn_highlights_per_component": false,
+          "mip_camera_shutter": 4.0,
+          "mip_cm2_factor": 1.0,
+          "mip_crush_blacks": 0.5,
+          "mip_f_number": 1.0,
+          "mip_film_iso": 100.0,
+          "mip_gamma": 2.200000047683716,
+          "mip_saturation": 1.0,
+          "mip_vignetting": 0.00019999999494757503,
+          "mip_whitepoint": [
+            1.0,
+            1.0,
+            1.0,
+            1.0
+          ],
+          "tm_enable_tonemapper": true,
+          "tm_tonemapper": "mia_exposure_photographic"
+        }
+      },
+      "extras": {
+        "resolution": [
+          640,
+          480
+        ]
+      },
+      "name": "default",
+      "perspective": {
+        "aspectRatio": 1.3333333730697632,
+        "yfov": 0.9272952079772949,
+        "zfar": 1000.0,
+        "znear": 0.1
+      },
+      "type": "perspective"
+    }
+  ],
+  "extensions": {
+    "KHR_lights_punctual": {
+      "lights": [
+        {
+          "color": [
+            1.0,
+            1.0,
+            1.0
+          ],
+          "intensity": 1000.0,
+          "name": "light",
+          "type": "point"
+        }
+      ]
+    },
+    "NV_MDL": {
+      "modules": [
+        "mdl::base",
+        "mdl::nvidia::core_definitions",
+        "mdl::state"
+      ],
+      "shaders": [
+        {
+          "definition": "mdl::base::environment_spherical(texture_2d)",
+          "name": "env_shd"
+        },
+        {
+          "arguments": {
+            "base_color": [
+              0.0055217444896698,
+              0.36859095096588135,
+              0.0041161770932376385
+            ],
+            "normal=": 2
+          },
+          "definition": "mdl::nvidia::core_definitions::flex_material",
+          "name": "cube_instance_material"
+        },
+        {
+          "definition": "mdl::state::normal()",
+          "name": "mdl::state::normal_341"
+        },
+        {
+          "arguments": {
+            "base_color": [
+              0.0055217444896698,
+              0.36859095096588135,
+              0.0041161770932376385
+            ],
+            "normal=": 4
+          },
+          "definition": "mdl::nvidia::core_definitions::flex_material",
+          "name": "cube_instance_material"
+        },
+        {
+          "definition": "mdl::state::normal()",
+          "name": "mdl::state::normal_341"
+        }
+      ]
+    }
+  },
+  "extensionsUsed": [
+    "NV_MDL",
+    "NV_Iray",
+    "KHR_lights_punctual"
+  ],
+  "materials": [
+    {
+      "doubleSided": true,
+      "extras": {
+        "mdl_shader": 1
+      },
+      "name": "cube_instance_material",
+      "pbrMetallicRoughness": {
+        "baseColorFactor": [
+          0.0055217444896698,
+          0.36859095096588135,
+          0.0041161770932376385,
+          1.0
+        ]
+      }
+    }
+  ],
+  "meshes": [
+    {
+      "name": "cube",
+      "primitives": [
+        {
+          "attributes": {
+            "POSITION": 0
+          },
+          "indices": 1,
+          "material": 0,
+          "mode": 4
+        }
+      ]
+    }
+  ],
+  "nodes": [
+    {
+      "camera": 0,
+      "extensions": {
+        "NV_Iray": {
+          "iview:fkey": -1,
+          "iview:fov": 53.130104064941406,
+          "iview:interest": [
+            0.1342654824256897,
+            -0.14356137812137604,
+            0.037264324724674225
+          ],
+          "iview:position": [
+            0.9729073643684387,
+            1.2592438459396362,
+            2.4199187755584717
+          ],
+          "iview:roll": 0.0,
+          "iview:up": [
+            0.0,
+            1.0,
+            0.0
+          ]
+        }
+      },
+      "matrix": [
+        0.9432751389105357,
+        -1.1769874756875739e-16,
+        -0.3320120665176343,
+        0.0,
+        -0.16119596696756341,
+        0.8742297945345237,
+        -0.45797175303889276,
+        0.0,
+        0.290254840694694,
+        0.48551237507207207,
+        0.8246392308792816,
+        0.0,
+        0.9729073377709113,
+        1.2592438262175363,
+        2.419918748461627,
+        1.0
+      ],
+      "name": "CamInst"
+    },
+    {
+      "extensions": {
+        "NV_Iray": {
+          "caustic": true,
+          "caustic_cast": true,
+          "caustic_recv": true,
+          "face_back": true,
+          "face_front": true,
+          "finalgather": true,
+          "finalgather_cast": true,
+          "finalgather_recv": true,
+          "globillum": true,
+          "globillum_cast": true,
+          "globillum_recv": true,
+          "material=": 3,
+          "pickable": true,
+          "reflection_cast": true,
+          "reflection_recv": true,
+          "refraction_cast": true,
+          "refraction_recv": true,
+          "shadow_cast": true,
+          "shadow_recv": true,
+          "transparency_cast": true,
+          "transparency_recv": true,
+          "visible": true
+        }
+      },
+      "mesh": 0,
+      "name": "cube_instance"
+    },
+    {
+      "extensions": {
+        "KHR_lights_punctual": {
+          "light": 0
+        },
+        "NV_Iray": {
+          "shadow_cast": true,
+          "visible": false
+        }
+      },
+      "matrix": [
+        0.6988062355563571,
+        -7.76042172309776e-17,
+        -0.7153110128800992,
+        -0.0,
+        -0.4276881690736487,
+        0.8015668284138362,
+        -0.41781987700564616,
+        -0.0,
+        0.57336957992379,
+        0.5979051928078428,
+        0.5601398979107212,
+        0.0,
+        2.3640632834071384,
+        2.465226204472449,
+        2.309515908392224,
+        1.0
+      ],
+      "name": "light_inst"
+    }
+  ],
+  "scene": 0,
+  "scenes": [
+    {
+      "extensions": {
+        "NV_Iray": {
+          "CP_canny_threshold1": 40,
+          "CP_canny_threshold2": 120,
+          "CP_color_quantization": 8,
+          "IVP_color": [
+            1.0,
+            0.0,
+            0.0,
+            1.0
+          ],
+          "TM_drago_bias": 0.8500000238418579,
+          "TM_drago_gamma2": 2.200000047683716,
+          "TM_drago_saturation": 1.0,
+          "TM_durand_contrast": 4.0,
+          "TM_durand_gamma": 2.200000047683716,
+          "TM_durand_saturation": 1.0,
+          "TM_durand_sigma_color": 2.0,
+          "TM_durand_sigma_space": 2.0,
+          "TM_linear_gamma": 2.200000047683716,
+          "TM_reinhard_color_adapt": 0.8999999761581421,
+          "TM_reinhard_gamma": 1.0,
+          "TM_reinhard_intensity": 0.0,
+          "TM_reinhard_light_adapt": 1.0,
+          "TM_reye_Ywhite": 1000000.0,
+          "TM_reye_bsize": 2,
+          "TM_reye_bthres": 3.0,
+          "TM_reye_gamma": 2.200000047683716,
+          "TM_reye_key": 0.5,
+          "TM_reye_saturation": 1.0,
+          "TM_reye_whitebalance": [
+            1.0,
+            0.9965101480484009,
+            0.9805564880371094,
+            0.0
+          ],
+          "environment_dome_depth": 200.0,
+          "environment_dome_height": 200.0,
+          "environment_dome_mode": "infinite",
+          "environment_dome_position": [
+            0.0,
+            0.0,
+            0.0
+          ],
+          "environment_dome_radius": 100.0,
+          "environment_dome_rotation_angle": 0.0,
+          "environment_dome_width": 200.0,
+          "environment_function=": 0,
+          "environment_function_intensity": 1.9900000095367432,
+          "iray_instancing": "off",
+          "iview::inline_color": [
+            1.0,
+            0.0,
+            0.0,
+            1.0
+          ],
+          "iview::inline_width": 1.0,
+          "iview::magnifier_size": 300,
+          "iview::offset": 10.0,
+          "iview::outline_color": [
+            0.0,
+            0.0,
+            0.0,
+            1.0
+          ],
+          "iview::outline_width": 2.0,
+          "iview::overview": true,
+          "iview::zoom_factor": 1.0,
+          "samples": 4.0,
+          "shadow_cast": true,
+          "shadow_recv": true
+        }
+      },
+      "nodes": [
+        0,
+        1,
+        2
+      ]
+    }
+  ]
+}

tests/tester.cc

@@ -93,6 +93,59 @@ TEST_CASE("extension-with-empty-object", "[issue-97]") {
 
 }
 
+TEST_CASE("extension-overwrite", "[issue-261]") {
+
+  tinygltf::Model model;
+  tinygltf::TinyGLTF ctx;
+  std::string err;
+  std::string warn;
+
+  bool ret = ctx.LoadASCIIFromFile(&model, &err, &warn, "../models/Extensions-overwrite-issue261/issue-261.gltf");
+  if (!err.empty()) {
+    std::cerr << err << std::endl;
+  }
+  REQUIRE(true == ret);
+
+  REQUIRE(model.extensionsUsed.size() == 3);
+  {
+    bool has_ext_lights = false;
+    has_ext_lights |= (model.extensionsUsed[0].compare("KHR_lights_punctual") == 0);
+    has_ext_lights |= (model.extensionsUsed[1].compare("KHR_lights_punctual") == 0);
+    has_ext_lights |= (model.extensionsUsed[2].compare("KHR_lights_punctual") == 0);
+
+    REQUIRE(true == has_ext_lights);
+  }
+
+  {
+    REQUIRE(model.extensions.size() == 2);
+    REQUIRE(model.extensions.count("NV_MDL"));
+    REQUIRE(model.extensions.count("KHR_lights_punctual"));
+  }
+
+  // TODO(syoyo): create temp directory.
+  {
+    ret = ctx.WriteGltfSceneToFile(&model, "issue-261.gltf", true, true);
+    REQUIRE(true == ret);
+
+    tinygltf::Model m;
+
+    // read back serialized glTF
+    bool ret = ctx.LoadASCIIFromFile(&m, &err, &warn, "issue-261.gltf");
+    if (!err.empty()) {
+      std::cerr << err << std::endl;
+    }
+    REQUIRE(true == ret);
+
+    REQUIRE(m.extensionsUsed.size() == 3);
+
+    REQUIRE(m.extensions.size() == 2);
+    REQUIRE(m.extensions.count("NV_MDL"));
+    REQUIRE(m.extensions.count("KHR_lights_punctual"));
+
+  }
+
+}
+
 TEST_CASE("invalid-primitive-indices", "[bounds-checking]") {
   tinygltf::Model model;
   tinygltf::TinyGLTF ctx;

tiny_gltf.h

@@ -52,6 +52,7 @@
 
 #include <array>
 #include <cassert>
+#include <cmath>  // std::fabs
 #include <cstdint>
 #include <cstdlib>
 #include <cstring>
@@ -59,7 +60,6 @@
 #include <map>
 #include <string>
 #include <vector>
-#include <cmath> // std::fabs
 
 #ifndef TINYGLTF_USE_CPP14
 #include <functional>
@@ -2662,12 +2662,10 @@ bool WriteWholeFile(std::string *err, const std::string &filepath,
                     const std::vector<unsigned char> &contents, void *) {
 #ifdef _WIN32
 #if defined(__GLIBCXX__)  // mingw
-  int file_descriptor =
+  int file_descriptor = _wopen(UTF8ToWchar(filepath).c_str(),
-      _wopen(UTF8ToWchar(filepath).c_str(), _O_CREAT | _O_WRONLY |
+                               _O_CREAT | _O_WRONLY | _O_TRUNC | _O_BINARY);
-                                            _O_TRUNC | _O_BINARY);
+  __gnu_cxx::stdio_filebuf<char> wfile_buf(
-  __gnu_cxx::stdio_filebuf<char> wfile_buf(file_descriptor,
+      file_descriptor, std::ios_base::out | std::ios_base::binary);
-                                           std::ios_base::out |
-                                           std::ios_base::binary);
   std::ostream f(&wfile_buf);
 #elif defined(_MSC_VER)
   std::ofstream f(UTF8ToWchar(filepath).c_str(), std::ofstream::binary);
@@ -2722,8 +2720,7 @@ static void UpdateImageObject(Image &image, std::string &baseDir, int index,
   if (image.uri.size()) {
     filename = GetBaseFilename(image.uri);
     ext = GetFilePathExtension(filename);
-  }
+  } else if (image.bufferView != -1) {
-  else if (image.bufferView != -1) {
     // If there's no URI and the data exists in a buffer,
     // don't change properties or write images
   } else if (image.name.size()) {
@@ -5687,7 +5684,9 @@ bool TinyGLTF::LoadFromString(Model *model, std::string *err, std::string *warn,
 
       for (auto &target : primitive.targets) {
         for (auto &attribute : target) {
-          model->bufferViews[size_t(model->accessors[size_t(attribute.second)].bufferView)]
+          model
+              ->bufferViews[size_t(
+                  model->accessors[size_t(attribute.second)].bufferView)]
               .target = TINYGLTF_TARGET_ARRAY_BUFFER;
         }
       }
@@ -6423,12 +6422,10 @@ static bool SerializeGltfBufferData(const std::vector<unsigned char> &data,
                                     const std::string &binFilename) {
 #ifdef _WIN32
 #if defined(__GLIBCXX__)  // mingw
-  int file_descriptor =
+  int file_descriptor = _wopen(UTF8ToWchar(binFilename).c_str(),
-      _wopen(UTF8ToWchar(binFilename).c_str(), _O_CREAT | _O_WRONLY |
+                               _O_CREAT | _O_WRONLY | _O_TRUNC | _O_BINARY);
-                                               _O_TRUNC | _O_BINARY);
+  __gnu_cxx::stdio_filebuf<char> wfile_buf(
-  __gnu_cxx::stdio_filebuf<char> wfile_buf(file_descriptor,
+      file_descriptor, std::ios_base::out | std::ios_base::binary);
-                                           std::ios_base::out |
-                                           std::ios_base::binary);
   std::ostream output(&wfile_buf);
   if (!wfile_buf.is_open()) return false;
 #elif defined(_MSC_VER)
@@ -7087,8 +7084,7 @@ static void SerializeGltfCamera(const Camera &camera, json &o) {
     // ???
   }
 
-  if(camera.extras.Type() != NULL_TYPE)
+  if (camera.extras.Type() != NULL_TYPE) {
-  {
     SerializeValue("extras", camera.extras, o);
   }
   SerializeExtensionMap(camera.extensions, o);
@@ -7181,11 +7177,6 @@ static void SerializeGltfModel(Model *model, json &o) {
     JsonAddMember(o, "bufferViews", std::move(bufferViews));
   }
 
-  // Extensions used
-  if (model->extensionsUsed.size()) {
-    SerializeStringArrayProperty("extensionsUsed", model->extensionsUsed, o);
-  }
-
   // Extensions required
   if (model->extensionsRequired.size()) {
     SerializeStringArrayProperty("extensionsRequired",
@@ -7296,7 +7287,9 @@ static void SerializeGltfModel(Model *model, json &o) {
   // EXTENSIONS
   SerializeExtensionMap(model->extensions, o);
 
-  // LIGHTS as KHR_lights_cmn
+  auto extensionsUsed = model->extensionsUsed;
+
+  // LIGHTS as KHR_lights_punctual
   if (model->lights.size()) {
     json lights;
     JsonReserveArray(lights, model->lights.size());
@@ -7311,7 +7304,7 @@ static void SerializeGltfModel(Model *model, json &o) {
 
     {
       json_const_iterator it;
-      if (!FindMember(o, "extensions", it)) {
+      if (FindMember(o, "extensions", it)) {
         JsonAssign(ext_j, GetValue(it));
       }
     }
@@ -7319,6 +7312,23 @@ static void SerializeGltfModel(Model *model, json &o) {
     JsonAddMember(ext_j, "KHR_lights_punctual", std::move(khr_lights_cmn));
 
     JsonAddMember(o, "extensions", std::move(ext_j));
+
+    // Also add "KHR_lights_punctual" to `extensionsUsed`
+    {
+      auto has_khr_lights_punctual = std::find_if(
+          extensionsUsed.begin(), extensionsUsed.end(), [](const std::string &s) {
+            return (s.compare("KHR_lights_punctual") == 0);
+          });
+
+      if (has_khr_lights_punctual == extensionsUsed.end()) {
+        extensionsUsed.push_back("KHR_lights_punctual");
+      }
+    }
+  }
+
+  // Extensions used
+  if (model->extensionsUsed.size()) {
+    SerializeStringArrayProperty("extensionsUsed", extensionsUsed, o);
   }
 
   // EXTRAS
@@ -7338,12 +7348,10 @@ static bool WriteGltfFile(const std::string &output,
 #if defined(_MSC_VER)
   std::ofstream gltfFile(UTF8ToWchar(output).c_str());
 #elif defined(__GLIBCXX__)
-  int file_descriptor =
+  int file_descriptor = _wopen(UTF8ToWchar(output).c_str(),
-      _wopen(UTF8ToWchar(output).c_str(), _O_CREAT | _O_WRONLY |
+                               _O_CREAT | _O_WRONLY | _O_TRUNC | _O_BINARY);
-                                          _O_TRUNC | _O_BINARY);
+  __gnu_cxx::stdio_filebuf<char> wfile_buf(
-  __gnu_cxx::stdio_filebuf<char> wfile_buf(file_descriptor,
+      file_descriptor, std::ios_base::out | std::ios_base::binary);
-                                           std::ios_base::out |
-                                           std::ios_base::binary);
   std::ostream gltfFile(&wfile_buf);
   if (!wfile_buf.is_open()) return false;
 #else
@@ -7429,12 +7437,10 @@ static void WriteBinaryGltfFile(const std::string &output,
 #if defined(_MSC_VER)
   std::ofstream gltfFile(UTF8ToWchar(output).c_str(), std::ios::binary);
 #elif defined(__GLIBCXX__)
-  int file_descriptor =
+  int file_descriptor = _wopen(UTF8ToWchar(output).c_str(),
-      _wopen(UTF8ToWchar(output).c_str(), _O_CREAT | _O_WRONLY |
+                               _O_CREAT | _O_WRONLY | _O_TRUNC | _O_BINARY);
-                                          _O_TRUNC | _O_BINARY);
+  __gnu_cxx::stdio_filebuf<char> wfile_buf(
-  __gnu_cxx::stdio_filebuf<char> wfile_buf(file_descriptor,
+      file_descriptor, std::ios_base::out | std::ios_base::binary);
-                                           std::ios_base::out |
-                                           std::ios_base::binary);
   std::ostream gltfFile(&wfile_buf);
 #else
   std::ofstream gltfFile(output.c_str(), std::ios::binary);
@@ -7479,7 +7485,8 @@ bool TinyGLTF::WriteGltfSceneToStream(Model *model, std::ostream &stream,
 
       std::string dummystring = "";
       // UpdateImageObject need baseDir but only uses it if embeddedImages is
-	  // enabled, since we won't write separate images when writing to a stream we
+      // enabled, since we won't write separate images when writing to a stream
+      // we
       UpdateImageObject(model->images[i], dummystring, int(i), false,
                         &this->WriteImageData, this->write_image_user_data_);
       SerializeGltfImage(model->images[i], image);
@@ -7533,7 +7540,8 @@ bool TinyGLTF::WriteGltfSceneToFile(Model *model, const std::string &filename,
       } else {
         std::string binSavePath;
         std::string binUri;
-        if (!model->buffers[i].uri.empty() && !IsDataURI(model->buffers[i].uri)) {
+        if (!model->buffers[i].uri.empty() &&
+            !IsDataURI(model->buffers[i].uri)) {
           binUri = model->buffers[i].uri;
         } else {
           binUri = defaultBinFilename + defaultBinFileExt;