View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0031733Open CASCADEOCCT:Visualizationpublic2020-08-25 12:422020-12-02 17:13
ReporterosaAssigned Tobugmaster  
PrioritynormalSeverityfeature 
Status closedResolutionfixed 
Target Version7.5.0Fixed in Version7.5.0 
Summary0031733: Visualization, Prs3d_ToolQuadric - create indexed arrays
DescriptionAdd possibility to return indexed arrays without duplicated vertices from methods Prs3d_ToolQuadric::FillArray().
Steps To ReproduceNot required
TagsNo tags attached.
Test case numberNot required

Relationships

parent of 0031773 closedmzernova Visualization - add Prs3d_ToolTorus 
parent of 0031876 closedbugmaster Configuration - Prs3d_Arrow::DrawShaded() crashes in msvc10 win32 Release build 
child of 0028010 closedapn Visualization, Prs3d_Arrow - add Shading presentation builder 

Activities

git

2020-08-31 17:28

administrator   ~0093771

Branch CR31733 has been created by mzernova.

SHA-1: 9676d418304602b1c577cc54851aa95b0ca5544c


Detailed log of new commits:

Author: mzernova
Date: Mon Aug 31 17:28:25 2020 +0300

    0031733: Visuaization, Prs3d_ToolQuadric - add indexed arrays
    
    The IsIndexedArray flag has been added as an argument to the Prs3d_ToolQuadric::FillArray() function.
    It indicates that the array should be returned indexed, without duplicated vertices.

mzernova

2020-08-31 17:30

developer   ~0093772

The patch CR31733 is ready to review

kgv

2020-08-31 17:50

developer   ~0093773 

-void Prs3d_ToolQuadric::FillArray (Handle(Graphic3d_ArrayOfTriangles)& theArray, const gp_Trsf& theTrsf)
+void Prs3d_ToolQuadric::FillArray (Handle(Graphic3d_ArrayOfTriangles)& theArray, const gp_Trsf& theTrsf, Standard_Boolean theIsIndexedArray)

Please split long lines (per argument).

-    theArray = new Graphic3d_ArrayOfTriangles (aTrianglesNb * 3, 0, Standard_True);
+    theArray = new Graphic3d_ArrayOfTriangles (
+      theIsIndexedArray ? (mySlicesNb + 1) * (myStacksNb + 1) : aTrianglesNb * 3,
+      theIsIndexedArray ? aTrianglesNb * 3 : 0,
+      Standard_True);

Please use another Graphic3d_ArrayOfTriangles constructor taking Graphic3d_ArrayFlags_VertexNormal bit-mask instead of anonymous Boolean flags.

-  Standard_EXPORT void FillArray (Handle(Graphic3d_ArrayOfTriangles)& theArray, const gp_Trsf& theTrsf);
+  Standard_EXPORT void FillArray (Handle(Graphic3d_ArrayOfTriangles)& theArray,
+                                  const gp_Trsf& theTrsf,
+                                  Standard_Boolean theIsIndexedArray = Standard_False);

Please set flag to TRUE and update existing code where necessary.

   //! Generate primitives for 3D quadric surface presentation and fill the given array and poly triangulation structure. Optional transformation is applied.
-  Standard_EXPORT void FillArray (Handle(Graphic3d_ArrayOfTriangles)& theArray, Handle(Poly_Triangulation)& theTriangulation, const gp_Trsf& theTrsf);
+  Standard_EXPORT void FillArray (Handle(Graphic3d_ArrayOfTriangles)& theArray,
+                                  Handle(Poly_Triangulation)& theTriangulation,
+                                  const gp_Trsf& theTrsf,
+                                  Standard_Boolean theIsIndexedArray = Standard_False);

I think this method is not used anywhere now.
I propose revising its code in scope of this patch:
- Define FillArray() filling in Poly_Triangulation without Graphic3d_ArrayOfTriangles
  (add wrapper with old syntax redirecting to 2 other methods, consider indexed is TRUE).
