Skip to content

Commit e035b12

Browse files
committed
Fix PlausibilityMonitor not picking up changes & extend checks to Bdv/Bvv
... while at it: - improve min-max detection - Fix NPE - Improve nearest point detection - Fix selection in Bdv
1 parent 920bc64 commit e035b12

5 files changed

Lines changed: 193 additions & 40 deletions

File tree

‎src/main/java/sc/fiji/snt/CurationManager.java‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -676,14 +676,15 @@ private JPanel buildLiveParamsPanel() {
676676
angleCheck != null && angleCheck.isEnabled());
677677
branchAngleMinCheckbox.setToolTipText("Warns when a child path emerges nearly parallel to its parent");
678678
branchAngleMinSpinner = new JSpinner(new SpinnerNumberModel(
679-
angleCheck != null ? angleCheck.getMinAngleDeg() : 10.0, 0.0, 90.0, 5.0));
679+
angleCheck != null ? angleCheck.getMinAngleDeg() : 10.0, 0.0, 180.0, 5.0));
680680
branchAngleMinCheckbox.addActionListener(e -> {
681681
if (angleCheck != null) angleCheck.setEnabled(
682682
branchAngleMinCheckbox.isSelected() || branchAngleMaxCheckbox.isSelected());
683683
branchAngleMinSpinner.setEnabled(branchAngleMinCheckbox.isSelected());
684684
});
685685
branchAngleMinSpinner.addChangeListener(e -> {
686686
if (angleCheck != null) angleCheck.setMinAngleDeg((Double) branchAngleMinSpinner.getValue());
687+
coupleBranchAngleBounds();
687688
});
688689
// Histogram button: both BranchAngle spinners share the same OUTSIDE-flagged
689690
// distribution. Clicking either button opens the same chart with both markers.
@@ -701,14 +702,15 @@ private JPanel buildLiveParamsPanel() {
701702
angleCheck != null && angleCheck.isEnabled());
702703
branchAngleMaxCheckbox.setToolTipText("Warns when a child path doubles back, nearly anti-parallel to its parent");
703704
branchAngleMaxSpinner = new JSpinner(new SpinnerNumberModel(
704-
angleCheck != null ? angleCheck.getMaxAngleDeg() : 170.0, 90.0, 180.0, 5.0));
705+
angleCheck != null ? angleCheck.getMaxAngleDeg() : 170.0, 0.0, 180.0, 5.0));
705706
branchAngleMaxCheckbox.addActionListener(e -> {
706707
if (angleCheck != null) angleCheck.setEnabled(
707708
branchAngleMinCheckbox.isSelected() || branchAngleMaxCheckbox.isSelected());
708709
branchAngleMaxSpinner.setEnabled(branchAngleMaxCheckbox.isSelected());
709710
});
710711
branchAngleMaxSpinner.addChangeListener(e -> {
711712
if (angleCheck != null) angleCheck.setMaxAngleDeg((Double) branchAngleMaxSpinner.getValue());
713+
coupleBranchAngleBounds();
712714
});
713715
final JButton branchAngleMaxHist = (angleCheck == null) ? null
714716
: CurationHistograms.button("Fork angle", null,
@@ -718,6 +720,7 @@ private JPanel buildLiveParamsPanel() {
718720
() -> ((Number) branchAngleMinSpinner.getValue()).doubleValue(),
719721
CurationHistograms.Side.OUTSIDE_FLAGGED, p);
720722
addCheckRow(p, c, branchAngleMaxCheckbox, spinnerWithHistogram(branchAngleMaxSpinner, branchAngleMaxHist));
723+
coupleBranchAngleBounds();
721724

722725
// Direction continuity
723726
final PlausibilityCheck.DirectionContinuity dirCheck =
@@ -1899,6 +1902,8 @@ private void addCheckRow(final JPanel panel, final GridBagConstraints c,
18991902
private JComponent spinnerWithHistogram(final JSpinner spinner, final JButton histButton) {
19001903
tightenSpinner(spinner);
19011904
if (histButton == null) return spinner;
1905+
// Listeners fire last-added first: commit pending typed text before the plot reads values
1906+
histButton.addActionListener(e -> commitSpinnerEdit(spinner));
19021907
final JPanel p = new JPanel(new BorderLayout(0, 0));
19031908
p.setOpaque(false);
19041909
p.add(spinner, BorderLayout.CENTER);
@@ -1912,11 +1917,33 @@ private JComponent spinnerWithHistogram(final JSpinner spinner, final JButton hi
19121917
* default of ~9 columns wastes horizontal space and pushes the checkbox
19131918
* label into truncation.
19141919
*/
1920+
/** Keeps min <= max: each fork angle spinner is bounded by the other's value */
1921+
private void coupleBranchAngleBounds() {
1922+
if (branchAngleMinSpinner == null || branchAngleMaxSpinner == null) return;
1923+
if (branchAngleMinSpinner.getModel() instanceof SpinnerNumberModel minModel
1924+
&& branchAngleMaxSpinner.getModel() instanceof SpinnerNumberModel maxModel) {
1925+
minModel.setMaximum(((Number) maxModel.getValue()).doubleValue());
1926+
maxModel.setMinimum(((Number) minModel.getValue()).doubleValue());
1927+
}
1928+
}
1929+
1930+
private static void commitSpinnerEdit(final JSpinner spinner) {
1931+
try {
1932+
spinner.commitEdit();
1933+
} catch (final java.text.ParseException ignored) {
1934+
// invalid text: keep last valid value
1935+
}
1936+
}
1937+
19151938
private static void tightenSpinner(final JSpinner spinner) {
19161939
if (spinner == null) return;
19171940
final JComponent editor = spinner.getEditor();
19181941
if (editor instanceof JSpinner.DefaultEditor de) {
19191942
de.getTextField().setColumns(4);
1943+
// Commit typed values as soon as they are valid: the histogram and
1944+
// Scan buttons are non-focusable, so no focus-lost commit happens
1945+
if (de.getTextField().getFormatter() instanceof javax.swing.text.DefaultFormatter df)
1946+
df.setCommitsOnValidEdit(true);
19201947
}
19211948
}
19221949

‎src/main/java/sc/fiji/snt/SNT.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5237,9 +5237,12 @@ protected void updateTracingViewers(final boolean includeLegacy3Dviewer, final b
52375237
}
52385238
}
52395239
if (getUI() != null && getUI().bdvSNT != null) {
5240-
// Bdv has no updateSelection()-equivalent yet (unlike Bvv): always do a full
5241-
// rebuild, even for selectionOnly changes. Hopefully this is cheap enough
5242-
new Thread(() -> getUI().bdvSNT.syncPathManagerList()).start();
5240+
if (selectionOnly) {
5241+
// Same rationale as for Bvv above: run on the EDT so the repaint is flushed immediately
5242+
SwingUtilities.invokeLater(() -> getUI().bdvSNT.updateSelection());
5243+
} else {
5244+
new Thread(() -> getUI().bdvSNT.syncPathManagerList()).start();
5245+
}
52435246
}
52445247
if (includeLegacy3Dviewer) update3DViewerContents();
52455248
}

‎src/main/java/sc/fiji/snt/viewer/AbstractBigViewer.java‎

Lines changed: 139 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3177,13 +3177,8 @@ protected abstract class AbstractTracer extends MouseAdapter {
31773177
// by the time mouseReleased(MouseEvent) fires, silently dropping a fork request (see #handleClick)
31783178
private boolean pressAltDown, pressShiftDown;
31793179

3180-
// Screen-space pick radius for snapping a click to a fork point (see #handleClick). It gets
3181-
// converted to a world-space distance via pickRadiusWorld() so it stays perceptually constant
3182-
// across zoom levels. Safe to keep generous: the search that consumes this is a single path
3183-
// (findNearestRenderedPath()), so a large radius can only ever match somewhere along that one
3184-
// path. Manually-placed tracing waypoints can be much sparser than densely auto-traced ones: a
3185-
// click needs to land within this radius of *some* node for a fork point to be found at all in
3186-
// the first place (see NearPoint#getInsertionIndex())
3180+
// Screen-space pick radius for snapping a click to a fork point (see #findForkHit). Also converted
3181+
// to a world-space distance via pickRadiusWorld() for the "Continue Extending" end-point tolerance
31873182
private static final double FORK_POINT_PICK_RADIUS_SCREEN_PX = 150;
31883183

31893184
// Wall-clock time (System.currentTimeMillis()) until which trace clicks are deferred because a
@@ -3311,40 +3306,36 @@ private void handleClick(final MouseEvent e) {
33113306
// modifier is an explicit request to attach to an existing branch, so if that request
33123307
// can't be honored, the click should be a clean no-op
33133308
showViewerMessage("Forking...");
3314-
// Auto-select the rendered path nearest the click, then restrict the fork-point search
3315-
// to just that one path, instead of scanning every path in the project
3316-
final Path nearestPath = findNearestRenderedPath(cc1);
3317-
// Pick radius is defined in screen pixels (see pickRadiusWorld()) rather than a fixed
3318-
// world-space distance, so "close enough to fork" stays consistent across zoom levels
3319-
final double pickRadius = pickRadiusWorld(FORK_POINT_PICK_RADIUS_SCREEN_PX);
3320-
final NearPoint nearPoint = (nearestPath == null) ? null : snt.getPathAndFillManager()
3321-
.nearestPointOnAnyPath(Collections.singletonList(nearestPath), cc1[0], cc1[1], cc1[2], pickRadius);
3322-
if (nearPoint == null) {
3323-
final String detail = (nearestPath == null) ? "No paths are currently rendered to fork from."
3324-
: String.format("No point within %.2f%s of the cursor was found on the nearest path ('%s').",
3325-
pickRadius, calUnit, nearestPath.getName());
3309+
// Fork point search happens in viewer (screen) space: depth along the viewing axis is ignored,
3310+
// since the picked 3D position (slice cursor in BDV, ray-max in BVV) rarely lies exactly on the
3311+
// path that is visibly under the cursor. A 3D distance would reject such clicks
3312+
final ForkHit hit = findForkHit(cc1, FORK_POINT_PICK_RADIUS_SCREEN_PX);
3313+
if (hit == null) {
33263314
final String mod = (renderingOptions.requireShiftToFork) ? "Alt+Shift" : "Alt";
3327-
getGuiUtils().error("<html>" + detail
3328-
+ "<br>Move closer to the target path, or click without " + mod + " to start an unconnected path.</html>",
3329-
"No Fork Point Found");
3315+
getGuiUtils().error(String.format("<html>No rendered path within %.0f screen pixels of the cursor."
3316+
+ "<br>Move closer to the target path, or click without %s to start an unconnected path.</html>",
3317+
FORK_POINT_PICK_RADIUS_SCREEN_PX, mod), "No Fork Point Found");
33303318
return;
33313319
}
3320+
final Path nearestPath = hit.path();
33323321

33333322
// Fork point found: only now commit to starting the path
33343323
snt.selectPath(nearestPath, false);
33353324
previousNode = new Path.PathNode(cc1);
33363325
highlightClickedLocation(previousNode, true);
3337-
// Prefer the interpolated point on the path's line segments over the nearest raw node:
3338-
// manually-placed waypoints can be sparser than pickRadius, so the closest actual node
3339-
// may sit well past it even when the click lands right on the path itself. Materialize
3340-
// that position as a real node so the branch point is geometrically accurate and the
3341-
// parent path visibly gains a node at the fork location
3342-
final int insertionIndex = nearPoint.getInsertionIndex();
3343-
if (insertionIndex < 0) {
3344-
previousForkPoint = nearPoint.getNode(); // already an existing node
3326+
// Materialize the closest position on the path's line segments as a real node, so that the
3327+
// branch point is geometrically accurate even when waypoints are sparse
3328+
if (hit.t() <= 1e-6) {
3329+
previousForkPoint = nearestPath.getNode(hit.segStart());
3330+
} else if (hit.t() >= 1 - 1e-6) {
3331+
previousForkPoint = nearestPath.getNode(hit.segStart() + 1);
33453332
} else {
3346-
nearestPath.insertNode(insertionIndex, nearPoint.getClosestIntersectionPoint());
3347-
previousForkPoint = nearestPath.getNode(insertionIndex);
3333+
final Path.PathNode n1 = nearestPath.getNode(hit.segStart());
3334+
final Path.PathNode n2 = nearestPath.getNode(hit.segStart() + 1);
3335+
final double t = hit.t();
3336+
nearestPath.insertNode(hit.segStart() + 1, new PointInImage(n1.x + t * (n2.x - n1.x),
3337+
n1.y + t * (n2.y - n1.y), n1.z + t * (n2.z - n1.z)));
3338+
previousForkPoint = nearestPath.getNode(hit.segStart() + 1);
33483339
syncOverlays(); // so the new node is reflected next repaint, not just on the next full sync
33493340
}
33503341
return;
@@ -3585,6 +3576,116 @@ protected double pickRadiusWorld(final double screenPx) {
35853576
return (screenPxPerWorldUnit > 0) ? screenPx / screenPxPerWorldUnit : screenPx;
35863577
}
35873578

3579+
// Screen-space distance within which two fork-point candidates are considered equally close, in which
3580+
// case the one nearer the click's depth (viewer Z) wins (see #findForkHit)
3581+
private static final double FORK_POINT_TIE_TOLERANCE_SCREEN_PX = 3;
3582+
3583+
private record ForkHit(Path path, int segStart, double t, double distPx, double absDz) {
3584+
boolean isBetterThan(final ForkHit other) {
3585+
if (other == null) return true;
3586+
if (Math.abs(distPx - other.distPx) <= FORK_POINT_TIE_TOLERANCE_SCREEN_PX)
3587+
return absDz < other.absDz;
3588+
return distPx < other.distPx;
3589+
}
3590+
}
3591+
3592+
/**
3593+
* Finds the point on any rendered path closest to a world position in viewer (screen) space,
3594+
* i.e., ignoring depth along the viewing axis. Distances are in screen pixels
3595+
*
3596+
* @param worldPos calibrated (x,y,z) position
3597+
* @param maxPx maximum accepted distance, in screen pixels
3598+
* @return the closest hit (segment start index and parametric position along that segment),
3599+
* or {@code null} if nothing is within {@code maxPx}
3600+
*/
3601+
private ForkHit findForkHit(final double[] worldPos, final double maxPx) {
3602+
final AffineTransform3D tf = getViewerTransform();
3603+
if (tf == null) return null;
3604+
final double[] c = new double[3];
3605+
tf.apply(worldPos, c);
3606+
ForkHit best = null;
3607+
final double[] a = new double[3];
3608+
final double[] b = new double[3];
3609+
final double[] w = new double[3];
3610+
for (final Tree tree : getRenderedTrees()) {
3611+
for (final Path path : tree.list()) {
3612+
if (!path.isActiveVersion() || path.size() == 0) continue;
3613+
Path.PathNode node = path.getNode(0);
3614+
w[0] = node.x; w[1] = node.y; w[2] = node.z;
3615+
tf.apply(w, a);
3616+
if (path.size() == 1) {
3617+
final double d = Math.hypot(a[0] - c[0], a[1] - c[1]);
3618+
if (d <= maxPx) {
3619+
final ForkHit h = new ForkHit(path, 0, 0, d, Math.abs(a[2] - c[2]));
3620+
if (h.isBetterThan(best)) best = h;
3621+
}
3622+
continue;
3623+
}
3624+
for (int i = 1; i < path.size(); i++) {
3625+
node = path.getNode(i);
3626+
w[0] = node.x; w[1] = node.y; w[2] = node.z;
3627+
tf.apply(w, b);
3628+
final double dx = b[0] - a[0];
3629+
final double dy = b[1] - a[1];
3630+
final double len2 = dx * dx + dy * dy;
3631+
double t = (len2 > 0) ? ((c[0] - a[0]) * dx + (c[1] - a[1]) * dy) / len2 : 0;
3632+
t = Math.max(0, Math.min(1, t));
3633+
final double d = Math.hypot(a[0] + t * dx - c[0], a[1] + t * dy - c[1]);
3634+
if (d <= maxPx) {
3635+
final double dz = Math.abs(a[2] + t * (b[2] - a[2]) - c[2]);
3636+
final ForkHit h = new ForkHit(path, i - 1, t, d, dz);
3637+
if (h.isBetterThan(best)) best = h;
3638+
}
3639+
System.arraycopy(b, 0, a, 0, 3);
3640+
}
3641+
}
3642+
}
3643+
return best;
3644+
}
3645+
3646+
private sc.fiji.snt.analysis.curation.PlausibilityMonitor liveMonitor() {
3647+
final sc.fiji.snt.SNTUI ui = snt.getUI();
3648+
if (ui == null) return null;
3649+
final sc.fiji.snt.analysis.curation.PlausibilityMonitor m = ui.getPlausibilityMonitor();
3650+
return (m != null && m.isEnabled()) ? m : null;
3651+
}
3652+
3653+
private void clearLiveCheckContext() {
3654+
final sc.fiji.snt.SNTUI ui = snt.getUI();
3655+
if (ui != null) ui.getPlausibilityMonitor().clearForkContext();
3656+
}
3657+
3658+
/**
3659+
* Mirrors SNT's classic hooks (fork initiated + segment completed) for the first segment of a
3660+
* new path: a forked segment already carries its parent path and branch point index
3661+
*/
3662+
private void runLiveChecksOnFirstSegment(final Path first) {
3663+
final sc.fiji.snt.analysis.curation.PlausibilityMonitor m = liveMonitor();
3664+
if (m == null) return;
3665+
final Path parent = first.getParentPath();
3666+
if (parent == null) {
3667+
m.clearForkContext();
3668+
return;
3669+
}
3670+
m.onForkInitiated(parent, first.getBranchPointIndex());
3671+
reportLiveWarnings(m.onSegmentCompleted(first));
3672+
}
3673+
3674+
/** Mirrors SNT's classic path finalization hook */
3675+
private void runLiveChecksOnFinishedPath(final Path finished) {
3676+
final sc.fiji.snt.analysis.curation.PlausibilityMonitor m = liveMonitor();
3677+
if (m == null) return;
3678+
if (finished.getParentPath() == null) {
3679+
m.clearForkContext();
3680+
return;
3681+
}
3682+
reportLiveWarnings(m.onPathFinalized(finished));
3683+
}
3684+
3685+
private void reportLiveWarnings(final java.util.List<sc.fiji.snt.analysis.curation.PlausibilityCheck.Warning> w) {
3686+
if (w != null && !w.isEmpty()) showViewerMessage("Warning: " + w.getFirst().message());
3687+
}
3688+
35883689
/**
35893690
* Commits {@code tempPath} to the Path Manager and resets tracing state. Called directly
35903691
* from {@code mouseReleased} when no segment is in flight, or deferred from {@code done()}
@@ -3619,6 +3720,7 @@ private void finishPath() {
36193720
snt.getPathAndFillManager().addPath(tempPath);
36203721
if (renderingOptions.activateFinishedPath) snt.selectPath(tempPath, false);
36213722
showViewerMessage("Path finished");
3723+
runLiveChecksOnFinishedPath(tempPath);
36223724
}
36233725
clearTracingOverlayContents();
36243726
syncPathManagerList();
@@ -3708,6 +3810,7 @@ protected void discardCurrentPath() {
37083810
setToBeCombinedPath(null); // discard a pending "Continue Extending" request too, if any
37093811
previousNode = null;
37103812
tempPath = null;
3813+
clearLiveCheckContext();
37113814
pendingFinish = false;
37123815
computing = false;
37133816
confirmedSegmentSizes.clear();
@@ -3775,6 +3878,7 @@ protected void done() {
37753878
// so the in-progress segment visually pairs with the path it will extend
37763879
if (toBeCombinedPath != null) tempPath.setSelected(true);
37773880
pushConfirmedSegmentSize(result.size());
3881+
runLiveChecksOnFirstSegment(result);
37783882
} else {
37793883
final int sizeBefore = tempPath.size();
37803884
tempPath.add(result);
@@ -3930,9 +4034,11 @@ public void actionPerformed(final java.awt.event.ActionEvent e) {
39304034
}
39314035

39324036
private void highlightClickedLocation(final Path.PathNode node, final boolean highlightAsForkPoint) {
4037+
// Bdv returns null here (bookmarks use their own color cycle)
4038+
final Color base = (getDefaultMarkerColor() != null) ? getDefaultMarkerColor() : Color.YELLOW;
39334039
ensureTracingOverlay().addAnnotation(node,
39344040
(highlightAsForkPoint) ? getDefaultMarkerSize() * 2 : getDefaultMarkerSize(),
3935-
(highlightAsForkPoint) ? SNTColor.contrastHueColor(getDefaultMarkerColor(), Color.BLACK) : getDefaultMarkerColor());
4041+
(highlightAsForkPoint) ? SNTColor.contrastHueColor(base, Color.BLACK) : base);
39364042
}
39374043

39384044
private void drawSegment(final Path segment) {

0 commit comments

Comments
 (0)