View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0030020CommunityOCCT:Modeling Datapublic2018-08-07 09:152021-04-06 12:36
Reporterdenix56 Assigned Tomsv 
PrioritynormalSeverityminor 
Status assignedResolutionopen 
PlatformWindowsOSVC++ 2015 
Product Version7.3.0 
Summary0030020: Modeling Data - Incorrect bspline visualization
DescriptionWhen I draw the spline as a line where the fit points (we`ll number it 1, 2, 3) are located in order 1, 3, 2 and vusialize it with VTK using IVtkTools_ShapeDataSource, the connection between points 2 and 3 is not drawn.

It is caused by the bug in GCPnts_TangentialDeflection::PerformCurve, where the check isLine returns true and only first and last points are connected.

The patch to fix this bug is provided.
Steps To Reproduce1. Define 3d fit points that lie on the line.
2. Interpolate them in order (1, 3, 2)
3. Create edge from spline
4. Visualize using IVtkTools_ShapeDataSource

# Draw script 1
set F [open pnts w]
puts $F "3 3d"
puts $F "0 0 0"
puts $F "2 0 0"
puts $F "1 0 0"
close $F
interpol c pnts
file delete pnts
crvtpoints r c 0.1 1

# Draw script 2
bsplinecurve c 2 2 0 3 146.299 3 17 -38 0 1 165.693 -38.4947 0 1 21.7888 -38 0 1
crvtpoints r c 0.4 12
TagsNo tags attached.
Test case number

Attached Files

  • GCPnts_TangentialDeflection.patch (2,158 bytes)
  • GCPnts_TangentialDeflectionV2.patch (2,783 bytes)
  • GCPnts_TangentialDeflectionV3.patch (2,524 bytes)
  • GCPnts_TangentialDeflectionV4.patch (3,091 bytes)
  • GCPnts_TangentialDeflectionV5.patch (3,423 bytes)
  • bug_example.txt (76 bytes)
  • GCPnts_TangentialDeflectionV6.patch (5,678 bytes)
  • incmesh_c7_21.brep (6,311 bytes)
  • incmesh_c7_21_1.draw (1,855 bytes)
  • crvtpoints_master.PNG (6,575 bytes)
  • crvtpoints_patch.PNG (7,362 bytes)
  • FixSplineDiscret.patch (30,175 bytes)
  • FixSplineDiscretV2.patch (30,326 bytes)
  • patches.zip (8,961 bytes)
  • BRepMesh_DiscretFactory.cxx.patch (1,015 bytes)

Relationships

related to 0032265 closedbugmaster Community Modeling Algorithms - some trivial code improvements in CSLib_Class2d and GCPnts_TangentialDeflection 

Activities

denix56

2018-08-07 09:15

reporter  

GCPnts_TangentialDeflection.patch (2,158 bytes)

denix56

2018-08-07 10:55

reporter  

GCPnts_TangentialDeflectionV2.patch (2,783 bytes)

denix56

2018-08-07 11:00

reporter   ~0078450

Fixed incorrect delta in V2

msv

2018-08-07 11:19

developer   ~0078451

The fix makes the algorithm insert 4 points in the result. Though it is enough to insert only 3 points.

denix56

2018-08-07 11:44

reporter   ~0078454

Last edited: 2018-08-07 11:46

 No, there will be only 3 points, the first, then 1 in loop [2, minNbPoints), where minNbPoints == 3 and the last one

Or maybe we talk about different lines

The old algorithm inserted more points then needed.

git

2018-08-07 12:01

administrator   ~0078456

Branch CR30020 has been created by msv.

SHA-1: 47bfc98f1134be50f788210384e0c8e6cc9fbe06


Detailed log of new commits:

Author: msv
Date: Tue Aug 7 11:47:59 2018 +0300

    0030020: Incorrect bspline visualization
    
    Let's we have a spline having fit points located on a direct line (let's number them 1, 2, 3), but they are located in order 1, 3, 2. There was a bug in GCPnts_TangentialDeflection that only first and last points were put in the result. Now more point is put, so that the result polygon made back movement.
    
    The command crvtpoints has been made to accept angular value in degrees instead of radians.

msv

2018-08-07 12:07

developer   ~0078457

I have prepared the branch for integration. It will be certified.

msv

2018-08-07 15:38

developer   ~0078466

Jenkins job is CR30020-master-msv. It shows regressions that must be analyzed. The branch did not pass tests.

denix56

2018-08-07 17:40

reporter   ~0078472

Where can I see which tests failed?

denix56

2018-08-07 18:15

reporter   ~0078476

Last edited: 2018-08-07 18:31

 gp_XYZ V3 = MiddlePoint.XYZ() - prevMiddlePoint.XYZ();

            if(IsLine && IsSequential)
           {
                Standard_Real L3 = V3.Modulus();
                IsSequential = (L3 <= L1);
            }
           }
          prevMiddlePoint = MiddlePoint;

Why was it removed from branch?
It will not work correctly without it.

One thing that could be done is to move inside if(IsLine && IsSequential)

Could you please delete old files& I cannot do it (or don`t see the button)

