View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0024509CommunityOCCT:Application Frameworkpublic2014-01-02 01:492014-11-11 12:59
Reporterbarbier Assigned Toapn  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Target Version6.8.0Fixed in Version6.8.0 
Summary0024509: Suspect unused variable in TPrsStd_ConstraintTools.cxx
DescriptionHello,

Lines 295--306 of this file are:

    if (is_directed) {
      GetGoodShape(shape3);
      const TopoDS_Edge& E = TopoDS::Edge(shape3);
      BRepAdaptor_Curve CURVE(E);
      Handle_Geom_Geometry aGeomGeometry = CURVE.Curve().Curve()->Transformed(CURVE.Trsf()) ;
      gp_Dir Dir = ((Handle(Geom_Line)&) aGeomGeometry)->Lin().Direction();
      gp_Dir xdir(aplane->Pln().Position().XDirection());
      if (Dir.IsParallel(xdir,Precision::Confusion()))
        typedist = AIS_TOD_Horizontal;
      else
        typedist = AIS_TOD_Vertical;
    }
  }

But since typedist is not used afterwards, this whole block looks pretty useless.
Additional information
and documentation updates		TPrsStdConstaraintTool::ComuteDistance() that builds length dimensions, was revised and redesigned to remove unused variables and to update method behavior after AIS_LengthDimension redesign (#24133).

1) AIS_TypeOfDist in TPrsStdConstaraintTool::ComuteDistance() method became unused.
In old dimension implementation it defined what type (vertical or horizontal relatively to screen ) dimension would be.
In the current implementation dimension line is parallel to attachment points line, so there is no need in AIS_TypeOfDist object.

2) TPrsStd length constraint now can build dimension not only:
    - on two vertices
    - on two faces
    - on edge and face,

but also:
    - on two edges
    - on one edge

3) Dimension in TPrsStd_ConstraintTool is build on one or two shapes, but not on three. Third shape was used earlier to determine direction of dimension (vertical or horizontal), but now dimension direction is computed as perpendicular to the attachment points line.
TagsNo tags attached.
Test case numberNot needed

Activities

szy

2014-03-20 18:34

manager   ~0028409

The corresponding part of code was removed by you by integration from branch 24133 at 31.10.2013. After that the mentioned variable 'typedist/ became unused.
Could you check it once again and explain your idea. This class is not used during dimension calculation and designed for another purpose.
szy

san

2014-04-28 12:13

developer   ~0029093

dear aba,

Can you please process the issue, as it looks quite simple?

aba

2014-04-29 13:11

developer   ~0029128

In the old AIS_LengthDimension implementation the variable of AIS_TypeOfDist type is used to compute direction of dimension flyouts.
Dimension flyout direction was fully determined by plane local coordinate system (plane's X and Y axes).

Changes:
1) In the current dimension implementation direction of flyout is computed from attachment points line and given working plane without need of additional parameters, so there is no need in AIS_TypeOfDist object.
2) Now AIS_TypeOfDist is used only in relation classes.
3) Dimension in TPrsStd_ConstraintTool is build on one or two shapes,
but not on three. Third shape was used earlier to determine direction
of dimension (vertical or horizontal), but now dimension direction is computed as perpendicular to the attachment points line.

aba

2014-04-29 13:13

developer   ~0029130

The git branch CR24509 is ready to be reviewed.

Dear san,

please review.

san

2014-05-13 15:40

developer   ~0029298

Branch CR24509 reviewed without remarks, ready for testing.

apn

2014-05-16 14:37

administrator   ~0029372

Dear BugMaster,

Branch CR24509 (and products from GIT master) was compiled on Linux, Windows and MacOS platforms and tested.
SHA-1: 6e20d78bfca479a504be64c7166b6e91ff331522

Number of compiler warnings:

occt component :
Linux: 18 (17 on master)
TPrsStd_ConstraintTools.cxx:346, GNU C Compiler 4 (gcc), Priority: Normal
suggest parentheses around ‘&&’ within ‘||’

Windows: 0 (0 on master)

