View Issue Details

IDProjectCategoryView StatusLast Update
0024955Open CASCADEOCCT:Codingpublic2014-11-11 12:52
ReporterabvAssigned Toapn  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
PlatformWindowsOSVC++ 2010 
Product Version6.7.1 
Target Version6.8.0Fixed in Version6.8.0 
Summary0024955: Misuse of DownCast
DescriptionIn some places over OCCT code DownCast() is used between incompatible types, e.g. casting from Geom_TrimmedCurve to Geom_BSplineCurve. Such cast will never succeed, thus the code must be revised.

The problem becomes apparent when method DownCast is made inline and template, using static_cast<> for casting pointer (made in the frames of 0024947), as compiler errors emerge:

GeomFill\GeomFill_NSections.cxx(537): error C2440: 'static_cast' : cannot convert from 'Geom_TrimmedCurve *' to 'Geom_BSplineCurve *
BRepFill\BRepFill_NSections.cxx(153): error C2440: 'static_cast' : cannot convert from 'Geom_TrimmedCurve *' to 'Geom_BSplineCurve *'
ShapeUpgrade\ShapeUpgrade_ShapeDivideAngle.cxx(76): error C2440: 'static_cast' : cannot convert from 'ShapeUpgrade_FaceDivide *' to 'ShapeUpgrade_SplitSurfaceAngle *'
AIS\AIS_Dimension.cxx(1109):error C2440: 'static_cast' : cannot convert from 'Geom_TrimmedCurve *' to 'Geom_Circle *'
\AIS\AIS_InteractiveObject.cxx(541): error C2440: 'static_cast' : cannot convert from 'PrsMgr_Presentation *' to 'Prs3d_Presentation *'
BRepOffsetAPI\BRepOffsetAPI_ThruSections.cxx(931): error C2440: 'static_cast' : cannot convert from 'Geom_TrimmedCurve *' to 'Geom_BSplineCurve *'

TagsNo tags attached.
Test case numberNot needed

Relationships

child of 0024947 closedabv Redesign OCCT legacy type system 

Activities

abv

2014-05-26 17:45

manager   ~0029518

Fix pushed to CR24955, please review

ifv

2014-05-27 10:23

developer   ~0029529

In new method EdgeToBSpline(...):

Handle(Geom_Curve) aCurve = BRep_Tool::Curve (theEdge, aLoc, aFirst, aLast);
....
if (aCurve->IsKind(STANDARD_TYPE(Geom_Conic)))
{
}

If aCurve returned by BRep_Tool::Curve is Geom_TrimmedCurve with basic curve of type Geom_Conic, test for Geom_Conic is wrong.

ifv

2014-05-27 10:46

developer   ~0029530

In addition to previous note:
all manipulation with aCurve are inside condition
if(aCurve.IsNull())
{
...
}

it seems to be misprint, correct test is
if(aBSCurve.IsNull())
...
So, this method must be rewritten

abv

2014-05-27 12:11

manager   ~0029536

Thank you for remarks! Please review corrected version in the same branch

ifv

2014-05-27 13:41

developer   ~0029544

Ok

apn

2014-05-29 11:12

administrator   ~0029595

Last edited: 2014-05-29 11:12

Dear BugMaster,

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

Number of compiler warnings:

occt component :
Linux: 17 (17 on master)
Windows: 0 (0 on master)

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

Regressions/Differences:
bugs modalg_2 bug23436
http://occt-tests/CR24955-master-occt/Debian60-64/bugs/modalg_2/bug23436.html
http://occt-tests/CR24955-master-occt/Windows-32-VC9/bugs/modalg_2/bug23436.html

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 360003780 / 359732036
Total CPU difference: 50183.990000000034 / 51526.570000000116

Testing on Windows:
Total MEMORY difference: 381930432 / 382719888
Total CPU difference: 39107.484375 / 37501.296875

There are no differences in images found by testdiff.

abv

2014-05-29 15:10

manager   ~0029610

I have pushed fix to CR24955, please review.

The problem was that previous fix used simplified treatment for b-splines, which allowed periodic curves to be passed to GeomFill_AppSurf. That class obviously does not support periodic b-splines on input, and produces inconsistent result in that case (poles and knots as for periodic curve). This can be considered as separate issue.

Now all curves, including b-splines, are converted to b-spline using GeomConvert (thus loose periodicity, original behavior is restored).

ifv

2014-05-29 15:41

developer   ~0029611

The same error as was in first version:
...
 Handle(Geom_Curve) aCurve = BRep_Tool::Curve (theEdge, aLoc, aFirst, aLast);

