MantisBT - Community
View Issue Details
0024860Community[OCCT] OCCT:Modeling Datapublic2014-04-21 02:582019-09-06 15:54
Roman Lygin 
bugmaster 
normalmajor 
feedbackno change required 
[OCCT] 6.7.1 
[OCCT] 7.5.0* 
0024860: [Regression] Modeling Data - OCC 6.7.1 beta BRepBuilderAPI_Transform loses pcurves
(Initially reported at http://dev.opencascade.org/index.php?q=node/954#comment-313 [^])

Scaling loses p-curves (perhaps on planes only).
The C++ reproducer: see BRepTools_ModifierTest.cxx. The failure happens at line:
QVERIFY (!aPCurve.IsNull());
after
BRep_Tool::CurveOnSurface (aNewEdge, aPCurve, aSurf, aLoc, aNewF, aNewL);

DRAW reproducer:
restore face-plane.brep f
dump f #pcurves are present
tscale f 0 0 0 0.001
dump f #no pcurves

See test models in the subdirectory face (face-plane.brep, etc)
No tags attached.
related to 0024652new ifv Open CASCADE It is necessary to always store p-curves on planes 
related to 0022614closed bugmaster Community Scaling fails with linear (p-)curves 
related to 0027457assigned ifv Open CASCADE Modeling - Raise exception if scaled transformation is used for shape location 
zip occ671feedback-scale.zip (4,266) 2014-04-21 02:58
https://tracker.dev.opencascade.org/
? box-no3dcurves.brep (2,460) 2014-04-29 11:56
https://tracker.dev.opencascade.org/
Issue History
2014-04-21 02:58Roman LyginNew Issue
2014-04-21 02:58Roman LyginAssigned To => ifv
2014-04-21 02:58Roman LyginFile Added: occ671feedback-scale.zip
2014-04-29 10:33abvNote Added: 0029118
2014-04-29 10:33abvAssigned Toifv => Roman Lygin
2014-04-29 10:33abvStatusnew => feedback
2014-04-29 10:33abvRelationship addedrelated to 0024652
2014-04-29 10:33abvRelationship addedrelated to 0022614
2014-04-29 11:52Roman LyginNote Added: 0029123
2014-04-29 11:52Roman LyginAssigned ToRoman Lygin => ifv
2014-04-29 11:52Roman LyginStatusfeedback => assigned
2014-04-29 11:56Roman LyginNote Added: 0029124
2014-04-29 11:56Roman LyginFile Added: box-no3dcurves.brep
2014-04-29 12:33abvTarget Version => 6.8.0
2014-05-23 12:22ifvAssigned Toifv => nbv
2014-05-26 16:39nbvNote Added: 0029516
2014-05-26 16:39nbvAssigned Tonbv => ifv
2014-05-26 16:39nbvStatusassigned => resolved
2014-05-26 16:52abvNote Added: 0029517
2014-05-26 16:52abvAssigned Toifv => nbv
2014-05-26 16:52abvStatusresolved => assigned
2014-09-26 12:18abvTarget Version6.8.0 => 7.1.0
2015-03-18 09:32gitNote Added: 0038457
2015-03-18 10:06msvNote Added: 0038458
2015-04-18 17:47abvPrioritynormal => high
2015-04-18 17:47abvTarget Version7.1.0 => 6.9.0
2015-04-20 12:32msvAssigned Tonbv => msv
2015-04-30 17:20nbvNote Added: 0040511
2015-04-30 17:21msvTarget Version6.9.0 => 7.1.0
2015-12-21 16:45gitNote Added: 0049376
2015-12-21 16:51ifvNote Added: 0049378
2015-12-21 16:51ifvStatusassigned => resolved
2015-12-22 00:23msvNote Added: 0049408
2015-12-22 00:23msvAssigned Tomsv => ifv
2015-12-22 00:23msvStatusresolved => assigned
2015-12-24 15:49gitNote Added: 0049569
2015-12-24 15:57gitNote Added: 0049572
2015-12-24 16:04ifvNote Added: 0049573
2015-12-24 16:04ifvAssigned Toifv => msv
2015-12-24 16:04ifvStatusassigned => resolved
2015-12-24 18:42gitNote Added: 0049580
2015-12-24 18:47msvNote Added: 0049581
2015-12-24 18:47msvAssigned Tomsv => bugmaster
2015-12-24 18:47msvStatusresolved => reviewed
2015-12-25 14:26apvAssigned Tobugmaster => apv
2015-12-28 13:19apvNote Added: 0049633
2015-12-28 13:19apvAssigned Toapv => ifv
2015-12-28 13:19apvStatusreviewed => assigned
2015-12-28 13:20apvNote Added: 0049634
2016-03-25 12:12gitNote Added: 0051987
2016-03-25 12:19ifvAssigned Toifv => msv
2016-03-25 12:19ifvStatusassigned => resolved
2016-03-28 11:22msvNote Added: 0052056
2016-03-28 11:22msvAssigned Tomsv => bugmaster
2016-03-28 11:22msvStatusresolved => reviewed
2016-03-28 19:23mkvAssigned Tobugmaster => apv
2016-03-29 12:19gitNote Added: 0052103
2016-03-29 12:20apvNote Added: 0052104
2016-03-30 11:50apvNote Added: 0052164
2016-03-30 11:50apvAssigned Toapv => ifv
2016-03-30 11:50apvStatusreviewed => assigned
2016-03-30 11:51apvNote Added: 0052165
2016-03-30 12:00apvNote Edited: 0052164bug_revision_view_page.php?bugnote_id=52164#r13340
2016-05-11 10:24msvRelationship addedrelated to 0027457
2016-10-25 15:46msvTarget Version7.1.0 => 7.2.0
2016-10-28 14:22apvNote Added: 0059319
2017-07-20 15:30msvTarget Version7.2.0 => 7.3.0
2017-12-05 17:09msvTarget Version7.3.0 => 7.4.0
2018-03-13 17:33gitNote Added: 0074447
2019-08-12 18:01msvTarget Version7.4.0 => 7.5.0*
2019-09-04 12:20kgvSummary[Regression] OCC 6.7.1 beta BRepBuilderAPI_Transform loses pcurves => [Regression] Modeling Data - OCC 6.7.1 beta BRepBuilderAPI_Transform loses pcurves
2019-09-06 15:54msvNote Added: 0086894
2019-09-06 15:54msvAssigned Toifv => bugmaster
2019-09-06 15:54msvPriorityhigh => normal
2019-09-06 15:54msvStatusassigned => feedback
2019-09-06 15:54msvResolutionopen => no change required

Notes
(0029118)
abv   
2014-04-29 10:33   
Hello Roman,

I deem this is not really a regression, as pcurves on planes are not necessary for OCCT, so they are lost safely without affecting validity of shape. On other types of surfaces pcurves do not get lost.

Do you have a particular use case when explicit pcurves on planes are necessary?

If so, this can be treated in context of 0024652. Recall also 0022614.
(0029123)
Roman Lygin   
2014-04-29 11:52   
Andrey,

The use case is CAD Exchanger importers which have to scale the model according to the factors specified in the file (e.g. ACIS or Parasolid). Scaling may happen prior to Shape Healing and when only 2D representation was constructed from the input file.

Of course, it will be up to you, as an architect, to eventually triage this as a regression or not.

I consider it as a regression because it changes previous behavior for worse: 6.7.0 did preserve pcurves and 6.7.1 does not, and having no pcurves has performance penalty (as pcurves have to be recomputed on the fly each time).

The fact that the shape without pcurves on planes is considered acceptable by OCC is just a matter of convention. The scaling (or any other modifying algorithm) should not unexpectedly downgrade the quality of the input shape without good reason for that. The change introduced in 6.7.1 with this side effect just feels as some oversight that should be analyzed and addressed.

Thank you,
Roman

P.S. Returning back to Igor, please feel free to reassign ownership.
(0029124)
Roman Lygin   
2014-04-29 11:56   
Additional note: I made further investigation to check how the scaling algorithm would work on the model without 3D curves:
- restore box-no3dcurves.brep b
- tscaling b 0 0 0 0.1
- dump b

PCurves are preserved, what leads to an assumption that the algorithm *decides* to drop pcurves if there are 3D curves. In this case (unlike more fundamental flaw which would be when it always drops pcurves) it should hopefully be easier to restore previous (version 6.7.0) behavior.
(0029516)
nbv   
2014-05-26 16:39   
Dear Igor.

Please review branch CR24860.

Main changes are in BRep_Tool::CurveOnSurface() function. Found by projection algorithm, pcurve on planar face is added into BRep_ListOfCurveRepresentation for given edge. After that, it can be read from this list (it is not necessary to find it with projection algorithm again).
(0029517)
abv   
2014-05-26 16:52   
BRep_Tool must not change anything in the shape. Please consider restoring previous behavior (before regression) instead.
(0038457)
git   
2015-03-18 09:32   
Branch CR24860 has been updated forcibly by nbv.

SHA-1: f97feed05cb8991f345b2d521e4d493fb94138bf
(0038458)
msv   
2015-03-18 10:06   
I agree with Andrey: BRep_Tool is used to request shape properties only, and it must not change anything in a shape.
And I agree with Roman: the behavior of 6.7.0 must be restored, when transformation algorithm translates pcurves in the result. If this restore procedure leads to regressions they should be eliminated without sacrificing this functionality.
(0040511)
nbv   
2015-04-30 17:20   
The reason of pcurve losing is fix for bug #0024204, file src\BRepTools\BRepTools_TrsfModification.cxx, method BRepTools_TrsfModification::NewCurve2d(...)
(0049376)
git   
2015-12-21 16:45   
Branch CR24860 has been updated forcibly by ifv.

SHA-1: b0e576c6a91c215b8a79117ea797f9bb7cb074ca
(0049378)
ifv   
2015-12-21 16:51   
Branch CR24860 is ready for review
(0049408)
msv   
2015-12-22 00:23   
Remarks:

src\BRep\BRep_Tool.cxx

1) Lines 298-300, aGTrsf.Trsf2d() is called thrice. Make a local variable.
2) Line 300, isn't it better to call aPC->Transform()?

