0026199: Visualization - use NCollection_IndexedMap instead of NCollection_Sequence in OpenGl_BVHClipPrimitiveSet to improve performance

This patch improves performance of OpenGl_BVHClipPrimitiveSet. In particular, on the attached test case for 90 000 spheres the results are the following:

1) Master: vdisplay - 55 sec, 5 FPS, 1.4 GB memory, vclear - 180 sec.
2) Branch: vdisplay - 1.3 sec, 5 FPS, 1.4 GB memory, vclear - 90 sec.

So, the patch improves vdisplay in ~40 times, and vclear in ~2 times without extra memory and with no impact on rendering performance. However, the vclear time is still significant. According to profile results, it is due to Graphic3d_Structure::DisconnectAll method.

Test case for issue CR26199

Fix invalid warnings in 'vdefaults' command.

src/OpenGl/OpenGl_BVHClipPrimitiveSet.cxx

@@ -41,7 +41,7 @@ Standard_Integer OpenGl_BVHClipPrimitiveSet::Size() const
 // =======================================================================
 Graphic3d_BndBox4f OpenGl_BVHClipPrimitiveSet::Box (const Standard_Integer theIdx) const
 {
-  return myStructs (theIdx + 1)->BoundingBox();
+  return myStructs.FindKey (theIdx + 1)->BoundingBox();
 }
 
 // =======================================================================
