View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0026747Open CASCADEOCCT:Modeling Algorithmspublic2015-10-02 17:032016-12-09 16:37
ReporternbvAssigned Tobugmaster  
PrioritylowSeveritytweak 
Status closedResolutionfixed 
Target Version7.1.0Fixed in Version7.1.0 
Summary0026747: Some constructors of gp_Parab2d class contain redundant parameters
DescriptionAccording to the theory, parabola is set by its directrix (is a simple line, not a vector) and focus point. The distances from every point of the parabola to the directrix and to the focus point are equal. The distance from the directrix to the focus point is equal to two focal length or to parameter of parabola. At that, there is a rule to direct X-axis (symmetry-axis) of local coordinate system (UCS) from the directrix to the focus point. Y-axis is always parallel to the directrix. In this case, every point of parabola is satisfied to the equation (in UCS) y(x)^2 = 2*p*x, where p is parameter of parabola.

Now, let consider existing constructors:

1. gp_Parab2d(const gp_Ax2d& D, const gp_Pnt2d& F, const Standard_Boolean Sense = Standard_True);

I do not understand, what should be the result of this constructor. Directrix D is directed line (has gp_Ax2d type). Does it mean that the direction of Y-axe should correspond to this direction? If it is not and the directrix direction is excess parameter then why we do not use gp_Lin2d class for directrix (indeed, line has direction, but it is its internal value as distinct from vector). Otherwise (if Y-direction correspond to the direction of the directrix, i.e. is defined), X-direction is defined by the directrix and the focus point (see above). Then, what is the role of "Sense" parameter? I cannot answer.

2. Constructor gp_Parab2d(const gp_Ax22d& D, const gp_Pnt2d& F).

We have analogical situation here: gp_Ax22d class sets full coordinate system XOY. Additionally, we need in focal-length only (which must be laid in positive direction of the X-axis). Why do we use focus point? More over, what kind of parabola should we obtain if the focus point does not lie in the X-axis? I cannot answer.

P.S.

Results of parabola creation for some constructor seems to be wrong. See Steps To Reproduce.
Steps To ReproduceTest cases have been created and pushed to the branch.

          Parabola 1 (see parab1.png):
// On the current MASTER, we have empty-parabola and exception in Debug-mode.
GCE2d_MakeParabola aPrb(gp_Ax2d(gp_Pnt2d(0.0, 3.0), gp_Dir2d(0.0, 1.0)), gp_Pnt2d(1.0, 3.0));

          Parabola 2 (see parab2.png):
// Left-handed coordinate system is used (Sense = Standard_False).
// However, the parabola is oriented as when right-handed coordinate system is used.
// More over, the parabola intersects its directrix. It is good, if and only if
// the focal length is equal to zero. But aPrb.Value()->Parab2d().Focal() returns 1.5 (on the current MASTER).
GCE2d_MakeParabola aPrb(gp_Ax2d(gp_Pnt2d(0.0, 0.0), gp_Dir2d(0.0, 1.0)), gp_Pnt2d(-1.0, 3.0), Standard_False);

          Parabola 3 (see parab3.png):
// Focus point lies in the directrix. Consequently, focal length is really equal to zero.
// But aPrb.Value()->Parab2d().Focal() returns 1.5 (on the current MASTER).
GCE2d_MakeParabola aPrb(gp_Ax2d(gp_Pnt2d(0.0, 0.0), gp_Dir2d(0.0, 1.0)), gp_Pnt2d(0.0, 3.0), Standard_False);
Additional information
and documentation updates		All changes have been documented in dox/dev_guides/upgrade/upgrade.md file.
TagsNo tags attached.
Test case numberbugs modalg_6 bug26747_1, bug26747_2, bug26747_3

Attached Files

  • parab1.png (10,421 bytes)
  • parab2.png (10,633 bytes)
  • parab3.png (10,685 bytes)

Relationships

related to 0025605 closedmsv Optimize methods of gp_* classes to avoid sqrt() function calling 

Activities

nbv

2015-10-02 17:11

developer   ~0046413

Dear Mikhail,

There are two ways for fixing problem 1 in Bug description:

