Fix Sketcher split edge dropping coincident and dimensional constrain…

[cray98-code]

Fixes #28240

The Sketcher Split Edge tool drops coincident and dimensional constraints
when splitting a line. Parallel constraints survive correctly. This is a
regression from 1.0.2.

Root cause: In SketchObject::split(), the erase_if filter before calling
deriveConstraintsForPieces() keeps only PointPos::none constraints, silently
discarding anything referencing PointPos::start, PointPos::end, or
PointPos::mid, which includes coincident and dimensional constraints.

Fix: Only exclude InternalAlignment constraints and start/end/mid constraints
(which are handled separately by transferConstraints()). All other constraints
including dimensional ones are correctly passed to deriveConstraintsForPieces()
for remapping.

Tested against 1.1rc2, constraints now survive the split correctly.

Example from 1.1rc2

[PaddleStroke]

@AjinkyaDahale please check, it most likely comes from the refactor so you are best to know if the fix is the correct one. Thanks

[AjinkyaDahale]

Unfortunately I'll be mostly unavailable for another week. From a cursory glance, I see that this commit edits split() directly. If there is a reasonable possibility that these constraints can apply when trim and extend are called, even if they do not in 1.0, this functionality should be handled in deriveConstraintsForPieces (I might not have written the name exactly as it is in the code, please confirm). For this, you might need to treat coincidence and distance constrains separately.

Also, as a general note, please try to limit the increase in cognitive complexity.

Finally, some parts of this code (particularly trim, split, join and extend) may be considered a "rewrite" rather than "refactor", so some functionality might have been dropped.

[maxwxyz]

Tested

[maxwxyz]

@cray98-code PR has a conflict

[yorikvanhavre]

Waiting for @theo-vt 's review

[theo-vt]

@hyarion I think it's good!

[AjinkyaDahale]

In addition to thw comments, please confirm if this applies only to split, or if other operations can also benefit from this.

[AjinkyaDahale]

This and the next two checks can (should?) be combined into one with || for reducing cognitive complexity.

[AjinkyaDahale]

What was the reason to remove this condition and replace it with three?

[AjinkyaDahale]

Since the array holds only indices involving the geoid and any posid start/end/mid/none, excluding "none" automatically implies the erased indices involve start/mid/end.

So, we can check for internal alignment and NOT none.

The one new thing this covers is if start/mid/end AND none are somehow both involved. Is that the case?

[maxwxyz]

@cray98-code friendly reminder, PR also has a conflict.

[AjinkyaDahale]

Does not seem to work completely. Here, splitting one line keeps the distance, but not the case for the other.
pre-split-28244.zip

[AjinkyaDahale]

Tested

@maxwxyz could you please attach a file that works with this PR? The one I attached doesn't work for point on object at all, and only half for the distance constraint.

[luzpaz]

@maxwxyz could you please attach a file that works with this PR? The one I attached doesn't work for point on object at all, and only half for the distance constraint.

[maxwxyz]

Indeed it does not work with your test file

[cray98-code]

After reviewing the feedback I realise this is beyond my current coding ability to properly address

I don't want to hold up a fix for this bug, so I'm going to close this PR. I hope the description of the root cause and the attempted fix here can be useful as a starting point for someone more experienced to take over.