MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0027903Community[OCCT] OCCT:Modeling Datapublic2016-09-26 17:022017-12-05 17:09
Reporterdrazmyslovich 
Assigned Tomsv 
PrioritynormalSeverityintegration request 
StatusassignedResolutionopen 
PlatformWindowsOSVC++ 2010OS Version64 bit
Product Version[OCCT] 7.0.0 
Target Version[OCCT] 7.3.0*Fixed in Version 
Summary0027903: Patch AdvApp2Var_ApproxAFunc2Var to handle the non-uniform bspline segmentation conditions
DescriptionThe current implementation of AdvApp2Var_ApproxAFunc2Var allows only to specify the total amount of patches necessary. Still, it makes sense to have an opportunity to influence the segmentation in U, V directions separately. The committed patch represents the possible modification of the class to handle the desired behavior.
Steps To Reproducetest bugs modalg_6 bug27903
TagsNo tags attached.
Test case number
Attached Files

- Relationships
has duplicate 0027904closedbugmaster Patch GeomPlate_MakeApprox to handle the non-uniform approximation parameters: degree, continuity, number of patches 

-  Notes
(0058167)
git (administrator)
2016-09-26 17:05

Branch CR27903 has been created by drazmyslovich.

SHA-1: f5b581b856089dfdc535e854952de71dbdcefb16


Detailed log of new commits:

Author: razmyslovich
Date: Mon Sep 26 16:04:46 2016 +0200

    0027903: Patch AdvApp2Var_ApproxAFunc2Var to handle the non-uniform bspline segmentation conditions
(0058168)
drazmyslovich (developer)
2016-09-26 17:06

The patch is submitted, please, review.
(0058175)
msv (developer)
2016-09-26 17:27

Remarks:

1. Please provide a test case that allows reproducing the issue. If DRAW misses the needed commands you may implement the overall test case in a new command in the file src/QABugs/QABugs_20.cxx.

2. Please rework your changes in the code so as to avoid reformatting as much as possible. The fix must introduce functional changes only, without making any changes that serve just to "improve readability". For latter kind of changes, a special issue must be added if needed, in order for the commit contained reformatting changes only.
(0062835)
git (administrator)
2017-01-20 14:59

Branch CR27903_1 has been created by drazmyslovich.

SHA-1: 1be4eb79ac470fff9e33c89717e06f73bc115939


Detailed log of new commits:

Author: razmyslovich
Date: Fri Jan 20 12:59:06 2017 +0100

    0027903: Patch AdvApp2Var_ApproxAFunc2Var and GeomPlate_MakeApprox to handle the non-uniform approximation parameters: degree, continuity, number of patches
(0062836)
drazmyslovich (developer)
2017-01-20 15:00

The updated patch is submitted, please, review.
(0063337)
msv (developer)
2017-02-02 10:07

Dear Igor, please review.
(0063393)
ifv (developer)
2017-02-03 14:33

The algorithm works in such a way that creates the right amount of spans in U and V directions to achieve the required approximation accuracy.
A limit on the total number of spans of surface (parameter MaxPatch) serves to prevent the generation of an infinite number of spans and emergency exit from the algorithm if there are any problems with the approximation, but not for the possibility of creating a surface with a given number of spans.
If MaxPatch is set not large enough and the algorithm ends its work because it exceeded the total number of spans, the criteria of approximation are not satisfied.
The adding of separate parameters to limit the number of spans for each parameter of surface is nothing new to add - these parameters should be used by the same way for emergency stop of the program in critical situations, and not to limit the number of spans in normal situations, which leads to bad approximation.
Furthermore, since now all three parameters are simultaneously used in the methods ComputePatches(...) and only one (MaxPatch) in methods ComputeConstraints(...), you must specify them in concert with each other, this complicates the use of the algorithm.
So adding of restrictions on the number of spans for each parameter is not advisable.
It seems likely that changes in the method InitGrid(...) and method
void AdvApp2Var_ApproxAFunc2Var::ComputePatches(const AdvApprox_Cutting& UChoice,
                     const AdvApprox_Cutting& VChoice,
                     const AdvApp2Var_EvaluatorFunc2Var& Func,
                     const AdvApp2Var_Criterion& Crit)
