View Issue Details

IDProjectCategoryView StatusLast Update
0027903CommunityOCCT:Modeling Datapublic2023-01-21 19:32
Reporterdrazmyslovich Assigned Tobugmaster  
PrioritynormalSeverityintegration request 
Status closedResolutionwon't fix 
PlatformWindowsOSVC++ 2010 
Product Version7.0.0 
Target Version7.7.0 
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

Relationships

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

Activities

git

2016-09-26 17:05

administrator   ~0058167

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

drazmyslovich

2016-09-26 17:06

developer   ~0058168

The patch is submitted, please, review.

msv

2016-09-26 17:27

developer   ~0058175

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.

git

2017-01-20 14:59

administrator   ~0062835

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

drazmyslovich

2017-01-20 15:00

developer   ~0062836

The updated patch is submitted, please, review.

msv

2017-02-02 10:07

developer   ~0063337

Dear Igor, please review.

ifv

2017-02-03 14:33

developer   ~0063393

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

msv

2017-02-03 15:52

developer   ~0063409

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

drazmyslovich

2017-07-13 12:34

developer   ~0068253

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

drazmyslovich

2021-11-28 20:51

developer   ~0105497

The issue is obsolete for our usage case, therefore you can actually close the issue if you prefer.

msv

2021-11-28 23:23

developer   ~0105499

Dear Igor, what is your opinion, is this patch interesting for us to include into master?

szy

2022-01-13 18:01

manager   ~0106349

The customer agreed to closed the issue andtThe proposed fix is qualified as useless for OCCT.

git

2023-01-21 19:27

administrator   ~0112932

Branch CR27903 has been deleted by vglukhik.

SHA-1: f5b581b856089dfdc535e854952de71dbdcefb16

git

2023-01-21 19:32

administrator   ~0112933

Branch CR27903_1 has been deleted by vglukhik.

SHA-1: 1be4eb79ac470fff9e33c89717e06f73bc115939

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
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.3.0
2017-12-05 17:09 msv Target Version 7.3.0 => 7.4.0
2019-08-12 16:36 msv Target Version 7.4.0 => 7.5.0
2020-09-14 22:58 msv Target Version 7.5.0 => 7.6.0
2021-08-29 18:53 msv Target Version 7.6.0 => 7.7.0
2021-11-28 20:51 drazmyslovich Note Added: 0105497
2021-11-28 20:51 drazmyslovich Status assigned => resolved
2021-11-28 23:23 msv Note Added: 0105499
2021-11-28 23:23 msv Assigned To msv => ifv
2021-11-28 23:23 msv Status resolved => feedback
2022-01-13 18:01 szy Assigned To ifv => bugmaster
2022-01-13 18:01 szy Status feedback => closed
2022-01-13 18:01 szy Resolution open => won't fix
2022-01-13 18:01 szy Note Added: 0106349
2023-01-21 19:27 git Note Added: 0112932
2023-01-21 19:32 git Note Added: 0112933