MacOS: 207 (212 on master)
TPrsStd_ConstraintTools.cxx:345, Clang (LLVM based), Priority: Normal
'&&' within '||'
TPrsStd_ConstraintTools.cxx:346, Clang (LLVM based), Priority: Normal
'&&' within '||'

products component :
Linux: 11 (11 on master)
Windows: 2 (2 on master)

Regressions/Differences:
No regressions

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 355755200 / 355968116
Total CPU difference: 50096.739999999765 / 51289.87999999977

Testing on Windows:
Total MEMORY difference: 380262364 / 380248128
Total CPU difference: 38512.625 / 35796.140625

There are no differences in images found by testdiff.

aba

2014-05-19 11:36

developer   ~0029384

Last edited: 2014-05-19 11:36

 Warnings were corrected.

Dear Bugmaster,

please test.

apv

2014-05-20 15:40

tester   ~0029412

Dear BugMaster,

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

Number of compiler warnings:

occt component :
Linux: 17 (17 on master)
Windows: 0 (0 on master)
MacOS: 202 (205 on master)

products component :
Linux: 11 (11 on master)
Windows: 2 (2 on master)

Regressions/Differences:
No regressions

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 356334564 / 357316044
Total CPU difference: 57274.68000000003 / 56363.22999999993

Testing on Windows:
Total MEMORY difference: 379138220 / 379627876
Total CPU difference: 40654.53125 / 35503.40625

Related Changesets

occt: master 011b361d

2014-05-22 13:13:04

aba


Committer: apn Details Diff		 0024509: Suspect unused variable in TPrsStd_ConstraintTools.cxx

- AIS_TypeOfDist is not used for length dimension construction;
- Added opportunity to build TPrsSrd length constraint on one edge.
- length dimension in TPrsStd_ConstraintTool is build on one or two shapes, but not on three.
Added validation of dimension plane.
Warnings were corrected.		 Affected Issues
0024509
mod - src/TPrsStd/TPrsStd_ConstraintTools.cxx Diff File

Issue History

Date Modified Username Field Change
2014-01-02 01:49 barbier New Issue
2014-01-02 01:49 barbier Assigned To => szy
2014-03-20 18:34 szy Note Added: 0028409
2014-03-20 18:34 szy Assigned To szy => aba
2014-03-20 18:34 szy Status new => assigned
2014-04-28 12:13 san Note Added: 0029093
2014-04-28 12:13 san Status assigned => feedback
2014-04-28 12:18 san Target Version => 6.8.0
2014-04-29 13:05 aba Status feedback => assigned
2014-04-29 13:11 aba Note Added: 0029128
2014-04-29 13:13 aba Note Added: 0029130
2014-04-29 13:13 aba Assigned To aba => san
2014-04-29 13:13 aba Status assigned => resolved
2014-05-13 15:40 san Note Added: 0029298
2014-05-13 15:40 san Assigned To san => bugmaster
2014-05-13 15:40 san Status resolved => reviewed
2014-05-14 16:36 apn Assigned To bugmaster => apn
2014-05-16 14:37 apn Note Added: 0029372
2014-05-16 14:38 apn Test case number => Not needed
2014-05-16 14:38 apn Assigned To apn => aba
2014-05-16 14:38 apn Status reviewed => assigned
2014-05-19 11:35 aba Status assigned => resolved
2014-05-19 11:36 aba Note Added: 0029384
2014-05-19 11:36 aba Assigned To aba => bugmaster
2014-05-19 11:36 aba Status resolved => reviewed
2014-05-19 11:36 aba Note Edited: 0029384
2014-05-20 15:40 apv Note Added: 0029412
2014-05-20 15:42 apv Status reviewed => tested
2014-05-23 14:25 apn Changeset attached => occt master 011b361d
2014-05-23 14:25 apn Assigned To bugmaster => apn
2014-05-23 14:25 apn Status tested => verified
2014-05-23 14:25 apn Resolution open => fixed
2014-10-06 12:30 aba Additional Information Updated
2014-11-11 12:44 aiv Fixed in Version => 6.8.0
2014-11-11 12:59 aiv Status verified => closed