feat: 3D mesh labeling (Canvas3D, brush-based vertex labeling)#234
Open
vietanhdev wants to merge 9 commits into
Open
feat: 3D mesh labeling (Canvas3D, brush-based vertex labeling)#234vietanhdev wants to merge 9 commits into
vietanhdev wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces first-pass 3D mesh labeling support via a new Canvas3D (PyQt6 + PyVista/VTK) widget, integrates it into the existing labeling UI, and adds supporting persistence + sample assets.
Changes:
- Added a new 3D canvas (
Canvas3D) with brush-based per-vertex labeling and view controls, plus wiring inLabelingWidget. - Extended label serialization/deserialization to support mesh projects (RLE vertex label ids + vertex index shapes).
- Added synthetic sample meshes and a headless-focused unit test suite for the Canvas3D data model.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_canvas3d.py | Adds Canvas3D headless/data-model tests with CI skips and mode assertions. |
| sample_meshes/README.md | Documents bundled synthetic meshes and regeneration steps. |
| sample_meshes/cube.obj | Adds synthetic cube sample mesh asset. |
| sample_meshes/cone.obj | Adds synthetic cone sample mesh asset. |
| sample_meshes/torus.obj | Adds synthetic torus sample mesh asset. |
| pyproject.toml | Adds PyVista/PyVistaQt/Trimesh dependencies. |
| anylabeling/views/labeling/widgets/canvas3d.py | Introduces the new Canvas3D + ViewControls3D implementation. |
| anylabeling/views/labeling/widgets/canvas.py | Replaces repaint() calls with update() in mouse move handling. |
| anylabeling/views/labeling/widgets/init.py | Exports Canvas3D and ViewControls3D. |
| anylabeling/views/labeling/utils/init.py | Adds RLE encode/decode helpers for vertex label id persistence. |
| anylabeling/views/labeling/shape.py | Extends Shape to carry vertex_indices and adds painter-path caching. |
| anylabeling/views/labeling/label_widget.py | Integrates 3D canvas, mesh open/save flow, label sync, and folder label persistence. |
| anylabeling/views/labeling/label_file.py | Updates label file load/save behavior for mesh projects and adds vertex_indices support. |
| anylabeling/utils.py | Adds mesh file detection helpers (MESH_EXTENSIONS, is_mesh_file). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| from PyQt6.QtCore import QObject, pyqtSignal, pyqtSlot | ||
|
|
||
| MESH_EXTENSIONS = [".obj", ".stl", ".ply"] |
Comment on lines
+534
to
+535
| except Exception as e: | ||
| print(f"Error loading mesh: {e}") |
Comment on lines
+1100
to
+1102
| # Emit new_shape once per label for the label list | ||
| for label in self._shapes_by_label: | ||
| self.new_shape.emit() |
Comment on lines
+603
to
+605
| self._refresh_vertex_colors() | ||
| for _ in new_labels: | ||
| self.new_shape.emit() |
Comment on lines
+2411
to
+2413
| else: | ||
| self.central_stack.setCurrentIndex(0) | ||
| self.view_controls_3d_dock.hide() |
Comment on lines
+45
to
+49
| def decode_rle(rle): | ||
| """Decode data using Run-Length Encoding""" | ||
| res = [] | ||
| for i in range(0, len(rle), 2): | ||
| val = rle[i] |
|
|
||
|
|
||
| @unittest.skipIf(Canvas3D is None, f"mesh deps unavailable: {_IMPORT_ERROR}") | ||
| @unittest.skipIf(_IN_CI, "Canvas3D test requires a real display; CI runners segfault on init") |
Comment on lines
+159
to
+163
| def test_mode_constants_reduced_to_view_and_brush(self): | ||
| """Keypoint mode was removed for simplicity; verify.""" | ||
| self.assertEqual(Canvas3D.VIEW, "view") | ||
| self.assertEqual(Canvas3D.BRUSH, "brush") | ||
| self.assertFalse(hasattr(Canvas3D, "KEYPOINT")) |
Comment on lines
+2389
to
+2394
| self._sync_all_label_colors_3d() | ||
| if "vertex_label_ids" in self.other_data: | ||
| self.canvas_3d.load_vertex_label_ids( | ||
| self.other_data["vertex_label_ids"] | ||
| ) | ||
| self.load_labels(self.label_file.shapes) |
Adds Canvas3D widget for labeling vertices on triangulated meshes via a brush UI. Built on PyQt6 + PyVista/VTK (new dev deps: pyvista>=0.43, pyvistaqt>=0.11, trimesh>=4.0). Headless-instantiable: Canvas3D.__init__ guards enable_trackball_style() and the iren-based observer install/remove paths, so the widget can be constructed in CI / unit tests where no interactive VTK render window is available. tests/test_canvas3d.py exercises the data-model layer (vertex_label_ids, the label-to-lid mapping, mesh round-trip, mode switching). The class self-skips if the VTK backend can't initialise, so the existing matrix keeps passing on bare runners. sample_meshes/ ships four small synthetic .obj fixtures (cube, cone, sphere, torus — all generated by pyvista, ~190 KB total) so users can try the feature and the test has a real on-disk mesh to load.
The macOS PR 234 cells failed with Segmentation fault: 11 — the VTK wheel on Apple Silicon GitHub runners crashes the Python process when Canvas3D() initialises without a real GL context. SIGSEGV cannot be caught by Python, so the existing try/except + SkipTest in setUpClass doesn't fire. Skip the entire test class when CI=true (set on every GitHub Actions runner). Local development still runs all 7 tests, since they exercise real value (data-model layer of the brush + vertex_label_ids storage).
Per follow-up review: - **Drop keypoint mode** for simplicity (user request). Canvas3D is now view + brush only; keypoint button gone from ViewControls3D, the KEYPOINT class constant gone, the keypoint_3d shape_type filter gone, the create_keypoint_3d action and toolbar entry gone. - **In-place paint hot path.** _apply_colors_and_render used to call _redraw_mesh on every brush stroke, which called add_mesh and rebuilt the actor — O(actor rebuild) per stroke and the dominant cost on dense meshes. Now the first paint flips a one-shot flag (_scalar_mode_active) that switches PBR -> per-vertex scalar colouring, and every subsequent stroke just mutates _main_mesh.point_data['label_colors'] and re-renders. Microbench: 50 paint+render cycles on a 530-vert sphere ~ 2 ms/stroke. - **Reuse the cursor sphere actor.** _show_cursor used to build a fresh pv.Sphere and add_mesh it on every mouse move. Now there's one unit-sphere actor created lazily; _show_cursor just SetScale + SetPosition + SetVisibility. (Verified: actor id stable across 20 _show_cursor calls.) - **Shortcut keys.** Esc -> view mode, [ -> shrink brush, ] -> grow brush. Wired through label_widget actions so they sit alongside the existing Ctrl+B (brush). Disabled until a mesh is loaded. - **Tests.** Three new test_canvas3d.py cases covering the perf contract: in-place paint must not rebuild the actor once _scalar_mode_active is True; the cursor actor is reused; KEYPOINT attribute is gone. Existing 7 tests still pass.
Mouse-move + brush paint can fire >100 Hz; rendering at that rate saturates the compositor (especially on Wayland) and makes the canvas feel frozen. Coalesce pending paint renders behind a single-shot Qt timer so the renderer sees at most ~60 fps with the latest colour state. _on_left_release flushes any pending render immediately so the final stroke state is never dropped.
Shape.paint() caches _path/_vrtx_path as a perf optimization, but two bugs made the cache stale or wrong: 1. `self.selected` is set as a plain attribute (canvas.py, label_widget.py) with no hook to invalidate _vrtx_path. Since canvas repaints every shape on load (unselected) before a user can select it, _vrtx_path was already cached empty by the time selection happened, and the rebuild condition only fired for `selected and vrtx_path is None` — so vertex handles for polygon/rectangle/etc. silently never appeared after selecting an already-painted shape. This affected every 2D shape, not just mesh-related ones. Fixed by tracking _last_selected (mirroring the existing _last_scale pattern) and simplifying the rebuild condition to fire whenever *either* cache is empty, regardless of selected state — otherwise deselecting after a selected paint left _vrtx_path at None with no rebuild trigger, crashing on the next paint() (`drawPath(None)`). 2. contains_point() wrote make_path()'s output into the same _path cache paint() uses, but make_path() builds different geometry than paint() for several shape types (e.g. "point"). Any hit-test called before a shape's first paint() would poison the cache paint() later reads. Fixed by reading the cache if populated, never writing to it from contains_point(). Also fixes tests/test_segment_anything_utils.py: _sa_with_mocks() patched PyQt6.QtCore.QPointF via raw attribute assignment with no restore, which is harmless when that file mocks a fake PyQt6 module, but corrupts the *real* QtCore.QPointF for every test running afterward once some other test file has already imported real PyQt6 (as tests/test_shape.py, added here, now does under `unittest discover`). Switched to patch.object()+addCleanup so it always restores. New tests/test_shape.py covers both cache bugs plus a fuzz sweep across shape types and random select/scale toggling.
Fixes 7 issues flagged by Copilot's automated review, all verified
against the actual code (not taken at face value):
- MESH_EXTENSIONS only listed .obj/.stl/.ply despite the PR claiming
broader pyvista format support (sample_meshes/README.md already
documented .vtk/.vtu). Expanded to a curated set of actual mesh
formats pyvista supports.
- load_mesh() swallowed all exceptions via a bare print(), leaving
self._main_mesh pointing at a stale mesh after self.clear() had
already wiped the displayed scene. Now returns True/False, logs via
the app logger, and resets _main_mesh before the try block. The
label_widget.py call site now surfaces failures via error_message()
instead of silently proceeding as if the mesh loaded.
- Switching from a mesh back to a 2D image left the 3D-only actions
(Ctrl+B brush, [ ] brush size) enabled with the 3D canvas hidden.
- decode_rle() had no input validation; malformed RLE (odd length,
negative counts) silently produced wrong-length output instead of a
clear error. load_vertex_label_ids() already wraps the call in
try/except, so this is a debuggability improvement, not a new crash
risk.
- Mesh annotation persistence was fundamentally broken, from three
compounding bugs Copilot's review + my own follow-up tracing found
together:
1. format_shape() never included vertex_indices in the saved
"shapes" JSON, so mesh shape geometry was silently dropped on
every save.
2. load_vertex_label_ids() ran before load_labels()/load_shapes()
in load_file(), so canvas_3d's label<->id mapping didn't exist
yet — every reconstructed label id resolved to nothing.
3. Canvas3D.load_shapes()'s "skip if vertex labels already exist"
guard was checked per-shape *inside* the loop that itself
writes into _vertex_label_ids — so after the first label's
vertices were written, every subsequent label in the same load
call was silently skipped. Only the first painted label ever
survived a save+reload, regardless of load order.
Fixed all three together with tests/test_canvas3d.py's new
test_multi_label_save_load_round_trip, which fails without all three
fixes and passes with them.
- Both load_shapes() and load_vertex_label_ids() emitted new_shape per
label during bulk load, on top of LabelingWidget.load_shapes()
already adding every shape to the label list directly — causing
duplicate label-list entries and marking a just-opened file dirty
via the new_shape -> _on_new_shape_3d -> set_dirty() cascade. Removed
the redundant emissions; added set_clean() after a successful mesh
label load (matching the existing 2D-image load path, which the mesh
branch was missing).
- Removed the unused `trimesh` dependency (never imported anywhere in
the codebase).
Also makes tests/test_canvas3d.py's skip condition a real runtime
capability check instead of a CI/platform guess: it now verifies VTK
is actually running on vtkOSOpenGLRenderWindow (vtk-osmesa) rather than
checking a CI env var. This was verified against actual failure modes
in this session: a real DISPLAY does NOT reliably mean it's safe to
construct a live VTK window (still hard-crashes, uncatchable, on at
least one real X11/XWayland setup tested), so DISPLAY presence was
dropped as a signal entirely in favor of checking the actual render
window class. With vtk-osmesa installed
(pip install --index-url https://wheels.vtk.org vtk-osmesa), all 15
Canvas3D tests now genuinely run and pass, rather than skip. CI now
installs vtk-osmesa on Linux for Python 3.11/3.12 (no wheel published
for 3.13 yet) so these tests get real coverage instead of an
unconditional CI skip.
Full suite verified with the vtk-osmesa backend: 121/121 tests pass
(previously only 106 non-mesh tests could even run in this session's
environment; the other 15 hard-crashed the process without it).
Canvas3D() was constructed unconditionally in LabelingWidget.__init__, so any failure there (broken pyvistaqt/VTK install, no working GL context) took down app startup for every user — including the vast majority who never open a mesh file. This session repeatedly hit VTK hard-crashing (native abort, not a catchable exception) with the regular vtk wheel in a display-less environment, which is the motivating case here, though a try/except can only help the subset of failures VTK reports as a normal Python exception — nothing at the Python level can catch a genuine native abort. Wraps Canvas3D/ViewControls3D construction in try/except; on failure, self.canvas_3d stays None and 3D actions stay disabled (they start disabled and are only enabled inside the now Canvas3D-None-gated mesh-loading path). Added None-guards to the handful of methods reachable from plain 2D usage regardless of mesh support: _update_3d_active_label (fires on any label-list selection change), _sync_all_label_colors_3d (called from the "+Add Label" flow), load_shapes()'s canvas_3d.load_shapes() call (runs on every shape load, 2D or mesh), and the 2D-branch of load_file() which unconditionally hid the 3D dock. Opening a mesh file with no working Canvas3D now shows a clear error instead of an AttributeError crash. Verified end-to-end (not just reasoned about): constructed a full MainWindow with Canvas3D() monkeypatched to raise, confirmed the app starts, confirmed the guarded methods run cleanly, confirmed opening a mesh file surfaces a clean error instead of crashing, and confirmed normal 2D image annotation works entirely normally in this degraded mode.
Found while recording a real end-to-end video of the mesh-labeling feature (load -> paint -> save -> reopen) to verify the persistence fix — not by reading code. After a simulated reopen, load_shapes() correctly reconstructed the labeled Shape objects (visible in canvas_3d.shapes / vertex_label_ids), but the mesh rendered as plain unpainted gray. The paint color data was correct; it just never made it to the screen. Root cause: _apply_colors_and_render() unconditionally set _scalar_mode_active = True after calling _redraw_mesh(), regardless of which branch _redraw_mesh() actually took. load_shapes() calls clear_shapes() first when reopening a freshly-loaded (still unpainted) mesh; clear_shapes() -> _apply_colors_and_render() -> _redraw_mesh() with has_paint=False configures a plain PBR actor (no scalar coloring), but the caller then set the flag to True anyway. The very next call in the same load_shapes() invocation — the one loading real label data — saw _scalar_mode_active already True and took the fast in-place point_data-mutation path against an actor whose VTK mapper was never told to use scalar/rgb coloring at all, so the correct color data was written to the dataset but the renderer kept showing the actor's old solid PBR material. Fixed by making _redraw_mesh() the single source of truth: it now sets _scalar_mode_active = has_paint itself, based on which branch it actually took, instead of the caller guessing. Regression test asserts the real, previously-broken signal directly — the VTK actor's mapper.GetScalarVisibility() — after load_mesh() -> load_shapes() with real label data.
vietanhdev
force-pushed
the
feat/mesh-labeling
branch
from
37f5f0b to
00a5a91
Compare
Owner
Author
|
Thanks for the review — all 9 findings verified against the code (not taken at face value) and fixed. Summary, referencing where each landed:
Also found two further bugs while actually running the feature end-to-end (recording a video of load → paint → save → reopen) rather than just reading the diff — a 2D shape-selection vertex-handle regression unrelated to mesh code, and a paint-color rendering bug that only manifested after a reload. Full details and a recording are in the updated PR description. |
The runtime-capability check I added for tests/test_canvas3d.py's skip
condition was itself unsafe: it called vtk.vtkRenderWindow() just to
read GetClassName(), assuming bare construction was inert (true on
Linux — confirmed by testing). It is not true on Windows: with no GPU
and no osmesa.dll, instantiating vtkRenderWindow() to ask its class
immediately triggers a fatal Win32 pixel-format negotiation failure
that kills the whole process before any Python-level exception can
fire — the exact class of crash this check exists to avoid, caused by
the check itself. This broke all three Windows jobs in CI (previously
green) the moment this branch was pushed.
Replaced with a pure package-metadata check
(importlib.metadata.distribution("vtk-osmesa")) that never touches a
live VTK object at all. Verified: regular-vtk env now skips cleanly
(16 skipped, no crash) instead of the previous class-name probe, and
the vtk-osmesa env still runs and passes all 16 tests unchanged.
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
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 as 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.
What this adds
Canvas3D(anylabeling/views/labeling/widgets/canvas3d.py, ~1.1k LoC) — a PyQt6 widget that embeds a PyVista/VTK render window. Modes:view— trackball orbit/pan/zoombrush— paint a label across the vertex set under the cursor (radius-based, vtkPointLocator, in-place scalar-color repaint, ~60fps render throttle)LabelingWidgetalongside the existing 2D canvas.pyvista.read/pyvista.save:.obj,.ply,.stl,.vtk,.vtp,.vtu,.glb,.gltf.pyvista>=0.43.0,pyvistaqt>=0.11.0.Rebased + hardened for merge
This PR was rebased onto current
main(picking up #240/#241/#242, three earlier edge-case crash fixes incanvas.pythat this branch also touches) and went through a second pass: a Copilot review, a manual code review, and — critically — actually running the feature end-to-end with a real (vtk-osmesa) render backend rather than just reading the diff. That last part caught two bugs neither review found.Fixed, in order of severity:
QPainterPathcaching optimization inshape.pynever invalidated on selection change, so canvas repainting a shape once (unselected) before the user could click it left the vertex-handle cache permanently empty. This affected every 2D shape in the app, not just mesh-related ones — found by testing, not code reading.format_shape()never persistedvertex_indices(Copilot caught the load-order half of this; tracing it further turned up the other two):load_vertex_label_ids()ran before the label list was populated, andload_shapes()'s "already labeled" guard was checked inside the loop that mutates the array it's checking, so only the first label in a multi-label save ever survived a reopen._apply_colors_and_render()mis-flagged the actor as scalar-mode-ready after a redraw that actually configured plain PBR material (nothing was painted yet at that instant), so the next real paint call wrote correct color data to a mapper that was never told to display it.Canvas3D()construction could crash the whole app at startup, for every user, whether or not they ever open a mesh. Now wrapped intry/except; on failure the app degrades to full 2D-only mode with a clear error if a mesh is opened instead. (A native VTK abort can't be caught by Python — this covers the failures VTK reports as a normal exception.)print()instead of a real error on mesh load failure, an incomplete mesh-format allowlist, unvalidated RLE decode, and an unusedtrimeshdependency.Test infra:
tests/test_canvas3d.pywas previously unrunnable in most environments — the regularvtkwheel can hard-crash the process (an uncatchable native abort, not a Python exception) without a display. Installedvtk-osmesa(pure-software VTK) and changed the test skip condition to check the actual render backend at runtime instead of guessing from a CI env var. CI now installsvtk-osmesaon Linux (Python 3.11/3.12 — no wheel published for 3.13 yet) so these tests get real coverage instead of an unconditional skip. Added regression tests for all of the above, including one that fails without the fix and passes with it for the multi-label persistence bug.Verified locally, live:
vtk-osmesabackend (previously only ~106 non-mesh tests could even run in a plain env).Canvas3Dwidget: load mesh → paint two labels in brush mode → orbit camera → simulate close+reopen → both labels correctly restored and rendered.What I deliberately did not touch
What's next (suggested)
Ctrl+Bbrush) once stable.