Uploaded patch V3

denix56

2018-08-07 18:28

reporter  

GCPnts_TangentialDeflectionV3.patch (2,524 bytes)

msv

2018-08-08 12:41

developer   ~0078486

The test results can be seen only in our intranet. However, if the used data model is not confidential I can provide the data and steps to reproduce the regression.

msv

2018-08-08 12:44

developer   ~0078487

I removed L3 from the branch by logical reasons. I thought that we must measure the distance of the middle point always from the starting point. Otherwise, it is not logical to compare the distance of some insiding segment with the distance between start and end points.

msv

2018-08-08 12:45

developer   ~0078488

Anyway, I will try your solution on the tests with regressions to see if it works.

msv

2018-08-08 12:48

developer   ~0078489

The most markable regression can be reproduced using bug 0022304.

denix56

2018-08-08 14:40

reporter   ~0078490

We compare the distances between (2 middle points) and (middle poin and start point) to detect if the current middle point lies outside (first - last).
The comparison of L1 and L2 does not gie us such information, because l2 could be less than l1, but point lies outside, i.e. 2--1-------3

msv

2018-08-08 16:05

developer   ~0078494

I see your point. But then the following case will not work: 1----2--4--3. To respect both cases it is needed to check both conditions L2<=L1 && L3<=L1.

msv

2018-08-08 16:10

developer   ~0078495

BTW, I found out the cause of regressions. If the firstu and lastu delimit one interval, the conditions of rejecting neighboring intervals [...,firstu] and [lastu,...] does not work, and then both L2 and L3 can be much greater than L1. It leads to isSequential == false evel for quite sequential points. So, it is needed to elaborate more accurate interval rejecting in the beginning of the loop "for (i = 1; i <= NbInterv && IsLine; ++i)".

git

2018-08-08 16:32

administrator   ~0078496

Branch CR30020 has been updated by msv.

SHA-1: 0ba065098f0056bedcd4e2d669b7dd7f023fca80


Detailed log of new commits:

Author: msv
Date: Wed Aug 8 16:30:25 2018 +0300

    # fighting against regressions

Author: msv
Date: Wed Aug 8 12:15:08 2018 +0300

    #return back to original patch by denix56

msv

2018-08-08 16:35

developer   ~0078497

Dear denix56, I think that prevMiddlePoint must be initialized before entering the loop on intervals, so that it did not reset to the start point with each interval.

I have updated the branch and launched tests. Could you review it meanwhile?

git

2018-08-08 16:37

administrator   ~0078498

Branch CR30020 has been updated forcibly by msv.

SHA-1: 1d29c498ca9317d99cc3d7510b512f60208f46da

denix56

2018-08-08 17:11

reporter  

GCPnts_TangentialDeflectionV4.patch (3,091 bytes)

denix56

2018-08-08 17:16

