MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0027870Open CASCADE[OCCT] OCCT:Modeling Algorithmspublic2016-09-14 00:462019-02-14 12:04
Reporterabv 
Assigned Toapn 
PrioritynormalSeverityminor 
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version[OCCT] 7.1.0Fixed in Version[OCCT] 7.1.0 
Summary0027870: Modeling - refactoring of HLR algorithms
DescriptionCurrent implementation of HLR algorithms is ugly and undebuggable, due to extensive usage of low-level types (Standard_Address) and macros involving low-level casts to access particular data elements, instead of using typed data structures and accessing them using names. As result, no static type checking is performed, and debugger cannot be used normally to view content of "variables" (actually macros) used throughout the code.

See e.g. HLRAlgo_PolyData.cxx. For instance, method HLRAlgo_PolyData::UpdateGlobalMinMax() should take Bnd_Box or equivalent structure, or at least double[2][3] as its argument, and not Standard_Address!
Steps To ReproduceN/A
TagsNo tags attached.
Test case numberNot needed
Attached Files

- Relationships

-  Notes
(0058154)
msv (developer)
2016-09-26 12:31

Dear Julia, please take care of this issue.
(0058496)
git (administrator)
2016-10-07 18:31

Branch CR27870 has been created by abk.

SHA-1: 4d969765e4ac4786dcb3e51954d12918067600a6


Detailed log of new commits:

Author: abk
Date: Wed Oct 5 13:49:44 2016 +0300

    HLRAlgo: Standard_Real[]
    
    HLRAlgo_EdgesBlock::MinMaxIndices
    
    PINod
    
    PISeg
    
    4
    
    3
    
    2
    
    1
(0058497)
git (administrator)
2016-10-07 18:42

Branch CR27870 has been updated forcibly by abk.

SHA-1: cfe0e49fd55834f26a57b82e59adfbaa47169b35
(0058502)
git (administrator)
2016-10-07 19:14

Branch CR27870 has been updated forcibly by abk.

SHA-1: 900b67e5ceac77518e2c283782a53653e4808b6c
(0058511)
abv (manager)
2016-10-10 07:41
edited on: 2016-10-10 09:09

Some remarks on first changes:

1. In HLRAlgo_BiPoint::HLRAlgo_BiPoint(), when initializing myPoints:

> myPoints.Pnt1 = gp_XYZ(X1, Y1, Z1);

can be better replaced by

> myPoints.Pnt1.SetCoord (X1, Y1, Z1);

2. In declaration of HLRAlgo::UpdateMinMax(), it is better to specify dimension of arrays: Standard_Real Min[16]. Casts of Min to Standard_Real* are not needed.

3. In HLRAlgo_BiPoint.hxx: (a) when defining a structure, it is better to define each field on its own line, (b) when defining single-line inline method, keep "{" and "}" on separate lines (unless whole method fits single line).

4. HLRAlgo_PolyData.hxx: why enum HLR_PolyMask is defined here? It is not used in this header at all! Please define it where it is used, as locally as possible.

(0058829)
git (administrator)
2016-10-17 21:18

Branch CR27870 has been updated forcibly by abk.

SHA-1: aba21216a01a9b6a7f00cdfd64f08cfa8162d29c
(0059064)
msv (developer)
2016-10-25 12:04

Dear Julia, please review the fix.
(0059245)
jgv (developer)
2016-10-27 12:48

My remarks:

1. HLRAlgo_PolyInternalData.cxx
line 323: why
Nod3RValues.Normal *= 1 / aNorm;
but not
Nod3RValues.Normal = aXYZ / aNorm; ?

2. HLRAlgo_PolyInternalData.cxx lines 698, 739;
HLRBRep_PolyAlgo.cxx lines 2471, 2942, 3125, 3135 :
there is a risk of usage of null pointer, it is better to avoid it like it is done in fix to OCC25214.
(0059260)
msv (developer)
2016-10-27 15:22

I propose to rebase the branch on CR25214, as it has already been tested.
(0059357)
git (administrator)
2016-10-28 16:46

Branch CR27870 has been updated forcibly by abk.

SHA-1: 6490d024bddca285f14032f9dc58d2d6148b63e8
(0059362)
abk (developer)
2016-10-28 16:52

The remarks were considered, the branch was rebased to the master.
Please review ASAP.
(0059363)
abk (developer)
2016-10-28 16:57

