MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0026265Open CASCADE[OCCT] OCCT:Modeling Algorithmspublic2015-05-25 19:442017-12-05 17:09
Reportermsv 
Assigned Toisn 
PrioritynormalSeverityminor 
StatusassignedResolutionopen 
PlatformOSOS Version
Product Version[OCCT] 6.9.0 
Target Version[OCCT] 7.4.0*Fixed in Version 
Summary0026265: Incorrect result of 2d offset
DescriptionIncorrect result of 2d offset operation.
It is reproduced in the test case "offset wire_unclosed_outside_0_025 B4".
Steps To Reproduceoffset wire_unclosed_outside_0_025 B4
TagsNo tags attached.
Test case number
Attached Filespng file icon offset_KO.PNG (9,997 bytes) 2015-05-25 19:44
png file icon offset_OK.PNG (9,341 bytes) 2015-05-25 19:45
? file icon offset_OK.brep (87,761 bytes) 2015-05-25 19:46

- Relationships
related to 0027890closedkgv Open CASCADE BndLib_Add2dCurve::Add(..) works incorrect on some curves 
related to 0028571newmsv Community BRepOffsetAPI_MakeOffset build invalid wire 

-  Notes
(0041530)
msv (developer)
2015-05-25 19:48

The correct result should look as in the picture offset_OK.png
The shape offset_OK.brep has been got by manually splitting intersected edges in Draw and collecting necessary edges in the compound.
(0043418)
git (administrator)
2015-07-24 15:48

Branch CR26265 has been created by isn.

SHA-1: c18ad6f7d4cc17cd308c7c2f21917c688dbb3571


Detailed log of new commits:

Author: isn
Date: Fri Jul 24 12:54:16 2015 +0300

    0026265: Incorrect result of 2d offset
    very draft version
(0043431)
git (administrator)
2015-07-24 17:24

Branch CR26265 has been updated by isn.

SHA-1: 20251080e1742bc6117bb3b31809cf426ec6920c


Detailed log of new commits:

Author: isn
Date: Fri Jul 24 17:24:36 2015 +0300

    get rid of mutable key on listoflinks

(0043446)
git (administrator)
2015-07-27 11:32

Branch CR26265 has been updated by isn.

SHA-1: 70e6ed80d4eb27deeab3a4050d2c0768ba81d88a


Detailed log of new commits:

Author: isn
Date: Mon Jul 27 11:32:31 2015 +0300

    locations of curves have been added

(0043447)
git (administrator)
2015-07-27 11:35

Branch CR26265 has been updated forcibly by isn.

SHA-1: 62bfe55abac418895a07053618a25a7a027423c6
(0043558)
git (administrator)
2015-07-27 14:08

Branch CR26265 has been updated forcibly by isn.

SHA-1: 69e05a4acbf0d9631ca64f76c1edebaca4a8375b
(0043606)
git (administrator)
2015-07-27 20:25

Branch CR26265 has been updated by isn.

SHA-1: 33135e743db92af19ec0599b9edea12d4b4a9f60


Detailed log of new commits:

Author: isn
Date: Mon Jul 27 20:25:30 2015 +0300

    1) more accurate method to find links with same nodes but diff. orientation
    2) take into account a self-intersected wires..

(0043636)
git (administrator)
2015-07-28 15:02

Branch CR26265 has been updated by isn.

SHA-1: 461217b0503237a20685b12330d3591d75e61ef7


Detailed log of new commits:

Author: isn
Date: Tue Jul 28 15:02:14 2015 +0300

    improving of stability

(0043843)
git (administrator)
2015-08-05 13:27

Branch CR26265 has been updated by isn.

SHA-1: d77104e39597eeb20bcbaffe07d2bc62bb579ed5


Detailed log of new commits:

Author: isn
Date: Wed Aug 5 13:27:09 2015 +0300

    comments

(0043847)
git (administrator)
2015-08-05 14:08

Branch CR26265 has been updated forcibly by isn.

SHA-1: 293d0f6ddebc674a17d6b7d2349d2071116ddfd8
(0043876)
git (administrator)
2015-08-06 13:05

Branch CR26265 has been updated by isn.

SHA-1: 9c5f8aa02426037afac80ca5e792610880347fb0


Detailed log of new commits:

Author: isn
Date: Thu Aug 6 11:25:53 2015 +0300

    get rid of std::max
    eliminate warnings
    check BRepLib_MakeEdge on IsDone()

(0043927)
git (administrator)
2015-08-07 17:23

Branch CR26265 has been updated by isn.

SHA-1: 9eb81cc39a67f44f14b9e4a5c12317f081622ef9


Detailed log of new commits:

Author: isn
Date: Fri Aug 7 17:22:22 2015 +0300

    remarks # 1

(0043984)
git (administrator)
2015-08-11 17:26

Branch CR26265 has been updated by isn.

SHA-1: 25c914a60422d285940f89bfc77c489d4a8418d7


Detailed log of new commits:

Author: isn
Date: Tue Aug 11 17:25:47 2015 +0300

    extract one-edge-loops from wire before passing to MakeLoops algo

(0043999)
git (administrator)
2015-08-11 19:05

Branch CR26265 has been updated forcibly by isn.

SHA-1: a02c7b3cd23472f0db71708f79a1ffe3309705f1
(0044019)
git (administrator)
2015-08-12 10:48

Branch CR26265 has been updated forcibly by isn.

SHA-1: dc494f8429d5880b8d4722cad3353536fa1205b9
(0044020)
git (administrator)
2015-08-12 11:03

Branch CR26265 has been updated forcibly by isn.

SHA-1: fc5d3cab9fd7d0613bb0c2977074b391f14e16f8
(0046098)
git (administrator)
2015-09-24 17:10

Branch CR26265_1 has been created by msv.

SHA-1: 707cb1776b2c8b9ec44ca3bbbe9dbe0438c3112b


Detailed log of new commits:

Author: isn
Date: Fri Jul 24 12:54:16 2015 +0300

    0026265: Incorrect result of 2d offset
    
    Additional treatment has been added as the last step of the offset procedure to remove wrong parts (loops) from the result.
(0046099)
msv (developer)
2015-09-24 17:11

The fix has been re-based on current master and combined in one commit.
(0057746)
git (administrator)
2016-09-13 16:04

Branch CR26265_2 has been created by isn.

SHA-1: 54c708c94594eab8930784724505a2559fb161eb


Detailed log of new commits:

Author: isn
Date: Fri Jul 24 12:54:16 2015 +0300

    0026265: Incorrect result of 2d offset
    
    Additional treatment has been added as the last step of the offset procedure to remove wrong parts (loops) from the result.
(0058237)
git (administrator)
2016-09-28 11:22

Branch CR26265_3 has been created by isn.

SHA-1: 73e83cf785f01bba2b32f368313e6d5656e4c8b5


Detailed log of new commits:

Author: isn
Date: Wed Sep 28 11:20:01 2016 +0300

    0026265: Incorrect result of 2d offset
    
    complete redesign of prev. version + optimizations + regressions

Author: isn
Date: Tue Sep 27 20:19:31 2016 +0300

    reversed meaning of -o option

Author: isn
Date: Mon Sep 26 11:48:39 2016 +0300

    actual fix

Author: isn
Date: Thu Sep 22 14:42:30 2016 +0300

    0027890: BndLib_Add2dCurve::Add(..) works incorrect on some curves
    
    new testcase: extension for 'gbounding' command. now this command allows to pass curve2d as args and works in two modes ('normal' and optimal)
(0058239)
isn (developer)
2016-09-28 11:37
edited on: 2016-09-28 11:38

this new branch is on top of cr27890, just because the changes from 27890 are not yet rebased on current master

(0058262)
msv (developer)
2016-09-28 14:46

Dear Ilya, please explain here what is relation with 27890.
(0058264)
isn (developer)
2016-09-28 15:09
edited on: 2016-09-28 15:10

