MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0024509Community[OCCT] OCCT:Application Frameworkpublic2014-01-02 01:492014-11-11 12:59
Reporterbarbier 
Assigned Toapn 
PrioritynormalSeverityminor 
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version[OCCT] 6.8.0Fixed in Version[OCCT] 6.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
Attached Files

- Relationships

-  Notes
(0028409)
szy (administrator)
2014-03-20 18:34

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
(0029093)
san (developer)
2014-04-28 12:13

dear aba,

Can you please process the issue, as it looks quite simple?
(0029128)
aba (developer)
2014-04-29 13:11

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.
(0029130)
aba (developer)
2014-04-29 13:13

The git branch CR24509 is ready to be reviewed.

Dear san,

please review.
(0029298)
san (developer)
2014-05-13 15:40

Branch CR24509 reviewed without remarks, ready for testing.
(0029372)
apn (administrator)
2014-05-16 14:37

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.
(0029384)
aba (developer)
2014-05-19 11:36
edited on: 2014-05-19 11:36

Warnings were corrected.

Dear Bugmaster,

please test.

(0029412)
apv (tester)
2014-05-20 15:40

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
Timestamp: 2014-05-22 13:13:04
Author: 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.
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 View Revisions
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 View Revisions
2014-11-11 12:44 user533 Fixed in Version => 6.8.0
2014-11-11 12:59 user533 Status verified => closed


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker