Use curve to digitize Polylines/Polygons#5842
Use curve to digitize Polylines/Polygons#5842qsavoye wants to merge 8 commits intoopengisch:masterfrom
Conversation
| height: visible ? 40 : 0 | ||
| padding: 2 | ||
| round: true | ||
| visible: dashBoard.activeLayer && (dashBoard.activeLayer.geometryType() === Qgis.GeometryType.Polygon || dashBoard.activeLayer.geometryType() === Qgis.GeometryType.Line) |
There was a problem hiding this comment.
I think we want this to be dependent on QgsWkbTypes.isCurvedType()
There was a problem hiding this comment.
In my example , my layer is not a curve type at all and i use QgsCompoundCurve::curveToLine function to set geometry drawn into my none curved layer. This helps me to drawn faster instead of set vertex by vertex.
But you are right i am not taking into account the possibility to set directly the qgscompoundcurve geometry into the new feature.
How do you think ?
There was a problem hiding this comment.
Interesting, I was mostly seeing this in the context of surveying / cadastre, where curves are often a requirement. My main motivation here would be to keep the interface as light as possible and ask users to have a curve layer if they want this feature. What do you think?
There was a problem hiding this comment.
I am using it to draw underground maps, such as caves or old quarries. My polygon layer corresponds to some galleries and i no need to have a specific curved layer but some times digitizing arcs like in qgis help me.
Maybe we can allow it only for curved layer (QgsWkbTypes.isCurvedType()) or for advanced digitizing user ( which could be set in settings).
Tell me what you think ?
Sorry for the delay, i had some stuff to do last week.
There was a problem hiding this comment.
Interesting job you have!
I'd be interested to hear @nirvn's point of view
There was a problem hiding this comment.
@m-kuhn , @qsavoye , hey there, sorry for the delay in chipping in.
I do think the feature is useful; I would indeed prefer if this option was only visible for curved type layers, I feel that'd be a good way to enable this while not cluttering UI by limiting to an explicit intent by the user / project manager.
| confirm(); | ||
| } else { | ||
| addVertex(); | ||
| if (settings.valueBool("/QField/Digitizing/CurveEdition", false) == true) { |
There was a problem hiding this comment.
I think we need an additional check here to avoid curved edition when people are freehand drawing, otherwise worlds will collide.
|
@9ls1 I noted you added a 👍 to this, do you have a specific use case in mind for which this would be interesting? |
|
@m-kuhn No, just in general thinking about saving time digitizing shapes like the one shown here and curved roads. |
|
@qsavoye , you have not been forgotten :) I'll review this in the coming day. Thanks for your patience. |
|
|
||
| private: | ||
| QVector<QgsPoint> mPointList; | ||
| QgsCompoundCurve mCompoundCurve; |
There was a problem hiding this comment.
I'm a little bit worried of the extra cost of moving from a simple QVector to a QgsAbstractGeometry-based curve here. I am wondering whether we should keep both the fast point list vector (used by our tracking system among other things), and have the mCompoundCurve as an alternative path for curved geometries.
| QgsCoordinateTransform ct( mCrs, crs, QgsProject::instance()->transformContext() ); | ||
|
|
||
| for ( const QgsPoint &pt : mPointList ) | ||
| for ( const QgsPoint &pt : vertices() ) |
There was a problem hiding this comment.
Let's do that instead.
| for ( const QgsPoint &pt : vertices() ) | |
| const QVector<QgsPoint> vertices = vertices(); | |
| for ( const QgsPoint &pt : vertices ) |
| QgsCoordinateTransform ct( mCrs, crs, QgsProject::instance()->transformContext() ); | ||
|
|
||
| for ( const QgsPoint &pt : mPointList ) | ||
| for ( const QgsPoint &pt : vertices() ) |
| geometry = deduplicatedGeometry; | ||
| } | ||
|
|
||
| if ( QgsWkbTypes::isCurvedType( layer()->wkbType() ) == true ) |
There was a problem hiding this comment.
You'll need more than this to preserve the curves here. You'll need to edit the Geometry::asQgsGeometry (
Line 27 in f0e0aeb
| return FeatureIterator( layer, request ); | ||
| } | ||
|
|
||
| bool LayerUtils::isCurvedGeometry( QgsVectorLayer *layer ) |
There was a problem hiding this comment.
Note to self: we should make the QgswkbTypes' isCurvedType Q_INVOKABLE upstream, that way we don't need an extra utils function.
That being said, I think I'd rather have that in the geometryutils, and on the QML side of things you can call a vector layer's wkbType() function (it's invokable).
There was a problem hiding this comment.
@nirvn
Do you prefer : make a patch when compiling qgis to modify QgsWkbTypes.h and erase what I did on LayerUtils or move the method into GeometryUtils ?
There was a problem hiding this comment.
@qsavoye , I'd love for you to submit a patch in QGIS upstream (we're 5 days away from 3.44 release), then we can update QField to 3.44 when released and we'll be able to make sure of that newly-made invokable function.
There was a problem hiding this comment.
@qsavoye , I've taken care of upstream here: qgis/QGIS#62306
|
|
||
| if ( QgsWkbTypes::isCurvedType( geom.wkbType() ) == true ) | ||
| { | ||
| geom.convertToStraightSegment(); |
There was a problem hiding this comment.
I'm worried about this change, did you give it a try? Was that giving you usable scenarios?
|
@qsavoye , are you planning to work on this further? |
|
@qsavoye , no problem, take your time. You're contributions are appreciated, just glad to hear you're planning to come back to it :) |
|
@qsavoye , if you don't mind, I will close this pull request for now, it'll need some work rebasing against latest master, etc. I do hope you will find some time to get back to coding this feature, your contributions are most welcome! |
Use curve to digitize Polylines/Polygons
Precision of the final arc can be set in QField Parameters page (slider)