[processing] restore network analysis algorithms

[alexbruy]

Description

Restore all network analysis algorithms

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 containt 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

[alexbruy]

@nyalldawson what do you think about adding to Point and Extent parameters additional property to hold CRS of the point/extent? This should make it possible to reproject them into layer CRS and prevent dependency on iface and issues like https://issues.qgis.org/issues/15680

[nyalldawson]

What do you think about making this and OUTPUT_POLYGON optional outputs? I think it's unlikely that models will usually require both outputs, and there's potentially a speed boost by not calculating unwanted convex hulls (not to mention the memory savings from avoiding creating the unwanted memory layers or cost of writing to a disk based format).

I'd leave both of them created by default (i.e. not taking advantage of the change in #4868)

[nyalldawson]

You could add a destinationCrs to the request here to match the CRS for layer - then we'd get transparent handling of network/points in different CRS.

[nyalldawson]

Looks good :D - I've posted some potential areas for improvement (as in - not related to the port, but ways we could improve these algorithms in general now)

Another potential improvement would be to modify QgsVectorLayerDirector to take a QgsFeatureSource as input instead of a QgsVectorLayer. I've had a look and QgsVectorLayerDirector only uses methods from the layer which are in the QgsFeatureSource interface anyway, so it should be a straightforward change.

Another good improvement would be to make QgsGraphDirector::makeGraph take a QgsFeedback object so that the graph building could be cancelled, and use that for the progress reports during makeGraph instead of the buildProgress() signal. Again, this should be quite straightforward to add if you're interested.

[nyalldawson]

@alexbruy what do you think about adding to Point and Extent parameters additional property to hold CRS of the point/extent? This should make it possible to reproject them into layer CRS and prevent dependency on iface and issues like https://issues.qgis.org/issues/15680

Agreed - we need to do something like this. I'm not sure how adding it to the parameter would help though. Wouldn't that "lock in" the crs that algorithm?

My original intention behind #4720 was to handle this. I thought we could have referenced QgsRectangle + QgsPoint classes which are metatyped so they can be stored in QVariants. Then the CRS of the parameter value will be available. We could add logic like "if a extent parameter is specified as a QgsRectangle - assume it's in a matching CRS. if it's a QgsReferencedRectangle then reproject as required to match CRS". The gui wrappers for point/extent would return QgsReferencedRectangles/QgsReferencedPoints in the CRS of the canvas.

Does that make sense to you?

[alexbruy]

Agreed - we need to do something like this. I'm not sure how adding it to the parameter would help though. Wouldn't that "lock in" the crs that algorithm?

If reprojection enabled, then Extent and Point parameters will return values in canvas CRS while they will be applied to layer which has different CRS. For example, in case of GDAL algorithms, clipping and similar operations will fail if extent defined in different CRS. Having CRS associated with these parameters will allow us to reproject into layer CRS and run algorithm.

Seems your work in #4720 is exactly what I'm looking for.