Skip to content

Conversation

@ThomasKierski
Copy link
Contributor

Should be rebased if #63 is merged

This PR modifies the Fast Marching editor effect to store and display a segmentation preview in a new segmentation node, rather than overwriting the input segment with preview data. This has the benefit of not losing the original segment data in certain scenarios, eg, the user saving the scene and closing during the preview step.

I am marking this PR as draft for the time being because I have a couple of questions:

  1. Would it be preferable to have this effect inherit from AbstractScriptedSegmentEditorAutoCompleteEffect? I started down this path initially but found that it would require a bit more work than originally anticipated to get things working. The work in this PR was the path of least resistance.
  2. Should the original segment be hidden during preview to maintain the look and feel of the Fast Marching effect in its current state?

@lassoan
Copy link
Owner

lassoan commented Sep 23, 2025

Thanks a lot for working on this! I think with such changes this effect will work much better!

Yes, we should use AbstractScriptedSegmentEditorAutoCompleteEffect as a basis, as it makes implementation, behavior, and appearance more consistent between various seed effects (grow from seeds, fill between slices, watershed). It also answers your question of how to display the preview exactly (the base class takes care of it), it has some extra features (allows showing of preview in 3D, etc.), and works well if the scene is saved and reloaded. In the future if we fix anything in the common base or add new features, then this effect will automatically benefit from it, too.

Since computation of this effect is much faster than most others (almost instant, while others may take a few seconds or more), it could make sense to use a shorter delay if the update is triggered by moving the slider (e.g., it could be 0.1s instead of the current 1.0s). If the input segment is changed (e.g., you go to paint or erase effect and make some changes) then for those updates keeping the current 1s update delay is fine.

Since this effect only works on a single segment, you probably need to add a flag to the base class that determines what segments are used by the effect (all visible segments - as it is done in all existing effects; or the currently selected segment only).

If you invest time into this effect and make it so much better then it would worth implementing masking for it, too (#61). It would make this effect much more powerful and versatile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants