[yongyin-leon]

Summary

Point targets currently snap even when the Vertex option is off, while MultiPoint members are never offered as snap targets. Make both representations obey the Vertex switch and offer each MultiPoint member in its existing order. Neither representation supplies line ends or implicit edges between members.

Preserve whole-feature exclusion for the source Point/MultiPoint, point/end-before-edge priority, strict distance ties, layer visibility/scale rules, tolerance and projection handling. This complements #818's line/polygon source-identity change without changing its behavior. No commands, parameters, dependencies, data model or project format changes.

Testing

Previously completed scoped validation, reused for submission:

Platform	Engine type regressions	UI gesture regressions	Existing edit/snapping regression
Windows x86_64, Rust 1.98.0	7 passed	5 passed	1 passed
macOS ARM64, Rust 1.95.0	7 passed	5 passed	1 passed
Linux ARM64, Rust 1.95.0	7 passed	5 passed	1 passed

The existing case is tests::modify_tools_snapping_topology_relationships_and_elevation. The two minimal regressions failed on the unchanged baseline before the fix. Coverage includes master/type switches, empty and repeated members, nearest/tie ordering, mixed point/line/polygon priority, tolerance, eligibility, projected invalid members, read-only queries and whole-source exclusion. UI tests use real pointer events and inspect the painted snap ring, rotated dragging, single-step commit/undo/redo, cancellation, read-only behavior and creation preview/placement parity.

Also completed on Windows: affected engine/UI libraries and integration targets passed Clippy with -D warnings; the engine's wasm32-unknown-unknown type check passed; two offscreen UI screenshots were inspected. Engine Rustdoc including private items built with eight pre-existing public-to-private link warnings, each matched to the baseline source; the added docs introduced none.

Executable code and all tests are unchanged from the three-platform validated patch. The shared target rules are documented after snap_at's summary, using "vertices and line ends" for the priority rule; snap_except retains its summary and links to that contract. Formatting and diff checks passed, and a temporary-index three-way application to current upstream was clean. No compilation or tests were rerun for submission or the Rustdoc-only follow-up. Full workspace CI, Linux x86_64 runtime and browser runtime are not claimed.

Related issue

Follow-up to the vertex-edit workflow in #143 / #428; complements #818's source-node exclusion with consistent point-target type switches.

Summary by CodeRabbit

• Bug Fixes

  • Point features now snap only when vertex snapping is enabled. MultiPoint members act as individual vertex targets, not edges or line ends.
  • When dragging a Point or MultiPoint member, its entire source feature is excluded from snapping.

• Documentation

  • Clarified how Point and MultiPoint features behave with snapping.

[coderabbitai]

Caution

Review failed

The pull request is closed.

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 6ada1c1f-f24c-4ad9-8a15-574d54b23527

Commits

Reviewing files that changed from the base of the PR and between c9efee9 and 7afe380.

Files selected for processing: 4

• crates/engine/src/cmd/design.rs
• crates/engine/tests/snapping_point_types.rs
• crates/ui-egui/tests/snapping_point_types.rs
• docs/user-guide/editing.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

Point features and individual MultiPoint members are snapping targets only when vertex snapping is enabled. Engine and map-view tests cover target selection, dragging, point creation, and related editing behavior. The editing guide documents these snapping rules and source-feature exclusion.

Changes: Point and MultiPoint snapping

Layer / File(s)	Summary
Target eligibility and engine coverage crates/engine/src/cmd/design.rs, crates/engine/tests/snapping_point_types.rs, docs/user-guide/editing.md	Point and MultiPoint geometries are treated as vertex targets when vertex snapping is enabled. Engine tests cover target selection and related snapping rules. The guide documents vertex-only targets and exclusion of the entire source feature when dragging a Point or MultiPoint member.
Map-view editing interactions crates/ui-egui/tests/snapping_point_types.rs	Integration tests cover dragging and point creation with Point and MultiPoint targets, including previews, commits, cancellation, read-only mode, source-feature exclusion, undo, and redo.

Priority: Not assessed

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Possibly related issues

• Help test RustGIS v0.4.0: new features and existing workflows #748 — The issue requests testing editing workflows, including vertex edits. This PR adds tests for vertex snapping during editing.

Possibly related PRs

• opengeos/RustGIS#818 — This PR changes the same snapping implementation and preserves whole-feature exclusion while changing Point and MultiPoint target eligibility.

Suggested reviewers: giswqs

Merge Risk: Minimal · up to 0f59c

The snapping changes have no established actionable merge-blocking risk.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly and concisely describes the primary change: Point and MultiPoint targets now honor vertex snapping.
Description check		The description includes all required sections. It clearly explains the change, testing performed, related issues, preserved behavior, and known validation limits.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 3 files. (1 skipped: 1…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 3 files. (1 skipped: 1 unsupported.)

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit tests each vertex near,
A Point snaps when the switch is clear.
MultiPoint members join the queue,
Drag previews show the target true.
Undo hops back; redo hops through.}

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Nit: the new doc lines sit above the existing summary and split it. The rendered doc now starts with details and the summary [snap_at], leaving out one feature… ends up mid-paragraph. Also "Points and line ends take priority" reads ambiguously (it means vertices and ends). Put the summary first:

Suggested change

	/// row and member order.
	/// [`snap_at`], leaving out one feature (layer id, row): a vertex being dragged doesn't snap to
	/// the shape it is changing.
	///
	/// Point features and MultiPoint members are vertex targets only, never edges or line ends.
	/// Vertices and line ends take priority over edges. Equal distances keep the original layer,
	/// row and member order.

