View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0024370 | Community | OCCT:Shape Healing | public | 2013-11-16 23:30 | 2014-05-05 13:37 |
Reporter | Roman Lygin | Assigned To | bugmaster | ||
Priority | normal | Severity | minor | ||
Status | closed | Resolution | fixed | ||
Platform | A | OS | L | ||
Product Version | 6.7.0 | ||||
Target Version | 6.7.1 | Fixed in Version | 6.7.1 | ||
Summary | 0024370: [Regression] 6.7.0beta ShapeFix_EdgeProjAux breaks conventions on using IsDone flag | ||||
Description | Up to 6.6.0 inclusive, ShapeFix_EdgeProjAux used a straightforward convention (even if not explicitly documented): if the edge's vertices cannot be projected within requested tolerance then IsDone (first and/or last) was set to false. 6.7.0 beta broke this convention and sets IsDone to true unless projection completely failed and the returned distance is infinite. This broken logic does not allow calling algorithms to identify the case when projection failed within initially requested tolerance. Moreover returned values of parameters (FirstParam() and LastParam() now correspond to pcurve ends, not to really projected values). Apparently regression is introduced by fixing a bug 0024206. The fix should be reworked so that the previous logic is preserved. | ||||
Steps To Reproduce | See the enclosed reproducer. | ||||
Tags | No tags attached. | ||||
Test case number | bugs heal bug24370 | ||||
|
testcase.zip (4,450 bytes) |
|
I would like to note that this fix is not regression but changing behavior. After this fix flags IsFirstDone() and IsLastDone() set to false only for cases when projection for corresponding parameter was not made. If projection is performed with distance more than specified precision then it is possible to get this solution and decide to use it or not. In previous version it was necessary to repeat projection with greater value of specified precision from time to time. Please note that class ShapeFix_EdgeProjAux is not API class. |
|
Galina, please have a look at the reproducer. The real projecting parameters correspond to those given in case 2: [5.2435913905776836e-005, 0.99999998584035321]. If you call an algorithm with precision of Precision::Confusion() it will return you flags as IsDone() and the range [0, 1]. In no way can you understand that actually the algorithm failed to project and you have to increase the tolerance to, say 0.2, to get a real range [5.24e-5, 0.999.584]. Previous behavior was intuitively clear and returned you IsDone false unless it could reach the requested projection. In new behavior the tolerance stops making any sense. Note that the new behavior not just triggered regressions in CAD Exchanger. It jeopardizes own OCC STEP importer (StepToTopoDS_TranslateEdgeLoop) which relies on logic of ShapeFix_EdgeProjAux: myEdgePro->Init (Face, edge); myEdgePro->Compute(preci); if (myEdgePro->IsFirstDone() && myEdgePro->IsLastDone()) { if (Abs (myEdgePro->FirstParam() - myEdgePro->LastParam()) < Precision::PConfusion()) continue; B.Range(edge, Face,myEdgePro->FirstParam(), myEdgePro->LastParam()); } else { RemoveSinglePCurve(edge, Face); #ifdef DEBUG cout <<"Removing after prj"<<endl; #endif } Where in the past, false IsDone() would cause ignoring a wrong pcurve, it will now be accepted almost certainly (given that IsDone() would return false only if projection screwed up completely). This will lead to an edge containing inconsistent 3D and 2D curves (and with incorrect ranges!) leading to more broken shape downstream. That is why, I believe the previous logic should be preserved and it is an underlying implementation that needs to be changed. If for whatever reason you do not want to revert the original behavior (and I don't see a strong argument why), here is an alternative: - do not accept any user-defined tolerance - always do projection (e.g. calling ShapeAnalysis_Curve::Project() with Precision::Infinite()) - return [square] distance from curve points to respective vertices - this will allow to take a decision of successful/failed projection by the user outside of the algorithm. |
|
Branch CR24370 is ready to be reviewed. |
|
In order to avoid removing 2D curve written in file following behavior is implemented now: If projection of points from 3D curve corresponding range of edge was performed then obtained parameters on 2D curve are used independent from distance points from 3D curve for projected points. |
|
Branch CR24370_1 is ready to be reviewed |
|
No remarks, please test |
|
Dear BugMaster, Branch CR24370_1 (and products from GIT master) was compiled on Linux and Windows platforms and tested. SHA-1: bf7b996e99ef5f40239c1152a6210fc0280efd7e Number of compiler warnings: occt component : Linux: 320 (320 on master) Windows: 0 (0 on master) products component : Linux: 188 (188 on master) Windows: 286 (286 on master) Regressions/Differences: http://occt-tests/CR24370-1-master-occt/Debian60-64/de/iges_1/P5.html http://occt-tests/CR24370-1-master-occt/Windows-32-VC9/de/iges_1/P5.html de iges_1(001) P5: FAILED http://jenkins-test-02.nnov.opencascade.com:8080/user/mnt/my-views/view/CR24370_1/job/mnt-CR24370_1-master_products_tests_linux_start/label=Debian60-64,tests_group=sat,tests_subgroup=002/2/HTML_Report/? http://jenkins-test-02.nnov.opencascade.com:8080/user/mnt/my-views/view/CR24370_1/job/mnt-CR24370_1-master_products_tests_windows_start/label=windows_test,tests_group=sat,tests_subgroup=002/2/HTML_Report/? sat doc_2(002) A4, A5, A9, F7, H1, I7 Testing cases: Testing on Linux: Total MEMORY difference: 357511528 / 358410160 Total CPU difference: 54597.54999999992 / 43957.070000000385 Testing on Windows: Total MEMORY difference: 412851588 / 412662448 Total CPU difference: 31332.234375 / 32172.234375 There are not differences in images found by testdiff. |
|
Test cases : de iges_1 P5, sat doc_2 A4, A5, A9, F7, H1, I7 are not regressions. Could you please to regenerate these tests. |
|
Dear abv, could you please review new draw-command for issue CR24370. (See CR24370_1) |
|
Dear BugMaster, Branch CR24370_1 (and products from GIT master) was compiled on Linux and Windows platforms and tested. SHA-1: 7ff733395e79c2b40767e6f5c38a1f2925abf7bd Number of compiler warnings: occt component : Linux: 103 (103 on master) Windows: 0 (0 on master) products component : Linux: 188 (188 on master) Windows: 237 (237 on master) Regressions/Differences: No regressions/differences Testing cases: It's necessary to create test case Testing on Linux: Total MEMORY difference: 358781576 / 358643576 Total CPU difference: 43486.840000000055 / 45654.32000000018 Testing on Windows: Total MEMORY difference: 413244472 / 412408396 Total CPU difference: 34745.5625 / 33977.296875 There are no differences in images found by testdiff. |
|
PRODUCTS branch CR24370_1 (contains changes in reference data in test cases sat/doc_2/A4 A5 A9 F7 H1 was created. |
|
Dear BugMaster, Test case (bugs/heal/bug24370) is bad (input shape are bad) pload QAcommands OCC24370 bug24370_edge.brep bug24370_pcurve_draw bug24370_surface_draw 0.001 OCC24370 Eror: Null input edge |
|
Dear BugMaster, Test case bugs/heal/bug24370 was corrected. It is OK now. Branch CR24370_1 is TESTED. |
occt: master e3096dec 2014-01-16 09:11:45
Committer: bugmaster Details Diff |
0024370: [Regression] 6.7.0beta ShapeFix_EdgeProjAux breaks conventions on using IsDone flag. Projection of 3d points corresponding to range of edge on 2d curve considered as successful for all cases to except for cases when projection is not made. Initialization initial values of flags Modification initial values of flags setting status Done for first and last parameters Adding new draw-command for issue CR24370 Test case for issue CR24370 |
Affected Issues 0024370 |
|
mod - src/QABugs/QABugs_19.cxx | Diff File | ||
mod - src/ShapeFix/ShapeFix_EdgeProjAux.cxx | Diff File | ||
add - tests/bugs/heal/bug24370 | Diff File | ||
mod - tests/de/step_3/E6 | Diff File | ||
mod - tests/de/step_4/C9 | Diff File | ||
mod - tests/de/step_4/D1 | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2013-11-16 23:30 | Roman Lygin | New Issue | |
2013-11-16 23:30 | Roman Lygin | Assigned To | => gka |
2013-11-16 23:30 | Roman Lygin | File Added: testcase.zip | |
2013-11-21 18:29 |
|
Status | new => assigned |
2013-11-22 11:18 |
|
Note Added: 0026835 | |
2013-11-22 11:18 |
|
Assigned To | gka => Roman Lygin |
2013-11-22 11:18 |
|
Status | assigned => feedback |
2013-11-22 11:45 | Roman Lygin | Assigned To | Roman Lygin => gka |
2013-11-22 11:45 | Roman Lygin | Note Added: 0026839 | |
2013-11-22 11:45 | Roman Lygin | Status | feedback => assigned |
2013-11-25 16:47 |
|
Note Added: 0026890 | |
2013-11-25 16:47 |
|
Assigned To | gka => abv |
2013-11-25 16:47 |
|
Status | assigned => resolved |
2013-11-25 17:06 |
|
Note Added: 0026893 | |
2013-11-26 16:24 |
|
Note Added: 0026923 | |
2013-11-26 17:08 |
|
Note Added: 0026926 | |
2013-11-26 17:08 |
|
Assigned To | abv => bugmaster |
2013-11-26 17:08 |
|
Status | resolved => reviewed |
2013-11-27 11:31 |
|
Note Added: 0026946 | |
2013-11-27 11:32 |
|
Assigned To | bugmaster => gka |
2013-11-27 11:32 |
|
Status | reviewed => assigned |
2013-11-27 19:46 |
|
Note Added: 0026968 | |
2013-11-27 19:46 |
|
Status | assigned => resolved |
2013-11-27 19:47 |
|
Status | resolved => reviewed |
2013-11-28 15:18 |
|
Note Added: 0026985 | |
2013-11-28 15:19 |
|
Assigned To | gka => abv |
2013-11-28 15:19 |
|
Status | reviewed => feedback |
2013-11-28 15:19 |
|
Note Edited: 0026985 | |
2013-11-28 17:16 |
|
Assigned To | abv => gka |
2013-12-04 10:13 |
|
Status | feedback => reviewed |
2013-12-04 11:41 |
|
Assigned To | gka => apn |
2013-12-05 13:02 | apn | Assigned To | apn => mkv |
2013-12-05 14:51 | apn | Note Added: 0027107 | |
2013-12-05 14:52 | apn | Note Edited: 0027107 | |
2013-12-05 14:53 | apn | Note Added: 0027108 | |
2013-12-05 14:53 | apn | Status | reviewed => feedback |
2013-12-21 10:13 |
|
Target Version | 6.7.0 => 6.7.1 |
2014-01-15 11:43 |
|
Note Added: 0027490 | |
2014-01-15 11:43 |
|
Assigned To | mkv => abv |
2014-01-15 15:40 |
|
Note Added: 0027498 | |
2014-01-15 15:40 |
|
Test case number | => bugs heal bug24370 |
2014-01-15 15:41 |
|
Assigned To | abv => bugmaster |
2014-01-15 15:41 |
|
Status | feedback => tested |
2014-01-21 10:41 |
|
Relationship added | parent of 0024543 |
2014-01-21 11:22 | bugmaster | Changeset attached | => occt master e3096dec |
2014-01-21 11:22 | bugmaster | Status | tested => verified |
2014-01-21 11:22 | bugmaster | Resolution | open => fixed |
2014-05-05 13:36 |
|
Status | verified => closed |
2014-05-05 13:37 |
|
Fixed in Version | => 6.7.1 |