@@ -51,12 +51,10 @@ Graphic3d_BndBox4f OpenGl_BVHClipPrimitiveSet::Box (const Standard_Integer theId
 Standard_ShortReal OpenGl_BVHClipPrimitiveSet::Center (const Standard_Integer theIdx,
                                                        const Standard_Integer theAxis) const
 {
-  Graphic3d_BndBox4f aBndBox = myStructs (theIdx + 1)->BoundingBox();
+  Graphic3d_BndBox4f aBndBox = myStructs.FindKey (theIdx + 1)->BoundingBox();
-  Standard_ShortReal aCenter =  theAxis == 0 ? (aBndBox.CornerMin().x() + aBndBox.CornerMax().x()) * 0.5f
+
-                             : (theAxis == 1 ? (aBndBox.CornerMin().y() + aBndBox.CornerMax().y()) * 0.5f
+  return (aBndBox.CornerMin()[theAxis] +
-                             : (theAxis == 2 ? (aBndBox.CornerMin().z() + aBndBox.CornerMax().z()) * 0.5f
+          aBndBox.CornerMax()[theAxis]) * 0.5f;
-                             : (aBndBox.CornerMin().w() + aBndBox.CornerMax().w()) * 0.5f));
-  return aCenter;
 }
 
 // =======================================================================
@@ -66,10 +64,7 @@ Standard_ShortReal OpenGl_BVHClipPrimitiveSet::Center (const Standard_Integer th
 void OpenGl_BVHClipPrimitiveSet::Swap (const Standard_Integer theIdx1,
                                        const Standard_Integer theIdx2)
 {
-  const OpenGl_Structure* aStruct1 = myStructs (theIdx1 + 1);
+  myStructs.Swap (theIdx1 + 1, theIdx2 + 1);
-  const OpenGl_Structure* aStruct2 = myStructs (theIdx2 + 1);
-  myStructs.ChangeValue (theIdx1 + 1) = aStruct2;
-  myStructs.ChangeValue (theIdx2 + 1) = aStruct1;
 }
 
 // =======================================================================
@@ -80,14 +75,16 @@ void OpenGl_BVHClipPrimitiveSet::Assign (const OpenGl_ArrayOfStructure& theStruc
 {
   myStructs.Clear();
 
-  const Standard_Integer aNbPriorities = theStructs.Length();
+  for (Standard_Integer aPriorityIdx = 0, aNbPriorities = theStructs.Length(); aPriorityIdx < aNbPriorities; ++aPriorityIdx)
-  for (Standard_Integer aPriorityIdx = 0; aPriorityIdx < aNbPriorities; ++aPriorityIdx)
   {
     for (OpenGl_SequenceOfStructure::Iterator aStructIter (theStructs (aPriorityIdx)); aStructIter.More(); aStructIter.Next())
     {
       const OpenGl_Structure* aStruct = aStructIter.Value();
+
       if (!aStruct->IsAlwaysRendered())
-        myStructs.Append (aStruct);
+      {
+        myStructs.Add (aStruct);
+      }
     }
   }
 
@@ -100,8 +97,12 @@ void OpenGl_BVHClipPrimitiveSet::Assign (const OpenGl_ArrayOfStructure& theStruc
 // =======================================================================
 void OpenGl_BVHClipPrimitiveSet::Add (const OpenGl_Structure* theStruct)
 {
-  myStructs.Append (theStruct);
+  const Standard_Integer aSize = myStructs.Size();
-  MarkDirty();
+
+  if (myStructs.Add (theStruct) > aSize) // new structure?
+  {
+    MarkDirty();
+  }
 }
 
 // =======================================================================
@@ -110,14 +111,13 @@ void OpenGl_BVHClipPrimitiveSet::Add (const OpenGl_Structure* theStruct)
 // =======================================================================
 void OpenGl_BVHClipPrimitiveSet::Remove (const OpenGl_Structure* theStruct)
 {
-  for (Standard_Integer anIdx = 1; anIdx <= myStructs.Size(); ++anIdx)
+  const Standard_Integer anIndex = myStructs.FindIndex (theStruct);
+
+  if (anIndex != 0)
   {
-    if (myStructs (anIdx) == theStruct)
+    myStructs.Swap (Size(), anIndex);
-    {
+    myStructs.RemoveLast();
-      myStructs.Remove (anIdx);
+    MarkDirty();
-      MarkDirty();
-      break;
-    }
   }
 }
 
@@ -137,5 +137,5 @@ void OpenGl_BVHClipPrimitiveSet::Clear()
 // =======================================================================
 const OpenGl_Structure* OpenGl_BVHClipPrimitiveSet::GetStructureById (Standard_Integer theId)
 {
-  return myStructs (theId + 1);
+  return myStructs.FindKey (theId + 1);
 }

src/OpenGl/OpenGl_BVHClipPrimitiveSet.hxx

@@ -18,8 +18,8 @@
 
 #include <BVH_PrimitiveSet.hxx>
 #include <NCollection_Array1.hxx>
+#include <NCollection_IndexedMap.hxx>
 
-#include <OpenGl_Vec.hxx>
 #include <OpenGl_Structure.hxx>
 #include <OpenGl_SequenceOfStructure.hxx>
 
@@ -40,14 +40,14 @@ public:
   //! Returns total number of structures.
   virtual Standard_Integer Size() const;
 
-  //! Returns AABB of a structure.
+  //! Returns AABB of the structure.
   virtual Graphic3d_BndBox4f Box (const Standard_Integer theIdx) const;
 
-  //! Calculates center of the AABB projection onto given axis.
+  //! Calculates center of the AABB along given axis.
   virtual Standard_ShortReal Center (const Standard_Integer theIdx,
                                      const Standard_Integer theAxis) const;
 
-  //! Swaps given AABBs.
+  //! Swaps structures with the given indices.
   virtual void Swap (const Standard_Integer theIdx1,
                      const Standard_Integer theIdx2);
 
@@ -55,21 +55,21 @@ public:
   //! if each structure is cullable or not.
   void Assign (const OpenGl_ArrayOfStructure& theStructs);
 
-  //! Adds structure theStruct to the set.
+  //! Adds structure to the set.
   void Add (const OpenGl_Structure* theStruct);
 
-  //! Removes the given OpenGl_Structure from the set.
+  //! Removes the given structure from the set.
   void Remove (const OpenGl_Structure* theStruct);
 
   //! Cleans the whole primitive set.
   void Clear();
 
-  //! Returns the structure corresponding to the given id.
+  //! Returns the structure corresponding to the given ID.
   const OpenGl_Structure* GetStructureById (Standard_Integer theId);
 
 private:
 
-  NCollection_Sequence<const OpenGl_Structure*> myStructs;    //!< Sequence of structures
+  NCollection_IndexedMap<const OpenGl_Structure*> myStructs;    //!< Indexed map of structures.
 
 };
 

src/ViewerTest/ViewerTest_ViewerCommands.cxx

@@ -7426,9 +7426,9 @@ static int VDefaults (Draw_Interpretor& theDi,
       // currently HLRDeviationAngle is used instead of DeviationAngle in most places
       aDefParams->SetHLRAngle (M_PI * Draw::Atof (theArgVec[anArgIter]) / 180.0);
     }
-    if (anArg == "-AUTOTR"
+    else if (anArg == "-AUTOTR"
-     || anArg == "-AUTOTRIANG"
+          || anArg == "-AUTOTRIANG"
-     || anArg == "-AUTOTRIANGULATION")
+          || anArg == "-AUTOTRIANGULATION")
     {
       if (++anArgIter >= theArgsNb)
       {

tests/bugs/vis/bug26199

@@ -0,0 +1,70 @@
+puts "=========="
+puts "OCC26199"
+puts "=========="
+puts ""
+############################################################
+# Visualization - use NCollection_IndexedMap instead of NCollection_Sequence in OpenGl_BVHClipPrimitiveSet to improve performance
+############################################################
+
+psphere s 0.5
+tclean  s
+incmesh s 0.1
+trinfo  s
+
+vinit View1
+vclear
+vaxo
+vsetdispmode 1
+
+vdefaults -defl 1.0 -autoTriang off
+# Warning, unknown argument '-DEFL'
+
+vdisplay s
+
+set aNb1 100
+
+# display 100x100 connected instances of single presentation
+puts "Creating [expr $aNb1*$aNb1] instances..."
+set t1 [time {for {set i 0} {$i < $aNb1} {incr i} {for {set j 0} {$j < $aNb1} {incr j} {vconnectto s_${i}_${j} ${i} ${j} 0 s -noupdate}}}]
+
+# the following command is slow
+set t2 [time {vclear}]
+
+set d1 [lindex $t1 0]
+puts "vconnectto done in $d1 microseconds!\n"
+
+set d2 [lindex $t2 0]
+puts "vclear done in $d2 microseconds!\n"
+
+set t1_sec [expr $d1 * 1.e-6]
+set t2_sec [expr $d2 * 1.e-6]
+
+if { [regexp {Debug mode} [dversion]] } {
+  if { [regexp {Windows} [dversion]] } {
+    set max_time_vconnectto 5
+    set max_time_vclear 5
+  } else {
+    set max_time_vconnectto 5
+    set max_time_vclear 5
+  }
+} else {
+  if { [regexp {Windows} [dversion]] } {
+    set max_time_vconnectto 1
+    set max_time_vclear 1
+  } else {
+    set max_time_vconnectto 1
+    set max_time_vclear 1
+  }
+}
+
+if { ${t1_sec} > ${max_time_vconnectto} } {                                         
+    puts "Elapsed time of vconnectto is more than ${max_time_vconnectto} seconds - Error"
+} else {
+    puts "Elapsed time of vconnectto is less than ${max_time_vconnectto} seconds - OK"
+}
+
+if { ${t2_sec} > ${max_time_vclear} } {                                         
+    puts "Elapsed time of vclear is more than ${max_time_vclear} seconds - Error"
+} else {
+    puts "Elapsed time of vclear is less than ${max_time_vclear} seconds - OK"
+}