src\BRepCheck\BRepCheck_Edge.cxx

3) If myShape is reversed then on closed surface PC will be the same for both lines 305 and 329.

src\BRepTest\BRepTest_CurveCommands.cxx

4) Includes in lines 71 and 72 are duplicate.
(0049569)
git   
2015-12-24 15:49   
Branch CR24860 has been updated forcibly by ifv.

SHA-1: a947550b47c3705e81952fc76b0b2ee597bd8af5
(0049572)
git   
2015-12-24 15:57   
Branch CR24860 has been updated forcibly by ifv.

SHA-1: 41b2865950f870b1cef6a0ecb9bd1c5e718e345c
(0049573)
ifv   
2015-12-24 16:04   
CR24860 is updated according to remarks.

2) Line 300, isn't it better to call aPC->Transform()?
It changes directly the pcurve attached to edge, it is modification of shape, so we should using copy of pcurve.
(0049580)
git   
2015-12-24 18:42   
Branch CR24860 has been updated forcibly by ifv.

SHA-1: 44c2f7b29ccb7f09a830abc1ced5a6791c972a3c
(0049581)
msv   
2015-12-24 18:47   
Reviewed.
(0049633)
apv   
2015-12-28 13:19   
Dear BugMaster,

Branch CR24860 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: 44c2f7b29ccb7f09a830abc1ced5a6791c972a3c

