MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0024860Community[OCCT] OCCT:Modeling Datapublic2014-04-21 02:582017-12-05 17:09
ReporterRoman Lygin 
Assigned Toifv 
PriorityhighSeveritymajor 
StatusassignedResolutionopen 
PlatformOSOS Version
Product Version[OCCT] 6.7.1 
Target Version[OCCT] 7.3.0*Fixed in Version 
Summary0024860: [Regression] OCC 6.7.1 beta BRepBuilderAPI_Transform loses pcurves
Description(Initially reported at http://dev.opencascade.org/index.php?q=node/954#comment-313 [^])

Scaling loses p-curves (perhaps on planes only).
Steps To ReproduceThe 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)
TagsNo tags attached.
Test case number
Attached Fileszip file icon occ671feedback-scale.zip (4,266 bytes) 2014-04-21 02:58
? file icon box-no3dcurves.brep (2,460 bytes) 2014-04-29 11:56

- Relationships
related to 0024652newifv Open CASCADE It is necessary to always store p-curves on planes 
related to 0022614closedbugmaster Community Scaling fails with linear (p-)curves 
related to 0027457assignedifv Open CASCADE Modeling - Raise exception if scaled transformation is used for shape location 

-  Notes
(0029118)
abv (manager)
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 (developer)
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 (developer)
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 (developer)
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 (manager)
2014-05-26 16:52

BRep_Tool must not change anything in the shape. Please consider restoring previous behavior (before regression) instead.
(0038457)
git (administrator)
2015-03-18 09:32

Branch CR24860 has been updated forcibly by nbv.

SHA-1: f97feed05cb8991f345b2d521e4d493fb94138bf
(0038458)
msv (developer)
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 (developer)
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 (administrator)
2015-12-21 16:45

Branch CR24860 has been updated forcibly by ifv.

SHA-1: b0e576c6a91c215b8a79117ea797f9bb7cb074ca
(0049378)
ifv (developer)
2015-12-21 16:51

Branch CR24860 is ready for review
(0049408)
msv (developer)
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 (administrator)
2015-12-24 15:49

Branch CR24860 has been updated forcibly by ifv.

SHA-1: a947550b47c3705e81952fc76b0b2ee597bd8af5
(0049572)
git (administrator)
2015-12-24 15:57

Branch CR24860 has been updated forcibly by ifv.

SHA-1: 41b2865950f870b1cef6a0ecb9bd1c5e718e345c
(0049573)
ifv (developer)
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 (administrator)
2015-12-24 18:42

Branch CR24860 has been updated forcibly by ifv.

SHA-1: 44c2f7b29ccb7f09a830abc1ced5a6791c972a3c
(0049581)
msv (developer)
2015-12-24 18:47

Reviewed.
(0049633)
apv (tester)
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 (tester)
2015-12-28 13:20

Dear ifv,

Branch CR24860 has been rejected due to:
- regressions/differences/improvements
- differences in images
(0051987)
git (administrator)
2016-03-25 12:12

Branch CR24860 has been updated forcibly by ifv.

SHA-1: 452cb0ea25a944921b9e74d2b37a1bf1a7c930b0
(0052056)
msv (developer)
2016-03-28 11:22

Reviewed.
(0052103)
git (administrator)
2016-03-29 12:19

Branch CR24860 has been updated forcibly by apv.

SHA-1: c4da7b9a4bfb0656021d066ecabd2e9abbfb13cb
(0052104)
apv (tester)
2016-03-29 12:20

Branch CR24860 has been rebased on the IR-2016-03-28
(0052164)
apv (tester)
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 (tester)
2016-03-30 11:51

Dear ifv,

Branch CR24860 has been rejected due to:
- regressions/differences/improvements
- differences in images
(0059319)
apv (tester)
2016-10-28 14:22

Testing results are too old and will be deleted.

- Issue History
Date Modified Username Field Change
2014-04-21 02:58 Roman Lygin New Issue
2014-04-21 02:58 Roman Lygin Assigned To => ifv
2014-04-21 02:58 Roman Lygin File Added: occ671feedback-scale.zip
2014-04-29 10:33 abv Note Added: 0029118
2014-04-29 10:33 abv Assigned To ifv => Roman Lygin
2014-04-29 10:33 abv Status new => feedback
2014-04-29 10:33 abv Relationship added related to 0024652
2014-04-29 10:33 abv Relationship added related to 0022614
2014-04-29 11:52 Roman Lygin Note Added: 0029123
2014-04-29 11:52 Roman Lygin Assigned To Roman Lygin => ifv
2014-04-29 11:52 Roman Lygin Status feedback => assigned
2014-04-29 11:56 Roman Lygin Note Added: 0029124
2014-04-29 11:56 Roman Lygin File Added: box-no3dcurves.brep
2014-04-29 12:33 abv Target Version => 6.8.0
2014-05-23 12:22 ifv Assigned To ifv => nbv
2014-05-26 16:39 nbv Note Added: 0029516
2014-05-26 16:39 nbv Assigned To nbv => ifv
2014-05-26 16:39 nbv Status assigned => resolved
2014-05-26 16:52 abv Note Added: 0029517
2014-05-26 16:52 abv Assigned To ifv => nbv
2014-05-26 16:52 abv Status resolved => assigned
2014-09-26 12:18 abv Target Version 6.8.0 => 7.1.0
2015-03-18 09:32 git Note Added: 0038457
2015-03-18 10:06 msv Note Added: 0038458
2015-04-18 17:47 abv Priority normal => high
2015-04-18 17:47 abv Target Version 7.1.0 => 6.9.0
2015-04-20 12:32 msv Assigned To nbv => msv
2015-04-30 17:20 nbv Note Added: 0040511
2015-04-30 17:21 msv Target Version 6.9.0 => 7.1.0
2015-12-21 16:45 git Note Added: 0049376
2015-12-21 16:51 ifv Note Added: 0049378
2015-12-21 16:51 ifv Status assigned => resolved
2015-12-22 00:23 msv Note Added: 0049408
2015-12-22 00:23 msv Assigned To msv => ifv
2015-12-22 00:23 msv Status resolved => assigned
2015-12-24 15:49 git Note Added: 0049569
2015-12-24 15:57 git Note Added: 0049572
2015-12-24 16:04 ifv Note Added: 0049573
2015-12-24 16:04 ifv Assigned To ifv => msv
2015-12-24 16:04 ifv Status assigned => resolved
2015-12-24 18:42 git Note Added: 0049580
2015-12-24 18:47 msv Note Added: 0049581
2015-12-24 18:47 msv Assigned To msv => bugmaster
2015-12-24 18:47 msv Status resolved => reviewed
2015-12-25 14:26 apv Assigned To bugmaster => apv
2015-12-28 13:19 apv Note Added: 0049633
2015-12-28 13:19 apv Assigned To apv => ifv
2015-12-28 13:19 apv Status reviewed => assigned
2015-12-28 13:20 apv Note Added: 0049634
2016-03-25 12:12 git Note Added: 0051987
2016-03-25 12:19 ifv Assigned To ifv => msv
2016-03-25 12:19 ifv Status assigned => resolved
2016-03-28 11:22 msv Note Added: 0052056
2016-03-28 11:22 msv Assigned To msv => bugmaster
2016-03-28 11:22 msv Status resolved => reviewed
2016-03-28 19:23 mkv Assigned To bugmaster => apv
2016-03-29 12:19 git Note Added: 0052103
2016-03-29 12:20 apv Note Added: 0052104
2016-03-30 11:50 apv Note Added: 0052164
2016-03-30 11:50 apv Assigned To apv => ifv
2016-03-30 11:50 apv Status reviewed => assigned
2016-03-30 11:51 apv Note Added: 0052165
2016-03-30 12:00 apv Note Edited: 0052164 View Revisions
2016-05-11 10:24 msv Relationship added related to 0027457
2016-10-25 15:46 msv Target Version 7.1.0 => 7.2.0
2016-10-28 14:22 apv Note Added: 0059319
2017-07-20 15:30 msv Target Version 7.2.0 => 7.2.1
2017-12-05 17:09 msv Target Version 7.2.1 => 7.3.0*


Copyright © 2000 - 2017 MantisBT Team
Powered by Mantis Bugtracker