The new method which called by BRepFill_OffsetWire algo uses bounding boxes to speed up intersection process between (offset) edges (more precisely, their pcurves). In some cases the retrieved result from BndLib_Add2dCurve::Add(..) is incorrect (wrong bnd box) which leads to reduction of number of found intersections. That's why this fix should be applied on top of the 27890...

(0058289)
msv (developer)
2016-09-28 21:12

- Please use a standard header before each function:
//=======================================================================
//function : <name>
//purpose :
//=======================================================================

- Please wrap long lines (> 100 characters).

- At 479-489: make all local functions static.

- At 1430: don't we need to put the new code under condition "!myIsOpenResult"?

- In many places local variables are named without prefix "a" starting with capital letter (IWires, HoleWires, ME2IP, IsMaxIsHole, Exp, etc.). Please, start them with "a", "is", or name starting with lowercase letter the names of iterators, like i, it, exp.

- Please make fake declarations in private section:
  BRepFill_BndBoxTreeSelector(const BRepFill_BndBoxTreeSelector& );
  BRepFill_BndBoxTreeSelector& operator=(const BRepFill_BndBoxTreeSelector& );

- In the method FindWireIntersections (line 3088), replace TopTools_SequenceOfShape with another container with quick direct access to items by index. I propose to use MapShapes to create IndexedMap.

- At 176, 255: creation of adaptor can be expensive operation, so it is better to cache them in some data map.

- Question at 3103: yes, use cache.

- At 194, it is better not to use 3d curves, instead use D0 of adaptor 2d, then D0 of the surface.

- At 276, location is not taken into account for p3d2.

- Why coefficient 1.01 at line 279 differs from the similar by meaning at line 206?

- Question at 183: if we create global map for all vertices at once then checking for coincidence will take too much time, and indeed we do not expect intersections with vertices from other edges here.

- Question at 214: it is OK.

- At 221: if end of one edge intersect the other edge in the middle we must split that edge, but not create new vertex. In your code, in such case the edge will not be split, which is incorrect.

- In the case of self-intersections, should not we consider the case of vertex-edge intersection?

- The method GetResult returns result map by assigning operator. It is not good. It is better to store the reference on the result container given in constructor, as it is done with mySeqOfEdges.

- Question at 322: for better readability you can use typedef.

- I see no need in using NCollection_DataMap<Standard_Real, TopoDS_Vertex> for each edge. Instead, you can use a list of structures containing param and vertex.

- At 3099: we know the size of BndBoxesOfEdges in advance. Therefore it is better to use Array1 instead of sequence for its type.

- Question at 3129: which "internal" do you mean? This function is static. It is enough to use local reshape.

- At 3136, use BRep_Tool::Range instead of Curve.

- At 3148: use std::sort instead of NCollection_QuickSort (see 0027915).

- Lines 3169-3170 can be moved after 3165.

- Question at 3189: Why reordering of wire is needed at all? Do some edges really need to be reversed? We should not allow reverse orientation of offsets of original edges. If this occurs with some edge we must skip such wire.

- Question at 3256: yes, use cached adaptor.

- Line 3277 is not needed, flags are initialized with this value by default.

- At 330: the name Poly_Helper makes me think that this class should be defined in the package Poly. Please use another prefix appropriate here.

- Line 338 is extra.

- At 345: no need to implement the copy constructor for this class. Make it and operator= private.

- Question at 3323: presence of hanging links may mean that some intersection/splitting was not done correctly. In this case, we can indeed skip the wire from fixing it, at least till the moment we know other cases when we should do something else.

- Question at 3342: non-closed wire cannot appear here; if it is then some error exists in the algorithm. You can put Standard_ASSERT_VOID here.

- Question at 3389: PerformInfinitePoint must never return status other than IN or OUT. You can put assert in other case.

- At 3356: it is unsafe to use a pure pointer. Please use here some kind of smart pointer, like NCollection_Handle.

- At 3395: why 0 is used for Eps parameter? Algorithm can run too long in this case.

