Fix an assert with negative animation index with Assimp plugin (#1611)

* animationManager: Fix a logic issue with negative animationIndex

* vtkF3DAssimpImporter: Improve animation index logic

* Tests: Add negative animation index testing

application/testing/CMakeLists.txt

@@ -530,6 +530,7 @@ if(F3D_PLUGIN_BUILD_ASSIMP)
   f3d_test(NAME TestFBXSkinningAnimation DATA punch.fbx ARGS --load-plugins=assimp --animation-time=1 --animation-progress)
   f3d_test(NAME TestVerboseAssimp DATA duck.fbx ARGS --verbose --load-plugins=assimp NO_BASELINE REGEXP "LOD3sp")
   f3d_test(NAME TestVerboseAssimpAnimationIndexError DATA animatedLights.fbx ARGS --load-plugins=assimp --animation-index=48 NO_BASELINE REGEXP "Specified animation index is greater than the highest possible animation index, enabling the first animation.")
+  f3d_test(NAME TestAssimpAnimationNegativeIndex DATA animatedLights.fbx ARGS --load-plugins=assimp --animation-index=-113 --animation-time=2 --animation-progress)
   f3d_test(NAME TestTGATextureFBX DATA duck.fbx ARGS --load-plugins=assimp)
   f3d_test(NAME TestDAE DATA duck.dae ARGS --load-plugins=assimp)
   f3d_test(NAME TestX DATA anim_test.x ARGS --load-plugins=assimp)

library/src/animationManager.cxx

@@ -58,17 +58,14 @@ bool animationManager::Initialize(
     this->ProgressWidget = nullptr;
   }
 
-  int animationIndex = options->scene.animation.index;
-  double animationTime = options->scene.animation.time;
-
   if (this->AvailAnimations <= 0)
   {
     log::debug("No animation available in this file");
-    if (animationIndex > 0)
+    if (options->scene.animation.index > 0)
     {
       log::warn("An animation index has been specified but there are no animation available.");
     }
-    if (animationTime != 0)
+    if (options->scene.animation.time != 0)
     {
       log::warn("No animation available, cannot load a specific animation time");
     }
@@ -314,7 +311,7 @@ std::string animationManager::GetAnimationName()
     return "No animation";
   }
 
-  if (this->AnimationIndex == -1)
+  if (this->AnimationIndex < 0)
   {
     return "All Animations";
   }
@@ -331,7 +328,7 @@ void animationManager::EnableOnlyCurrentAnimation()
   }
   for (int i = 0; i < this->AvailAnimations; i++)
   {
-    if (this->AnimationIndex == -1 || i == this->AnimationIndex)
+    if (this->AnimationIndex < 0 || i == this->AnimationIndex)
     {
       this->Importer->EnableAnimation(i);
     }

plugins/assimp/module/Testing/TestF3DAssimpImportError.cxx

@@ -65,5 +65,12 @@ int TestF3DAssimpImportError(int vtkNotUsed(argc), char* vtkNotUsed(argv)[])
     return EXIT_FAILURE;
   }
 
+  if (!importer->UpdateAtTimeValue(0))
+  {
+    std::cerr << "Importer did not return true with no animation enabled "
+              << "when calling UpdateAtTimeValue()" << std::endl;
+    return EXIT_FAILURE;
+  }
+
   return EXIT_SUCCESS;
 }

plugins/assimp/module/vtkF3DAssimpImporter.cxx

@@ -867,7 +867,7 @@ class vtkF3DAssimpImporter::vtkInternals
   std::vector<vtkSmartPointer<vtkPolyData>> Meshes;
   std::vector<vtkSmartPointer<vtkProperty>> Properties;
   std::vector<vtkSmartPointer<vtkTexture>> EmbeddedTextures;
-  vtkIdType ActiveAnimation = 0;
+  vtkIdType ActiveAnimation = -1; // -1 means no animation enabled here
   std::vector<std::pair<std::string, vtkSmartPointer<vtkLight>>> Lights;
   std::vector<
     std::pair<std::string, std::pair<vtkSmartPointer<vtkCamera>, vtkSmartPointer<vtkCamera>>>>
@@ -910,8 +910,11 @@ std::string vtkF3DAssimpImporter::GetOutputsDescription()
 //----------------------------------------------------------------------------
 bool vtkF3DAssimpImporter::UpdateAtTimeValue(double timeValue)
 {
-  assert(this->Internals->ActiveAnimation >= 0);
   assert(this->Internals->ActiveAnimation < this->GetNumberOfAnimations());
+  if (this->Internals->ActiveAnimation == -1)
+  {
+    return true;
+  }
 
   // get the animation tick
   double fps =
@@ -1034,26 +1037,30 @@ vtkIdType vtkF3DAssimpImporter::GetNumberOfAnimations()
 //----------------------------------------------------------------------------
 std::string vtkF3DAssimpImporter::GetAnimationName(vtkIdType animationIndex)
 {
-  assert(animationIndex >= 0);
   assert(animationIndex < this->GetNumberOfAnimations());
+  assert(animationIndex >= 0);
   return this->Internals->Scene->mAnimations[animationIndex]->mName.C_Str();
 }
 
 //----------------------------------------------------------------------------
 void vtkF3DAssimpImporter::EnableAnimation(vtkIdType animationIndex)
 {
+  assert(animationIndex < this->GetNumberOfAnimations());
+  assert(animationIndex >= 0);
   this->Internals->ActiveAnimation = animationIndex;
 }
 
 //----------------------------------------------------------------------------
 void vtkF3DAssimpImporter::DisableAnimation(vtkIdType vtkNotUsed(animationIndex))
 {
-  this->Internals->ActiveAnimation = 0;
+  this->Internals->ActiveAnimation = -1;
 }
 
 //----------------------------------------------------------------------------
 bool vtkF3DAssimpImporter::IsAnimationEnabled(vtkIdType animationIndex)
 {
+  assert(animationIndex < this->GetNumberOfAnimations());
+  assert(animationIndex >= 0);
   return this->Internals->ActiveAnimation == animationIndex;
 }
 
@@ -1062,6 +1069,9 @@ bool vtkF3DAssimpImporter::GetTemporalInformation(vtkIdType animationIndex,
   double vtkNotUsed(frameRate), int& vtkNotUsed(nbTimeSteps), double timeRange[2],
   vtkDoubleArray* vtkNotUsed(timeSteps))
 {
+  assert(animationIndex < this->GetNumberOfAnimations());
+  assert(animationIndex >= 0);
+
   double duration = this->Internals->Scene->mAnimations[animationIndex]->mDuration;
   double fps = this->Internals->Scene->mAnimations[animationIndex]->mTicksPerSecond;
   if (fps == 0.0)

plugins/assimp/module/vtkF3DAssimpImporter.h

@@ -52,6 +52,9 @@ class vtkF3DAssimpImporter : public vtkF3DImporter
   /**
    * Enable/Disable/Get the status of specific animations
    * Only one single animation can be enabled
+   * By default, no animation are enabled
+   * As specified in the vtkImporter API, animationIndex
+   * is expected to be 0 < GetNumberOfAnimations
    */
   void EnableAnimation(vtkIdType animationIndex) override;
   void DisableAnimation(vtkIdType animationIndex) override;