Skip to content

Polyline smoothing#262

Merged
oitel merged 15 commits intomasterfrom
polyline_relax
Jul 12, 2022
Merged

Polyline smoothing#262
oitel merged 15 commits intomasterfrom
polyline_relax

Conversation

@oitel
Copy link
Contributor

@oitel oitel commented Jul 12, 2022

No description provided.

@oitel oitel requested a review from Grantim July 12, 2022 12:48
Comment on lines +11 to +13
struct PolylineRelaxParams : RelaxParams
{
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need in empty struct

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment on lines +41 to +43
Vector3d sum;
sum += Vector3d( polyline.points[polyline.topology.dest( e0 )] );
sum += Vector3d( polyline.points[polyline.topology.dest( e1 )] );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

polyline.destPnt( e0 );
polyline.destPnt( e1 );

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment on lines +36 to +39
auto e0 = polyline.topology.edgeWithOrg( v );
auto e1 = polyline.topology.next( e0 );
if ( !e0.valid() || !e1.valid() )
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edges are always valid, better check

!polyline.topology.isLoneEdge( e0 )

e1 is always OK if e0 is OK. For boundary edges e1 == e0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment on lines +60 to +63
using VectorD = typename SubstType<double, V>::type;
VectorD sum;
sum += VectorD( polyline.destPnt( e0 ) );
sum += VectorD( polyline.destPnt( e1 ) );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can use V here, no need to convert to double

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment on lines +11 to +20
/// applies given number of relaxation iterations to the whole polyline ( or some region if it is specified )
/// \return true if was finished successfully, false if was interrupted by progress callback
MRMESH_API bool relax( Polyline2& polyline, const RelaxParams& params = {}, ProgressCallback cb = {} );
MRMESH_API bool relax( Polyline3& polyline, const RelaxParams& params = {}, ProgressCallback cb = {} );

/// applies given number of relaxation iterations to the whole polyline ( or some region if it is specified )
/// do not really keeps area but tries hard
/// \return true if was finished successfully, false if was interrupted by progress callback
MRMESH_API bool relaxKeepArea( Polyline2& polyline, const RelaxParams& params = {}, ProgressCallback cb = {} );
MRMESH_API bool relaxKeepArea( Polyline3& polyline, const RelaxParams& params = {}, ProgressCallback cb = {} );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mb better

template<typename V>
MRMESH_API bool relax( Polyline<V>& polyline, const RelaxParams& params = {}, ProgressCallback cb = {} );

template<typename V>
MRMESH_API bool relaxKeepArea( Polyline<V>& polyline, const RelaxParams& params = {}, ProgressCallback cb = {} );

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@oitel oitel requested a review from Fedr July 12, 2022 14:25
@@ -0,0 +1,24 @@
#pragma once
#include "MRMeshFwd.h"
#include "MRMeshRelax.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why include MeshRelax in PolylineRelax?

@@ -0,0 +1,130 @@
#include "MRBitSetParallelFor.h"
#include "MRPolyline.h"
#include "MRPolylineRelax.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better put it on top of all to make sure that it is self-sufficient

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@oitel oitel merged commit 5b311af into master Jul 12, 2022
@oitel oitel deleted the polyline_relax branch July 12, 2022 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants