Refactor setting face meshes in Shapes to support other planar axes #7622
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7622 +/- ##
==========================================
- Coverage 92.87% 92.84% -0.03%
==========================================
Files 630 630
Lines 58996 58994 -2
==========================================
- Hits 54792 54773 -19
- Misses 4204 4221 +17 ☔ View full report in Codecov by Sentry. |
Not sure how else to fix this; we could return dummy values for axis and value, but that seems worse than ignoring the type.
Czaki
approved these changes
Feb 19, 2025
| values = np.unique(points[:, axis_idx]) | ||
| if len(values) == 1: | ||
| return np.delete(points, axis_idx, axis=1), axis_idx, values[0] | ||
| return np.empty((0, 2), dtype=points.dtype), None, None |
There was a problem hiding this comment.
Does this will remove faces, or also edges?
There was a problem hiding this comment.
This only affects face triangulation. Edge triangulation is untouched.
jni
added a commit
to jni/napari
that referenced
this pull request
Mar 19, 2025
When napari#7622 is merged to main and main is merged here, these commits should disappear.
jni
added a commit
that referenced
this pull request
Apr 16, 2025
# References ~~Partial fix for~~ Closes #5673. (!!!) Closes #1653. 🥳 Closes #1058. 🥳 Closes #7799. Depends on #7622 # Description (Updated 29/2/2024.) (Updated 16/4/2025.) This PR adds three preprocessing steps to polygon data during meshing in Shapes layers: - discard duplicate polygon vertices, and recast edges in terms of unique vertices. - discard duplicate edges — these are found when going to interior polygons depicting holes and when coming back. - remove triangles from the triangulation that are not in the polygon. (only for the `triangle` backend, which otherwise includes these triangles.) For the VisPy step, we go directly into the Triangulation class, because the PolygonData class completely ignores input edges when building a triangulation. ## Current limitations: - ~~only works when `triangle` is installed (`py-triangle` in conda-forge)~~ **Edit:** nope, works with plain VisPy too! - ~~currently on macOS, I am getting random segfaults when calling triangle.triangulate. I don't know what triggers them, but I suspect we're just going to have to implement our own triangulation, maybe with Numba.~~ **Edit:** these were caused by repeated vertices, against which triangulation algorithms are generally not robust. Uniquifying the vertices fixed the problem! - ~~I still need to filter out the edges of the polygon for drawing, and maybe also for highlighting. Currently, the edges connecting the interior and exterior polygons are visible, which looks crap.~~ **Edit:** this is now done! - ~~the polygons are processed twice, once for the interiors and once for the edges.~~ **Edit:** fixed by @Czaki 🥳 - ~~the processing is probably a fair bit of overhead when many datasets would get no benefit. It might be worthwhile adding a flag to turn these checks on/off.~~ **Edit:** fixed by @Czaki 🥳 ## Summary of the issue and strategies: It turns out that what we actually need is a *constrained Delaunay triangulation*[1], that is, a Delaunay triangulation that preserves specific edges from the vertices. That’s what the 'p' option in `triangle.triangulate` does *but* it doesn’t work if the edges are actually not planar — that’s why the triangulation was failing for this polygon even if triangle was installed. This PR fixes that by finding edges that appear twice in the polygon and removing them. Additionally, certain triangles from the resulting triangulation need to be removed, and this is easy to do by running `skimage.measure.points_in_poly` on the triangle centers, which are fast to compute with NumPy indexing. [1]: http://en.wikipedia.org/wiki/Constrained_Delaunay_triangulation --------- Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com> Co-authored-by: Wouter-Michiel Vierdag <w-mv@hotmail.com> Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Tim Monko <timmonko@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied a
2FDF
s a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #7621
This also simplifies the many nested if/else clauses within the
Shape._set_meshes_pycode, which will be handy for #6654.