[needs-docs][FEATURE] Add reverse line maptools AKA swap direction

[lbartoletti]

Description

A new maptools to reverse a (multi)linestring. Very useful for water/road digitzing.

REF: qgis/QGIS-Enhancement-Proposals#106 6. Swap Direction

Demonstration

data's demonstration:

# LineString

## Base
wkt_geom
LineString (1980460.58086552447639406 5190699.52586492709815502, 1980462.61913713160902262 5190732.29500076361000538)

## Reversed
wkt_geom
LineString (1980462.61913713160902262 5190732.29500076361000538, 1980460.58086552447639406 5190699.52586492709815502)

# MultiLineString

## Base
wkt_geom
MultiLineString ((1980571.43148292158730328 5190702.03450690489262342, 1980560.92654463928192854 5190728.37524767313152552),(1980571.43148292158730328 5190716.92956864926964045, 1980580.52531009120866656 5190727.90487730223685503))

## Part 1 reversed 
wkt_geom
MultiLineString ((1980571.43148292158730328 5190702.03450690489262342, 1980560.92654463928192854 5190728.37524767313152552),(1980580.52531009120866656 5190727.90487730223685503, 1980571.43148292158730328 5190716.92956864926964045))

## Part 2 reversed
wkt_geom
MultiLineString ((1980560.92654463928192854 5190728.37524767313152552, 1980571.43148292158730328 5190702.03450690489262342),(1980580.52531009120866656 5190727.90487730223685503, 1980571.43148292158730328 5190716.92956864926964045))

# Curve

## Base
wkt_geom
CompoundCurve (CircularString (1980475.00555689726024866 5190698.27154393866658211, 1980499.93518655234947801 5190737.78265509009361267, 1980531.92037177016027272 5190705.79746987204998732))

## Reversed
wkt_geom
CompoundCurve (CircularString (1980531.92037177016027272 5190705.79746987204998732, 1980499.93518655234947801 5190737.78265509009361267, 1980475.00555689726024866 5190698.27154393866658211))

Checklist

Reviewing is a process done by project maintainers, mostly on a volunteer basis. We try to keep the overhead as small as possible and appreciate if you help us to do so by completing the following items. Feel free to ask in a comment if you have troubles with any of them.

• Commit messages are descriptive and explain the rationale for changes
• Commits which fix bugs include fixes #11111 in the commit message next to the description
• Commits which add new features are tagged with [FEATURE] in the commit message
• Commits which change the UI or existing user workflows are tagged with [needs-docs] in the commit message and contain sufficient information in the commit message to be documented
• I have read the QGIS Coding Standards and this PR complies with them
• This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
• New unit tests have been added for core changes
• I have run the scripts/prepare-commit.sh script before each commit

[nyalldawson]

Nice feature, and much needed!

However, I'm a -0 on adding this as a new map tool. I don't think adding extra map tools for every geometry operation like this will scale, especially when we're already fighting UI complexity with the current set of tools. I'd personally rather see this implemented using the approach from qgis/QGIS-Enhancement-Proposals#114 - where we reuse the Processing toolbox for additional geometry editing tools. I'm going to be launching a crowdfunding campaign for this shortly, so hopefully we'll get it implemented soon.

If everyone else agrees that reusing the toolbox is a better approach, I'd suggest we leave this PR open (fighting off the stale bot as we go!) and if QEP #114 isn't implemented by 3.4, then we merge this approach instead.

[nirvn]

@nyalldawson , I disagree here. While we definitively want a reverse line algorithm that'll be compatible with the processing's in-place editing, this is one function that very much deserves a tool of its own when digitizing features, it's a basic function that's no different from our reshape features tool.

More importantly here, solely having a processing in-place editing function is not enough, as we need to be able to reverse direction of single part(s) within a multipart line / polygon.

[nyalldawson]

I disagree here. While we definitively want a reverse line algorithm that'll be compatible with the processing's in-place editing, this is one function that very much deserves a tool of its own when digitizing features, it's a basic function that's no different from our reshape features tool.

That argument's a slippery slope. There's lot of other tools I'd consider "essential" which we don't have - e.g. split multipart feature to single parts. So I'd argue that any geometry operation which doesn't require a "draw something on the canvas" interaction should be done through the toolbox instead.

