View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0025159Open CASCADEOCCT:Codingpublic2014-08-18 10:122014-11-11 13:02
Reporteroan Assigned Tobugmaster  
PrioritynormalSeveritymajor 
Status closedResolutionfixed 
Target Version6.8.0Fixed in Version6.8.0 
Summary0025159: Collections and common types in BVH package are named in non-conformant manner
DescriptionIn OCCT we have convention that name of the global class should start from name of the package where it is defined, separated and the class, separated by underscore, and normally the class is defined in the file with same name (see http://dev.opencascade.org/doc/overview/html/occt_dev_guides__coding_rules.html#occt_coding_rules_2_2).

Functionality implemented within BVH package violates this convention multiple times for collection classes and base structures. Their definition is distributed between several files like BVH_Box.lxx, BVH_Properties.lxx, BVH_Types.hxx within BVHTools namespace. Custom types are defined in structures and have public availablity (e.g. BVH_Transform::BVH_MatNt, BVHTools::MatrixOp::BVH_Mat4t). At the same time these structures itself are named in non-conformant manner (e.g. BVHTools::MatrixOp (BVH_Properties.lxx), BVHTools::CenterAxis (BVH_Box.lxx)). Definition of several types within a single header file also takes place (e.g. BVH_Transform in BVH_Properties.hxx).

File BVH_Types.hxx does not contain neither namespace nor class corresponded to the name of this header file. It defines namespace BVHTools as well as several custom types such as BVH_Vec2i, BVH_Vec3i, BVH_Vec4i, etc. Note that BVH_Vec4i is widely used in OpenGL package.

The conformance to naming conventions is crucial in many situations, e.g. automatic tools working with the code, SWIG wrapper etc. Like this, it is quite important to put BVH package in order to fit the coding rules.
TagsNo tags attached.
Test case numberNot needed

Relationships

related to 0025314 closedbugmaster Usage of smart pointers for dynamically allocated memory in BVH 
related to 0025227 closedbugmaster Visualization - optimize BVH binned builder 
related to 0025234 closedbugmaster Implementing LBVH builder 

Activities

abv

2014-09-23 14:27

manager   ~0032002

Just to add to the above list, some functions are declared in LXX files, e.g. BHVTools::ArrayOp is defined in BVH_Tree.lxx. This is very confusing; it is almost impossible to find something in that code as our usual practices and even common sense is disturbed, very irritating.

abv

2014-09-23 16:57

manager   ~0032029

One general remark: the code seems to be underdocumented, e.g. I see no documentation for most of types defined in BVH_Types.*xx. Parameters of templates seem to be never documented.

git

2014-09-25 10:56

administrator   ~0032113

Branch CR25159 has been created by dbp.

SHA-1: 4e054c926077a40dbf99133cbe0920560483e9a6


Detailed log of new commits:

Author: dbp
Date: Thu Sep 25 10:56:30 2014 +0400

    Revise BVH types header.

git

2014-09-25 11:41

administrator   ~0032115

Branch CR25159 has been updated by dbp.

SHA-1: 1d96d0a5cf7dd8bdbda9a6888a67bebe15f3828c


Detailed log of new commits:

Author: dbp
Date: Thu Sep 25 11:41:25 2014 +0400

    Refactoring BVH box class.

git

2014-09-25 12:14

administrator   ~0032126

Branch CR25159 has been updated by dbp.

SHA-1: 9020f3cc1f57007a161ba090d67d11a390f8b6e6


Detailed log of new commits:

Author: dbp
Date: Thu Sep 25 12:14:45 2014 +0400

    Revising BVH object set.

git

2014-09-25 12:54

administrator   ~0032132

Branch CR25159 has been updated by dbp.

SHA-1: a93748666702ec3c3fcb48f85c63e82b3879d243


Detailed log of new commits:

Author: dbp
Date: Thu Sep 25 12:54:11 2014 +0400

    Update.

git

2014-09-25 14:37

administrator   ~0032146

Branch CR25159 has been updated by dbp.

SHA-1: 9646c2c458e8fc5c1e8b95db84762dd9afa4cde0


Detailed log of new commits:

Author: dbp
Date: Thu Sep 25 14:37:21 2014 +0400

    Update.

git

2014-09-25 14:50

administrator   ~0032149

Branch CR25159_1 has been created by dbp.

SHA-1: d928fc7c6c5ef8dfea60e378cbff05eeb76037d9


Detailed log of new commits:

Author: dbp
Date: Thu Sep 25 14:49:56 2014 +0400

    0025159: Collections and common types in BVH package are named in non-conformant manner

dbp

2014-09-25 14:51

developer   ~0032152

Dear abv,

please review the patch in branch CR25159_1.

abv

2014-10-01 20:02

manager   ~0032540

Partially reviewed in branch CR25159_1, please test. This branch includes fixes for 0025234 and #252227 which are thus also reviewed, to be tested together.

Note that not all problems reported in the bug description are eliminated. It seems to be necessary to elaborate special coding rules to apply to code heavily using templates.

One additional issue is registered as 0025314

apv

2014-10-02 16:56

tester   ~0032595

Dear dbp,

Could you please rebase branch CR25159_1 on new master due conflicts in source code.

git

2014-10-02 17:20

administrator   ~0032598

Branch CR25159_2 has been created by dbp.

SHA-1: 33499475c9b361f0165a5ade6d7cc20b94ee35fd


Detailed log of new commits:

Author: dbp
Date: Thu Sep 25 14:49:56 2014 +0400

    0025159: Collections and common types in BVH package are named in non-conformant manner

Author: dbp
Date: Wed Sep 24 19:32:40 2014 +0400

    0025234: Implementing LBVH builder
    
    Performs fast BVH construction using LBVH building approach. Algorithm uses spatial Morton codes to reduce the BVH construction problem to a sorting problem (radix sort -- O(N) complexity). This Linear Bounding Volume Hierarchy (LBVH) builder produces BVH trees of lower quality compared to SAH-based BVH builders but it is over an order of magnitude faster (up to 4M triangles per second).

Author: dbp
Date: Wed Sep 24 12:03:22 2014 +0400

    0025227: Visualization - optimize BVH binned builder
    BVH binned builder is used for different rendering aspects, such as view frustum culling, ray-tracing, and (in future) for selection. It is desirable to improve builder performance. This simple patch decreases BVH building time for 30-35%.

dbp

2014-10-02 17:24

developer   ~0032600

Dear apv,

patch was rebased and pushed in branch CR25159_2.

apv

2014-10-03 12:17

tester   ~0032617

Last edited: 2014-10-03 12:19

 Dear BugMaster,

There is error during branch CR25159_2 building:
Error : File BVH_Types.lxx could not be found

More information could be found by the following link:
http://jenkins-test-02.nnov.opencascade.com/user/mnt/my-views/view/CR25159_2/job/mnt-CR25159_2-master_prepare_occt_linux/2/parsed_console/

git

2014-10-03 13:49

administrator   ~0032627

Branch CR25159_2 has been updated by dbp.

SHA-1: 59ed7bcd888963e41b1d89df34739a92ce5b0b47


Detailed log of new commits:

Author: dbp
Date: Fri Oct 3 13:48:07 2014 +0400

    Fix file list in BVH package.

dbp

2014-10-03 13:50

developer   ~0032628

Dear apv,

compilation error was fixed. Plese test (branch CR25159_2).

git

2014-10-03 16:40

administrator   ~0032650

Branch CR25159_2 has been updated by dbp.

SHA-1: f9fdb9f58b35d831f1995d4651e03e1bed7248bc


Detailed log of new commits:

Author: dbp
Date: Fri Oct 3 16:40:16 2014 +0400

    Fix error in BVH_Box.

git

2014-10-03 16:56

administrator   ~0032656

Branch CR25159_2 has been updated forcibly by apv.

SHA-1: 3da0d683fb1867490873d9781f40b2e0227a518f

git

2014-10-03 17:04

administrator   ~0032658

Branch CR25159_2 has been updated by dbp.

SHA-1: 3ce44c1581d885d580d918cf19bfeac312f2b432


Detailed log of new commits:

Author: dbp
Date: Fri Oct 3 17:04:00 2014 +0400

    Fix error in BVH_Box.lxx.

apv

2014-10-03 19:22

tester   ~0032672

Dear BugMaster,

There are 5 errors during branch CR25159_2 compilation on Linux:
1. ../../../../inc/BVH_LinearBuilder.lxx:152: error: expected primary-expression before ‘const’
2. ../../../../inc/BVH_LinearBuilder.lxx:152: error: expected ‘;’ before ‘const’
3. ../../../../inc/BVH_LinearBuilder.lxx:156: error: ‘aBox’ was not declared in this scope
4. ../../../../inc/BVH_LinearBuilder.lxx:161: error: ‘aBox’ was not declared in this scope
5. ../../../../inc/BVH_LinearBuilder.lxx:182: error: ‘myLeafNodeSize’ was not declared in this scope
More information could be found by the following link:
http://jenkins-test-02.nnov.opencascade.com/user/mnt/my-views/view/CR25159_2/job/mnt-CR25159_2-master_build_occt_linux/1/parsed_console/

Moreover, 2 compilation errors are detected on MacOS:
1. /Users/mnt/tools/WOK671/wok_entities/LOC/dev/CR25159-2-master-occt/inc/BVH_LinearBuilder.lxx:152:24: error: expected a qualified name after 'typename'
2. /Users/mnt/tools/WOK671/wok_entities/LOC/dev/CR25159-2-master-occt/inc/BVH_LinearBuilder.lxx:182:29: error: use of undeclared identifier 'myLeafNodeSize'
More information:
http://jenkins-test-02.nnov.opencascade.com/user/mnt/my-views/view/CR25159_2/job/mnt-CR25159_2-master_prepare_build_occt_MacOS/1/parsed_console/

git

2014-10-03 19:26

administrator   ~0032674

Branch CR25159_3 has been created by dbp.

SHA-1: 4160c134e49e69c8568fa1619fb7e1796ec72360


Detailed log of new commits:

Author: dbp
Date: Fri Oct 3 19:25:28 2014 +0400

    0025159: Collections and common types in BVH package are named in non-conformant manner

Author: dbp
Date: Wed Sep 24 19:32:40 2014 +0400

    0025234: Implementing LBVH builder
    
    Performs fast BVH construction using LBVH building approach. Algorithm uses spatial Morton codes to reduce the BVH construction problem to a sorting problem (radix sort -- O(N) complexity). This Linear Bounding Volume Hierarchy (LBVH) builder produces BVH trees of lower quality compared to SAH-based BVH builders but it is over an order of magnitude faster (up to 4M triangles per second).

Author: dbp
Date: Wed Sep 24 12:03:22 2014 +0400

    0025227: Visualization - optimize BVH binned builder
    BVH binned builder is used for different rendering aspects, such as view frustum culling, ray-tracing, and (in future) for selection. It is desirable to improve builder performance. This simple patch decreases BVH building time for 30-35%.

git

2014-10-06 11:16

administrator   ~0032691

Branch CR25159_3 has been updated by dbp.

SHA-1: 35662661462945cc502ba7bbbfdf2e6d5e74cebd


Detailed log of new commits:

Author: dbp
Date: Mon Oct 6 11:16:01 2014 +0400

    Fix GCC compilation errors.

dbp

2014-10-06 11:17

developer   ~0032692

Dear kgv,

please review patch corrections in branch CR25159_3.

dbp

2014-10-06 11:22

developer   ~0032693

Dear bugmaster,

please test the patch in branch CR25159_3.

apv

2014-10-07 16:30

tester   ~0032779

Dear BugMaster,

Branch CR25159_3 (and products from GIT master) was compiled on Linux, Windows and MacOS platforms and tested.
SHA-1: 35662661462945cc502ba7bbbfdf2e6d5e74cebd

Number of compiler warnings:
occt component:
   Linux: 15 (15 on master)
   Windows: 0 (0 on master)
products component:
   Linux: 11 (11 on master)
   Windows: 1 (1 on master)

Regressions/Differences:
Not detected

Testing case:
Not needed

Testing on Linux:
Total MEMORY difference: 397826608 / 398004436
Total CPU difference: 46360.77000000022 / 47309.68000000025

Testing on Windows:
Total MEMORY difference: 278176200 / 278287368
Total CPU difference: 35521.453125 / 32988.859375

git

2014-10-21 16:43

administrator   ~0033442

Branch CR25159_3 has been deleted by inv.

SHA-1: 35662661462945cc502ba7bbbfdf2e6d5e74cebd

git

2014-10-21 16:43

administrator   ~0033443

Branch CR25159_2 has been deleted by inv.

SHA-1: 3ce44c1581d885d580d918cf19bfeac312f2b432

git

2014-10-21 16:44

administrator   ~0033471

Branch CR25159_1 has been deleted by inv.

SHA-1: d928fc7c6c5ef8dfea60e378cbff05eeb76037d9

git

2014-10-21 16:44

administrator   ~0033472

Branch CR25159 has been deleted by inv.

SHA-1: 9646c2c458e8fc5c1e8b95db84762dd9afa4cde0

Related Changesets

occt: master 679d3878

2014-10-09 11:31:24

dbp


Committer: bugmaster Details Diff		 0025159: Collections and common types in BVH package are named in non-conformant manner

Fix GCC compilation errors.		 Affected Issues
0025159
mod - src/BVH/BVH_BinnedBuilder.lxx Diff File
mod - src/BVH/BVH_Box.hxx Diff File
mod - src/BVH/BVH_Box.lxx Diff File
mod - src/BVH/BVH_Builder.hxx Diff File
mod - src/BVH/BVH_Geometry.hxx Diff File
mod - src/BVH/BVH_LinearBuilder.lxx Diff File
mod - src/BVH/BVH_Object.hxx Diff File
mod - src/BVH/BVH_ObjectSet.hxx Diff File
mod - src/BVH/BVH_ObjectSet.lxx Diff File
mod - src/BVH/BVH_PrimitiveSet.hxx Diff File
mod - src/BVH/BVH_Set.hxx Diff File
mod - src/BVH/BVH_SweepPlaneBuilder.lxx Diff File
mod - src/BVH/BVH_Tree.hxx Diff File
mod - src/BVH/BVH_Tree.lxx Diff File
mod - src/BVH/BVH_Triangulation.hxx Diff File
mod - src/BVH/BVH_Triangulation.lxx Diff File
mod - src/BVH/BVH_Types.hxx Diff File
rm - src/BVH/BVH_Types.lxx Diff File
mod - src/BVH/FILES Diff File

Issue History

Date Modified Username Field Change
2014-08-18 10:12 oan New Issue
2014-08-18 10:12 oan Assigned To => abv
2014-08-29 20:16 abv Assigned To abv => dbp
2014-08-29 20:16 abv Status new => assigned
2014-09-11 18:05 abv Target Version => 6.8.0
2014-09-23 14:27 abv Note Added: 0032002
2014-09-23 16:57 abv Note Added: 0032029
2014-09-25 10:56 git Note Added: 0032113
2014-09-25 11:41 git Note Added: 0032115
2014-09-25 12:14 git Note Added: 0032126
2014-09-25 12:54 git Note Added: 0032132
2014-09-25 14:37 git Note Added: 0032146
2014-09-25 14:50 git Note Added: 0032149
2014-09-25 14:51 dbp Note Added: 0032152
2014-09-25 14:51 dbp Assigned To dbp => abv
2014-09-25 14:51 dbp Status assigned => resolved
2014-10-01 19:47 abv Relationship added related to 0025314
2014-10-01 20:02 abv Note Added: 0032540
2014-10-01 20:04 abv Assigned To abv => bugmaster
2014-10-01 20:04 abv Status resolved => reviewed
2014-10-01 20:12 abv Relationship added related to 0025227
2014-10-01 20:12 abv Relationship added related to 0025234
2014-10-02 16:11 apv Assigned To bugmaster => apv
2014-10-02 16:56 apv Note Added: 0032595
2014-10-02 16:56 apv Assigned To apv => dbp
2014-10-02 16:56 apv Status reviewed => feedback
2014-10-02 17:20 git Note Added: 0032598
2014-10-02 17:24 dbp Note Added: 0032600
2014-10-02 17:24 dbp Assigned To dbp => apv
2014-10-02 17:24 dbp Status feedback => assigned
2014-10-03 12:17 apv Assigned To apv => dbp
2014-10-03 12:17 apv Note Added: 0032617
2014-10-03 12:19 apv Note Edited: 0032617
2014-10-03 13:49 git Note Added: 0032627
2014-10-03 13:50 dbp Assigned To dbp => apv
2014-10-03 13:50 dbp Note Added: 0032628
2014-10-03 16:40 git Note Added: 0032650
2014-10-03 16:56 git Note Added: 0032656
2014-10-03 17:04 git Note Added: 0032658
2014-10-03 19:17 apv Assigned To apv => dbp
2014-10-03 19:22 apv Note Added: 0032672
2014-10-03 19:26 git Note Added: 0032674
2014-10-06 11:16 git Note Added: 0032691
2014-10-06 11:17 dbp Note Added: 0032692
2014-10-06 11:17 dbp Assigned To dbp => kgv
2014-10-06 11:17 dbp Status assigned => resolved
2014-10-06 11:22 dbp Note Added: 0032693
2014-10-06 11:22 dbp Assigned To kgv => bugmaster
2014-10-06 11:22 dbp Status resolved => assigned
2014-10-06 17:49 bugmaster Assigned To bugmaster => apv
2014-10-07 14:50 abv Status assigned => feedback
2014-10-07 16:27 apv Test case number => Not needed
2014-10-07 16:30 apv Note Added: 0032779
2014-10-07 16:30 apv Status feedback => tested
2014-10-07 16:30 apv Assigned To apv => bugmaster
2014-10-13 17:52 bugmaster Changeset attached => occt master 679d3878
2014-10-13 17:52 bugmaster Status tested => verified
2014-10-13 17:52 bugmaster Resolution open => fixed
2014-10-21 16:43 git Note Added: 0033442
2014-10-21 16:43 git Note Added: 0033443
2014-10-21 16:44 git Note Added: 0033471
2014-10-21 16:44 git Note Added: 0033472
2014-11-11 12:43 aiv Fixed in Version => 6.8.0
2014-11-11 13:02 aiv Status verified => closed