1. To eliminate strange constructors and to keep only three:
a) gp_Parab2d(const gp_Ax2d& theDirectrix, const Standard_Real FocalLength, const Standard_Boolean Sense).

b) gp_Parab2d(const gp_Ax22d& theDirectrix, const Standard_Real FocalLength).

c) gp_Parab2d(const gp_Ax2d& theDirectrix, const gp_Pnt2d& theFocus). X-axe is defined here by theFocus location.

2. To keep these constructors and make them well-documented. E.g. "theFocus is used here for computation of focal length only. Sometimes, its position can be changed in order to obtain correct parabola (which does not intersects with the directrix)."

What is the better way for us? TIA.

msv

2015-10-07 15:39

developer   ~0046567

Dear Igor, your feedback is very appreciated.

ifv

2015-10-12 10:10

developer   ~0046665

In my opinion, we can remove strange constructor, I cannot think any case when that constructor will be useful.

msv

2015-10-12 10:22

developer   ~0046667

Dear Nikolay, so please make the fix removing useless constructor.
Process it with low priority, please.

git

2016-04-11 15:12

administrator   ~0052571

Branch CR26747 has been created by nbv.

SHA-1: 6fbe89bf607bd18fa96f6e74e8fac94fffbc9c6c


Detailed log of new commits:

Author: nbv
Date: Mon Apr 11 11:12:45 2016 +0300

    0026747: Some constructors of gp_Parab2d classes have not understandable interface and create wrong parabola
    
    1. Useless constructors have been deleted.
    2. New constructor gp_Parab2d(const gp_Ax2d& theDirectrix, const gp_Pnt2d& theFocus, const Standard_Boolean theSense) has been added.
    3. Documentation of gp_Parab2d class has been updated.
    
    Creation of test cases for this issue.

nbv

2016-04-11 15:13

developer   ~0052572

Dear Mikhail,

Please review CR26747 branch.

abv

2016-04-12 06:20

manager   ~0052594

The commit comment is apparently incorrect and incomplete:
- constructor gp_Parab2d(const gp_Ax2d& D, const gp_Pnt2d& F, const Standard_Boolean Sense) was existing before the fix
- method Directrix() is changed and will return different result

git

2016-04-12 10:07

administrator   ~0052595

Branch CR26747 has been updated forcibly by nbv.

SHA-1: a46b2ced68da54294f41e9d6ad19fae125a4c2c5

nbv

2016-04-12 10:08

developer   ~0052596

Commit message has been corrected according to Andrey's remarks.

git

2016-04-26 10:09

administrator   ~0053571

Branch CR26747 has been updated forcibly by nbv.

SHA-1: 8f6e50c9506b978263d3a3e233bd4951cf2a81f9

nbv

2016-04-26 10:10

developer   ~0053572

CR26747 has been rebased on the current MASTER

msv

2016-04-28 15:26

developer   ~0053683

1) Replace the word 'triple' with 'coordinate system' in doxygen comments.
2) Do not use Standard_EXPORT declaration for inline methods.

git

2016-04-28 15:51

administrator   ~0053684

Branch CR26747_1 has been created by nbv.

SHA-1: b5743bddadec5cbc5f742ca85ea15214f8c15f75


Detailed log of new commits:

Author: nbv
Date: Mon Apr 11 11:12:45 2016 +0300

    0026747: Some constructors of gp_Parab2d classes have not understandable interface and create wrong parabola
    
    1. Useless constructors have been deleted.
    2. Value returned by gp_Parab2d::Directrix() method has been corrected according to common sense.
    3. Documentation of gp_Parab2d class has been updated (in hxx-files).
    
    Creation of test cases for this issue.

nbv

2016-04-28 15:52

developer   ~0053685

Dear Mikhail,

Please review CR26747_1 branch.

msv

2016-04-28 16:47

developer   ~0053693

Please avoid code duplication as much as possible. For that, make only one draw function OCC26747 that will use branches inside according to the command name, and reuse the common code.

git

2016-05-04 16:20

administrator   ~0053813

Branch CR26747_2 has been created by nbv.

SHA-1: cc78b97e892f8ab067c315a1ac3c42f6d804a10f


Detailed log of new commits:

Author: nbv
Date: Mon Apr 11 11:12:45 2016 +0300

    0026747: Some constructors of gp_Parab2d classes have not understandable interface and create wrong parabola
    
    1. Useless constructors have been deleted.
    2. Value returned by gp_Parab2d::Directrix() method has been corrected according to common sense.
    3. Documentation of gp_Parab2d class has been updated (in hxx-files).
    
    Creation of test cases for this issue.

nbv

2016-05-04 16:21

developer   ~0053814

Dear Mikhail,

Please review CR26747_2 branch.

msv

2016-05-25 16:18

developer   ~0054402

- Please name global variables in namespace Parab2d_Bug26747 starting with capital letter. The prefix 'a' should be used for local variables only.

git

2016-05-25 16:52

administrator   ~0054406

Branch CR26747_2 has been updated forcibly by nbv.

SHA-1: b36626432a45c2f87993668998a97305dd23a8c8

nbv

2016-05-25 16:55

developer   ~0054407

Dear Mikhail,

Please review the current state of CR26747_2 branch. This branch has already been rebased on the current MASTER.

msv

2016-05-25 19:44

developer   ~0054417

Reviewed.

mkv

2016-05-26 15:32

tester   ~0054450

Dear nbv,
could you please rebase branch CR26747_2 on IR-2016-05-26 branch, there are conflict files.

git

2016-05-26 16:13

administrator   ~0054453

Branch CR26747_3 has been created by nbv.

SHA-1: f3cc5da50de216e8206d0ef242894e4aee96aeba


Detailed log of new commits:

Author: nbv
Date: Mon Apr 11 11:12:45 2016 +0300

    0026747: Some constructors of gp_Parab2d classes have not understandable interface and create wrong parabola
    
    1. Useless constructors have been deleted.
    2. Value returned by gp_Parab2d::Directrix() method has been corrected according to common sense.
    3. Documentation of gp_Parab2d class has been updated (in hxx-files).
    
    Creation of test cases for this issue.

nbv

2016-05-26 16:14

developer   ~0054454

Done.

Please test CR26747_3 branch.

mkv

2016-05-27 13:12

tester   ~0054490

Dear BugMaster,
Branch CR26747_3 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
SHA-1: f3cc5da50de216e8206d0ef242894e4aee96aeba

Number of compiler warnings:

occt component :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)

products component :
Linux: 72 (72 on master)
Windows: 0 (4 on master)
MacOS : 1158

Regressions/Differences/Improvements:
No regressions/differences

Testing cases:
http://occt-tests/CR26747-3-master-OCCT/Debian70-64/bugs/modalg_6/bug26747_1.html
http://occt-tests/CR26747-3-master-OCCT/Windows-64-VC10/bugs/modalg_6/bug26747_1.html
bugs modalg_6 bug26747_1: OK
http://occt-tests/CR26747-3-master-OCCT/Debian70-64/bugs/modalg_6/bug26747_2.html
http://occt-tests/CR26747-3-master-OCCT/Windows-64-VC10/bugs/modalg_6/bug26747_2.html
bugs modalg_6 bug26747_2: OK
http://occt-tests/CR26747-3-master-OCCT/Debian70-64/bugs/modalg_6/bug26747_3.html
http://occt-tests/CR26747-3-master-OCCT/Windows-64-VC10/bugs/modalg_6/bug26747_3.html
bugs modalg_6 bug26747_3: OK

Testing on Linux:
occt component :
Total MEMORY difference: 88370541 / 88307858 [+0.07%]
Total CPU difference: 18610.110000000037 / 18553.190000000006 [+0.31%]
products component :
Total MEMORY difference: 27267555 / 27256815 [+0.04%]
Total CPU difference: 5203.3899999999785 / 5188.639999999986 [+0.28%]

Testing on Windows:
occt component :
Total MEMORY difference: 55934537 / 55926481 [+0.01%]
Total CPU difference: 18088.565551598855 / 18435.979778598783 [-1.88%]
products component :
Total MEMORY difference: 18919594 / 18883714 [+0.19%]
Total CPU difference: 5006.274891299942 / 5050.469974599949 [-0.88%]

There are no differences in images found by testdiff.

mkv

2016-05-27 13:12

tester   ~0054491

Dear BugMaster,
Branch CR26747_3 is TESTED.

abv

2016-06-02 14:22

manager   ~0054630

Please correct issue summary and commit message to provide meaningful description of the problem and the changes made. Avoid vague wording like "not understandable", "common sense" etc. If behavior of the API class changes (I suppose that gp_Parab2d being a data structure, is an API class), please describe that change in Upgrade Guide (dox/upgrade/upgrade.md) so that projects which use that API can recognize the change, and adapt their code accordingly.

nbv

2016-06-02 16:54

developer  

parab1.png (10,421 bytes)

nbv

2016-06-02 16:54

developer  

parab2.png (10,633 bytes)

nbv

2016-06-02 16:55

developer  

parab3.png (10,685 bytes)

git

2016-06-02 17:31

administrator   ~0054646

Branch CR26747_4 has been created by nbv.

SHA-1: b4bf2a84a572be3b3ab240071a470b9739a994ae


Detailed log of new commits:

Author: nbv
Date: Mon Apr 11 11:12:45 2016 +0300

    0026747: Some constructors of gp_Parab2d class contain redundant parameters
    
    1. Useless constructors have been deleted.
    2. Value returned by gp_Parab2d::Directrix() method has been corrected (there is no point in reversing directrix).
    3. Documentation of gp_Parab2d class has been updated (in hxx-file).
    4. Release Notes have been updated according to corrections made in this issue.
    
    Creation of test cases for this issue.

nbv

2016-06-02 17:35

developer   ~0054647

Dear Andrei,

Please review the current state of CR26747_4 branch.

git

2016-06-02 17:59

administrator   ~0054651

Branch CR26747_5 has been created by nbv.

SHA-1: c1c48e19f7850eb33dc621e5bcb0ed685348e561


Detailed log of new commits:

Author: nbv
Date: Mon Apr 11 11:12:45 2016 +0300

    0026747: Some constructors of gp_Parab2d class contain redundant parameters
    
    1. Useless constructors have been deleted.
    2. Value returned by gp_Parab2d::Directrix() method has been corrected to exclude reversing the directrix.
    3. Documentation of gp_Parab2d class has been updated (in hxx-file).
    4. Upgrade Guide has been updated according to corrections made in this issue.
    
    Creation of test cases for this issue.

abv

2016-06-02 18:09

manager   ~0054652

Branch CR26747_5 is reviewed, please consider as tested (there are no differences in code from CR26747_3)

nbv

2016-06-03 09:21

developer   ~0054657

Description has been updated.

git

2016-06-03 13:41

administrator   ~0054682

Branch CR26747_5 has been updated forcibly by mkv.

SHA-1: 0511704ede514b58b26593edc1d55db543f6d1b9

mkv

2016-06-06 18:57

tester   ~0054741

Dear BugMaster,
Branch CR26747_5 was rebased on current master of occt git-repository.
SHA-1: 0511704ede514b58b26593edc1d55db543f6d1b9

mkv

2016-06-06 18:57

tester   ~0054742

Dear BugMaster,
Branch CR26747_5 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
SHA-1: 0511704ede514b58b26593edc1d55db543f6d1b9

Number of compiler warnings:

occt component :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)

products component :
Linux: 72 (72 on master)
Windows: 4 (4 on master)
MacOS : 1148

mkv

2016-06-06 18:57

tester   ~0054743

Dear BugMaster,
Branch CR26747_5 is TESTED.

git

2016-06-17 12:14

administrator   ~0055151

Branch CR26747_5 has been deleted by inv.

SHA-1: 0511704ede514b58b26593edc1d55db543f6d1b9

git

2016-06-17 12:14

administrator   ~0055153

Branch CR26747_4 has been deleted by inv.

SHA-1: b4bf2a84a572be3b3ab240071a470b9739a994ae

git

2016-06-17 12:14

administrator   ~0055155

Branch CR26747_3 has been deleted by inv.

SHA-1: f3cc5da50de216e8206d0ef242894e4aee96aeba

git

2016-06-17 12:14

administrator   ~0055156

Branch CR26747_2 has been deleted by inv.

SHA-1: b36626432a45c2f87993668998a97305dd23a8c8

