0029825: Foundation Classes, NCollection_Vec4 - workaround gcc optimizer issues with...

Methods of NCollection_Vec3 and NCollection_Vec3 that returned reference to internal buffer as vector of lower dimension (non-const xy(), xyz() etc.) are eliminated.
Use of these methods could led to generation of incorrect binary code by GCC.
Instead added new method SetValues() accepting vector of lower dimension and additional value.

DRAW test command QANColTestVec4 reproducing one situation where the bug occurs is added, along with a test case.

diff --git a/src/NCollection/NCollection_Vec3.hxx b/src/NCollection/NCollection_Vec3.hxx
index ed6cf98b5eb1d6ede7dcc1a038cc748b834c7f3d..165b6d5801024c3ec2ff78f43f51ceabc633aec9 100644 (file)
--- a/src/NCollection/NCollection_Vec3.hxx
+++ b/src/NCollection/NCollection_Vec3.hxx
@@ -65,12 +65,12 @@ public:
     v[2] = theZ;
   }
 
-  //! Constructor from 2-components vector.
-  explicit NCollection_Vec3 (const NCollection_Vec2<Element_t>& theVec2)
+  //! Constructor from 2-components vector + optional 3rd value.
+  explicit NCollection_Vec3 (const NCollection_Vec2<Element_t>& theVec2, Element_t theZ = Element_t(0))
   {
     v[0] = theVec2[0];
     v[1] = theVec2[1];
-    v[2] = Element_t(0);
+    v[2] = theZ;
   }
 
   //! Assign new values to the vector.
@@ -83,6 +83,14 @@ public:
     v[2] = theZ;
   }
 
+  //! Assign new values to the vector.
+  void SetValues (const NCollection_Vec2<Element_t>& theVec2, Element_t theZ)
+  {
+    v[0] = theVec2.x();
+    v[1] = theVec2.y();
+    v[2] = theZ;
+  }
+
   //! Alias to 1st component as X coordinate in XYZ.
   Element_t x() const { return v[0]; }
 
@@ -127,18 +135,6 @@ public:
   //! Alias to 3rd component as BLUE channel in RGB.
   Element_t& b() { return v[2]; }
 
-  //! @return XY-components modifiable vector
-  NCollection_Vec2<Element_t>& xy()
-  {
-    return *((NCollection_Vec2<Element_t>* )&v[0]);
-  }
-
-  //! @return YZ-components modifiable vector
-  NCollection_Vec2<Element_t>& yz()
-  {
-    return *((NCollection_Vec2<Element_t>* )&v[1]);
-  }
-
   //! Check this vector with another vector for equality (without tolerance!).
   bool IsEqual (const NCollection_Vec3& theOther) const
   {
@@ -295,7 +291,7 @@ public:
   }
 
   //! Compute per-component division by scale factor.
-  NCollection_Vec3 operator/ (const Element_t theInvFactor)
+  NCollection_Vec3 operator/ (const Element_t theInvFactor) const
   {
     NCollection_Vec3 aResult (*this);
     return aResult /= theInvFactor;

diff --git a/src/NCollection/NCollection_Vec4.hxx b/src/NCollection/NCollection_Vec4.hxx
index 08704121e3ac12be62d4a3961012f6e7e3bfa5b6..c3cb93a712b1f2aafd61bee586274de92fba6441 100644 (file)
--- a/src/NCollection/NCollection_Vec4.hxx
+++ b/src/NCollection/NCollection_Vec4.hxx
@@ -66,18 +66,11 @@ public:
     v[2] = v[3] = Element_t (0);
   }
 
-  //! Constructor from 3-components vector.
-  explicit NCollection_Vec4(const NCollection_Vec3<Element_t>& theVec3)
+  //! Constructor from 3-components vector + optional 4th value.
+  explicit NCollection_Vec4(const NCollection_Vec3<Element_t>& theVec3, const Element_t theW = Element_t(0))
   {
     std::memcpy (this, &theVec3, sizeof(NCollection_Vec3<Element_t>));
-    v[3] = Element_t (0);
-  }
-
-  //! Constructor from 3-components vector + alpha value.
-  explicit NCollection_Vec4(const NCollection_Vec3<Element_t>& theVec3,
-                            const Element_t                    theAlpha) {
-    std::memcpy (this, &theVec3, sizeof(NCollection_Vec3<Element_t>));
-    v[3] = theAlpha;
+    v[3] = theW;
   }
 
   //! Assign new values to the vector.