reporter  

GCPnts_TangentialDeflectionV5.patch (3,423 bytes)

denix56

2018-08-08 17:18

reporter   ~0078499

Last edited: 2018-08-08 17:21

 I added V5, where fixed incorrect visualization when spline is nearly but not linear and point 2 is outside and located far from 1 and 3.

Fixed by replacement sun check with the cos one (problem happens when angle is > 90)

Reviewed current branch, everything`s look ok.

msv

2018-08-08 17:49

developer   ~0078501

Could you attach the shape to reproduce the problem in your last message? You can use the method GeomTools::Write to obtain a file.
If so, then patch V5 could be also taken.

denix56

2018-08-08 18:00

reporter  

bug_example.txt (76 bytes)

denix56

2018-08-08 18:00

reporter   ~0078503

If I`ve done it right, you will see that connection to 2nd point disappears.

msv

2018-08-08 18:24

developer   ~0078505

Last edited: 2018-08-08 18:27

 I have tried this curve, it is always tessallated well, even without patch V5.

msv

2018-08-08 18:26

developer   ~0078506

Which parameters do you pass to the algorithm?

git

2018-08-08 18:43

administrator   ~0078508

Branch CR30020 has been updated by msv.

SHA-1: d9a0f9af17f0f4ab63934f0ef2d80253e947c80b


Detailed log of new commits:

Author: msv
Date: Wed Aug 8 17:54:58 2018 +0300

    # accomodate patch V5

denix56

2018-08-08 18:43

reporter   ~0078509

I use bigger dev_coeff for IVtkOCC_ShapeMesher 0.001

msv

2018-08-08 18:58

developer   ~0078510

I do not use VTK, and it is hard for me derive GCPnts_TangentialDeflection parameters from IVtkOCC_ShapeMesher parameters. Could you break in the algorithm to see the parameters?

denix56

2018-08-08 19:19

reporter   ~0078511

curvatureDeflection = 0.41189937809767357
angularDeflection = 0.20943951023931953
MinimumOfPoints = 2
UTol = 9.9999999999999986e-10
theMinLen = 9.9999999999999995e-08

msv

2018-08-08 21:32

developer   ~0078512

OK, thanks, now I could reproduce the bug.

msv

2018-08-08 21:44

developer   ~0078513

Tests have not been passed with patch v5. A lot of errors and differences in output meshes.

denix56

2018-08-08 23:49

reporter   ~0078514

Last edited: 2018-08-09 08:12

 Only one thing that could be changed is to compare the dot product with greater or equal sign (if points are same dot product return 0)

Do you any ideas what causes these problems?)

If it`s possible, could you provide the small example (as I did) of what error or difference produced?

denix56

2018-08-09 08:57

reporter  

GCPnts_TangentialDeflectionV6.patch (5,678 bytes)

denix56

2018-08-09 08:58

reporter   ~0078515

Uploaded V6 patch, some improvements and maybe it will fix errors (don`t know).

git

2018-08-09 19:13

administrator   ~0078542

Branch CR30020 has been updated by msv.

SHA-1: 9926564df2deedee6b2c0f07f0d3393545e31861


Detailed log of new commits:

Author: msv
Date: Thu Aug 9 19:11:24 2018 +0300

    # patch V6

msv

2018-08-09 19:37

developer  

incmesh_c7_21.brep (6,311 bytes)

msv

2018-08-09 19:41

developer   ~0078543

With the patch V6 many regressions were gone. However, some remain. I have attached a face from one test data model. It is meshed and visualized on master version, but fails with the patch.

Commands to reproduce in Draw:
restore incmesh_c7_21.brep f
incmesh f 0.1
trinfo f

msv

2018-08-09 20:08

developer  

incmesh_c7_21_1.draw (1,855 bytes)

msv

2018-08-09 20:10

developer  

crvtpoints_master.PNG (6,575 bytes)

msv

2018-08-09 20:10

developer  

crvtpoints_patch.PNG (7,362 bytes)