Number of compiler warnings:
occt component:
   Linux: 0 (0 on master)
   Windows: 0 (0 on master)
   MasOS: 134 (134 on master)
products component:
   Linux: 37 (37 on master)
   Windows: 0 (0 on master)

Regressions/Differences:
http://occt-tests/CR24860-master-occt-64/Debian70-64/summary.html [^]
http://occt-tests/CR24860-master-occt-64/Windows-64-VC10/summary.html [^]
blend simple Q6
bugs vis bug641
http://occt-tests/CR24860-master-products-64/Windows-64-VC10/summary.html [^]
sat doc_5 H6

Testing on Linux:
Total MEMORY difference: 89033984 / 89634264 [-0.67%]
Total CPU difference: 19017.849999999813 / 19153.59000000006 [-0.71%]

Testing on Windows:
Total MEMORY difference: 56819954 / 57251886 [-0.75%]
Total CPU difference: 18181.04294439915 / 18699.106665299056 [-2.77%]

There are differences in images found by testdiff:
http://occt-tests/CR24860-master-occt-64/Debian70-64/diff-Debian70-64.html [^]
http://occt-tests/CR24860-master-occt-64/Windows-64-VC10/diff-Windows-64-VC10.html [^]
boolean gdml_private ZJ7
boolean gdml_private ZJ5
boolean gdml_private F3
boolean gdml_private E8
boolean gdml_private E9
boolean gdml_private F1
boolean gdml_private ZI6
boolean gdml_private E4
boolean gdml_private ZJ3
boolean gdml_private I8
boolean gdml_private E5
boolean gdml_private ZJ6
boolean gdml_private E6
boolean gdml_private ZJ4
boolean gdml_private E7
boolean gdml_private O3
boolean gdml_private W1
boolean gdml_private T2
boolean gdml_private ZH3
boolean gdml_private F8
boolean gdml_private F2
boolean gdml_private P6
boolean gdml_private G1
boolean gdml_private O1
boolean gdml_private ZI7
boolean volumemaker A3
boolean volumemaker A7
boolean volumemaker B5
boolean volumemaker C2
boolean volumemaker D2
boolean volumemaker D8
boolean volumemaker C4
boolean volumemaker A8
bugs modalg_1 bug10232
bugs vis bug22240
bugs modalg_2 bug472_3
bugs modalg_2 bug472_2
bugs modalg_2 bug472_1
bugs modalg_6 bug26953_2
bugs modalg_6 bug26288
(0049634)
apv   
2015-12-28 13:20   
Dear ifv,

Branch CR24860 has been rejected due to:
- regressions/differences/improvements
- differences in images
(0051987)
git   
2016-03-25 12:12   
Branch CR24860 has been updated forcibly by ifv.

SHA-1: 452cb0ea25a944921b9e74d2b37a1bf1a7c930b0
(0052056)
msv   
2016-03-28 11:22   
Reviewed.
(0052103)
git   
2016-03-29 12:19   
Branch CR24860 has been updated forcibly by apv.

SHA-1: c4da7b9a4bfb0656021d066ecabd2e9abbfb13cb
(0052104)
apv   
2016-03-29 12:20   
Branch CR24860 has been rebased on the IR-2016-03-28
(0052164)
apv   
2016-03-30 11:50   
(edited on: 2016-03-30 12:00)
Dear BugMaster,

Branch CR24860 from occt git-repository (and IR-2016-03-28 from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: c4da7b9a4bfb0656021d066ecabd2e9abbfb13cb

Number of compiler warnings:
occt component:
   Linux: 0 (0 on master)
   Windows: 0 (0 on master)
   MasOS: 0 (0 on master)
products component:
   Linux: 68
   Windows: 0
   MacOS: 1125

Regressions/Differences:
http://occt-tests/CR24860-IR-2016-03-28-OCCT/Debian70-64/summary.html [^]
http://occt-tests/CR24860-IR-2016-03-28-OCCT/Windows-64-VC10/summary.html [^]
blend simple Q6

Testing on Linux:
Total MEMORY difference: 89161408 / 90131416 [-1.08%]
Total CPU difference: 19360.570000000163 / 19403.840000000106 [-0.22%]

Testing on Windows:
Total MEMORY difference: 57521886 / 57534725 [-0.02%]
Total CPU difference: 17950.80106849889 / 18043.200460798813 [-0.51%]

There are differences in images found by testdiff:
http://occt-tests/CR24860-IR-2016-03-28-OCCT/Debian70-64/diff-Debian70-64.html [^]
http://occt-tests/CR24860-IR-2016-03-28-OCCT/Windows-64-VC10/diff-Windows-64-VC10.html [^]
blend simple E1
blend simple D3
blend simple E3
blend simple D9
blend simple D7
blend simple D8
blend simple V6
blend simple D5
blend simple E2
blend buildevol B8
blend buildevol B9
blend buildevol C1
bugs moddata_1 bug22761
bugs vis bug22240

(0052165)
apv   
2016-03-30 11:51   
Dear ifv,

Branch CR24860 has been rejected due to:
- regressions/differences/improvements
- differences in images
(0059319)
apv   
2016-10-28 14:22   
Testing results are too old and will be deleted.
(0074447)
git   
2018-03-13 17:33   
Branch CR24860 has been updated forcibly by ifv.

SHA-1: 15a57ca9682ed87432bc046f00592444d3412b3e
(0086894)
msv   
2019-09-06 15:54   
OCCT does not store pcurves of planes in newly created shapes. Scaling operation (using BRepBuilderAPI_Transform) produces new shapes. They follow the rule of "no pcurves on planes".

So, I consider this issue is not a bug and must be closed.