lines 592 – 606:

    if (Regular && decision>0) {
      switch (decision)
      {
      case 1: ++NbU; break;
      case 2: ++NbV; break;
      case 3: ++NbU; ++NbV; break;
      default:
        {
          myHasResult = myDone = Standard_False;
          Standard_ConstructionError::Raise ("AdvApp2Var_ApproxAFunc2Var : Surface Approximation Error");
        }
      }
      InitGrid(NbU, NbV);

can help to create a more optimal (with a smaller total number of spans) surface, but unfortunately the sample test absolutely does not affect this branch of the algorithm.
This part of patch can be integrated, if corresponding test case will be presented
(0063409)
msv (developer)
2017-02-03 15:52

Dear drazmyslovich,
First of all, thank you very much for your contribution.
As ifv wrote, your idea is worth to consider partly, only concerning the method InitGrid, which really will benefit taking two parameters nbU,nbV instead of one nbInt. So, I propose you to reduce your changes to only this scope.
Also, the test case you attached does not allow to reproduce any wrong behavior. Do you have another case that shows improvement of the patch in comparison with master version?
Regards,
Mikhail
(0068253)
drazmyslovich (developer)
2017-07-13 12:34

Dear ifv and msv,
first of all, sorry for the delayed answer to your comments and thank you for reviewing my changes.
The point of the patch is not fixing some wrong behavior of the algorithm, but extending the algorithm possibilities in order to be able to generate the surface approximations with the predefined parameters. This is a prerequisite for the advanced users of our software, which definitely know what kind of surface approximation is necessary. So, for the modified algorithm we don't provide the parameters ourselves, but the user enters the parameters over the interface.
Therefore, I would kindly ask you to reconsider the proposed changes as an extension of the algorithm, which provides a better flexibility in choosing parameters.
Thank you!
Regards,
Dmitry

- Issue History
Date Modified Username Field Change
2016-09-26 17:02 drazmyslovich New Issue
2016-09-26 17:02 drazmyslovich Assigned To => drazmyslovich
2016-09-26 17:05 git Note Added: 0058167
2016-09-26 17:06 drazmyslovich Note Added: 0058168
2016-09-26 17:06 drazmyslovich Assigned To drazmyslovich => msv
2016-09-26 17:06 drazmyslovich Status new => resolved
2016-09-26 17:17 drazmyslovich Relationship added related to 0027904
2016-09-26 17:27 msv Note Added: 0058175
2016-09-26 17:27 msv Assigned To msv => drazmyslovich
2016-09-26 17:27 msv Status resolved => assigned
2016-10-25 15:36 msv Target Version 7.1.0 => 7.2.0
2017-01-20 14:59 git Note Added: 0062835
2017-01-20 15:00 drazmyslovich Note Added: 0062836
2017-01-20 15:00 drazmyslovich Assigned To drazmyslovich => msv
2017-01-20 15:00 drazmyslovich Status assigned => resolved
2017-01-20 15:00 drazmyslovich Steps to Reproduce Updated View Revisions
2017-02-01 18:33 msv Relationship replaced has duplicate 0027904
2017-02-02 10:07 msv Note Added: 0063337
2017-02-02 10:07 msv Assigned To msv => ifv
2017-02-03 14:33 ifv Note Added: 0063393
2017-02-03 14:33 ifv Assigned To ifv => msv
2017-02-03 14:33 ifv Status resolved => reviewed
2017-02-03 15:36 mkv Assigned To msv => mkv
2017-02-03 15:46 msv Assigned To mkv => msv
2017-02-03 15:46 msv Status reviewed => assigned
2017-02-03 15:52 msv Note Added: 0063409
2017-02-03 15:52 msv Assigned To msv => drazmyslovich
2017-07-13 12:34 drazmyslovich Note Added: 0068253
2017-07-13 12:35 drazmyslovich Assigned To drazmyslovich => msv
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