- It is unclear why we reject the cases indicated by the condition on line 3418.

- At 3380-3391: It is unclear why cannot we determine the wire is hole or not by checking the sign of surface area. We could get rid of necessity of creation of FClass2d for each wire. Instead, just to remember 'af' face to create FClass2d later.

- Question at 3379: for the most outer wire the mass cannot be very small.
(0063793)
git (administrator)
2017-02-14 12:21

Branch CR26265_3 has been updated by isn.

SHA-1: f4729d5ded78c3d9528f3b545e68cffe02fad048


Detailed log of new commits:

Author: isn
Date: Thu Jan 12 19:28:29 2017 +0300

    processing of E/E and E/V intersection segments

Author: isn
Date: Mon Oct 3 11:03:28 2016 +0300

    first remarks

(0064587)
msv (developer)
2017-03-21 17:51

Remarks:

- Please use a standard header before each function.
- make methods IsEqual and HashCode inline instead of static.
- empty ctor of BRepFill_IntersectionParam must initialize the field param.
- 224: this optimization may not help, leave for a while.
- 298: use Max(TolE1, TolE2)
- 310: use Square(TolV+aFutTol)
- 413: in this case it is better to avoid usage of map, just get two vertices and compare. BTW, put computation of invariant out of the loop.
- 435: not for a while
- 439: the declared here check is not performed. Can the method ProcessIPoints be adopted for this case?
- 649: add 'static' declaration.
- 650-687: such declarations are used when the usage of function precedes its definition. You placed definitions before their usage. It can (and does!) lead to errors. Look at the function FindWireIntersections. Its last argument 'theAddtEdgesInt' has different types in declaration and in definition, and this fact makes declaration not relevant to the definition without any compilation warning. You should either remove all these declarations (this is preferred) or place functions definitions in different order.
- 1628: don't we need to put the new code under condition "!myIsOpenResult"?
- 3345: yes, if all splits are considered. And the argument theAddtEdgesInt is extra here. Moreover, I think the map theAddtEdgesInt is not needed at all. It just makes the code complex. And it duplicates info from ME2IP.
- 3357: use TopExp::Vertices to get vertices of an edge.
- 3365,3366: too long lines.
- 3366-3375: why indentation is 3?
- 3382-3383: use TopExp::Vertices to get both vertices of an edge.
- 3411-3421: avoid map rebuilding, just skip unnecessary items when it is used.
- 3434: use const &
- 3467: make the function BRepFill_ParamComp inline.
- 3499: it's better to insert operator<() in the struct BRepFill_IntersectionParam and use function sort with two arguments.
- 3478: remove this comment.
- 3486: use const &
- 3517: OK
- 3523: it is better to make check in 3308, making sure there is no deg edges in processing at all.
- 3532: you control building of wire yourself, so it's better to use low-level builder here.
- 3555: usage NCollection_IndexedMap as item in SV2CE is not justified. It's better to check for existence of added edge by iteration in UpdateSV2CEMap.
- 3568: use const &. Removal from CEM is not necessary, it's better just to iterate all its items.
- 3569: usage NCollection_IndexedMap as item in AllGE is not justified. It's better to use the list.
- 3613: too long line
- 3633-3634: use TopExp::Vertices to get both vertices of an edge.
- 3709: too long line
- 3721: use const &
- 3723: use BRep_Builder to make wire here.
- 3735: set the flag yourself.
- 3745,3747: too long line
- 3766: use BRep_Builder
- 3774: avoid creation of classifier for each wire. First select the most outer using only SurfaceProperties. And only for it create classifier. Then no smart pointer of BRepTopAdaptor_FClass2d will be needed.
- Use dedicated handle class BRepAdaptor_HCurve2d instead of NCollection_Handle<BRepAdaptor_Curve2d>.
- 3848-3854: you can use the method BRep_Tool::MaxTolerance instead.
- 3855: I think it is needed to limit it to be not greater than 1% of max extension (bounding box of all vertices).
- 3857: why oriented hasher is used here? Analyzing the further code does not prove its necessity. It's better to use TopTools_IndexedDataMapOfShapeListOfShape.
- 3894: iteration on data map with key depending on memory address can lead to unstable run of the algorithm. If you use indexed data map this pb will be gone.
- 3860: extra container. It is enough to use allV.
- 3870: it is needed to add vertex tolerance as gap to the box.
- 3905: If wire has no intersections but vertices were fused we should apply this fusion to output (change wire).
- 3947: use 'the' prefix only for function arguments.
- use TopTools_IndexedDataMapOfShapeShape instead of NCollection_IndexedDataMap<TopoDS_Edge, TopoDS_Edge>.
- use TopTools_IndexedMapOfShape instead of NCollection_IndexedMap<TopoDS_Vertex,TopTools_ShapeMapHasher>
- use TopTools_IndexedMapOfShape instead of NCollection_IndexedMap<TopoDS_Edge>
(0066503)
msv (developer)
2017-05-24 11:18

Ilya, what is the current state of this bug?

- Issue History
Date Modified Username Field Change
2015-05-25 19:44 msv New Issue
2015-05-25 19:44 msv Assigned To => msv
2015-05-25 19:44 msv File Added: offset_KO.PNG
2015-05-25 19:45 msv File Added: offset_OK.PNG
2015-05-25 19:46 msv File Added: offset_OK.brep
2015-05-25 19:48 msv Note Added: 0041530
2015-05-26 10:45 msv Assigned To msv => isn
2015-05-26 10:45 msv Status new => assigned
2015-07-24 15:48 git Note Added: 0043418
2015-07-24 17:24 git Note Added: 0043431
2015-07-27 11:32 git Note Added: 0043446
2015-07-27 11:35 git Note Added: 0043447
2015-07-27 14:08 git Note Added: 0043558
2015-07-27 20:25 git Note Added: 0043606
2015-07-28 15:02 git Note Added: 0043636
2015-08-05 13:27 git Note Added: 0043843
2015-08-05 14:08 git Note Added: 0043847
2015-08-06 13:05 git Note Added: 0043876
2015-08-07 17:23 git Note Added: 0043927
2015-08-11 17:26 git Note Added: 0043984
2015-08-11 19:05 git Note Added: 0043999
2015-08-12 10:48 git Note Added: 0044019
2015-08-12 11:03 git Note Added: 0044020
2015-09-24 17:10 git Note Added: 0046098
2015-09-24 17:11 msv Note Added: 0046099
2016-09-13 16:04 git Note Added: 0057746
2016-09-28 11:22 git Note Added: 0058237
2016-09-28 11:23 isn Assigned To isn => msv
2016-09-28 11:23 isn Status assigned => resolved
2016-09-28 11:37 isn Note Added: 0058239
2016-09-28 11:38 isn Note Edited: 0058239 View Revisions
2016-09-28 14:45 msv Relationship added related to 0027890
2016-09-28 14:46 msv Note Added: 0058262
2016-09-28 15:09 isn Note Added: 0058264
2016-09-28 15:10 isn Note Edited: 0058264 View Revisions
2016-09-28 21:12 msv Note Added: 0058289
2016-09-28 21:12 msv Assigned To msv => isn
2016-09-28 21:12 msv Status resolved => assigned
2016-10-28 16:28 msv Target Version 7.1.0 => 7.2.0
2017-02-14 12:21 git Note Added: 0063793
2017-02-14 12:33 isn Assigned To isn => msv
2017-02-14 12:33 isn Status assigned => feedback
2017-03-20 09:53 msv Relationship added related to 0028571
2017-03-21 17:51 msv Note Added: 0064587
2017-03-21 17:51 msv Assigned To msv => isn
2017-03-21 17:51 msv Status feedback => assigned
2017-05-24 11:18 msv Note Added: 0066503
2017-07-24 09:22 msv Target Version 7.2.0 => 7.3.0
2017-12-05 17:09 msv Target Version 7.3.0 => 7.4.0*


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker