Merge topic 'improve_gltf_image_error_codepath'

11a016ac GLTFImporter: Add testing for invalid image
4a1bdcab GLTFImporter: Fix a potential when creating textures

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: buildbot <buildbot@kitware.com>
Reviewed-by: Ryan Krattiger <ryan.krattiger@kitware.com>
Merge-request: !11385

IO/Import/Testing/Cxx/CMakeLists.txt

@@ -68,11 +68,21 @@ vtk_add_test_cxx(vtkIOImportCxxTests tests
   TestOBJImporter-Malformed,TestOBJImporter.cxx,NO_VALID
     DATA{../Data/Input/malformed.obj}
 )
+
+# Test an file with an invalid image
+vtk_add_test_cxx(vtkIOImportCxxTests tests
+  TestGLTFImporter-Invalid,TestGLTFImporterInvalid.cxx,NO_VALID
+    DATA{../Data/glTF/invalid.gltf}
+    DATA{../Data/glTF/Duck0.bin}
+)
+
 # (vtk/vtk#19097) This test is skipped on wasm architecture.
 if (NOT CMAKE_SYSTEM_NAME STREQUAL "Emscripten") 
   set_tests_properties(VTK::IOImportCxx-TestOBJImporter-Malformed PROPERTIES PASS_REGULAR_EXPRESSION "Unexpected point indice value")
   set_tests_properties(VTK::IOImportCxx-TestOBJImporter-Malformed PROPERTIES FAIL_REGULAR_EXPRESSION "")
 endif ()
+  set_tests_properties(VTK::IOImportCxx-TestGLTFImporter-Invalid PROPERTIES PASS_REGULAR_EXPRESSION "Image mimeType not supported")
+  set_tests_properties(VTK::IOImportCxx-TestGLTFImporter-Invalid PROPERTIES FAIL_REGULAR_EXPRESSION "")
 
 vtk_add_test_cxx(vtkIOImportCxxTests tests
   TestVRMLImporter.cxx,NO_VALID

IO/Import/Testing/Cxx/TestGLTFImporterInvalid.cxx

+// SPDX-FileCopyrightText: Copyright (c) Ken Martin, Will Schroeder, Bill Lorensen
+// SPDX-License-Identifier: BSD-3-Clause
+
+#include "vtkGLTFImporter.h"
+#include "vtkNew.h"
+#include "vtkRenderWindow.h"
+#include "vtkRenderWindowInteractor.h"
+#include "vtkRenderer.h"
+
+//------------------------------------------------------------------------------
+int TestGLTFImporterInvalid(int argc, char* argv[])
+{
+  if (argc < 2)
+  {
+    std::cout << "expected testname file1.gltf" << std::endl;
+    return EXIT_FAILURE;
+  }
+
+  // Just not segfaulting is considered a success
+  vtkNew<vtkGLTFImporter> importer;
+  importer->SetFileName(argv[1]);
+
+  vtkNew<vtkRenderWindow> renderWindow;
+  importer->SetRenderWindow(renderWindow);
+
+  vtkNew<vtkRenderer> renderer;
+  renderWindow->AddRenderer(renderer);
+  renderer->SetBackground(.0, .0, .2);
+
+  vtkNew<vtkRenderWindowInteractor> renderWindowInteractor;
+  renderWindowInteractor->SetRenderWindow(renderWindow);
+
+  importer->Update();
+  return EXIT_SUCCESS;
+}

IO/Import/Testing/Data/glTF/Duck0.bin.sha512

+713f861afe182eb509d1fa283ac442e15142d9659188bc13053321b7eb2ca7a174b6bdee1094c076810cac00f804c166083f19fa05056490d664006b6264db44

IO/Import/Testing/Data/glTF/invalid.gltf.sha512

+e41e02224b7e71b26b44dc345b847d525c143e5174f72b42fdc99cb745bce305dc8f179a881c424fea48b9ea442004cf3df5bcb50545a897803d7bbeed053e2c

IO/Import/vtkGLTFImporter.cxx

@@ -192,7 +192,7 @@ bool PrimitiveNeedsTangents(const std::shared_ptr<vtkGLTFDocumentLoader::Model>
 }
 
 //------------------------------------------------------------------------------
-void ApplyGLTFMaterialToVTKActor(std::shared_ptr<vtkGLTFDocumentLoader::Model> model,
+bool ApplyGLTFMaterialToVTKActor(std::shared_ptr<vtkGLTFDocumentLoader::Model> model,
   vtkGLTFDocumentLoader::Primitive& primitive, vtkSmartPointer<vtkActor> actor,
   std::map<int, vtkSmartPointer<vtkTexture>>& existingTextures, vtkGLTFImporter* parent)
 {
@@ -240,6 +240,10 @@ void ApplyGLTFMaterialToVTKActor(std::shared_ptr<vtkGLTFDocumentLoader::Model> m
     // set albedo texture
     vtkSmartPointer<vtkTexture> baseColorTex;
     baseColorTex = CreateVTKTextureFromGLTFTexture(model, texIndex, existingTextures, parent);
+    if (!baseColorTex)
+    {
+      return false;
+    }
     baseColorTex->UseSRGBColorSpaceOn();
     property->SetBaseColorTexture(baseColorTex);
   }
@@ -300,6 +304,10 @@ void ApplyGLTFMaterialToVTKActor(std::shared_ptr<vtkGLTFDocumentLoader::Model> m
       }
       auto materialTex =
         CreateVTKTextureFromGLTFTexture(model, pbrTexIndex, existingTextures, parent);
+      if (!materialTex)
+      {
+        return false;
+      }
       property->SetORMTexture(materialTex);
     }
   }
@@ -310,6 +318,10 @@ void ApplyGLTFMaterialToVTKActor(std::shared_ptr<vtkGLTFDocumentLoader::Model> m
   {
     auto emissiveTex =
       CreateVTKTextureFromGLTFTexture(model, emissiveTexIndex, existingTextures, parent);
+    if (!emissiveTex)
+    {
+      return false;
+    }
     emissiveTex->UseSRGBColorSpaceOn();
     property->SetEmissiveTexture(emissiveTex);
   }
@@ -320,6 +332,10 @@ void ApplyGLTFMaterialToVTKActor(std::shared_ptr<vtkGLTFDocumentLoader::Model> m
     actor->GetProperty()->SetNormalScale(material.NormalTextureScale);
     auto normalTex =
       CreateVTKTextureFromGLTFTexture(model, normalMapIndex, existingTextures, parent);
+    if (!normalTex)
+    {
+      return false;
+    }
     property->SetNormalTexture(normalTex);
   }
 
@@ -335,6 +351,7 @@ void ApplyGLTFMaterialToVTKActor(std::shared_ptr<vtkGLTFDocumentLoader::Model> m
     b = pow(b, 1.f / 2.2f);
     actor->GetProperty()->SetColor(r, g, b);
   }
+  return true;
 }
 
 //------------------------------------------------------------------------------
@@ -548,7 +565,12 @@ void vtkGLTFImporter::ImportActors(vtkRenderer* renderer)
         if (primitive.Material >= 0 &&
           primitive.Material < static_cast<int>(model->Materials.size()))
         {
-          ApplyGLTFMaterialToVTKActor(model, primitive, actor, this->Textures, this);
+          if (!ApplyGLTFMaterialToVTKActor(model, primitive, actor, this->Textures, this))
+          {
+            vtkErrorMacro("Could not apply GLTF material to VTK actor, aborting.");
+            this->SetUpdateStatus(vtkImporter::UpdateStatusEnum::FAILURE);
+            return;
+          }
         }
         renderer->AddActor(actor);