@@ -92,6 +85,15 @@ public:
     v[3] = theW;
   }
 
+  //! Assign new values as 3-component vector and a 4-th value.
+  void SetValues (const NCollection_Vec3<Element_t>& theVec3, const Element_t theW)
+  {
+    v[0] = theVec3.x();
+    v[1] = theVec3.y();
+    v[2] = theVec3.z();
+    v[3] = theW;
+  }
+
   //! Alias to 1st component as X coordinate in XYZW.
   Element_t x() const { return v[0]; }
 
@@ -157,36 +159,6 @@ public:
   //! Alias to 4th component as ALPHA channel in RGBA.
   Element_t& a() { return v[3]; }
 
-  //! @return XY-components modifiable vector
-  NCollection_Vec2<Element_t>& xy()
-  {
-    return *((NCollection_Vec2<Element_t>* )&v[0]);
-  }
-
-  //! @return YZ-components modifiable vector
-  NCollection_Vec2<Element_t>& yz()
-  {
-    return *((NCollection_Vec2<Element_t>* )&v[1]);
-  }
-
-  //! @return YZ-components modifiable vector
-  NCollection_Vec2<Element_t>& zw()
-  {
-    return *((NCollection_Vec2<Element_t>* )&v[2]);
-  }
-
-  //! @return XYZ-components modifiable vector
-  NCollection_Vec3<Element_t>& xyz()
-  {
-    return *((NCollection_Vec3<Element_t>* )&v[0]);
-  }
-
-  //! @return YZW-components modifiable vector
-  NCollection_Vec3<Element_t>& yzw()
-  {
-    return *((NCollection_Vec3<Element_t>* )&v[1]);
-  }
-
   //! Check this vector with another vector for equality (without tolerance!).
   bool IsEqual (const NCollection_Vec4& theOther) const
   {

diff --git a/src/OpenGl/OpenGl_BackgroundArray.cxx b/src/OpenGl/OpenGl_BackgroundArray.cxx
index 0f5723b4197ac23dabeeeadc28636ff3e200ff60..f760dcfc9820b102d204d6118db1cb7c11e2d609 100644 (file)
--- a/src/OpenGl/OpenGl_BackgroundArray.cxx
+++ b/src/OpenGl/OpenGl_BackgroundArray.cxx
@@ -211,24 +211,24 @@ Standard_Boolean OpenGl_BackgroundArray::createGradientArray() const
   {
     case Aspect_GFM_HOR:
     {
-      aCorners[0] = myGradientParams.color2.xyz().ChangeData();
-      aCorners[1] = myGradientParams.color2.xyz().ChangeData();
-      aCorners[2] = myGradientParams.color1.xyz().ChangeData();
-      aCorners[3] = myGradientParams.color1.xyz().ChangeData();
+      aCorners[0] = myGradientParams.color2.ChangeData();
+      aCorners[1] = myGradientParams.color2.ChangeData();
+      aCorners[2] = myGradientParams.color1.ChangeData();
+      aCorners[3] = myGradientParams.color1.ChangeData();
       break;
     }
     case Aspect_GFM_VER:
     {
-      aCorners[0] = myGradientParams.color2.xyz().ChangeData();
-      aCorners[1] = myGradientParams.color1.xyz().ChangeData();
-      aCorners[2] = myGradientParams.color2.xyz().ChangeData();
-      aCorners[3] = myGradientParams.color1.xyz().ChangeData();
+      aCorners[0] = myGradientParams.color2.ChangeData();
+      aCorners[1] = myGradientParams.color1.ChangeData();
+      aCorners[2] = myGradientParams.color2.ChangeData();
+      aCorners[3] = myGradientParams.color1.ChangeData();
       break;
     }
     case Aspect_GFM_DIAG1:
     {
-      aCorners[0] = myGradientParams.color2.xyz().ChangeData();
-      aCorners[3] = myGradientParams.color1.xyz().ChangeData();
+      aCorners[0] = myGradientParams.color2.ChangeData();
+      aCorners[3] = myGradientParams.color1.ChangeData();
       aDiagCorner1[0] = aDiagCorner2[0] = 0.5f * (aCorners[0][0] + aCorners[3][0]);
       aDiagCorner1[1] = aDiagCorner2[1] = 0.5f * (aCorners[0][1] + aCorners[3][1]);
       aDiagCorner1[2] = aDiagCorner2[2] = 0.5f * (aCorners[0][2] + aCorners[3][2]);
@@ -238,8 +238,8 @@ Standard_Boolean OpenGl_BackgroundArray::createGradientArray() const
     }
     case Aspect_GFM_DIAG2:
     {
-      aCorners[1] = myGradientParams.color1.xyz().ChangeData();
-      aCorners[2] = myGradientParams.color2.xyz().ChangeData();
+      aCorners[1] = myGradientParams.color1.ChangeData();
+      aCorners[2] = myGradientParams.color2.ChangeData();
       aDiagCorner1[0] = aDiagCorner2[0] = 0.5f * (aCorners[1][0] + aCorners[2][0]);
       aDiagCorner1[1] = aDiagCorner2[1] = 0.5f * (aCorners[1][1] + aCorners[2][1]);
       aDiagCorner1[2] = aDiagCorner2[2] = 0.5f * (aCorners[1][2] + aCorners[2][2]);
@@ -254,18 +254,18 @@ Standard_Boolean OpenGl_BackgroundArray::createGradientArray() const
       aVertices[2] = OpenGl_Vec2(float(myViewWidth), 0.0f);
       aVertices[3] = OpenGl_Vec2(0.0f,               0.0f);
 
-      aCorners[0] = myGradientParams.color2.xyz().ChangeData();
-      aCorners[1] = myGradientParams.color1.xyz().ChangeData();
-      aCorners[2] = myGradientParams.color2.xyz().ChangeData();
-      aCorners[3] = myGradientParams.color2.xyz().ChangeData();
+      aCorners[0] = myGradientParams.color2.ChangeData();
+      aCorners[1] = myGradientParams.color1.ChangeData();
+      aCorners[2] = myGradientParams.color2.ChangeData();
+      aCorners[3] = myGradientParams.color2.ChangeData();
       break;
     }
     case Aspect_GFM_CORNER2:
     {
-      aCorners[0] = myGradientParams.color2.xyz().ChangeData();
-      aCorners[1] = myGradientParams.color1.xyz().ChangeData();
-      aCorners[2] = myGradientParams.color2.xyz().ChangeData();
-      aCorners[3] = myGradientParams.color2.xyz().ChangeData();
+      aCorners[0] = myGradientParams.color2.ChangeData();
+      aCorners[1] = myGradientParams.color1.ChangeData();
+      aCorners[2] = myGradientParams.color2.ChangeData();
+      aCorners[3] = myGradientParams.color2.ChangeData();
       break;
     }
     case Aspect_GFM_CORNER3:
@@ -275,18 +275,18 @@ Standard_Boolean OpenGl_BackgroundArray::createGradientArray() const
       aVertices[2] = OpenGl_Vec2(float(myViewWidth), 0.0f);
       aVertices[3] = OpenGl_Vec2(0.0f,               0.0f);
 
-      aCorners[0] = myGradientParams.color2.xyz().ChangeData();
-      aCorners[1] = myGradientParams.color2.xyz().ChangeData();
-      aCorners[2] = myGradientParams.color1.xyz().ChangeData();
-      aCorners[3] = myGradientParams.color2.xyz().ChangeData();
+      aCorners[0] = myGradientParams.color2.ChangeData();
+      aCorners[1] = myGradientParams.color2.ChangeData();
+      aCorners[2] = myGradientParams.color1.ChangeData();
+      aCorners[3] = myGradientParams.color2.ChangeData();
       break;
     }
     case Aspect_GFM_CORNER4:
     {
-      aCorners[0] = myGradientParams.color2.xyz().ChangeData();
-      aCorners[1] = myGradientParams.color2.xyz().ChangeData();
-      aCorners[2] = myGradientParams.color1.xyz().ChangeData();
-      aCorners[3] = myGradientParams.color2.xyz().ChangeData();
+      aCorners[0] = myGradientParams.color2.ChangeData();
+      aCorners[1] = myGradientParams.color2.ChangeData();
+      aCorners[2] = myGradientParams.color1.ChangeData();
+      aCorners[3] = myGradientParams.color2.ChangeData();
       break;
     }
     case Aspect_GFM_NONE:

diff --git a/src/OpenGl/OpenGl_Context.cxx b/src/OpenGl/OpenGl_Context.cxx
index 487e4e034911b0716f029d874745e4a3b937fc5c..4fa36fffae67d603fb25452238be30ab144a841c 100644 (file)
--- a/src/OpenGl/OpenGl_Context.cxx
+++ b/src/OpenGl/OpenGl_Context.cxx
@@ -3365,12 +3365,16 @@ void OpenGl_Context::SetTextureMatrix (const Handle(Graphic3d_TextureParams)& th
     }
 
     // pack transformation parameters
-    OpenGl_Vec4 aTrsf[2];
-    aTrsf[0].x()  = -theParams->Translation().x();
-    aTrsf[0].y()  = -theParams->Translation().y();
-    aTrsf[0].zw() = theParams->Scale();
-    aTrsf[1].x()  = std::sin (-theParams->Rotation() * static_cast<float> (M_PI / 180.0));
-    aTrsf[1].y()  = std::cos (-theParams->Rotation() * static_cast<float> (M_PI / 180.0));
+    OpenGl_Vec4 aTrsf[2] =
+    {
+      OpenGl_Vec4 (-theParams->Translation().x(),
+                   -theParams->Translation().y(),
+                    theParams->Scale().x(),
+                    theParams->Scale().y()),
+      OpenGl_Vec4 (static_cast<float> (std::sin (-theParams->Rotation() * M_PI / 180.0)),
+                   static_cast<float> (std::cos (-theParams->Rotation() * M_PI / 180.0)),
+                   0.0f, 0.0f)
+    };
     myActiveProgram->SetUniform (this, aUniLoc, 2, aTrsf);
     return;
   }