- Avoid creation of temporary Poly_Array1OfTriangle / TColgp_Array1OfPnt.

kgv

2020-08-31 18:42

developer   ~0093776 

-void Prs3d_ToolQuadric::FillArray (Handle(Graphic3d_ArrayOfTriangles)& theArray, const gp_Trsf& theTrsf)
+void Prs3d_ToolQuadric::FillArray (Handle(Graphic3d_ArrayOfTriangles)& theArray, const gp_Trsf& theTrsf, Standard_Boolean theIsIndexedArray)

theIsIndexedArray could be eliminated.
For NULL theArray it should be considered TRUE and for non-NULL - it should be determined from Edges allocation within theArray.

git

2020-09-01 13:43

administrator   ~0093814

Branch CR31733 has been updated by mzernova.

SHA-1: 3e30444ff309879ce8c0c095aa76357379de1910


Detailed log of new commits:

Author: mzernova
Date: Tue Sep 1 13:33:08 2020 +0300

    remarks from kgv

git

2020-09-01 13:43

administrator   ~0093815

Branch CR31733_1 has been created by mzernova.

SHA-1: e9e6865211221f02079f648b763147dfa9be8bdb


Detailed log of new commits:

Author: mzernova
Date: Mon Aug 31 17:28:25 2020 +0300

    0031733: Visuaization, Prs3d_ToolQuadric - add indexed arrays
    
    By default, the Prs3d_ToolQuadric::FillArray() function returns an indexed array.
    To use the old functionality, you can pass a non-indexed array as an argument to the function.
    The Prs3d_ToolQuadric::FillArray() method was defined to fill only the Poly_Triangulation without Graphic3d_ArrayOfTriangles.

mzernova

2020-09-01 13:43

developer   ~0093816

The patch CR31733 is ready to review

kgv

2020-09-01 14:18

developer   ~0093819 

@@ -57,25 +57,43 @@ void Prs3d_ToolQuadric::FillArray (Handle(Graphic3d_ArrayOfTriangles)& theArray,
   const Standard_Integer aTrianglesNb = TrianglesNb();
   if (theArray.IsNull())
   {
-    theArray = new Graphic3d_ArrayOfTriangles (aTrianglesNb * 3, 0, Standard_True);
+    theArray = new Graphic3d_ArrayOfTriangles ((mySlicesNb + 1) * (myStacksNb + 1),
+                                               aTrianglesNb * 3,

It is expected that Prs3d_ToolQuadric will provide a method returning a number of vertices within indexed array in similar way as method TrianglesNb() returns number of triangles.

+  const Standard_Integer aMaxEdges = aNbTris
+                                   + (aNbTrisTube > 0 ? 2 : 0)
+                                   + (aNbTrisCone > 0 ? 4 : 0);
+  Handle(Graphic3d_ArrayOfTriangles) anArray = new Graphic3d_ArrayOfTriangles (aMaxEdges , aNbTris * 3, Standard_True);

Why aMaxEdges, when first argument of Graphic3d_ArrayOfTriangles takes number of vertices?
Please also replace Standard_True with vertex attribute(s) bitmask here.

+  Standard_Boolean isIndexedArray = !theArray->Indices().IsNull() && theArray->Indices()->NbMaxElements() >= aTrianglesNb * 3;

const bool isIndexed = theArray->EdgeNumberAllocated() > 0;

+  Standard_Integer aSlicesNb = isIndexedArray ? mySlicesNb + 1 : mySlicesNb;
+  Standard_Integer aStacksNb = isIndexedArray ? myStacksNb + 1 : myStacksNb;

const

+  for (Standard_Integer aU = 0; aU < aSlicesNb; ++aU)
   {
+    for (Standard_Integer aV = 1; aV <= aStacksNb; ++aV)
     {
+      if (isIndexedArray)
+      {

I think code will be more clear if loops will be duplicated for indexed/non-indexed cases.

+void Prs3d_ToolQuadric::FillArray (Handle(Graphic3d_ArrayOfTriangles)& theArray,
+                                   Handle(Poly_Triangulation)& theTriangulation,
+                                   const gp_Trsf& theTrsf)
+{
+  theArray = new Graphic3d_ArrayOfTriangles ((mySlicesNb + 1) * (myStacksNb + 1), TrianglesNb() * 3);
+  FillArray (theArray, theTrsf);

I suppose theArray will be allocated anyway by next FillArray() call.

   //! Generate primitives for 3D quadric surface and fill the given array. Optional transformation is applied.
   Standard_EXPORT void FillArray (Handle(Graphic3d_ArrayOfTriangles)& theArray, const gp_Trsf& theTrsf);
 
+  //! Generate primitives for 3D quadric surface presentation and fill the given poly triangulation structure. Optional transformation is applied.
+  Standard_EXPORT void FillArray (Handle(Poly_Triangulation)& theTriangulation, const gp_Trsf& theTrsf);

Could you please improve documentation of these methods in scope of this patch?
E.g. describe @param arguments and elaborate behavior of NULL/non-NULL array input.

git

2020-09-02 13:44

administrator   ~0093863

Branch CR31733 has been updated by mzernova.

SHA-1: 6602d48872769f6f5670e8793aa9480937d34ad6


Detailed log of new commits:

Author: mzernova
Date: Wed Sep 2 13:45:58 2020 +0300

    remarks from kgv

git

2020-09-02 13:44

administrator   ~0093864

Branch CR31733_1 has been updated forcibly by mzernova.

SHA-1: ae27eabe4b5ad7c9e8fa312495daf2d481b5e8cc

mzernova

2020-09-02 17:49

developer   ~0093905

The patch CR31733 is ready to review

kgv

2020-09-02 20:14

developer   ~0093932

Last edited: 2020-09-03 00:23

 
@@ -57,25 +57,43 @@ void Prs3d_ToolQuadric::FillArray (Handle(Graphic3d_ArrayOfTriangles)& theArray,
   const Standard_Integer aTrianglesNb = TrianglesNb();
   if (theArray.IsNull())
   {
-    theArray = new Graphic3d_ArrayOfTriangles (aTrianglesNb * 3, 0, Standard_True);
+    theArray = new Graphic3d_ArrayOfTriangles (VerticesNb(), aTrianglesNb * 3, Graphic3d_ArrayFlags_VertexNormal);
   }
 
-  Poly_Array1OfTriangle aPolyTriangles (1, aTrianglesNb);
   TColgp_Array1OfPnt anArray (1, aTrianglesNb * 3);
   NCollection_Array1<gp_Dir> aNormals (1, aTrianglesNb * 3);
   fillArrays (theTrsf, anArray, aNormals);

The code performs redundant actions - creates temporary arrays which could be avoided.
Please refactor method to fill in Graphic3d_ArrayOfTriangles directly.
It might be simpler dropping fillArrays() method at all.

+void Prs3d_ToolQuadric::FillArray (Handle(Poly_Triangulation)& theTriangulation, const gp_Trsf& theTrsf)
 {
   const Standard_Integer aTrianglesNb = TrianglesNb();
+  theTriangulation = new Poly_Triangulation (aTrianglesNb * 3, aTrianglesNb, Standard_False);

It looks unexpected allocating Poly_Triangulation with number of nodes larger than Prs3d_ToolQuadric::VerticesNb().

+  //! @param theArray [out] the array of verticies
+  //!                       when array is NULL function create indexed array
+  //!                       when array is not NULL new vertices are added to it.

@param theArray [in] [out] the array of vertices;
when NULL, function will create an indexed array;
when not NULL, triangles will be appended to the end of array
(will raise an exception if reserved array size is not large enough).

+  Standard_EXPORT void FillArray (Handle(Poly_Triangulation)& theTriangulation, const gp_Trsf& theTrsf);

As Poly_Triangulation is unconditionally created - could be just a return value.
The method could be called CreatePolyTriangulation().
And similar method CreateTriangulation() returning Graphic3d_ArrayOfTriangles and redirecting to FillArray().

+  Standard_EXPORT void FillArray (Handle(Graphic3d_ArrayOfTriangles)& theArray,
+                                  Handle(Poly_Triangulation)& theTriangulation,
+                                  const gp_Trsf& theTrsf);

Please mark as Standard_DEPRECATED

kgv

2020-09-03 00:30

developer   ~0093952

Last edited: 2020-09-03 00:31

 Please also make sure that triangulator sorts vertices within a quad in the order expected by Graphic3d_Aspects::ToSkipFirstEdge():
  //!     1------2
  //!    /      /   Triangle # 1: 2-0-1; Triangle # 2: 0-2-3
  //!   0------3

(you may try drawing some objects using quadrics with temporarily enabled mesh edges to verify).

git

2020-09-03 10:18

administrator   ~0093961

Branch CR31733 has been updated by mzernova.

SHA-1: 7f9d41480f3de9d0ca28718ece2a66dd2b41a25e


Detailed log of new commits:

Author: mzernova
Date: Thu Sep 3 10:20:08 2020 +0300

    remarks from kgv

git

2020-09-03 10:19

administrator   ~0093962

Branch CR31733_1 has been updated forcibly by mzernova.

SHA-1: 5a67993e75e442fe0bfc0faee4b2b87013f0c085

git

2020-09-03 10:42

administrator   ~0093963

Branch CR31733 has been updated by mzernova.

SHA-1: 11bd2a289e3d74780a08c141a78ba6d3cef3a33c


Detailed log of new commits:

Author: mzernova
Date: Thu Sep 3 10:43:49 2020 +0300

    fix vertices order

git

2020-09-03 10:42

administrator   ~0093964

Branch CR31733_1 has been updated forcibly by mzernova.

SHA-1: 9d6a57484fbb3127d3b9c9aef4d63e48e6ba2af5

mzernova

2020-09-03 10:44

developer   ~0093965

The patch CR31733 is ready to review

kgv

2020-09-03 11:01

developer   ~0093966

Please check also names for new methods within my edited message

git

2020-09-03 11:39

administrator   ~0093968

Branch CR31733 has been updated by mzernova.

SHA-1: 6f154e174129b91a879f431697b88b5ed3ee6746


Detailed log of new commits:

Author: mzernova
Date: Thu Sep 3 11:31:29 2020 +0300

    Created CreatePolyTriangulation() And CreateTriangulation() methods

git

2020-09-03 11:39

administrator   ~0093969

Branch CR31733_1 has been updated forcibly by mzernova.

SHA-1: 8289316b05140d5d394d9d277d50cfd881109ebe

git

2020-09-03 14:40

administrator   ~0093984

Branch CR31733_2 has been created by kgv.

SHA-1: 7269e3ca60237e501a37444ff0728be9c8a5b9d7


Detailed log of new commits:

Author: mzernova
Date: Mon Aug 31 17:28:25 2020 +0300

    0031733: Visualization, Prs3d_ToolQuadric - create indexed arrays
    
    Prs3d_ToolQuadric has been modified to return an indexed triangulation.
    Added methods Prs3d_ToolQuadric::CreateTriangulation() and Prs3d_ToolQuadric::CreateTriangulation()
    as more straightforward API returning generated triangulation.
    Added missing const to constant methods.
    
    Confising method Prs3d_ToolQuadric::FillArray() filling both
    Graphic3d_ArrayOfTriangles and Poly_Triangulation at once has been marked deprecated.
    
    V3d_Trihedron, AIS_ViewCube now set Closed flag to groups with shaded sphere and arrows.

git

2020-09-03 15:01

administrator   ~0093989

Branch CR31733_2 has been updated forcibly by kgv.

SHA-1: b420eb621ddc3a0bb64384f6f20779734243ca58

git

2020-09-03 15:10

administrator   ~0093990

Branch CR31733_2 has been updated forcibly by kgv.

SHA-1: 6eb6a6a6f3ec31fa8232b411249673910cf1666f

kgv

2020-09-03 17:17

developer   ~0093995

Please raise the patch
- OCCT branch: CR31733_2

bugmaster

2020-09-05 12:09

administrator   ~0094066

Combination -
OCCT branch : IR-2020-09-04
master SHA - a516227511f3452f9f55b79c961265b0bf210793
a206de37fbfa0bf71bd534ae47192bbec23b8522
Products branch : IR-2020-09-04 SHA - 134448d280fb82bfb14ffc6255ac1cb53f7fd1cf
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 17319.18000000012 / 17284.370000000214 [+0.20%]
Products
Total CPU difference: 12059.670000000087 / 12007.590000000067 [+0.43%]
Windows-64-VC14:
OCCT
Total CPU difference: 18776.390625 / 18770.796875 [+0.03%]
Products
Total CPU difference: 13251.640625 / 13222.90625 [+0.22%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2020-09-05 12:35

administrator   ~0094076

Branch CR31733_2 has been deleted by inv.

SHA-1: 6eb6a6a6f3ec31fa8232b411249673910cf1666f

git

2020-09-05 12:35

administrator   ~0094077

Branch CR31733_1 has been deleted by inv.

SHA-1: 8289316b05140d5d394d9d277d50cfd881109ebe

git

2020-09-05 12:35

administrator   ~0094078

Branch CR31733 has been deleted by inv.

SHA-1: 6f154e174129b91a879f431697b88b5ed3ee6746

Related Changesets

occt: master f0da4970

2020-08-31 14:28:25

mzernova


Committer: bugmaster Details Diff		 0031733: Visualization, Prs3d_ToolQuadric - create indexed arrays

Prs3d_ToolQuadric has been modified to return an indexed triangulation.
Added methods Prs3d_ToolQuadric::CreateTriangulation() and Prs3d_ToolQuadric::CreateTriangulation()
as more straightforward API returning generated triangulation.
Added missing const to constant methods.

Confusing method Prs3d_ToolQuadric::FillArray() filling both
Graphic3d_ArrayOfTriangles and Poly_Triangulation at once has been marked deprecated.

V3d_Trihedron, AIS_ViewCube and AIS_Manipulator
now set Closed flag to groups with shaded sphere and arrows.		 Affected Issues
0031733
mod - src/AIS/AIS_Manipulator.cxx Diff File
mod - src/AIS/AIS_ViewCube.cxx Diff File
mod - src/Prs3d/Prs3d_Arrow.cxx Diff File
mod - src/Prs3d/Prs3d_ToolCylinder.cxx Diff File
mod - src/Prs3d/Prs3d_ToolCylinder.hxx Diff File
mod - src/Prs3d/Prs3d_ToolDisk.cxx Diff File
mod - src/Prs3d/Prs3d_ToolDisk.hxx Diff File
mod - src/Prs3d/Prs3d_ToolQuadric.cxx Diff File
mod - src/Prs3d/Prs3d_ToolQuadric.hxx Diff File
mod - src/Prs3d/Prs3d_ToolSector.cxx Diff File
mod - src/Prs3d/Prs3d_ToolSector.hxx Diff File
mod - src/Prs3d/Prs3d_ToolSphere.cxx Diff File
mod - src/Prs3d/Prs3d_ToolSphere.hxx Diff File
mod - src/V3d/V3d_Trihedron.cxx Diff File

Issue History

Date Modified Username Field Change
2020-08-25 12:42 osa New Issue
2020-08-25 12:42 osa Assigned To => kgv
2020-08-25 12:48 kgv Assigned To kgv => mzernova
2020-08-25 12:48 kgv Status new => assigned
2020-08-25 12:48 kgv Relationship added child of 0028010
2020-08-31 17:28 git Note Added: 0093771
2020-08-31 17:30 mzernova Note Added: 0093772
2020-08-31 17:30 mzernova Assigned To mzernova => osa
2020-08-31 17:30 mzernova Status assigned => resolved
2020-08-31 17:30 mzernova Steps to Reproduce Updated
2020-08-31 17:50 kgv Note Added: 0093773
2020-08-31 17:50 kgv Assigned To osa => mzernova
2020-08-31 17:50 kgv Status resolved => assigned
2020-08-31 18:42 kgv Note Added: 0093776
2020-09-01 13:43 git Note Added: 0093814
2020-09-01 13:43 git Note Added: 0093815
2020-09-01 13:43 mzernova Note Added: 0093816
2020-09-01 13:43 mzernova Assigned To mzernova => kgv
2020-09-01 13:43 mzernova Status assigned => resolved
2020-09-01 14:18 kgv Note Added: 0093819
2020-09-01 14:18 kgv Assigned To kgv => mzernova
2020-09-01 14:18 kgv Status resolved => assigned
2020-09-02 13:44 git Note Added: 0093863
2020-09-02 13:44 git Note Added: 0093864
2020-09-02 17:49 mzernova Note Added: 0093905
2020-09-02 17:49 mzernova Assigned To mzernova => kgv
2020-09-02 17:49 mzernova Status assigned => resolved
2020-09-02 20:14 kgv Note Added: 0093932
2020-09-02 20:14 kgv Assigned To kgv => mzernova
2020-09-02 20:14 kgv Status resolved => assigned
2020-09-03 00:23 kgv Note Edited: 0093932
2020-09-03 00:30 kgv Note Added: 0093952
2020-09-03 00:31 kgv Note Edited: 0093952
2020-09-03 10:18 git Note Added: 0093961
2020-09-03 10:19 git Note Added: 0093962
2020-09-03 10:42 git Note Added: 0093963
2020-09-03 10:42 git Note Added: 0093964
2020-09-03 10:44 mzernova Note Added: 0093965
2020-09-03 10:44 mzernova Assigned To mzernova => kgv
2020-09-03 10:44 mzernova Status assigned => resolved
2020-09-03 11:01 kgv Note Added: 0093966
2020-09-03 11:39 git Note Added: 0093968
2020-09-03 11:39 git Note Added: 0093969
2020-09-03 13:56 kgv Summary Visuaization, Prs3d_ToolQuadric - add indexed arrays => Visualization, Prs3d_ToolQuadric - create indexed arrays
2020-09-03 14:40 git Note Added: 0093984
2020-09-03 15:01 git Note Added: 0093989
2020-09-03 15:10 git Note Added: 0093990
2020-09-03 17:17 kgv Note Added: 0093995
2020-09-03 17:17 kgv Assigned To kgv => bugmaster
2020-09-03 17:17 kgv Status resolved => reviewed
2020-09-05 12:09 bugmaster Note Added: 0094066
2020-09-05 12:09 bugmaster Status reviewed => tested
2020-09-05 12:23 bugmaster Test case number => Not required
2020-09-05 12:25 bugmaster Changeset attached => occt master f0da4970
2020-09-05 12:25 bugmaster Status tested => verified
2020-09-05 12:25 bugmaster Resolution open => fixed
2020-09-05 12:35 git Note Added: 0094076
2020-09-05 12:35 git Note Added: 0094077
2020-09-05 12:35 git Note Added: 0094078
2020-09-18 09:16 kgv Relationship added parent of 0031773
2020-10-24 20:58 kgv Relationship added parent of 0031876
2020-12-02 16:22 emo Fixed in Version => 7.5.0
2020-12-02 17:13 emo Status verified => closed