Make avoid overlap work with more digitizing tools (fixes #50433)

[JuhoErvasti]

Description

Use the QgsAvoidIntersectionsOperation class to remove overlap from geometries which are edited with the move, rotate, scale and offset curve tools.

Copy and move is not included in this, since the QgsAvoidIntersectionsOperation is part of "app", but the copying and moving is done in core code (QgsVectorLayerTools) and I assume you're not supposed to include headers from app in core.

[github-actions]

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit 18e78a1)

[troopa81]

Thank you for this work!

You deal with "move tool" but forget to deal with "copy-move tool"

Do you mind adding test for each tool?

[JuhoErvasti]

@troopa81 Thank you for the review!

I don't mind adding tests for the tools, but it's probably going to take some time.

About the copy-move feature, do you have a suggestion on how to approach it?

Currently the copy-moving is being done in QgsVectorLayerTools which is core code whereas the QgsAvoidIntersectionsOperation is part of the app code. I'm not too familiar with the code-base, but I assume you're not supposed to include code from the maptools to core.

[troopa81]

I don't mind adding tests for the tools, but it's probably going to take some time.

I don't mind waiting, the rest of the PR is good now (except maybe the question regarding copy-move) so as soon as there is test for each tool, I'll merge. You can also split this PR in several ones if you want, one for each tool for instance, so you get your work merged progressively as you write the tests.

About the copy-move feature, do you have a suggestion on how to approach it?

Currently the copy-moving is being done in QgsVectorLayerTools which is core code whereas the QgsAvoidIntersectionsOperation is part of the app code. I'm not too familiar with the code-base, but I assume you're not supposed to include code from the maptools to core.

No, you're not. Actually QgsMapToolMoveFeature uses a QgsGuiVectorLayerTool which lives in app. So If I were to do this, I think I would override copyMoveFeatures (the method is virtual) add add a virtual protected method avoidIntersection() that would call QgsAvoidIntersectionOperation in QgsGuiVectorLayerTool and would propose another implementation in core without all the Gui stuff. Something like that.

But It would be better to have all this logic in a different PR. I'm a little bothered to have a move tool that behaves differently from the copy-move tool but all the tool are supposed to support avoid intersection so I think the situation would be better after your PR even if the copy-move part is not implemented.

[JuhoErvasti]

Okay, thank you. I think I'd prefer to add the tests for each tool in this PR and add the copy-move part in a different one. I think I can get the tests done before the end of this week but the copy-move logic might take a bit longer.

[troopa81]

Okay, thank you. I think I'd prefer to add the tests for each tool in this PR and add the copy-move part in a different one. I think I can get the tests done before the end of this week but the copy-move logic might take a bit longer.

👍

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.

[troopa81]

@JuhoErvasti Is this PR ready or is there remaining work to do ? all the comments have been addressed?

[JuhoErvasti]

@troopa81 Yes, this PR should be ready. I've addressed the comments and added tests for each tool.

As for the copy-move implementation; I started preparing a fix and it's mostly working but still needs some development, but that'd be a different PR regardless.

[troopa81]

Great work, thanks 👍

As for the copy-move implementation; I started preparing a fix and it's mostly working but still needs some development, but that'd be a different PR regardless.

👍