-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add functionality for reparameterizing B-splines without changing geometry (Fix issue #890) #1007
Conversation
@svengoldberg Great effort! I saw that you chose a piecewise linear approach. Doesn't it introduce c1-discontinuities (i.e. jumps in the curve's gradient) at the edges of each curve segment? If so, this could be a problem for the lofting algorithm as it can introduce g1-discontinuities during lofting (i.e. undesired kinks). We had the same idea back then with Merlin (who implemented the Gordon Surface Algorithm), where we first decided to implement a piecewise linear reparametrization and experienced these problems (kinks) in some of the gordon surface test cases. As a consequence, we chose the continuous approximative reparametrization, which keeps the degree and a smooth reparametrization curve - however with the overshooting problems you discovered. |
I think for the case to introduce kinks into the surface, your method is correct. But I recommend also looking at cases where the curve is reparametrized but without introducing any kinks. |
Good point! Let's check what happens if we have two curves without kinks, reparametrize at least one of them piecewisely linear and create a loft. @rainman110 I didn't know you tried this with Merlin back then, good to know and thanks for the heads up! Anyway, with @svengoldberg's changes we now have two algorithms, the approximate one from before and the piecewise linear one, both with their pros and cons |
Yes, I think it is a great addition. Ideally, the algorithm is chosen "dynamically" depending on what is needed. |
@joergbrech already had an idea to restructure the additions and I implemented it locally. I will push the changes later (after some more testing) |
@rainman110 I took the same CPACS file as at the top just left out all kinks in the profile definition. It looks like this should not be a problem: |
Have a look with the zebra plot and a high tesselation. These problems are not easy to catch visually, but can be indeed an aerodynamic problem. The zebra stripes need to pass all "edges" in a smooth way |
@rainman110 I did some testing and changed the parameters in a way that I took different number of parameters on two lofted profiles and raised the "veloctity" of the curve between two points. I could not reproduce this problem... To me, it looked like, the zebra stripes do pass in a smooth way. Do you maybe still have an idea about a configuration that you tested which created this problem of missing C1-continuity? |
Have a look at the gordon surface case Here, profiles should be reparametrized, such that all intersections are at the same curve parameter. Then create a skinned surface. Ideally, the skinned surface is
Alternatively, replace your new reparametrization algorithm with the one used by the gordon surface method:
The resulting surface needs to interpolate the input curves. Back then, they did not exactly (it was not easy too see though). This should give as a good feeling, for which algorithm your new reparametrization function can be used (and for what not). |
When there are parameters defined in the CPACS profile, the reparameterization in CTiglInterpolatePointsWithKinks() does not get along with the following reparametrizeBSplineNiceKnots() which was implemented for performance reasons. In this case, the nice knots change the geometry in a too drastic way (cf. unittest BSplineInterpolation.reparameterizePiecewiseLinear). To still account for these performance advantages in most cases, the function is reactivated. But only if there are no parameters defined in the CPACS profile.
@rainman110 We investigated the application of the new algorithm within the Gordon surface method. However, the guide curve's poles of the resulting object looked not very nice (not equidistant, stretched/clinched in 'guide curve direction') and we could not really discuss the continuity property. In addtion to that, 3 other Gordon surface tests did not pass when both (profiles and guides) are reparameterized using the new algorithm. Nevertheless, the test you mentioned passes. When only the profiles are reparameterized this way, everything works fine and all tests pass. |
@svengoldberg Thanks for the update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @svengoldberg! I have a few comments, mostly cosmetics.
Tolerance was only implemented in knot removal before, but now for compatability also added for knot insertion. There existed CPACS files that crashed without this tolerance for adding knots in the first step of the reparameterization algorithm
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1007 +/- ##
==========================================
+ Coverage 69.81% 69.91% +0.09%
==========================================
Files 299 299
Lines 24166 24244 +78
==========================================
+ Hits 16871 16949 +78
Misses 7295 7295
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥳 Thanks! Everything looks good now.
Description
This PR implements a method to apply exact reparameterizations of B-splines used for profile curves. The underlying algorithm was found in The NURBS Book (1997; 2nd edtion; Piegl and Tiller), p. 251. The reparameterization is mandatory for lofting (correct connection of kinks in two profiles, since they are connected based on parametric values of the curves).
The current implemented reparameterization creates overshoots of the profile curves (cf. issue #890) due to change of the geometry. The approach in this PR does not change the geometric structure but still executes a reparameterization.
The wanted parameters are interpolated picewise linearly (to mimic a B-spline structure of the reparameterization function) and to not increase the degree of the resulting B-spline curve. After that, the above mentiond algorithm is applied based on this reparameterization function.
To come into account, a new function and some helper functions are implemented based on OpenCASCADE B-spline handling.
Changes in src/fuselage/CCPACSFuselageProfile.cpp :
Apply the reparameterization on a B-spline. Similar to the current behaviour.
Changes in src/geometry/CTiglBSplineAlgorithms.* :
Implement the new reparameterization (
bsplineReparameterizePicewiseLinear
) with the use of a few helper functions.Changes in src/geometry/CTiglInterpolatePointsWithKinks.* :
The function
computeParams
is now needed also in CCPACSFuselageProfile.cpp and is therefore put into a non-anonymous namespace.Fixes issue #890
EDITS:
reparametrizeBSplineNiceKnots()
seems not to work as the geometry is changed in a too drastic way when the parameters are predefined. However, since the performance advantage should not be rejected in the most cases, the function is only used when there are no parameters defined in the CPACS profile.How Has This Been Tested?
The test
bsplineReparameterizePicewiseLinear
is added to check if the reparameterization is applied correct and if the geometric structure is kept. Also, in the attached screenshots, the behaviour of the old vs. new implementation is compared:Overshoot after reparameterization
No overshoot after reparameterization
Checklist: