View Issue Details

IDProjectCategoryView StatusLast Update
0026083Open CASCADEOCCT:Codingpublic2021-08-23 22:59
Reporterabv Assigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version6.9.0 
Target Version7.6.0Fixed in Version7.6.0 
Summary0026083: Coding Rules - Poor design of Graphic3d_GraduatedTrihedron
DescriptionImplementation of Graphic3d_GraduatedTrihedron class is poor:

- Graphic3d_GraduatedTrihedron.hxx contains definition of the class Graphic3d_AxisAspect which should be either made separate class (if it is useful outside of GraduatedTrihedron) or made nested class (or even simple structure)

- It uses public fields (pointers!) for callback function and Visual3d_View which normally should be (a) private and get/set by appropriate methods and (b) objects (e.g. Handle for view) rather than plain pointers
Steps To ReproduceNot required
TagsNo tags attached.
Test case numberNot required

Relationships

related to 0025611 closedbugmaster Community Displaying 'zbufftrihedron' interferes with 'graduatedtrihedron' 

Activities

git

2021-08-04 14:53

administrator   ~0102978

Branch CR26083 has been created by CheskoArt.

SHA-1: 0649c22f81e152239a55bc3b1347139f24c44c2c


Detailed log of new commits:

Author: CheskoArt
Date: Wed Aug 4 14:37:46 2021 +0300

    0026083: Poor design of Graphic3d_GraduatedTrihedron
    
    - Made class Graphic3d_AxisAspect publicly nested in Graphic3d_GraduatedTrihedron.
    - Removed public PtrView field from trihedron and obtained it from OpenGL_Workspace while rendering.
    - Made CubicAxesCallback protected and provided corresponding getter/setter.
    - Renamed AxisAspect() to AxisAspectAt().

kgv

2021-08-04 21:06

developer   ~0102990

+  MinMaxValuesCallback CubicAxesCallback() const { return myCubicAxesCallback; }

I think there is no need exposing a getter to this class field, as it is used only internally. Only setter is needed.

CheskoArt

2021-08-05 12:42

developer   ~0102994

OpenGl_GraduatedTrihedron makes use of this callback, so it's used not only internally.

