0031315: Visualization - marker texture is lost after multi-textured object

OpenGl_Context::BindTextures() now iterates over pair of texture sets
considering inconsistent texture unit ranges.

src/Graphic3d/Graphic3d_TextureSet.hxx

@@ -18,6 +18,7 @@
 #include <NCollection_Array1.hxx>
 
 //! Class holding array of textures to be mapped as a set.
+//! Textures should be defined in ascending order of texture units within the set.
 class Graphic3d_TextureSet : public Standard_Transient
 {
   DEFINE_STANDARD_RTTIEXT(Graphic3d_TextureSet, Standard_Transient)

src/OpenGl/FILES

@@ -56,6 +56,7 @@ OpenGl_TextureFormat.cxx
 OpenGl_TextureFormat.hxx
 OpenGl_TextureSet.cxx
 OpenGl_TextureSet.hxx
+OpenGl_TextureSetPairIterator.hxx
 OpenGl_Resource.hxx
 OpenGl_Resource.cxx
 OpenGl_NamedResource.hxx

src/OpenGl/OpenGl_AspectsTextureSet.cxx

@@ -195,6 +195,7 @@ void OpenGl_AspectsTextureSet::build (const Handle(OpenGl_Context)& theCtx,
 
   Standard_Integer& aTextureSetBits = myTextures[0]->ChangeTextureSetBits();
   aTextureSetBits = Graphic3d_TextureSetBits_NONE;
+  Standard_Integer aPrevTextureUnit = -1;
   if (theAspect->ToMapTexture())
   {
     Graphic3d_TextureSet::Iterator aTextureIter (aNewTextureSet);
@@ -212,6 +213,11 @@ void OpenGl_AspectsTextureSet::build (const Handle(OpenGl_Context)& theCtx,
           if (aResource->Init(theCtx, aTexture))
           {
             aResIter0.ChangeUnit() = aResource->Sampler()->Parameters()->TextureUnit();
+            if (aResIter0.Unit() < aPrevTextureUnit)
+            {
+              throw Standard_ProgramError("Graphic3d_TextureMap defines texture units in non-ascending order");
+            }
+            aPrevTextureUnit = aResIter0.Unit();
             aResource->Sampler()->SetParameters(aTexture->GetParams());
             aResource->SetRevision (aTexture->Revision());
           }
@@ -262,6 +268,11 @@ void OpenGl_AspectsTextureSet::build (const Handle(OpenGl_Context)& theCtx,
         // update occupation of texture units
         const Graphic3d_TextureUnit aTexUnit = aResource->Sampler()->Parameters()->TextureUnit();
         aResIter0.ChangeUnit() = aTexUnit;
+        if (aResIter0.Unit() < aPrevTextureUnit)
+        {
+          throw Standard_ProgramError("Graphic3d_TextureMap defines texture units in non-ascending order");
+        }
+        aPrevTextureUnit = aResIter0.Unit();
         if (aTexUnit >= Graphic3d_TextureUnit_0 && aTexUnit <= Graphic3d_TextureUnit_5)
         {
           aTextureSetBits |= (1 << int(aTexUnit));

src/OpenGl/OpenGl_Context.cxx

@@ -31,6 +31,7 @@
 #include <OpenGl_FrameStats.hxx>
 #include <OpenGl_Sampler.hxx>
 #include <OpenGl_ShaderManager.hxx>
+#include <OpenGl_TextureSetPairIterator.hxx>
 #include <OpenGl_Workspace.hxx>
 #include <OpenGl_Aspects.hxx>
 #include <Graphic3d_TransformUtils.hxx>
@@ -3428,93 +3429,56 @@ Handle(OpenGl_TextureSet) OpenGl_Context::BindTextures (const Handle(OpenGl_Text
   if (myActiveTextures != theTextures)
   {
     Handle(OpenGl_Context) aThisCtx (this);
-    OpenGl_TextureSet::Iterator aTextureIterOld (myActiveTextures), aTextureIterNew (theTextures);
+    for (OpenGl_TextureSetPairIterator aSlotIter (myActiveTextures, theTextures); aSlotIter.More(); aSlotIter.Next())
-    for (;;)
     {
-      if (!aTextureIterNew.More())
+      const Graphic3d_TextureUnit aTexUnit = aSlotIter.Unit();
+      const OpenGl_Texture* aTextureOld = aSlotIter.Texture1();
+      const OpenGl_Texture* aTextureNew = aSlotIter.Texture2();
+      if (aTextureNew == aTextureOld)
       {
-        for (; aTextureIterOld.More(); aTextureIterOld.Next())
-        {
-          if (const Handle(OpenGl_Texture)& aTextureOld = aTextureIterOld.Value())
-          {
-            aTextureOld->Unbind (aThisCtx, aTextureIterOld.Unit());
-          #if !defined(GL_ES_VERSION_2_0)
-            if (core11 != NULL)
-            {
-              OpenGl_Sampler::resetGlobalTextureParams (aThisCtx, *aTextureOld, aTextureOld->Sampler()->Parameters());
-            }
-          #endif
-          }
-        }
-        break;
-      }
-
-      const Handle(OpenGl_Texture)& aTextureNew = aTextureIterNew.Value();
-      if (aTextureIterOld.More())
-      {
-        const Handle(OpenGl_Texture)& aTextureOld = aTextureIterOld.Value();
-        if (aTextureNew == aTextureOld
-         && aTextureIterNew.Unit() == aTextureIterOld.Unit())
-        {
-          aTextureIterNew.Next();
-          aTextureIterOld.Next();
-          continue;
-        }
-        else if (aTextureNew.IsNull()
-             || !aTextureNew->IsValid())
-        {
-          if (!aTextureOld.IsNull())
-          {
-            aTextureOld->Unbind (aThisCtx, aTextureIterOld.Unit());
-          #if !defined(GL_ES_VERSION_2_0)
-            if (core11 != NULL)
-            {
-              OpenGl_Sampler::resetGlobalTextureParams (aThisCtx, *aTextureOld, aTextureOld->Sampler()->Parameters());
-            }
-          #endif
-          }
-
-          aTextureIterNew.Next();
-          aTextureIterOld.Next();
-          continue;
-        }
-
-        aTextureIterOld.Next();
-      }
-      if (aTextureNew.IsNull())
-      {
-        aTextureIterNew.Next();
         continue;
       }
 
-      const Graphic3d_TextureUnit aTexUnit = aTextureIterNew.Unit();
+      if (aTextureNew != NULL
-      if (aTexUnit >= myMaxTexCombined)
+       && aTextureNew->IsValid())
       {
-        PushMessage (GL_DEBUG_SOURCE_APPLICATION, GL_DEBUG_TYPE_ERROR, 0, GL_DEBUG_SEVERITY_HIGH,
+        if (aTexUnit >= myMaxTexCombined)
-                     TCollection_AsciiString("Texture unit ") + aTexUnit + " for " + aTextureNew->ResourceId() + " exceeds hardware limit " + myMaxTexCombined);
+        {
-        aTextureIterNew.Next();
+          PushMessage (GL_DEBUG_SOURCE_APPLICATION, GL_DEBUG_TYPE_ERROR, 0, GL_DEBUG_SEVERITY_HIGH,
-        continue;
+                       TCollection_AsciiString("Texture unit ") + aTexUnit + " for " + aTextureNew->ResourceId() + " exceeds hardware limit " + myMaxTexCombined);
-      }
+          continue;
+        }
 
-      aTextureNew->Bind (aThisCtx, aTexUnit);
+        aTextureNew->Bind (aThisCtx, aTexUnit);
-      if (aTextureNew->Sampler()->ToUpdateParameters())
+        if (aTextureNew->Sampler()->ToUpdateParameters())
-      {
-        if (aTextureNew->Sampler()->IsImmutable())
         {
-          aTextureNew->Sampler()->Init (aThisCtx, *aTextureNew);
+          if (aTextureNew->Sampler()->IsImmutable())
+          {
+            aTextureNew->Sampler()->Init (aThisCtx, *aTextureNew);
+          }
+          else
+          {
+            OpenGl_Sampler::applySamplerParams (aThisCtx, aTextureNew->Sampler()->Parameters(), aTextureNew->Sampler().get(), aTextureNew->GetTarget(), aTextureNew->HasMipmaps());
+          }
         }
-        else
+      #if !defined(GL_ES_VERSION_2_0)
+        if (core11 != NULL)
         {
-          OpenGl_Sampler::applySamplerParams (aThisCtx, aTextureNew->Sampler()->Parameters(), aTextureNew->Sampler().get(), aTextureNew->GetTarget(), aTextureNew->HasMipmaps());
+          OpenGl_Sampler::applyGlobalTextureParams (aThisCtx, *aTextureNew, aTextureNew->Sampler()->Parameters());
         }
+      #endif
       }
-    #if !defined(GL_ES_VERSION_2_0)
+      else if (aTextureOld != NULL
-      if (core11 != NULL)
+            && aTextureOld->IsValid())
       {
-        OpenGl_Sampler::applyGlobalTextureParams (aThisCtx, *aTextureNew, aTextureNew->Sampler()->Parameters());
+        aTextureOld->Unbind (aThisCtx, aTexUnit);
+      #if !defined(GL_ES_VERSION_2_0)
+        if (core11 != NULL)
+        {
+          OpenGl_Sampler::resetGlobalTextureParams (aThisCtx, *aTextureOld, aTextureOld->Sampler()->Parameters());
+        }
+      #endif
       }
-    #endif
-      aTextureIterNew.Next();
     }
     myActiveTextures = theTextures;
   }

src/OpenGl/OpenGl_TextureSet.hxx

@@ -20,6 +20,7 @@
 class OpenGl_Texture;
 
 //! Class holding array of textures to be mapped as a set.
+//! Textures should be defined in ascending order of texture units within the set.
 class OpenGl_TextureSet : public Standard_Transient
 {
   DEFINE_STANDARD_RTTIEXT(OpenGl_TextureSet, Standard_Transient)
@@ -99,12 +100,18 @@ public:
   //! Return the first texture.
   Handle(OpenGl_Texture)& ChangeFirst() { return myTextures.ChangeFirst().Texture; }
 
+  //! Return the first texture unit.
+  Graphic3d_TextureUnit FirstUnit() const { return myTextures.First().Unit; }
+
   //! Return the last texture.
   const Handle(OpenGl_Texture)& Last() const { return myTextures.Last().Texture; }
 
   //! Return the last texture.
   Handle(OpenGl_Texture)& ChangeLast() { return myTextures.ChangeLast().Texture; }
 
+  //! Return the last texture unit.
+  Graphic3d_TextureUnit LastUnit() const { return myTextures.Last().Unit; }
+
   //! Return the last texture unit.
   Graphic3d_TextureUnit& ChangeLastUnit() { return myTextures.ChangeLast().Unit; }
 

src/OpenGl/OpenGl_TextureSetPairIterator.hxx

@@ -0,0 +1,104 @@
+// Copyright (c) 2020 OPEN CASCADE SAS
+//
+// This file is part of Open CASCADE Technology software library.
+//
+// This library is free software; you can redistribute it and/or modify it under
+// the terms of the GNU Lesser General Public License version 2.1 as published
+// by the Free Software Foundation, with special exception defined in the file
+// OCCT_LGPL_EXCEPTION.txt. Consult the file LICENSE_LGPL_21.txt included in OCCT
+// distribution for complete text of the license and disclaimer of any warranty.
+//
+// Alternatively, this file may be used under the terms of Open CASCADE
+// commercial license or contractual agreement.
+
+#ifndef _OpenGl_TextureSetPairIterator_Header
+#define _OpenGl_TextureSetPairIterator_Header
+
+#include <OpenGl_TextureSet.hxx>
+
+//! Class for iterating pair of texture sets through each defined texture slot.
+//! Note that iterator considers texture slots being in ascending order within OpenGl_TextureSet.
+class OpenGl_TextureSetPairIterator
+{
+public:
+
+  //! Constructor.
+  OpenGl_TextureSetPairIterator (const Handle(OpenGl_TextureSet)& theSet1,
+                                 const Handle(OpenGl_TextureSet)& theSet2)
+  : myIter1 (theSet1),
+    myIter2 (theSet2),
+    myTexture1 (NULL),
+    myTexture2 (NULL),
+    myUnitLower (IntegerLast()),
+    myUnitUpper (IntegerFirst()),
+    myUnitCurrent (0)
+  {
+    if (!theSet1.IsNull()
+     && !theSet1->IsEmpty())
+    {
+      myUnitLower = Min (myUnitLower, theSet1->FirstUnit());
+      myUnitUpper = Max (myUnitUpper, theSet1->LastUnit());
+    }
+    if (!theSet2.IsNull()
+     && !theSet2->IsEmpty())
+    {
+      myUnitLower = Min (myUnitLower, theSet2->FirstUnit());
+      myUnitUpper = Max (myUnitUpper, theSet2->LastUnit());
+    }
+    myUnitCurrent = myUnitLower;
+    myTexture1 = (myIter1.More() && myIter1.Unit() == myUnitCurrent)
+               ? myIter1.ChangeValue().get()
+               : NULL;
+    myTexture2 = (myIter2.More() && myIter2.Unit() == myUnitCurrent)
+               ? myIter2.ChangeValue().get()
+               : NULL;
+  }
+
+  //! Return TRUE if there are more texture units to pass through.
+  bool More() const { return myUnitCurrent <= myUnitUpper; }
+
+  //! Return current texture unit.
+  Graphic3d_TextureUnit Unit() const { return (Graphic3d_TextureUnit )myUnitCurrent; }
+
+  //! Access texture from first texture set.
+  const OpenGl_Texture* Texture1() const { return myTexture1; }
+
+  //! Access texture from second texture set.
+  const OpenGl_Texture* Texture2() const { return myTexture2; }
+
+  //! Move iterator position to the next pair.
+  void Next()
+  {
+    ++myUnitCurrent;
+    myTexture1 = myTexture2 = NULL;
+    for (; myIter1.More(); myIter1.Next())
+    {
+      if (myIter1.Unit() >= myUnitCurrent)
+      {
+        myTexture1 = myIter1.Unit() == myUnitCurrent ? myIter1.ChangeValue().get() : NULL;
+        break;
+      }
+    }
+    for (; myIter2.More(); myIter2.Next())
+    {
+      if (myIter2.Unit() >= myUnitCurrent)
+      {
+        myTexture2 = myIter2.Unit() == myUnitCurrent ? myIter2.ChangeValue().get() : NULL;
+        break;
+      }
+    }
+  }
+
+private:
+
+  OpenGl_TextureSet::Iterator myIter1;
+  OpenGl_TextureSet::Iterator myIter2;
+  OpenGl_Texture*  myTexture1;
+  OpenGl_Texture*  myTexture2;
+  Standard_Integer myUnitLower;
+  Standard_Integer myUnitUpper;
+  Standard_Integer myUnitCurrent;
+
+};
+
+#endif //_OpenGl_TextureSetPairIterator_Header

tests/bugs/vis/bug31315

@@ -0,0 +1,16 @@
+puts "============="
+puts "0031315: Visualization - marker texture is lost after multi-textured object"
+puts "============="
+
+pload MODELING VISUALIZATION
+vclear
+vinit View1
+box b 1 2 3
+vdisplay -dispMode 1 -highMode 1 b
+vtexture b -tex0 3 -tex1 4
+vfit
+vpoint p0 -1 0 0
+vaspects p0 -setMarkerSize 5
+if { [vreadpixel 15 310 -rgb -name] != "BLACK" } { puts "Error: Black color is expected" }
+
+vdump ${imagedir}/${casename}.png