diff --git a/src/OpenGl/OpenGl_Material.hxx b/src/OpenGl/OpenGl_Material.hxx
index 928f5a9e0c1476e1e88de7986ee0c725d9d7d232..f2225a4d78b71b0a95be9c4ef979d4afc9667731 100644 (file)
--- a/src/OpenGl/OpenGl_Material.hxx
+++ b/src/OpenGl/OpenGl_Material.hxx
@@ -39,8 +39,8 @@ struct OpenGl_Material
   void SetColor (const OpenGl_Vec4& theColor)
   {
     // apply the same formula as in Graphic3d_MaterialAspect::SetColor()
-    Ambient.xyz() = theColor.rgb() * 0.25f;
-    Diffuse.xyz() = theColor.rgb();
+    Ambient.SetValues (theColor.rgb() * 0.25f, Ambient.a());
+    Diffuse.SetValues (theColor.rgb(), Diffuse.a());
   }
 
   //! Initialize material

diff --git a/src/OpenGl/OpenGl_Text.cxx b/src/OpenGl/OpenGl_Text.cxx
index e12620fe094b06a603b785652d882171daf2b84b..70cd2fb3a37a4ca36f7f794dae29ad538d9ef0c7 100644 (file)
--- a/src/OpenGl/OpenGl_Text.cxx
+++ b/src/OpenGl/OpenGl_Text.cxx
@@ -219,8 +219,7 @@ void OpenGl_Text::Init (const Handle(OpenGl_Context)&     theCtx,
   }
   myIs2d       = true;
   myParams     = theParams;
