MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0025159Open CASCADE[OCCT] OCCT:Codingpublic2014-08-18 10:122014-11-11 13:02
Reporteroan 
Assigned Tobugmaster 
PrioritynormalSeveritymajor 
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version[OCCT] 6.8.0Fixed in Version[OCCT] 6.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
Attached Files

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

-  Notes
(0032002)
abv (manager)
2014-09-23 14:27

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.
(0032029)
abv (manager)
2014-09-23 16:57

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.
(0032113)
git (administrator)
2014-09-25 10:56

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.
(0032115)
git (administrator)
2014-09-25 11:41

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.

(0032126)
git (administrator)
2014-09-25 12:14

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.

(0032132)
git (administrator)
2014-09-25 12:54

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.

(0032146)
git (administrator)
2014-09-25 14:37

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.

(0032149)
git (administrator)
2014-09-25 14:50

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
(0032152)
dbp (developer)
2014-09-25 14:51

Dear abv,

please review the patch in branch CR25159_1.
(0032540)
abv (manager)
2014-10-01 20:02

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
(0032595)
apv (tester)
2014-10-02 16:56

Dear dbp,

Could you please rebase branch CR25159_1 on new master due conflicts in source code.
(0032598)
git (administrator)
2014-10-02 17:20

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%.
(0032600)
dbp (developer)
2014-10-02 17:24

Dear apv,

patch was rebased and pushed in branch CR25159_2.
(0032617)
apv (tester)
2014-10-03 12:17
edited on: 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/ [^]

(0032627)
git (administrator)
2014-10-03 13:49

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.

(0032628)
dbp (developer)
2014-10-03 13:50

Dear apv,

compilation error was fixed. Plese test (branch CR25159_2).
(0032650)
git (administrator)
2014-10-03 16:40

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.

(0032656)
git (administrator)
2014-10-03 16:56

Branch CR25159_2 has been updated forcibly by apv.

SHA-1: 3da0d683fb1867490873d9781f40b2e0227a518f
(0032658)
git (administrator)
2014-10-03 17:04

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.

(0032672)
apv (tester)
2014-10-03 19:22

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/ [^]
(0032674)
git (administrator)
2014-10-03 19:26

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%.
(0032691)
git (administrator)
2014-10-06 11:16

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.

(0032692)
dbp (developer)
2014-10-06 11:17

Dear kgv,

please review patch corrections in branch CR25159_3.
(0032693)
dbp (developer)
2014-10-06 11:22

Dear bugmaster,

please test the patch in branch CR25159_3.
(0032779)
apv (tester)
2014-10-07 16:30

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
(0033442)
git (administrator)
2014-10-21 16:43

Branch CR25159_3 has been deleted by inv.

SHA-1: 35662661462945cc502ba7bbbfdf2e6d5e74cebd
(0033443)
git (administrator)
2014-10-21 16:43

Branch CR25159_2 has been deleted by inv.

SHA-1: 3ce44c1581d885d580d918cf19bfeac312f2b432
(0033471)
git (administrator)
2014-10-21 16:44

Branch CR25159_1 has been deleted by inv.

SHA-1: d928fc7c6c5ef8dfea60e378cbff05eeb76037d9
(0033472)
git (administrator)
2014-10-21 16:44

Branch CR25159 has been deleted by inv.

SHA-1: 9646c2c458e8fc5c1e8b95db84762dd9afa4cde0

- Related Changesets
occt: master 679d3878
Timestamp: 2014-10-09 11:31:24
Author: dbp
Committer: bugmaster
Details ] Diff ]
0025159: Collections and common types in BVH package are named in non-conformant manner

Fix GCC compilation errors.
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 View Revisions
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


Copyright © 2000 - 2020 MantisBT Team
Powered by Mantis Bugtracker