More importantly here, solely having a processing in-place editing function is not enough, as we need to be able to reverse direction of single part(s) within a multipart line / polygon.

That's a fair point, I'll concede that one! (But I'll counter it by asking how often you really find you need to do that...)

[nirvn]

@nyalldawson , it's indeed a slippery slope here, various workflows would bring a different set of tools as "basic" for different users. That said, the multi-part handling here is IMHO what tips off the balance here :)

Similarly, while we have an offset / buffer algorithm, I still think the tool is useful as it can offset / buffer individual parts.

[nyalldawson]

Move initialization to header

[nyalldawson]

if ( f.hasGeometry() )

[nyalldawson]

There's an extra clone happening here which isn't needed - you should be able to get away with

geom = QgsGeometry( static_cast< const QgsCurve* >( f.geometry().constGet() )->reversed() );

[lbartoletti]

I agree that this can be done by an in place treatment through the toolbox, but I'm +1 with @nirvn From a draftsman's point of view, we more often use the "click-click" tools rather than the toolbox or the expression calc.

@nyalldawson thanks for the review

[nirvn]

@lbartoletti , regarding the "click-click" behavior, that's something that could be achieved via in-place processing algorithm (for e.g., we could "select" an algorithm, and a click on a given feature on the canvas could apply that algorithm to this singular feature). That could be nice in addition to "selected features".

[3nids]

I'm a bit concerned by adding more and more map tools too.
Processing sounds nice but I agree it might not be the best user experience.
If we add this as a map tool, what about embed it in the reshape tool (a menu button, like select tool or move feature tool) ?

[lbartoletti]

A small argument in favor of a graphical user interface for this kind of tool is that I have difficulty to imagine (more simply) the change of direction of only one part of a multilinestring since a select feature does not allow to choose the part of its geometry.

[nyalldawson]

@lbartoletti You may want to look at testqgsvertextool.cpp (if you haven't already!). @wonder-sk fought with Travis a lot to get that test working, so maybe there's some clues there which will help get your test passing on Travis too.

[lbartoletti]

@nyalldawson thanks, I changed the point so it clicks on a node and Travis is happy.

[pblottiere]

Do you think that a unique_ptr could be used instead?

[lbartoletti]

I had to remove the Z for circularstring because it was flaky on linux (debian)

[lbartoletti]

OK to merge?

[nyalldawson]

Just needs the conflicts fixed..

[lbartoletti]

Which conflicts?

[nyalldawson]

@lbartoletti There's rebase conflicts. Don't worry - a merge would still work.

@3nids is this OK to merge for you?

[3nids]

I proposed to embed it with the reshape tool. I am not fully convinced about this approach though.
So if the other are fine with new button, then let it be.

[nyalldawson]

Embedding with reshape action sounds reasonable to me

[haubourg]

Hi, we discussed this at the hackfest and opinions were 50/50. on one hand,, the UI is already too clutered. On the other hand, the reshape tool icon only has a polygon icon which makes it not so obvious for users to discover there. I'd more be in favor of merging the three "merge feature tools" if we want to declutter than hiding this one.

[andreasneumann]

I am not so convinced that introducing more and more submenus for buttons is the way to go. But you would probably hide away a lot of other toolbars.

If you are into digitizing, then you don't want a lot of these drop down sub menus. You want immediate access to the tools you need.

I have waited for ages for a tool to swap line directions. I have instructed my users to go into DB manager and do ST_Reverse in the combination with the primary key in the meantime. Very inconvenient. Now that we have such a tool, please don't hide it away in sub menus.

Now that we have user profiles and customization options, why don't we leave it to the user to introduce different profiles where they show and hide various combinations of toolbars and panels?

[3nids]

You are pretty convincing 😉 Indeed it might only make sense for variations of the same map tool and not for different ones.
Although a side note, the action would remember it's preferred/last used tool. But indeed, one of the 2 has to be hidden on first use.
So fine for me as is.

[nyalldawson]

@haubourg

I'd more be in favor of merging the three "merge feature tools" if we want to declutter than hiding this one.

+1 to that! (I'm curious what I'm missing here, but I only can see find 2 merge feature tools?)

[nyalldawson]

Right, let's merge. THere's universal agreement that the tool is useful, and the code is good. We can tweak ui later ;)