Please test.
(0059382)
git (administrator)
2016-10-28 18:41

Branch CR27870 has been updated forcibly by apv.

SHA-1: 847fe70ad1ad584ea140d6b8fffd9944c64eb1a2
(0059383)
apv (tester)
2016-10-28 18:42

Branch CR27870 has been rebased on the current master
(0059695)
apv (tester)
2016-11-01 15:03

Dear BugMaster,

Branch CR27870 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: 847fe70ad1ad584ea140d6b8fffd9944c64eb1a2

Number of compiler warnings:
occt component:
   Linux: 0 (0 on master)
   Windows: 0 (0 on master)
   MasOS: 0 (0 on master)
products component:
   Linux: 63
   Windows: 0
   MacOS: 1149

Regressions/Differences:
Not detected

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 91024361 / 90749168 [+0.30%]
Total CPU difference: 19467.219999999903 / 19313.639999999898 [+0.80%]

Testing on Windows:
Total MEMORY difference: 57306156 / 57321116 [-0.03%]
Total CPU difference: 17749.575378598707 / 18220.18359529866 [-2.58%]
(0059812)
apn (administrator)
2016-11-02 18:32

There are compilation errors in debug mode (build with preprocessor definition OCCT_DEBUG):

Building CXX object src/TKHLR/CMakeFiles/TKHLR.dir/__/HLRBRep/HLRBRep_ShapeBounds.cxx.o
/Users/mnt/builds/IR-2016-11-02_IR-2016-11-02/MacOS-deb/OCCT/src/HLRBRep/HLRBRep_PolyAlgo.cxx:906:12: error: use of undeclared identifier 'Nod11NrmX'
              if (Nod11NrmX*Nod12NrmX +
                  ^
/Users/mnt/builds/IR-2016-11-02_IR-2016-11-02/MacOS-deb/OCCT/src/HLRBRep/HLRBRep_PolyAlgo.cxx:906:22: error: use of undeclared identifier 'Nod12NrmX'
              if (Nod11NrmX*Nod12NrmX +
                            ^
/Users/mnt/builds/IR-2016-11-02_IR-2016-11-02/MacOS-deb/OCCT/src/HLRBRep/HLRBRep_PolyAlgo.cxx:907:5: error: use of undeclared identifier 'Nod11NrmY'
                  Nod11NrmY*Nod12NrmY +
                  ^
/Users/mnt/builds/IR-2016-11-02_IR-2016-11-02/MacOS-deb/OCCT/src/HLRBRep/HLRBRep_PolyAlgo.cxx:907:15: error: use of undeclared identifier 'Nod12NrmY'
                  Nod11NrmY*Nod12NrmY +
                            ^
/Users/mnt/builds/IR-2016-11-02_IR-2016-11-02/MacOS-deb/OCCT/src/HLRBRep/HLRBRep_PolyAlgo.cxx:908:15: error: use of undeclared identifier 'Nod12NrmZ'
                  Nod11NrmZ*Nod12NrmZ < 0) {
                            ^
/Users/mnt/builds/IR-2016-11-02_IR-2016-11-02/MacOS-deb/OCCT/src/HLRBRep/HLRBRep_PolyAlgo.cxx:908:5: error: use of undeclared identifier 'Nod11NrmZ'
                  Nod11NrmZ*Nod12NrmZ < 0) {
                  ^
/Users/mnt/builds/IR-2016-11-02_IR-2016-11-02/MacOS-deb/OCCT/src/HLRBRep/HLRBRep_PolyAlgo.cxx:1093:12: error: use of undeclared identifier 'Nod21NrmX'
              if (Nod21NrmX*Nod22NrmX +
                  ^
/Users/mnt/builds/IR-2016-11-02_IR-2016-11-02/MacOS-deb/OCCT/src/HLRBRep/HLRBRep_PolyAlgo.cxx:1093:22: error: use of undeclared identifier 'Nod22NrmX'
              if (Nod21NrmX*Nod22NrmX +
                  ^
/Users/mnt/builds/IR-2016-11-02_IR-2016-11-02/MacOS-deb/OCCT/src/HLRBRep/HLRBRep_PolyAlgo.cxx:1094:5: error: use of undeclared identifier 'Nod21NrmY'
                  Nod21NrmY*Nod22NrmY +
                  ^
/Users/mnt/builds/IR-2016-11-02_IR-2016-11-02/MacOS-deb/OCCT/src/HLRBRep/HLRBRep_PolyAlgo.cxx:1094:15: error: use of undeclared identifier 'Nod22NrmY'
                  Nod21NrmY*Nod22NrmY +
                  ^
/Users/mnt/builds/IR-2016-11-02_IR-2016-11-02/MacOS-deb/OCCT/src/HLRBRep/HLRBRep_PolyAlgo.cxx:1095:15: error: use of undeclared identifier 'Nod22NrmZ'
                  Nod21NrmZ*Nod22NrmZ < 0) {
                  ^
/Users/mnt/builds/IR-2016-11-02_IR-2016-11-02/MacOS-deb/OCCT/src/HLRBRep/HLRBRep_PolyAlgo.cxx:1095:5: error: use of undeclared identifier 'Nod21NrmZ'
                  Nod21NrmZ*Nod22NrmZ < 0) {
                  ^
/Users/mnt/builds/IR-2016-11-02_IR-2016-11-02/MacOS-deb/OCCT/src/HLRBRep/HLRBRep_PolyAlgo.cxx:2313:11: error: use of undeclared identifier 'Tri1Node1'
                cout << Tri1Node1 << " modifies : DX,DY ";
(0059843)
git (administrator)
2016-11-03 13:09

Branch CR27870 has been updated forcibly by abk.

SHA-1: e02a4642317745189856c90e5c3acda1abde4560
(0059847)
abk (developer)
2016-11-03 13:16

The cause of the compilation errors in the debug mode was fixed.
Please test again.
(0061249)
git (administrator)
2016-12-07 11:30

Branch CR27870 has been deleted by kgv.

SHA-1: e02a4642317745189856c90e5c3acda1abde4560

- Related Changesets
occt: master 681f3919
Timestamp: 2016-10-05 10:49:44
Author: abk
Committer: apn
Details ] Diff ]
0027870: Modeling - refactoring of HLR algorithms

Toolkit 'TKHLR' was fully refactored for 'Standard_Address' and macros except about half of package 'HLRBREP' there 'Standard_Address' is used through the 'generic' mechanism.
mod - src/Contap/Contap_ContAna.cxx Diff ] File ]
mod - src/Contap/Contap_Contour.cxx Diff ] File ]
mod - src/Contap/Contap_SurfFunction.cxx Diff ] File ]
mod - src/DBRep/DBRep_HideData.cxx Diff ] File ]
mod - src/HLRAlgo/FILES Diff ] File ]
mod - src/HLRAlgo/HLRAlgo.cxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo.hxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_BiPoint.cxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_BiPoint.hxx Diff ] File ]
rm - src/HLRAlgo/HLRAlgo_Coincidence.cxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_Coincidence.hxx Diff ] File ]
rm - src/HLRAlgo/HLRAlgo_Coincidence.lxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_EdgeIterator.cxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_EdgeIterator.hxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_EdgeIterator.lxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_EdgesBlock.cxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_EdgesBlock.hxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_PolyAlgo.cxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_PolyAlgo.hxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_PolyData.cxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_PolyData.hxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_PolyData.lxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_PolyHidingData.hxx Diff ] File ]
rm - src/HLRAlgo/HLRAlgo_PolyHidingData.lxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_PolyInternalData.cxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_PolyInternalData.hxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_PolyInternalNode.hxx Diff ] File ]
rm - src/HLRAlgo/HLRAlgo_PolyInternalNode.lxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_PolyInternalSegment.hxx Diff ] File ]
rm - src/HLRAlgo/HLRAlgo_PolyInternalSegment.lxx Diff ] File ]
add - src/HLRAlgo/HLRAlgo_PolyMask.hxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_PolyShellData.cxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_PolyShellData.hxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_PolyShellData.lxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_TriangleData.hxx Diff ] File ]
rm - src/HLRAlgo/HLRAlgo_TriangleData.lxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_WiresBlock.cxx Diff ] File ]
mod - src/HLRAlgo/HLRAlgo_WiresBlock.hxx Diff ] File ]
rm - src/HLRAlgo/HLRAlgo_WiresBlock.lxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_BiPnt2D.hxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_CLProps.hxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_CLPropsATool.hxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_CLPropsATool.lxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_CLProps_0.cxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_Curve.cxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_Curve.hxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_Curve.lxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_Data.cxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_Data.hxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_EdgeData.cxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_EdgeData.hxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_EdgeData.lxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_InternalAlgo.cxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_Intersector.cxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_Intersector.hxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_PolyAlgo.cxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_PolyAlgo.hxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_PolyHLRToShape.cxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_ShapeBounds.cxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_ShapeBounds.hxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_ShapeBounds.lxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_Surface.cxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_Surface.hxx Diff ] File ]
mod - src/HLRBRep/HLRBRep_Surface.lxx Diff ] File ]
mod - src/HLRTest/HLRTest_DrawablePolyEdgeTool.cxx Diff ] File ]
mod - src/HLRTopoBRep/HLRTopoBRep_Data.cxx Diff ] File ]
mod - src/HLRTopoBRep/HLRTopoBRep_Data.hxx Diff ] File ]
mod - src/HLRTopoBRep/HLRTopoBRep_DSFiller.cxx Diff ] File ]
mod - src/StdPrs/StdPrs_HLRPolyShape.cxx Diff ] File ]