-  myPoint.xy() = thePoint;
-  myPoint.z()  = 0.0f;
+  myPoint.SetValues (thePoint, 0.0f);
   myString.FromUnicode (theText.ToExtString());
 }
 

diff --git a/src/QANCollection/QANCollection_Test.cxx b/src/QANCollection/QANCollection_Test.cxx
index 78715b5123f4c90dc1961f0a09141ab9d79a7308..658aad87ec002f8f64823d7864861949a701654c 100644 (file)
--- a/src/QANCollection/QANCollection_Test.cxx
+++ b/src/QANCollection/QANCollection_Test.cxx
@@ -1143,10 +1143,49 @@ static Standard_Integer QATestAtof (Draw_Interpretor& di, Standard_Integer argc,
   return 0;
 }
 
+// Test operations with NCollection_Vec4 that caused generation of invalid code by GCC
+// due to reinterpret_cast conversions of Vec4 internal buffer to Vec3 (see #29825)
+static Standard_Integer QANColTestVec4 (Draw_Interpretor& theDI, Standard_Integer /*theNbArgs*/, const char** /*theArgVec*/)
+{
+  NCollection_Mat4<float> aMatrix;
+  aMatrix.Translate (NCollection_Vec3<float> (4.0f, 3.0f, 1.0f));
+
+  NCollection_Vec4<float> aPoints1[8];
+  for (int aX = 0; aX < 2; ++aX)
+  {
+    for (int aY = 0; aY < 2; ++aY)
+    {
+      for (int aZ = 0; aZ < 2; ++aZ)
+      {
+        aPoints1[aX * 2 * 2 + aY * 2 + aZ] = NCollection_Vec4<float> (-1.0f + 2.0f * float(aX),
+                                                                      -1.0f + 2.0f * float(aY),
+                                                                      -1.0f + 2.0f * float(aZ),
+                                                                       1.0f);
+      }
+    }
+  }
+
+  NCollection_Vec3<float> aPoints2[8];
+  for (int aPntIdx = 0; aPntIdx < 8; ++aPntIdx)
+  {
+    // NB: the evaluation of line below could be dropped by GCC optimizer
+    // while retrieving xyz() value the line after
+    aPoints1[aPntIdx] = aMatrix * aPoints1[aPntIdx];
+    aPoints2[aPntIdx] = aPoints1[aPntIdx].xyz() / aPoints1[aPntIdx].w();
+    //aPoints2[aPntIdx] = NCollection_Vec3<float> (aPoints1[aPntIdx].x(), aPoints1[aPntIdx].y(), aPoints1[aPntIdx].z()) / aPoints1[aPntIdx].w();
+  }
+
+  for (int aPntIter = 0; aPntIter < 8; ++aPntIter) { theDI << aPoints2[aPntIter].SquareModulus() << " "; }
+  if ((int )(aPoints2[7].SquareModulus() + 0.5f) != 45)
+  {
+    theDI << "Error: method 'NCollection_Vec4::xyz()' failed.";
+  }
+  return 0;
+}
+
 void QANCollection::CommandsTest(Draw_Interpretor& theCommands) {
   const char *group = "QANCollection";
 
-  // from agvCollTest/src/CollectionEXE/FuncTestEXE.cxx
   theCommands.Add("QANColTestArray1",         "QANColTestArray1 Lower Upper",
     __FILE__, QANColTestArray1, group);
   theCommands.Add("QANColTestArray2",         "QANColTestArray2 LowerRow UpperRow LowerCol UpperCol",
@@ -1160,5 +1199,6 @@ void QANCollection::CommandsTest(Draw_Interpretor& theCommands) {
   theCommands.Add("QANColTestSequence",       "QANColTestSequence",       __FILE__, QANColTestSequence,       group);  
   theCommands.Add("QANColTestVector",         "QANColTestVector",         __FILE__, QANColTestVector,         group);  
   theCommands.Add("QANColTestArrayMove",      "QANColTestArrayMove (is expected to give error)", __FILE__, QANColTestArrayMove, group);  
+  theCommands.Add("QANColTestVec4",           "QANColTestVec4 test Vec4 implementation", __FILE__, QANColTestVec4, group);
   theCommands.Add("QATestAtof", "QATestAtof [nbvalues [nbdigits [min [max]]]]", __FILE__, QATestAtof, group);
 }

diff --git a/src/Select3D/Select3D_SensitivePrimitiveArray.cxx b/src/Select3D/Select3D_SensitivePrimitiveArray.cxx
index a06de722d3faad5c0a62df28ef944c6cb1e254e2..9fb57165cd29a00bb881d9cc2dffe9b56e9da269 100644 (file)
--- a/src/Select3D/Select3D_SensitivePrimitiveArray.cxx
+++ b/src/Select3D/Select3D_SensitivePrimitiveArray.cxx
@@ -365,7 +365,7 @@ bool Select3D_SensitivePrimitiveArray::InitTriangulation (const Handle(Graphic3d
       const Graphic3d_Vec2& aNode1 = getPosVec2 (aTriNodes[0]);
       const Graphic3d_Vec2& aNode2 = getPosVec2 (aTriNodes[1]);
       const Graphic3d_Vec2& aNode3 = getPosVec2 (aTriNodes[2]);
-      aCenter.xy() += (aNode1 + aNode2 + aNode3) / 3.0;
+      aCenter += Graphic3d_Vec3((aNode1 + aNode2 + aNode3) / 3.0);
     }
     if (myBvhIndices.HasPatches())
     {
@@ -543,7 +543,7 @@ bool Select3D_SensitivePrimitiveArray::InitPoints (const Handle(Graphic3d_Buffer
     else
     {
       aPnt2d = &getPosVec2 (aPointIndex);
-      aCenter.xy() += *aPnt2d;
+      aCenter += Graphic3d_Vec3(*aPnt2d);
     }
 
     if (myBvhIndices.HasPatches())

diff --git a/tests/collections/n/vec4 b/tests/collections/n/vec4
new file mode 100644 (file)
index 0000000..42c7f5f
--- /dev/null
+++ b/tests/collections/n/vec4
@@ -0,0 +1,3 @@
+puts "Check NCollection_Vec4 functionality"
+
+QANColTestVec4