(Replace lines 1070–1074.)

[github-actions]

Code review

The fix is small and correct. snap_except now treats Point and every MultiPoint member as vertex candidates, and only when the "vertex" snapping switch is on. Before, a Point snapped even with vertices off, and MultiPoint never snapped. I read the surrounding function and the diff but didn't build or run anything, including the new tests.

Bugs

• None found. Feature exclusion, the extent pre-filter, the point-before-edge priority and the tie order are unchanged. Empty and non-finite MultiPoint members are handled safely, because projected_in and the extent check run before the new loop.

Security

• None.

Performance

• None. The loop adds one pass over the points of the features that pass the extent filter.

Quality

• Nit (high confidence): the new doc lines in crates/engine/src/cmd/design.rs come before the existing summary line and split the doc comment. "Points and line ends take priority over edges" should say vertices. I left an inline comment with a suggested reordering.
• Observation (low confidence, intentional): Point layers no longer snap when "vertex" is off, e.g. in an edge-only configuration. This is documented and tested, but it changes behaviour for anyone relying on it.

AGENTS.md

• No unwrap, expect or panic! in non-test code, and no new dependencies, UI strings or assets. The unwrap and expect calls are in test files only.
• The change adds tests in both crates and updates the user guide.
• The docs sentence "Dragging a Point or MultiPoint member excludes its entire source feature" documents existing behaviour that the PR now covers with tests.

Change size

Compared: the whole PR against main (merge base c281e2a to PR head 7afe380).

Metric	Value
Files changed	4 (2 added, 2 modified, 0 removed, 0 renamed)
Size before	65.9 KB
Size after	85.1 KB
Total size change	+19.2 KB

File	Before	After	Change
crates/ui-egui/tests/snapping_point_types.rs	0 B	9.6 KB	+9.6 KB
crates/engine/tests/snapping_point_types.rs	0 B	8.9 KB	+8.9 KB
crates/engine/src/cmd/design.rs	58.0 KB	58.5 KB	+477 B
docs/user-guide/editing.md	7.9 KB	8.1 KB	+196 B

[github-actions]

📖 Docs preview

Item	Value
Preview	https://preview.rustgis.app/pr-859/
Web app	https://preview.rustgis.app/pr-859/app/
Commit	7afe380

GitHub Pages takes a minute or two to publish a new commit.

[github-actions]

Code review

I found no bugs, security issues or performance problems in this PR. I read the full diff and checked the change in snap_except against the surrounding function. I did not compile or run anything, and I did not run the new tests.

What I checked

• crates/engine/src/cmd/design.rs (snap_except): Point and MultiPoint members now go through the sn.vertex gate. Before, a Point snapped even with Vertex off, and MultiPoint members were never offered.
  • The consider closure and its two slots (point/end versus edge) are unchanged, so vertices and line ends still beat edges.
  • Ties still resolve by layer, row and member order, because only a strictly smaller distance replaces the current best.
  • skip is applied per row before the geometry is looked at, so whole-feature exclusion covers every MultiPoint member.
  • The extent pre-filter, tolerance, scale range, group hold and projection handling are untouched.
  • There is no indexing, unwrap or panic. std::slice::from_ref and the &[] fallback are safe, and an empty MultiPoint is a no-op.
  • NaN or infinite members: the new tests cover this and the code handles it, but I didn't check how projected_in drops unprojectable members.
• Wasm and layering: there are no new dependencies or platform-specific APIs, and no std::time use.

Bugs

None found.

Security

None.

Performance

None. The change adds one more O(members) loop per MultiPoint row, and only for rows that already pass the extent pre-filter.

Quality

• Low confidence, test brittleness: crates/engine/tests/snapping_point_types.rs asserts s.revision == revision.wrapping_add(1) after edit.snap. This pins an incidental side effect of command dispatch, so the test breaks if that bookkeeping changes. The comment says it is intended, so this is only a nit.
• Nit, behavior change not called out: a Point layer that previously snapped with Vertex off no longer does. The docs and PR text say so, but docs/user-guide/editing.md could state it plainly (for example, "Point features only snap when Vertex is on"). The new sentence says Point and MultiPoint "use the vertex option", which already covers it.
• Nit, docs line length: the wrapped lines in editing.md are uneven. The "Dragging a Point or MultiPoint member excludes…" sentence runs past the width of the surrounding lines.

AGENTS.md

• No unwrap, expect or panic in non-test code. The test files use them, which is allowed.
• No new commands, parameters or UI strings, so the tl! and i18n rules don't apply.
• The PR says full workspace CI was not run (cargo xtask ci: fmt, clippy, tests, layers). Make sure CI passes before merging, including the new ui-egui kittest gesture tests, which are the most environment-sensitive.

Change size

Compared: the whole PR against main (merge base c281e2a to PR head 0f59c19).

Metric	Value
Files changed	4 (2 added, 2 modified, 0 removed, 0 renamed)
Size before	65.9 KB
Size after	85.1 KB
Total size change	+19.2 KB

File	Before	After	Change
crates/ui-egui/tests/snapping_point_types.rs	0 B	9.6 KB	+9.6 KB
crates/engine/tests/snapping_point_types.rs	0 B	8.9 KB	+8.9 KB
crates/engine/src/cmd/design.rs	58.0 KB	58.5 KB	+483 B
docs/user-guide/editing.md	7.9 KB	8.1 KB	+196 B