[digitizing] Split features: always give largest geometry to original feature

[JuhoErvasti]

Fixes #53696. Backport to 3.40 requested.

Funded by the National Land Survey of Finland.

[github-actions]

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit c9981ad)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit c9981ad)

[nyalldawson]

@JuhoErvasti do you currently have a use case for this beyond that described by #53696 ? If there's no concrete use case for custom sorting, then I'd avoid the complexity and just ALWAYS use the largest part (as described in #53696)

[JuhoErvasti]

@nyalldawson
Not immediately, there was a concern that #53696 could be too specific to the funder and therefore a solution would require giving options to different users. I reached out to Oslandia's devs and @troopa81 suggested the idea to use expressions.

If this is not necessary, I can of course change it. In this case since you can split a feature an arbitrary number of times should all of them still be sorted by area/length or just that the original feature will get the largest geometry?

[nyalldawson]

@JuhoErvasti

I'm wondering if instead it'd make more sense to handle this in the split tool itself, so that users can interactively pick on a split-by-split basis which feature is the "reference" feature.

Eg maybe something like ctrl+click with the tool sets the reference point, and then after the split the part closest to the reference point becomes the reference (original) feature geometry.

[JuhoErvasti]

@nyalldawson
That'd definitely make it quite flexible, but I think it also adds more complexity, at least from the user's perspective. For example in the use case behind #53696 there are rules for managing the lifecycle of features, one of which would be that when splitting the biggest geometry goes to the original feature.

You'd then have to remember to set the reference point each time and in some cases it can be difficult to eyeball which is the biggest feature, making it more error-prone. Also potentially if there's different rules for different layers you'd have to remember them and keep track of that as well.

I think that feature would work for some situations, but I think there's also a very solid case for being able to set a layer-level rule to avoid manual work and mistakes.

Edit: I do understand that this implementation does add complexity. We wanted make it an option preemptively in case other users/organizations have different needs. Would it be better from the complexity perspective if instead of an expression there were preset options (a bit similar to the Attributes Form split/duplicate policies?

[troopa81]

Just to complete @JuhoErvasti, my concerns was that user would be continuously asking for different way of choosing the reference feature according to their needs, so better have an expression and maybe a list of classic order (area, length).

Regarding the PR, not sure sorting order is needed. Because you can order in ascending with $area, and descending with 1/$area, no?

@nyalldawson Your proposal is interesting but it makes the UI a little bit complex with even more click when it is not (always) needed.

[nyalldawson]

@JuhoErvasti
@troopa81

That'd definitely make it quite flexible, but I think it also adds more complexity, at least from the user's perspective. For example in the use case behind #53696 there are rules for managing the lifecycle of features, one of which would be that when splitting the biggest geometry goes to the original feature.

You'd then have to remember to set the reference point each time and in some cases it can be difficult to eyeball which is the biggest feature, making it more error-prone. Also potentially if there's different rules for different layers you'd have to remember them and keep track of that as well.

What I'm proposing are two changes:

1. Default to ALWAYS retaining the biggest part. That becomes the standard behaviour, no GUI for changing or controlling it. This fixes the situation immediately and transparently for 99% of users, with no added complexity.
2. Add some way of picking the reference part in the GUI in an opt-in way, which doesn't change the tool at all for users who don't care about picking the reference part and are happy with the biggest-part logic to apply. That's why I'm thinking something like a ctrl+click in advance, as it's only ever going to be required/used in advanced circumstances and won't change any existing workflows at all

[JuhoErvasti]

@nyalldawson

Well, that would take care of the use case I'm trying to address. I'm okay to make that change if is no need for custom sorting. If I could then ask a few more questions:

• Would it be enough to just implement the behavior in #53696 to start with, and create a separate feature request for the CTRL+click behaviour?
  • If yes, could it also be backported into 3.40? Or would it be considered a new feature?
• Should the behavior be that if you're splitting the feature to more than 2 parts the split geometries are sorted or just that it finds the biggest geometry and that will be the original feature and the rest created in the random(ish) order like right now?

@troopa81

Regarding the PR, not sure sorting order is needed. Because you can order in ascending with $area, and descending with 1/$area, no?

That would work, I thought I'd add it for consistency since some other sorting features in the GUI have it as well (at least feature rendering order).

[nyalldawson]

@JuhoErvasti

Would it be enough to just implement the behavior in #53696 to start with, and create a separate feature request for the CTRL+click behaviour?

That's fine by me! I don't personally think the second part is a requirement, I just think it's a better approach to handling advanced situations vs the expression. So if you just want to do the "always biggest part" logic only, that's perfectly reasonable.

If yes, could it also be backported into 3.40? Or would it be considered a new feature?

I think it can be considered a bug fix, yes.

Should the behavior be that if you're splitting the feature to more than 2 parts the split geometries are sorted or just that it finds the biggest geometry and that will be the original feature and the rest created in the random(ish) order like right now?

I'm fine with just the single biggest part being treated special, and the rest in random order. Ordering for adding features usually isn't important anyway.

[JuhoErvasti]

@nyalldawson

I've force-pushed the change to just always give the biggest geom to the original feature and also updated the description if you want to take look.

I had to modify some existing tests, some of them were retrieving the new feature by a hard-coded number and the change also affected the split policy test.

[nyalldawson]

@JuhoErvasti looks good to me, thanks!

[nyalldawson]

We'll wait for @troopa81's feedback before merging

[JuhoErvasti]

@nyalldawson @troopa81

Thank you! Could this be tagged for a backport to 3.40 as well? 🙏

[troopa81]

LGTM