git

2016-06-17 12:14

administrator   ~0055158

Branch CR26747_1 has been deleted by inv.

SHA-1: b5743bddadec5cbc5f742ca85ea15214f8c15f75

git

2016-06-17 12:14

administrator   ~0055159

Branch CR26747 has been deleted by inv.

SHA-1: 8f6e50c9506b978263d3a3e233bd4951cf2a81f9

Related Changesets

occt: master c1609fbe

2016-04-11 08:12:45

nbv


Committer: bugmaster Details Diff		 0026747: Some constructors of gp_Parab2d class contain redundant parameters

1. Useless constructors have been deleted.
2. Value returned by gp_Parab2d::Directrix() method has been corrected to exclude reversing the directrix.
3. Documentation of gp_Parab2d class has been updated (in hxx-file).
4. Upgrade Guide has been updated according to corrections made in this issue.

Creation of test cases for this issue.		 Affected Issues
0026747
mod - dox/dev_guides/upgrade/upgrade.md Diff File
mod - src/gce/gce_MakeParab2d.cxx Diff File
mod - src/gce/gce_MakeParab2d.hxx Diff File
mod - src/GCE2d/GCE2d_MakeParabola.cxx Diff File
mod - src/GCE2d/GCE2d_MakeParabola.hxx Diff File
mod - src/gp/gp_Parab2d.cxx Diff File
mod - src/gp/gp_Parab2d.hxx Diff File
mod - src/gp/gp_Parab2d.lxx Diff File
mod - src/QABugs/QABugs_20.cxx Diff File
add - tests/bugs/modalg_6/bug26747_1 Diff File
add - tests/bugs/modalg_6/bug26747_2 Diff File
add - tests/bugs/modalg_6/bug26747_3 Diff File

Issue History

Date Modified Username Field Change
2015-10-02 17:03 nbv New Issue
2015-10-02 17:03 nbv Assigned To => msv
2015-10-02 17:03 nbv File Added: parab1.png
2015-10-02 17:03 nbv File Added: parab2.png
2015-10-02 17:11 nbv Note Added: 0046413
2015-10-02 17:11 nbv Status new => feedback
2015-10-02 17:43 nbv Relationship added related to 0025605
2015-10-07 15:39 msv Note Added: 0046567
2015-10-07 15:39 msv Assigned To msv => ifv
2015-10-12 10:07 ifv Assigned To ifv => msv
2015-10-12 10:10 ifv Note Added: 0046665
2015-10-12 10:22 msv Note Added: 0046667
2015-10-12 10:22 msv Assigned To msv => nbv
2015-10-12 10:22 msv Status feedback => assigned
2015-10-28 10:50 msv Target Version 7.0.0 => 7.1.0
2016-04-11 15:12 git Note Added: 0052571
2016-04-11 15:13 nbv Note Added: 0052572
2016-04-11 15:13 nbv Assigned To nbv => msv
2016-04-11 15:13 nbv Status assigned => resolved
2016-04-11 15:13 nbv Steps to Reproduce Updated
2016-04-12 06:20 abv Note Added: 0052594
2016-04-12 10:07 git Note Added: 0052595
2016-04-12 10:08 nbv Note Added: 0052596
2016-04-26 10:09 git Note Added: 0053571
2016-04-26 10:10 nbv Note Added: 0053572
2016-04-28 15:26 msv Note Added: 0053683
2016-04-28 15:26 msv Assigned To msv => nbv
2016-04-28 15:26 msv Status resolved => assigned
2016-04-28 15:51 git Note Added: 0053684
2016-04-28 15:52 nbv Note Added: 0053685
2016-04-28 15:52 nbv Assigned To nbv => msv
2016-04-28 15:52 nbv Status assigned => resolved
2016-04-28 16:47 msv Note Added: 0053693
2016-04-28 16:47 msv Assigned To msv => nbv
2016-04-28 16:47 msv Status resolved => assigned
2016-04-28 16:47 msv Priority normal => low
2016-05-04 16:20 git Note Added: 0053813
2016-05-04 16:21 nbv Note Added: 0053814
2016-05-04 16:21 nbv Assigned To nbv => msv
2016-05-04 16:21 nbv Status assigned => resolved
2016-05-25 16:18 msv Note Added: 0054402
2016-05-25 16:18 msv Assigned To msv => nbv
2016-05-25 16:18 msv Status resolved => assigned
2016-05-25 16:52 git Note Added: 0054406
2016-05-25 16:55 nbv Note Added: 0054407
2016-05-25 16:55 nbv Assigned To nbv => msv
2016-05-25 16:55 nbv Status assigned => resolved
2016-05-25 19:44 msv Note Added: 0054417
2016-05-25 19:44 msv Assigned To msv => bugmaster
2016-05-25 19:44 msv Status resolved => reviewed
2016-05-26 15:31 mkv Assigned To bugmaster => mkv
2016-05-26 15:32 mkv Note Added: 0054450
2016-05-26 15:32 mkv Assigned To mkv => nbv
2016-05-26 15:32 mkv Status reviewed => feedback
2016-05-26 16:13 git Note Added: 0054453
2016-05-26 16:14 nbv Assigned To nbv => mkv
2016-05-26 16:14 nbv Note Added: 0054454
2016-05-26 16:28 mkv Status feedback => reviewed
2016-05-27 13:12 mkv Note Added: 0054490
2016-05-27 13:12 mkv Note Added: 0054491
2016-05-27 13:12 mkv Assigned To mkv => bugmaster
2016-05-27 13:12 mkv Status reviewed => tested
2016-05-27 13:13 mkv Test case number => bugs modalg_6 bug26747_1, bug26747_2, bug26747_3
2016-06-02 14:22 abv Note Added: 0054630
2016-06-02 14:22 abv Assigned To bugmaster => nbv
2016-06-02 14:22 abv Status tested => assigned
2016-06-02 16:41 nbv Summary Some constructors of gp_Parab2d classes have not understandable interface and create wrong parabola => Some constructors of gp_Parab2d class contain redundant parameters
2016-06-02 16:41 nbv Description Updated
2016-06-02 16:41 nbv Steps to Reproduce Updated
2016-06-02 16:54 nbv File Deleted: parab1.png
2016-06-02 16:54 nbv File Deleted: parab2.png
2016-06-02 16:54 nbv File Added: parab1.png
2016-06-02 16:54 nbv File Added: parab2.png
2016-06-02 16:55 nbv File Added: parab3.png
2016-06-02 17:31 nbv Description Updated
2016-06-02 17:31 git Note Added: 0054646
2016-06-02 17:33 nbv Additional Information Updated
2016-06-02 17:35 nbv Note Added: 0054647
2016-06-02 17:35 nbv Assigned To nbv => abv
2016-06-02 17:35 nbv Status assigned => resolved
2016-06-02 17:59 git Note Added: 0054651
2016-06-02 18:09 abv Note Added: 0054652
2016-06-02 18:09 abv Assigned To abv => bugmaster
2016-06-02 18:09 abv Status resolved => reviewed
2016-06-03 09:21 nbv Note Added: 0054657
2016-06-03 09:21 nbv Description Updated
2016-06-03 13:21 mkv Assigned To bugmaster => mkv
2016-06-03 13:41 git Note Added: 0054682
2016-06-06 18:57 mkv Note Added: 0054741
2016-06-06 18:57 mkv Note Added: 0054742
2016-06-06 18:57 mkv Note Added: 0054743
2016-06-06 18:57 mkv Assigned To mkv => bugmaster
2016-06-06 18:57 mkv Status reviewed => tested
2016-06-10 13:12 bugmaster Changeset attached => occt master c1609fbe
2016-06-10 13:12 bugmaster Status tested => verified
2016-06-10 13:12 bugmaster Resolution open => fixed
2016-06-17 12:14 git Note Added: 0055151
2016-06-17 12:14 git Note Added: 0055153
2016-06-17 12:14 git Note Added: 0055155
2016-06-17 12:14 git Note Added: 0055156
2016-06-17 12:14 git Note Added: 0055158
2016-06-17 12:14 git Note Added: 0055159
2016-12-09 16:31 aiv Status verified => closed
2016-12-09 16:37 aiv Fixed in Version => 7.1.0