View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0024955 | Open CASCADE | OCCT:Coding | public | 2014-05-26 13:45 | 2014-11-11 12:52 |
Reporter | Assigned To | apn | |||
Priority | normal | Severity | minor | ||
Status | closed | Resolution | fixed | ||
Platform | Windows | OS | VC++ 2010 | ||
Product Version | 6.7.1 | ||||
Target Version | 6.8.0 | Fixed in Version | 6.8.0 | ||
Summary | 0024955: Misuse of DownCast | ||||
Description | In 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 *' | ||||
Tags | No tags attached. | ||||
Test case number | Not needed | ||||
child of | 0024947 | closed | Redesign OCCT legacy type system |
|
Fix pushed to CR24955, please review |
|
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. |
|
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 |
|
Thank you for remarks! Please review corrected version in the same branch |
|
Ok |
|
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. |
|
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). |
|
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 |
|
Sorry, you are right! Please check corrected version pushed to CR24955. Notably, tests seem to be completely insensitive to this change. |
|
Ok |
|
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. |
occt: master 5e5b6f81 2014-06-05 10:14:14
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 |
Date Modified | Username | Field | Change |
---|---|---|---|
2014-05-26 13:45 |
|
New Issue | |
2014-05-26 13:45 |
|
Assigned To | => kgv |
2014-05-26 13:46 |
|
Relationship added | child of 0024947 |
2014-05-26 17:45 |
|
Note Added: 0029518 | |
2014-05-26 17:45 |
|
Assigned To | kgv => ifv |
2014-05-26 17:45 |
|
Status | new => resolved |
2014-05-27 10:23 |
|
Note Added: 0029529 | |
2014-05-27 10:23 |
|
Assigned To | ifv => kgv |
2014-05-27 10:23 |
|
Status | resolved => feedback |
2014-05-27 10:46 |
|
Note Added: 0029530 | |
2014-05-27 12:11 |
|
Note Added: 0029536 | |
2014-05-27 12:11 |
|
Assigned To | kgv => ifv |
2014-05-27 12:11 |
|
Status | feedback => resolved |
2014-05-27 13:41 |
|
Note Added: 0029544 | |
2014-05-27 13:41 |
|
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 |
|
Note Added: 0029610 | |
2014-05-29 15:10 |
|
Assigned To | abv => ifv |
2014-05-29 15:10 |
|
Status | assigned => resolved |
2014-05-29 15:41 |
|
Note Added: 0029611 | |
2014-05-29 15:41 |
|
Status | resolved => feedback |
2014-05-29 15:41 |
|
Assigned To | ifv => abv |
2014-05-29 15:53 |
|
Note Added: 0029612 | |
2014-05-29 15:53 |
|
Assigned To | abv => ifv |
2014-05-29 15:53 |
|
Status | feedback => resolved |
2014-05-29 16:01 |
|
Note Added: 0029613 | |
2014-05-29 16:01 |
|
Status | resolved => reviewed |
2014-05-30 15:11 | apn | Assigned To | ifv => apn |
2014-06-04 12:02 |
|
Note Added: 0029669 | |
2014-06-04 12:03 |
|
Assigned To | apn => bugmaster |
2014-06-04 12:03 |
|
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 |
|
Fixed in Version | => 6.8.0 |
2014-11-11 12:52 |
|
Status | verified => closed |