Skip to content

Allow adding a new part to multipart geometries #4496

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.

Sign up for GitHub

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

Open
uclaros wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow adding a new part to multipart geometries #4496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions app/maptools/recordingmaptool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,36 @@ void RecordingMapTool::cancelGrab()
setActiveVertex( Vertex() );
}

void RecordingMapTool::startDigitizingNewPart()
{
// if maptool is in GRAB and VIEW state, no part should be added
if ( mState == RecordingMapTool::View || mState == RecordingMapTool::Grab )
Comment on lines +1155 to +1156

@tomasMizera tomasMizera Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@uclaros why not? :)

@tomasMizera tomasMizera Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm thinking View is a great state to add a new part

@uclaros uclaros Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The state is set to Record in qml before invoking this method. This is kind of a safeguard that the method doesn't get called properly.

Should I set the state in cpp and just call startDigitizingNewPart() from qml?

{
return;
}

QgsAbstractGeometry *geom = mRecordedGeometry.get();
if ( QgsGeometryCollection *collection = qgsgeometry_cast<QgsGeometryCollection *>( geom ) )
{
switch ( mRecordedGeometry.type() )
{
case Qgis::GeometryType::Line:
collection->addGeometry( new QgsLineString() );
setActivePartAndRing( collection->partCount() - 1, 0 );
break;
case Qgis::GeometryType::Polygon:
collection->addGeometry( new QgsPolygon( new QgsLineString(), QList<QgsLineString*>() ) );
setActivePartAndRing( collection->partCount() - 1, 0 );
break;
case Qgis::GeometryType::Point:
// MultiPoints do not need an empty placeholder part, new point part is directly appended when digitizing
case Qgis::GeometryType::Unknown:
case Qgis::GeometryType::Null:
break;
}
}
}

double RecordingMapTool::pixelsToMapUnits( double numPixels )
{
QgsRenderContext context = QgsRenderContext::fromMapSettings( mapSettings()->mapSettings() );
Expand Down
7 changes: 7 additions & 0 deletions app/maptools/recordingmaptool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,13 @@ class RecordingMapTool : public AbstractMapTool

Q_INVOKABLE void cancelGrab();

/**
* When this is called on a multipart geometry, a new empty part will be added to the geometry and
* activePart will be set to that new last part.
* For MultiPoints and singlepart geometries no new empty part will be added.
*/
Q_INVOKABLE void startDigitizingNewPart();

// Getters / setters
bool centeredToGPS() const;
void setCenteredToGPS( bool newCenteredToGPS );
Expand Down
48 changes: 45 additions & 3 deletions app/qml/map/MMMapController.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,17 +814,22 @@ Item {
return "invisible"

if (internal.splitGeometryButtonVisible) {
if (!internal.redrawGeometryButtonVisible && !internal.streamingModeButtonVisible)
if (!internal.redrawGeometryButtonVisible && !internal.streamingModeButtonVisible && !internal.addPartButtonVisible)
return "split"
}

if (internal.addPartButtonVisible) {
if (!internal.splitGeometryButtonVisible && !internal.streamingModeButtonVisible && !internal.redrawGeometryButtonVisible)
return "addPart"
}

if (internal.redrawGeometryButtonVisible) {
if (!internal.splitGeometryButtonVisible && !internal.streamingModeButtonVisible)
if (!internal.splitGeometryButtonVisible && !internal.streamingModeButtonVisible && !internal.addPartButtonVisible)
return "redraw"
}

if (internal.streamingModeButtonVisible) {
if (!internal.redrawGeometryButtonVisible && !internal.splitGeometryButtonVisible)
if (!internal.redrawGeometryButtonVisible && !internal.splitGeometryButtonVisible && !internal.addPartButtonVisible)
Comment thread
Withalion marked this conversation as resolved.
return "stream"
}
return "menu"
Expand All @@ -834,6 +839,9 @@ Item {
if (actionState === "split")
return __style.splitGeometryIcon

if (actionState === "addPart")
return __style.plusIcon

if (actionState === "redraw")
return __style.redrawGeometryIcon

Expand All @@ -847,6 +855,9 @@ Item {
if (actionState === "split")
return root.toggleSplitting()

if (actionState === "addPart")
return root.toggleAddPart()

if (actionState === "redraw")
return root.toggleRedraw()

Expand Down Expand Up @@ -977,6 +988,18 @@ Item {
}
}

MMListDelegate {
text: qsTr( "Add part" )
leftContent: MMIcon { source: __style.plusIcon }

visible: internal.addPartButtonVisible

onClicked: {
root.toggleAddPart()
moreToolsMenu.close()
}
}

MMListDelegate {
text: qsTr( "Redraw geometry" )
leftContent: MMIcon { source: __style.redrawGeometryIcon }
Expand Down Expand Up @@ -1262,6 +1285,7 @@ Item {

// visibility of buttons in "more" menu
property bool splitGeometryButtonVisible: !internal.isPointLayer && !root.isStreaming && root.state === "edit"
property bool addPartButtonVisible: internal.isMultiPartLayer && !root.isStreaming && root.state === "edit"
property bool redrawGeometryButtonVisible: root.state === "edit"
property bool streamingModeButtonVisible: !internal.isPointLayer || internal.isMultiPartLayer

Expand Down Expand Up @@ -1323,6 +1347,24 @@ Item {
}
}

function toggleAddPart() {
addPart( internal.featurePairToEdit )
}

Comment thread
Withalion marked this conversation as resolved.
function addPart( featurepair) {
__activeProject.setActiveLayer( featurepair.layer )
root.centerToPair( featurepair )

@uclaros uclaros Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
root.centerToPair( featurepair )

I think this is a bad idea and would like to remove it (also from split and redraw logic).
User is centered to the geometry when identifying it, then if he has panned the map he is re-centered a second time when tapping Edit geometry.
I think it is very probable that the user has already panned the map to where he wants to add the new part / split / redraw and we're just forcing him back to the original geometry location!

root.showInfoTextMessage( qsTr( "Add new part to the geometry" ) )

internal.featurePairToEdit = featurepair
Comment thread
Withalion marked this conversation as resolved.

// You should be already in state == "edit"
if ( recordingToolsLoader.active ) {
recordingToolsLoader.item.recordingMapTool.state = MM.RecordingMapTool.Record
recordingToolsLoader.item.recordingMapTool.startDigitizingNewPart()
}
}

function toggleSplitting() {
split(internal.featurePairToEdit)
}
Expand Down
Loading
Loading