DiscussionsIssue archiveOCCT:Modeling Data

Archived issue #0024860

[Regression] Modeling Data - OCC 6.7.1 beta BRepBuilderAPI_Transform loses pcurves

CommunityOCCT:Modeling Dataclosed28 public notes

Search issues

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 reproduce

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)

Public activity

28 archived notes

Participants are labeled by their role within this record.

01Commenter 1
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.
02Author
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.
03Author
Additional note: I made further investigation to check how the scaling algorithm would work on the model without 3D curves:
- restore Attachment 1 (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.
04Commenter 4
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).
05Commenter 5
BRep_Tool must not change anything in the shape. Please consider restoring previous behavior (before regression) instead.
06Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
07Commenter 7
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.
08Commenter 8
The reason of pcurve losing is fix for bug #0024204, file src\BRepTools\BRepTools_TrsfModification.cxx, method BRepTools_TrsfModification::NewCurve2d(...)
09Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
10Commenter 10
Branch CR24860 is ready for review
11Commenter 11
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.
12Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
13Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
14Commenter 14
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.
15Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
16Commenter 16
Reviewed.
17Commenter 17
Dear Commenter 1,

Branch CR24860 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
[revision removed]

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
18Commenter 18
Dear ifv,

Branch [archived branch] has been rejected due to:
- regressions/differences/improvements
- differences in images
19Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
20Commenter 20
Reviewed.
21Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
22Commenter 22
Branch [archived branch] has been rebased on the IR-2016-03-28
23Commenter 23
Dear Commenter 1,

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.
[revision removed]

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

24Commenter 24
Dear ifv,

Branch [archived branch] has been rejected due to:
- regressions/differences/improvements
- differences in images
25Commenter 25
Testing results are too old and will be deleted.
26Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
27Commenter 27
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.
28Commenter 2
Branch [archived branch] has been deleted by Participant.

[revision removed]

Related records