refactoring of QgsMapToolCapture to integrate shape map tools #46687

3nids · 2022-01-04T07:49:29Z

This moves part of the code from QgsMapToolDigitizeFeature to QgsMapToolCapture so the tool can actually capture point, line and polygons. It's mainly the 'cadCanvasReleaseEvent` which has been transfered.

This is a preliminary work to support shape digitizing in more map tools (like add part, add ring, fill ring or anything else).

The idea is that QgsMapToolCapture should actually perform the digitization of point/line/polygon and let subclasses deal to what they want to do with it (create feature, add part, add ring, etc).

Then, QgsMapToolCapture will be able to digitize shapes (by adding a new enum entry CaptureTechnique::ShapeDigitizing). The idea is that the shape toolbar will be turned into a checkable choice so that QgsMapToolCapture will be able to perform any of the capture technique entries:

StraightSegments
CircularString
Streaming
Shapes

I'd like to get opinions of the map tools experts @lbartoletti @nyalldawson. If you consider I should open a QEP for this discussion, let me know.

nyalldawson · 2022-01-04T08:20:31Z

I haven't looked at the code, but I fully support the design changes here. (I've wanted similar to allow the shape digitising for annotation items!). Great work @3nids!

lbartoletti · 2022-01-05T06:32:03Z

I can't review it this week, but it's a big +1 for this design. Thanks @3nids !

3nids · 2022-01-05T09:11:04Z

thanks @lbartoletti and @nyalldawson for the thumbs up

I have gone a bit furthe:

QgsMapToolCapture now has a capture technique defined
ground work to introduce the shape technique is done
add part has been turned to fully use QgsMapToolCapture (by using the parent cadCanvasReleaseEvent)

nyalldawson · 2022-01-06T01:56:45Z

src/gui/qgsmaptoolcapture.h

+     * Called when the geometry is captured
+     * \since QGIS 3.24
+     */
+    virtual void geometryCaptured( const QgsGeometry &geometry ) {Q_UNUSED( geometry )} SIP_FORCE


Shouldn't these be protected in order to allow python subclasses of the tool to function correctly?

I thought it was ok with forcing them with SIP_FORCE. Am I wrong?

I'm not sure either way -- it just looks problematic to me when a protected method is guaranteed to work correctly

quoting https://stackoverflow.com/a/2170696/1548052

This lets the derived classes override the function to customize the behavior as needed, without further exposing the virtual functions directly by making them callable by derived classes (as would be possible if the functions were just protected). The point is that virtual functions exist to allow customization; unless they also need to be invoked directly from within derived classes' code, there's no need to ever make them anything

And if you consider the idea of the additional class to handle the avoidIntersections, we'll be dealing with the private handler on the geometry and it will be better to just not forward the specific handlers.

src/gui/qgsmaptoolcapture.h

src/gui/qgsmaptoolcapture.cpp

3nids · 2022-01-13T15:01:35Z

Some progress have been made:

a registry of shape tools has been added 8649b64
a new class QgsMapToolsDigitizingTechniqueManager handles actions in app related to capture map tools 3874352 (this allows cleaning up quite some code in QgisApp, see f5d9e90#diff-a6c6864f28b601d260d5aff00f149039a0df622797f72f15311d052b1a38b77f )
the refactoring of existing shape tools is on-going ( 0ec45e8 )

Remains:

complete refactoring of shape tools
complete refactoring of capture map tools (add feature and add part are done for now, remains add/fill ring, etc.)
adapt existing shape tool tests to work with new code

I know this a big code change, but I would like if possible to merge it during this cycle.

3nids · 2022-01-13T20:44:52Z

Refactoring of existing shape tools is completed

3nids · 2022-01-13T21:16:41Z

And here are the curved rings:

3nids · 2022-01-13T21:56:35Z

3nids · 2022-02-21T09:38:30Z

Thanks a loooooot for the review @nyalldawson.
Comments have been addressed.

lbartoletti

Except for a few details, it's good for me.

lbartoletti · 2022-01-27T10:33:23Z

src/app/maptools/qgsmaptoolshapecircle2points.cpp

@@ -0,0 +1,85 @@
+/***************************************************************************
+    qgmaptoolcircle2points.cpp  -  map tool for adding circle


Suggested change

qgmaptoolcircle2points.cpp - map tool for adding circle

qgsmaptoolshapecircle2points.cpp - map tool for adding circle

Same for other files

lbartoletti · 2022-01-27T10:39:16Z

src/app/maptools/qgsmaptoolshapecircleabstract.cpp

 {
  if ( !mParentTool || mCircle.isEmpty() )
-  {


IMHO braces should be always present around statements even for one line if.

I personally like the readability when there is only one line.
The coding guidelines are not super explicit about this, but there is an example with it https://docs.qgis.org/3.16/en/docs/developers_guide/codingstandards.html#put-commands-on-separate-lines

python/gui/auto_generated/qgisinterface.sip.in

…void point duplication

github-actions · 2022-02-22T07:00:06Z

@3nids
A documentation ticket has been opened at qgis/QGIS-Documentation#7366
It is your responsibility to visit this ticket and add as much detail as possible for the documentation team to correctly document this change.
Thank you!

agiudiceandrea · 2022-10-01T16:14:27Z

Hi @3nids, could you please have a look at the issue #49541 which it seems to me it is related to this PR?

github-actions bot added this to the 3.24.0 milestone Jan 4, 2022

3nids added the Squash! Remember to squash this PR, instead of merging or rebasing label Jan 4, 2022

m-kuhn added Changelog Items that are queued to appear in the visual changelog - remove after harvesting Feature labels Jan 4, 2022

3nids force-pushed the capture-map-tool branch from 65aaad2 to 992e822 Compare January 4, 2022 15:15

3nids force-pushed the capture-map-tool branch from 992e822 to f1edb07 Compare January 5, 2022 09:08

3nids force-pushed the capture-map-tool branch 6 times, most recently from b5c145f to df6d2e1 Compare January 5, 2022 14:38

nyalldawson reviewed Jan 6, 2022

View reviewed changes

3nids force-pushed the capture-map-tool branch 3 times, most recently from 4703779 to 24ccf65 Compare January 6, 2022 14:54

3nids force-pushed the capture-map-tool branch from 54ac82b to 0ec45e8 Compare January 13, 2022 14:55

3nids removed the Squash! Remember to squash this PR, instead of merging or rebasing label Jan 13, 2022

3nids force-pushed the capture-map-tool branch 2 times, most recently from 13b583b to 7a8dbb9 Compare January 13, 2022 20:44

3nids changed the title ~~make QgsMapToolCapture capable of capturing point/line/polygons~~ refactoring of QgsMapToolCapture to integrate shape map tools Jan 14, 2022

3nids added the Digitizing Related to feature digitizing map tools or functionality label Jan 14, 2022

3nids force-pushed the capture-map-tool branch from a7bfcc6 to ed7f802 Compare January 14, 2022 10:36

3nids added 3 commits February 21, 2022 10:33

move layer specific part to specific tool

49b5926

fix typo

4f87cee

fix leak

3cccf4f

3nids force-pushed the capture-map-tool branch from ac7b351 to 3cccf4f Compare February 21, 2022 09:34

fix dox

8fbd474

lbartoletti approved these changes Feb 21, 2022

View reviewed changes

3nids added 7 commits February 21, 2022 17:00

fix segmentization

002fd4b

call map tool implementation of addCurve when adding trace curve to a…

b827f34

…void point duplication

call sub-class implementation

d8c6574

fix since 3.24 -> 3.26

efcf169

fix test

68702f1

add test to avoid extra curves when using tracing

f7cf0f5

fix headers

4185783

3nids merged commit 36a1019 into qgis:master Feb 22, 2022

3nids deleted the capture-map-tool branch February 22, 2022 06:59

qgis-bot mentioned this pull request Feb 22, 2022

refactoring of QgsMapToolCapture to integrate shape map tools (Request in QGIS) qgis/QGIS-Documentation#7366

Closed

Gustry added a commit to Gustry/QGIS that referenced this pull request Mar 17, 2022

Fix regression when adding feature without geom qgis#46687

dcd9ad8

Gustry added a commit to Gustry/QGIS that referenced this pull request Mar 17, 2022

Fix regression when adding feature without geom qgis#46687

ff87afd

Gustry mentioned this pull request Mar 17, 2022

Fix regression when adding feature without geom #46687 #47840

Merged

Gustry added a commit to Gustry/QGIS that referenced this pull request Mar 17, 2022

Fix regression when adding feature without geom qgis#46687

2a7cb4f

3nids pushed a commit that referenced this pull request Apr 4, 2022

Fix regression when adding feature without geom #46687

2282a05

zacharlie added ChangelogHarvested This PR description has been harvested in the Changelog already. and removed Changelog Items that are queued to appear in the visual changelog - remove after harvesting labels May 28, 2022

uclaros mentioned this pull request Jun 16, 2022

No rubberband when switching from "Add circular string" (and by radius) to "Add feature" tool #23011

Closed

agiudiceandrea mentioned this pull request Oct 1, 2022

Can not toggle between "shape digitization tool" (circle, square, etc) and the standard "Add polygon feature" #49541

Closed

2 tasks

agiudiceandrea mentioned this pull request Nov 28, 2022

Digitizing mode is geometry type agnostic #51014

Open

2 tasks

agiudiceandrea mentioned this pull request Feb 10, 2024

The description of the "Edit" menu is not up to date qgis/QGIS-Documentation#8853

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactoring of QgsMapToolCapture to integrate shape map tools #46687

refactoring of QgsMapToolCapture to integrate shape map tools #46687

3nids commented Jan 4, 2022

nyalldawson commented Jan 4, 2022

lbartoletti commented Jan 5, 2022

3nids commented Jan 5, 2022 •

edited

Loading

nyalldawson Jan 6, 2022

3nids Jan 6, 2022

nyalldawson Jan 7, 2022

3nids Jan 7, 2022

3nids commented Jan 13, 2022 •

edited

Loading

3nids commented Jan 13, 2022

3nids commented Jan 13, 2022

3nids commented Jan 13, 2022

3nids commented Feb 21, 2022

lbartoletti left a comment

lbartoletti Jan 27, 2022

lbartoletti Jan 27, 2022

3nids Feb 22, 2022

github-actions bot commented Feb 22, 2022

agiudiceandrea commented Oct 1, 2022 •

edited

Loading

		@@ -0,0 +1,85 @@
		/***************************************************************************
		qgmaptoolcircle2points.cpp - map tool for adding circle

	qgmaptoolcircle2points.cpp - map tool for adding circle
	qgsmaptoolshapecircle2points.cpp - map tool for adding circle

refactoring of QgsMapToolCapture to integrate shape map tools #46687

refactoring of QgsMapToolCapture to integrate shape map tools #46687

Conversation

3nids commented Jan 4, 2022

nyalldawson commented Jan 4, 2022

lbartoletti commented Jan 5, 2022

3nids commented Jan 5, 2022 • edited Loading

nyalldawson Jan 6, 2022

Choose a reason for hiding this comment

3nids Jan 6, 2022

Choose a reason for hiding this comment

nyalldawson Jan 7, 2022

Choose a reason for hiding this comment

3nids Jan 7, 2022

Choose a reason for hiding this comment

3nids commented Jan 13, 2022 • edited Loading

3nids commented Jan 13, 2022

3nids commented Jan 13, 2022

3nids commented Jan 13, 2022

3nids commented Feb 21, 2022

lbartoletti left a comment

Choose a reason for hiding this comment

lbartoletti Jan 27, 2022

Choose a reason for hiding this comment

lbartoletti Jan 27, 2022

Choose a reason for hiding this comment

3nids Feb 22, 2022

Choose a reason for hiding this comment

github-actions bot commented Feb 22, 2022

agiudiceandrea commented Oct 1, 2022 • edited Loading

3nids commented Jan 5, 2022 •

edited

Loading

3nids commented Jan 13, 2022 •

edited

Loading

agiudiceandrea commented Oct 1, 2022 •

edited

Loading