MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0031733Open CASCADE[OCCT] OCCT:Visualizationpublic2020-08-25 12:422020-09-18 09:16
Reporterosa 
Assigned Tobugmaster 
PrioritynormalSeverityfeature 
StatusverifiedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version[OCCT] 7.5.0Fixed in Version 
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
Attached Files

- Relationships
parent of 0031773assignedmzernova Visualization - add Prs3d_ToolTorus 
child of 0028010closedapn Visualization, Prs3d_Arrow - add Shading presentation builder 
Not all the children of this issue are yet resolved or closed.

-  Notes
(0093771)
git (administrator)
2020-08-31 17:28

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.
(0093772)
mzernova (developer)
2020-08-31 17:30

The patch CR31733 is ready to review
(0093773)
kgv (developer)
2020-08-31 17:50

-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.
(0093776)
kgv (developer)
2020-08-31 18:42

-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.
(0093814)
git (administrator)
2020-09-01 13:43

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

(0093815)
git (administrator)
2020-09-01 13:43

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.
(0093816)
mzernova (developer)
2020-09-01 13:43

The patch CR31733 is ready to review
(0093819)
kgv (developer)
2020-09-01 14:18

@@ -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.
(0093863)
git (administrator)
2020-09-02 13:44

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

(0093864)
git (administrator)
2020-09-02 13:44

Branch CR31733_1 has been updated forcibly by mzernova.

SHA-1: ae27eabe4b5ad7c9e8fa312495daf2d481b5e8cc
(0093905)
mzernova (developer)
2020-09-02 17:49

The patch CR31733 is ready to review
(0093932)
kgv (developer)
2020-09-02 20:14
edited on: 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

(0093952)
kgv (developer)
2020-09-03 00:30
edited on: 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).

(0093961)
git (administrator)
2020-09-03 10:18

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

(0093962)
git (administrator)
2020-09-03 10:19

Branch CR31733_1 has been updated forcibly by mzernova.

SHA-1: 5a67993e75e442fe0bfc0faee4b2b87013f0c085
(0093963)
git (administrator)
2020-09-03 10:42

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

(0093964)
git (administrator)
2020-09-03 10:42

Branch CR31733_1 has been updated forcibly by mzernova.

SHA-1: 9d6a57484fbb3127d3b9c9aef4d63e48e6ba2af5
(0093965)
mzernova (developer)
2020-09-03 10:44

The patch CR31733 is ready to review
(0093966)
kgv (developer)
2020-09-03 11:01

Please check also names for new methods within my edited message
(0093968)
git (administrator)
2020-09-03 11:39

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

(0093969)
git (administrator)
2020-09-03 11:39

Branch CR31733_1 has been updated forcibly by mzernova.

SHA-1: 8289316b05140d5d394d9d277d50cfd881109ebe
(0093984)
git (administrator)
2020-09-03 14:40

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.
(0093989)
git (administrator)
2020-09-03 15:01

Branch CR31733_2 has been updated forcibly by kgv.

SHA-1: b420eb621ddc3a0bb64384f6f20779734243ca58
(0093990)
git (administrator)
2020-09-03 15:10

Branch CR31733_2 has been updated forcibly by kgv.

SHA-1: 6eb6a6a6f3ec31fa8232b411249673910cf1666f
(0093995)
kgv (developer)
2020-09-03 17:17

Please raise the patch
- OCCT branch: CR31733_2
(0094066)
bugmaster (administrator)
2020-09-05 12:09

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
(0094076)
git (administrator)
2020-09-05 12:35

Branch CR31733_2 has been deleted by inv.

SHA-1: 6eb6a6a6f3ec31fa8232b411249673910cf1666f
(0094077)
git (administrator)
2020-09-05 12:35

Branch CR31733_1 has been deleted by inv.

SHA-1: 8289316b05140d5d394d9d277d50cfd881109ebe
(0094078)
git (administrator)
2020-09-05 12:35

Branch CR31733 has been deleted by inv.

SHA-1: 6f154e174129b91a879f431697b88b5ed3ee6746

- Related Changesets
occt: master f0da4970
Timestamp: 2020-08-31 14:28:25
Author: 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.
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 View Revisions
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 View Revisions
2020-09-03 00:30 kgv Note Added: 0093952
2020-09-03 00:31 kgv Note Edited: 0093952 View Revisions
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


Copyright © 2000 - 2020 MantisBT Team
Powered by Mantis Bugtracker