msv

2018-08-09 20:13

developer   ~0078544

From the face above, the problem edge is the first one. I have attached its curve in the last file incmesh_c7_21_1.draw.
The master version tesselates its end in another way than in the patch. See attached snapshots crvtpoints_master.PNG and crvtpoints_patch.PNG.
This causes the error "self intersecting wire" in the meshing algorithm.

denix56

2018-08-09 22:06

reporter   ~0078547

Unfortunately, I will away from my laptop till the end of the next week.
Could you please attach file so that I could read it with geom_tools read (if it is possible)?

msv

2018-08-10 09:54

developer   ~0078550

File is attached. To read by GeomTools just remove the first line from .draw file. The .brep file is to be read by BRepTools.
I will be away next 3 weeks.

denix56

2018-08-21 19:58

reporter   ~0078710

Last edited: 2018-08-22 10:19

 First look showed that on this curve some points are not sequential (or at least D0 returns such points). It happens int the end of the algorithm when we insert additional points. On some of them the middlePoint is located after P1 and P2, causing self-intersections. It might be problem with either the data or the D0 function, not algorithm.

I have plotted the points from draw file using some web tools - it shows that points form self-intersecting curve.
The tests passed before due to the incorrect work of the algorithm (sin was used instead of cos)

denix56

2018-09-24 11:41

reporter   ~0079349

Any updates on this issue?

msv

2018-09-24 14:48

developer   ~0079366

Sorry for delay, I have many work in other projects.

Indeed, this curve is self-intersceting and has many forward-back movements along it.
You are right, in the master version the algorithm behaves in different way and occasionally this face is triangulated without error.

Anyway, this fix introduces formal regression. It would be great if we could make workaround for this case. E.g., we can make two modes of work of the patched algorithm, and call it from BRepMesh using the old behavior.

denix56

2018-09-24 16:05

reporter   ~0079367

Last edited: 2018-09-24 16:27

 Calling from BRepMesh using the old behavior will produce incorrect results on good meshes while we create workaround for incorrect mesh.

I think that fixing that test file will be much better solution.

msv

2018-09-24 17:48

developer   ~0079372

Please provide a new test case that proves the necessity of this fix.
In the steps to reproduce only the case of direct calling the patched algorithm is provided. And now I mean that we need a test case that involves incmesh.

msv

2018-09-24 17:50

developer   ~0079373

Usually we avoid fixing input test data, because they provide true test case from a customer. Altering test file and patching the algorithm we can introduce a regression in some user application.

denix56

2018-09-25 11:29

reporter  

FixSplineDiscret.patch (30,175 bytes)

denix56

2018-09-25 11:31

reporter   ~0079383

Last edited: 2018-09-25 11:32

 I`ve added new patch that contains this workaround - by default, ocad will use old behavior in brep_mesh, and if user want to use new beahavior there is an argument to change it.
Also there is note about the fact that old behavior might cause incorrect results and provided only for backward compatibility.

denix56

2018-09-25 11:38

reporter  

FixSplineDiscretV2.patch (30,326 bytes)

msv

2018-09-28 12:11

developer   ~0079469

How did you get the patch file? I cannot apply your patch:
D:\git\occt1>git apply FixSplineDiscretV2.patch
error: corrupt patch at line 17

I use git version 2.16.1.windows.4.
Please provide a valid patch file.

denix56

2018-09-28 18:01

reporter  

patches.zip (8,961 bytes)

denix56

2018-09-28 18:02

reporter   ~0079493

It seems i have some problems with creation of the single patch, so I provided archive with separate patches for each file.

msv

2018-10-05 19:48

developer   ~0079738

These patches also have errors to apply. Eventually, I have managed to apply them. For that I had to manually change the lines like:
--- "a/D:\\dev-tools\\opencascade-7.3.0\\tmp\\BRepMesh_FastDiscret.hxx"
+++ "b/D:\\dev-tools\\opencascade-7.3.0\\src\\BRepMesh\\BRepMesh_FastDiscret.hxx"
to
--- "a/src\\BRepMesh\\BRepMesh_FastDiscret.hxx"
+++ "b/src\\BRepMesh\\BRepMesh_FastDiscret.hxx"

BTW, your patch misses the changes in the file BRepMesh_DiscretFactory.cxx.

In substance, the new parameter TDOldBehaviour looks ugly. Please give me a while for thinking about a way to avoid such impacting the algorithms.

Again, I repeat my request for test file to reproduce the initial problem using mesh algorithm. I highly need it to create a non-regression test.

git

2018-10-05 20:03

administrator   ~0079739

Branch CR30020 has been updated by msv.

SHA-1: 897bd7f6efd4ad2bed98b5741b1dd78ca22ccb17


Detailed log of new commits:

Author: msv
Date: Fri Oct 5 20:03:04 2018 +0300

    # patch with leaving old behavior, and adding new behavior by option

git

2018-10-05 20:05

administrator   ~0079740

Branch CR30020 has been updated forcibly by msv.

SHA-1: 83bc18b4f0769ddc2e0a177a3a31adddaf4d15bd

git

2018-10-08 10:14

administrator   ~0079769

Branch CR30020 has been updated by msv.

SHA-1: f9f5db1264e7efa8ff8e82ebb64cdeeb85478d76


Detailed log of new commits:

Author: msv
Date: Mon Oct 8 10:14:00 2018 +0300

    # correct test cases

denix56

2018-10-16 14:15

reporter   ~0080011

Last edited: 2018-10-16 16:02

 Sorry for delay with answer, has lots of other work to do.
Added patch for discretfactory.cxx

denix56

2018-10-16 14:15

reporter  

BRepMesh_DiscretFactory.cxx.patch (1,015 bytes)

denix56

2019-08-14 11:05

reporter   ~0086242

What the current progress on this bug?

denix56

2019-10-05 09:41

reporter   ~0087848

Reminder sent to: msv

Issue History

Date Modified Username Field Change
2018-08-07 09:15 denix56 New Issue
2018-08-07 09:15 denix56 Assigned To => msv
2018-08-07 09:15 denix56 File Added: GCPnts_TangentialDeflection.patch
2018-08-07 10:55 denix56 File Added: GCPnts_TangentialDeflectionV2.patch
2018-08-07 11:00 denix56 Note Added: 0078450
2018-08-07 11:19 msv Note Added: 0078451
2018-08-07 11:20 msv Steps to Reproduce Updated
2018-08-07 11:44 denix56 Note Added: 0078454
2018-08-07 11:45 denix56 Note Edited: 0078454
2018-08-07 11:45 denix56 Note Edited: 0078454
2018-08-07 11:46 denix56 Note Edited: 0078454
2018-08-07 11:46 denix56 Note Edited: 0078454
2018-08-07 12:01 git Note Added: 0078456
2018-08-07 12:07 msv Note Added: 0078457
2018-08-07 12:07 msv Status new => assigned
2018-08-07 15:38 msv Note Added: 0078466
2018-08-07 17:40 denix56 Note Added: 0078472
2018-08-07 18:15 denix56 Note Added: 0078476
2018-08-07 18:16 denix56 Note Edited: 0078476
2018-08-07 18:28 denix56 File Added: GCPnts_TangentialDeflectionV3.patch
2018-08-07 18:28 denix56 Note Edited: 0078476
2018-08-07 18:31 denix56 Note Edited: 0078476
2018-08-08 12:41 msv Note Added: 0078486
2018-08-08 12:44 msv Note Added: 0078487
2018-08-08 12:45 msv Note Added: 0078488
2018-08-08 12:48 msv Note Added: 0078489
2018-08-08 14:40 denix56 Note Added: 0078490
2018-08-08 16:05 msv Note Added: 0078494
2018-08-08 16:10 msv Note Added: 0078495
2018-08-08 16:32 git Note Added: 0078496
2018-08-08 16:35 msv Note Added: 0078497
2018-08-08 16:37 git Note Added: 0078498
2018-08-08 17:11 denix56 File Added: GCPnts_TangentialDeflectionV4.patch
2018-08-08 17:16 denix56 File Added: GCPnts_TangentialDeflectionV5.patch
2018-08-08 17:18 denix56 Note Added: 0078499
2018-08-08 17:19 denix56 Note Edited: 0078499
2018-08-08 17:21 denix56 Note Edited: 0078499
2018-08-08 17:49 msv Note Added: 0078501
2018-08-08 18:00 denix56 File Added: bug_example.txt
2018-08-08 18:00 denix56 Note Added: 0078503
2018-08-08 18:24 msv Note Added: 0078505
2018-08-08 18:26 msv Note Added: 0078506
2018-08-08 18:27 msv Note Edited: 0078505
2018-08-08 18:43 git Note Added: 0078508
2018-08-08 18:43 denix56 Note Added: 0078509
2018-08-08 18:58 msv Note Added: 0078510
2018-08-08 19:19 denix56 Note Added: 0078511
2018-08-08 21:32 msv Note Added: 0078512
2018-08-08 21:34 msv Steps to Reproduce Updated
2018-08-08 21:44 msv Note Added: 0078513
2018-08-08 23:49 denix56 Note Added: 0078514
2018-08-09 08:12 denix56 Note Edited: 0078514
2018-08-09 08:57 denix56 File Added: GCPnts_TangentialDeflectionV6.patch
2018-08-09 08:58 denix56 Note Added: 0078515
2018-08-09 19:13 git Note Added: 0078542
2018-08-09 19:37 msv File Added: incmesh_c7_21.brep
2018-08-09 19:41 msv Note Added: 0078543
2018-08-09 20:08 msv File Added: incmesh_c7_21_1.draw
2018-08-09 20:10 msv File Added: crvtpoints_master.PNG
2018-08-09 20:10 msv File Added: crvtpoints_patch.PNG
2018-08-09 20:13 msv Note Added: 0078544
2018-08-09 22:06 denix56 Note Added: 0078547
2018-08-10 09:54 msv Note Added: 0078550
2018-08-21 19:58 denix56 Note Added: 0078710
2018-08-22 10:19 denix56 Note Edited: 0078710
2018-09-24 11:41 denix56 Note Added: 0079349
2018-09-24 14:48 msv Note Added: 0079366
2018-09-24 16:05 denix56 Note Added: 0079367
2018-09-24 16:26 denix56 Note Edited: 0079367
2018-09-24 16:27 denix56 Note Edited: 0079367
2018-09-24 17:48 msv Note Added: 0079372
2018-09-24 17:50 msv Note Added: 0079373
2018-09-25 11:29 denix56 File Added: FixSplineDiscret.patch
2018-09-25 11:31 denix56 Note Added: 0079383
2018-09-25 11:32 denix56 Note Edited: 0079383
2018-09-25 11:38 denix56 File Added: FixSplineDiscretV2.patch
2018-09-28 12:11 msv Note Added: 0079469
2018-09-28 18:01 denix56 File Added: patches.zip
2018-09-28 18:02 denix56 Note Added: 0079493
2018-10-05 19:48 msv Note Added: 0079738
2018-10-05 20:03 git Note Added: 0079739
2018-10-05 20:05 git Note Added: 0079740
2018-10-08 10:14 git Note Added: 0079769
2018-10-16 14:15 denix56 Note Added: 0080011
2018-10-16 14:15 denix56 File Added: BRepMesh_DiscretFactory.cxx.patch
2018-10-16 16:02 denix56 Note Edited: 0080011
2019-08-14 11:05 denix56 Note Added: 0086242
2019-09-04 12:07 kgv Summary Incorrect bspline visualization => Modeling Data - Incorrect bspline visualization
2019-10-05 09:41 denix56 Note Added: 0087848
2021-04-06 12:36 kgv Relationship added related to 0032265