@@ -566,9 +566,9 @@ void OpenGl_GraduatedTrihedron::Render (const Handle(OpenGl_Workspace)& theWorks
   OpenGl_Vec3 anOldMin = myMin;
   OpenGl_Vec3 anOldMax = myMax;
 
-  if (myData.CubicAxesCallback)
+  if (myData.CubicAxesCallback() != NULL)
   {
-    myData.CubicAxesCallback (myData.PtrView);
+    myData.CubicAxesCallback() (theWorkspace->View());

kgv

2021-08-05 13:42

developer   ~0102997

> OpenGl_GraduatedTrihedron makes use of this callback,
> so it's used not only internally.
In this case, please redefine Graphic3d_GraduatedTrihedron::CubicAxesCallback() as a method taking arguments and calling a slot internally.

git

2021-08-06 12:31

administrator   ~0103023

Branch CR26083 has been updated forcibly by CheskoArt.

SHA-1: ff0975e6f3b6b99858fa84f60098b5a1e8f925f7

CheskoArt

2021-08-06 12:43

developer   ~0103024

Changed signature of Graphic3d_GraduatedTrihedron::CubicAxesCallback() to accept callback's parameter

git

2021-08-06 13:05

administrator   ~0103025

Branch CR26083 has been updated forcibly by CheskoArt.

SHA-1: 20d91ef39fb3da93666ceec0dc41a81955c08b5e

git

2021-08-06 13:20

administrator   ~0103026

Branch CR26083 has been updated forcibly by CheskoArt.

SHA-1: 0e5794db296dafd6e642d5c442cf196fa6db9e1b

git

2021-08-06 18:48

administrator   ~0103045

Branch CR26083 has been updated forcibly by CheskoArt.

SHA-1: 3a26eb529ca0d5e6d2ecf9794da5813eb91db3c6

CheskoArt

2021-08-06 18:49

developer   ~0103046

Ready for review

kgv

2021-08-07 14:05

developer   ~0103088

Patch should be rebased against the actual master before sending for testing/review.

git

2021-08-09 19:16

administrator   ~0103109

Branch CR26083 has been updated forcibly by CheskoArt.

SHA-1: 1b5eb2be231815cf76d379f441ada467ac6e44f7

CheskoArt

2021-08-09 19:19

developer   ~0103110

Updated branch. Ready for review.

bugmaster

2021-08-14 13:22

administrator   ~0103198

Combination -
OCCT branch : IR-2021-08-13
master SHA - 7b5f784419eb9fd9a1d3dc69eff89d3e720d6e97
a87b7ddc8cb44606b91e3f37113847c3f5f50fdc
Products branch : IR-2021-08-13 SHA - 8dc957c07e49e8db2f1330ec126160fe1c7eb89d
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: 17368.20000000041 / 17370.70000000038 [-0.01%]
Products
Total CPU difference: 11492.330000000085 / 11466.840000000084 [+0.22%]
Windows-64-VC14:
OCCT
Total CPU difference: 19143.8125 / 19180.875 [-0.19%]
Products
Total CPU difference: 12803.65625 / 12837.4375 [-0.26%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2021-08-23 22:59

administrator   ~0103352

Branch CR26083 has been deleted by kgv.

SHA-1: 1b5eb2be231815cf76d379f441ada467ac6e44f7

Related Changesets

occt: master 1b5eb2be

2021-08-04 11:37:46

achesnok

Details Diff
0026083: Coding Rules - Poor design of Graphic3d_GraduatedTrihedron

- Made class Graphic3d_AxisAspect publicly nested in Graphic3d_GraduatedTrihedron.
- Removed public PtrView field from trihedron and obtained it from OpenGL_Workspace while rendering.
- Made CubicAxesCallback protected and provided corresponding getter/setter.
- Renamed AxisAspect() to AxisAspectAt().
Affected Issues
0026083
mod - src/Graphic3d/Graphic3d_GraduatedTrihedron.hxx Diff File
mod - src/OpenGl/OpenGl_GraduatedTrihedron.cxx Diff File
mod - src/OpenGl/OpenGl_GraduatedTrihedron.hxx Diff File
mod - src/OpenGl/OpenGl_View.cxx Diff File

Issue History

Date Modified Username Field Change
2015-04-16 09:00 abv New Issue
2015-04-16 09:00 abv Assigned To => kgv
2015-04-16 09:13 kgv Relationship added related to 0025611
2016-11-01 06:41 abv Target Version 7.1.0 => 7.2.0
2017-07-20 11:38 kgv Target Version 7.2.0 => 7.4.0
2019-07-10 19:27 abv Target Version 7.4.0 => 7.5.0
2020-08-28 14:45 kgv Target Version 7.5.0 => 7.6.0
2021-08-02 11:40 age Assigned To kgv => CheskoArt
2021-08-04 14:33 CheskoArt Status new => assigned
2021-08-04 14:53 git Note Added: 0102978
2021-08-04 18:36 kgv Summary Poor design of Graphic3d_GraduatedTrihedron => Coding Rules - Poor design of Graphic3d_GraduatedTrihedron
2021-08-04 21:06 kgv Note Added: 0102990
2021-08-05 12:42 CheskoArt Note Added: 0102994
2021-08-05 13:42 kgv Note Added: 0102997
2021-08-06 12:31 git Note Added: 0103023
2021-08-06 12:43 CheskoArt Note Added: 0103024
2021-08-06 13:05 git Note Added: 0103025
2021-08-06 13:20 git Note Added: 0103026
2021-08-06 18:48 git Note Added: 0103045
2021-08-06 18:49 CheskoArt Note Added: 0103046
2021-08-06 18:49 CheskoArt Assigned To CheskoArt => kgv
2021-08-06 18:49 CheskoArt Status assigned => resolved
2021-08-06 18:49 CheskoArt Steps to Reproduce Updated
2021-08-07 14:05 kgv Note Added: 0103088
2021-08-07 14:05 kgv Assigned To kgv => CheskoArt
2021-08-07 14:05 kgv Status resolved => assigned
2021-08-09 19:16 git Note Added: 0103109
2021-08-09 19:19 CheskoArt Note Added: 0103110
2021-08-09 19:20 CheskoArt Assigned To CheskoArt => kgv
2021-08-09 19:20 CheskoArt Status assigned => resolved
2021-08-09 21:31 kgv Assigned To kgv => bugmaster
2021-08-09 21:31 kgv Status resolved => reviewed
2021-08-14 13:22 bugmaster Note Added: 0103198
2021-08-14 13:22 bugmaster Status reviewed => tested
2021-08-14 13:25 bugmaster Test case number => Not required
2021-08-14 13:31 Changeset attached => occt master 1b5eb2be
2021-08-14 13:31 bugmaster Status tested => verified
2021-08-14 13:31 bugmaster Resolution open => fixed
2021-08-23 22:59 git Note Added: 0103352