- Issue History
Date Modified Username Field Change
2016-09-14 00:46 abv New Issue
2016-09-14 00:46 abv Assigned To => msv
2016-09-26 12:31 msv Note Added: 0058154
2016-09-26 12:31 msv Assigned To msv => jgv
2016-09-26 12:31 msv Status new => assigned
2016-10-04 15:57 abv Assigned To jgv => abk
2016-10-07 18:31 git Note Added: 0058496
2016-10-07 18:42 git Note Added: 0058497
2016-10-07 19:14 git Note Added: 0058502
2016-10-10 07:41 abv Note Added: 0058511
2016-10-10 09:09 abv Note Edited: 0058511 View Revisions
2016-10-17 21:18 git Note Added: 0058829
2016-10-17 21:21 abk Assigned To abk => abv
2016-10-17 21:21 abk Status assigned => resolved
2016-10-17 21:21 abk Steps to Reproduce Updated View Revisions
2016-10-17 23:13 abv Assigned To abv => jgv
2016-10-25 12:04 msv Note Added: 0059064
2016-10-27 12:48 jgv Note Added: 0059245
2016-10-27 12:48 jgv Status resolved => assigned
2016-10-27 12:48 jgv Assigned To jgv => abk
2016-10-27 15:22 msv Note Added: 0059260
2016-10-28 16:46 git Note Added: 0059357
2016-10-28 16:52 abk Note Added: 0059362
2016-10-28 16:52 abk Assigned To abk => jgv
2016-10-28 16:52 abk Status assigned => resolved
2016-10-28 16:52 abk Steps to Reproduce Updated View Revisions
2016-10-28 16:57 abk Note Added: 0059363
2016-10-28 16:57 abk Assigned To jgv => bugmaster
2016-10-28 16:57 abk Status resolved => reviewed
2016-10-28 17:09 apv Assigned To bugmaster => apv
2016-10-28 18:41 git Note Added: 0059382
2016-10-28 18:42 apv Note Added: 0059383
2016-11-01 13:27 apv Test case number => Not needed
2016-11-01 15:03 apv Note Added: 0059695
2016-11-01 15:03 apv Assigned To apv => bugmaster
2016-11-01 15:03 apv Status reviewed => tested
2016-11-02 18:32 apn Note Added: 0059812
2016-11-02 18:32 apn Assigned To bugmaster => abk
2016-11-02 18:32 apn Status tested => feedback
2016-11-03 13:09 git Note Added: 0059843
2016-11-03 13:16 abk Note Added: 0059847
2016-11-03 13:16 abk Assigned To abk => apn
2016-11-03 13:16 abk Status feedback => reviewed
2016-11-03 17:06 apn Status reviewed => tested
2016-11-03 17:10 apn Changeset attached => occt master 681f3919
2016-11-03 17:10 apn Status tested => verified
2016-11-03 17:10 apn Resolution open => fixed
2016-12-07 11:30 git Note Added: 0061249
2016-12-08 18:12 kgv Relationship added parent of 0028202
2016-12-09 16:28 aiv Status verified => closed
2016-12-09 16:40 aiv Fixed in Version => 7.1.0
2018-10-16 20:00 msv Relationship added parent of 0030243
2019-02-14 12:04 bugmaster Project Internal => Open CASCADE


Copyright © 2000 - 2019 MantisBT Team
Powered by Mantis Bugtracker