aCurve can be Geom_TrimmedCurve, so test
...
if (aCurve->IsKind(STANDARD_TYPE(Geom_Conic)))
...
can be wrong

abv

2014-05-29 15:53

manager   ~0029612

Sorry, you are right! Please check corrected version pushed to CR24955.

Notably, tests seem to be completely insensitive to this change.

ifv

2014-05-29 16:01

developer   ~0029613

Ok

mkv

2014-06-04 12:02

tester   ~0029669

Dear BugMaster,

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

Number of compiler warnings:

occt component :
Linux: 16 (16 on master)
Windows: 0 (0 on master)
MacOS: 200 (203 on master)

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

Regressions/Differences:
No regressions/differences

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 361709680 / 361225452
Total CPU difference: 53757.34999999997 / 53202.74999999999

Testing on Windows:
Total MEMORY difference: 383369872 / 383800460
Total CPU difference: 40417.484375 / 42820.09375

There are no differences in images found by testdiff.

Related Changesets

occt: master 5e5b6f81

2014-06-05 10:14:14

abv


Committer: apn Details Diff
0024955: Misuse of DownCast

Code where DownCast was applied to Handle of the type being not a base class of the target one is revised and (hopefully) corrected.
Code corrected following review remarks
GeomConvert::CurveToBSplineCurve() is called even for b-spline curves to ensure that result is non-periodic
Check for conic is corrected
Affected Issues
0024955
mod - src/AIS/AIS_Dimension.cxx Diff File
mod - src/AIS/AIS_InteractiveObject.cxx Diff File
mod - src/BRepFill/BRepFill_NSections.cxx Diff File
mod - src/BRepOffsetAPI/BRepOffsetAPI_ThruSections.cxx Diff File
mod - src/GeomFill/GeomFill_NSections.cxx Diff File
mod - src/ShapeUpgrade/ShapeUpgrade_FaceDivide.cdl Diff File
mod - src/ShapeUpgrade/ShapeUpgrade_ShapeDivideAngle.cxx Diff File

Issue History

Date Modified Username Field Change
2014-05-26 13:45 abv New Issue
2014-05-26 13:45 abv Assigned To => kgv
2014-05-26 13:46 abv Relationship added child of 0024947
2014-05-26 17:45 abv Note Added: 0029518
2014-05-26 17:45 abv Assigned To kgv => ifv
2014-05-26 17:45 abv Status new => resolved
2014-05-27 10:23 ifv Note Added: 0029529
2014-05-27 10:23 ifv Assigned To ifv => kgv
2014-05-27 10:23 ifv Status resolved => feedback
2014-05-27 10:46 ifv Note Added: 0029530
2014-05-27 12:11 abv Note Added: 0029536
2014-05-27 12:11 abv Assigned To kgv => ifv
2014-05-27 12:11 abv Status feedback => resolved
2014-05-27 13:41 ifv Note Added: 0029544
2014-05-27 13:41 ifv Status resolved => reviewed
2014-05-27 14:11 kgv Assigned To ifv => bugmaster
2014-05-27 15:42 apn Assigned To bugmaster => apn
2014-05-29 11:12 apn Note Added: 0029595
2014-05-29 11:12 apn Note Edited: 0029595
2014-05-29 11:13 apn Test case number => Not needed
2014-05-29 11:13 apn Assigned To apn => abv
2014-05-29 11:13 apn Status reviewed => assigned
2014-05-29 15:10 abv Note Added: 0029610
2014-05-29 15:10 abv Assigned To abv => ifv
2014-05-29 15:10 abv Status assigned => resolved
2014-05-29 15:41 ifv Note Added: 0029611
2014-05-29 15:41 ifv Status resolved => feedback
2014-05-29 15:41 ifv Assigned To ifv => abv
2014-05-29 15:53 abv Note Added: 0029612
2014-05-29 15:53 abv Assigned To abv => ifv
2014-05-29 15:53 abv Status feedback => resolved
2014-05-29 16:01 ifv Note Added: 0029613
2014-05-29 16:01 ifv Status resolved => reviewed
2014-05-30 15:11 apn Assigned To ifv => apn
2014-06-04 12:02 mkv Note Added: 0029669
2014-06-04 12:03 mkv Assigned To apn => bugmaster
2014-06-04 12:03 mkv Status reviewed => tested
2014-06-06 12:04 apn Changeset attached => occt master 5e5b6f81
2014-06-06 12:04 apn Assigned To bugmaster => apn
2014-06-06 12:04 apn Status tested => verified
2014-06-06 12:04 apn Resolution open => fixed
2014-11-11 12:46 aiv Fixed in Version => 6.8.0
2014-11-11 12:52 aiv Status verified => closed