-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Paint tool on startup fixes #507
Conversation
✅ Deploy Preview for volview-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
src/store/tools/paint.ts
Outdated
if (!(segValue in segments.byValue)) | ||
throw new Error('Segment is not available for the active labelmap'); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain how this block of code affected the paint tool not loading properly through a session file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. usePaintStore.deserialize
calls setActiveSegment
before activeSegmentGroupID
is set. Later, activeSegmentGroupID will be set by useToolStore.setCurrentTool
-> usePaintStore.activateTool
-> setActiveLabelmapFromImage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think then that we can solve this by calling setActiveLabelmap(segmentGroupIDMap[paint.activeSegmentGroupID])
before setActiveSegment
in deserialize
. I think keeping the code in setActiveSegment
is good to ensure we are getting a valid segment value. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to keep the checks. I rebased a fix. Turns out segmentIDMap was not getting passed down to paint.deserialize
. Also, can't setActiveSegment
without a segmentGroup, so only calling setActiveSegment
if one is deserialized.
Overall LGTM! My quick functional test didn't reveal any issues. I just had one clarifying question. |
If session.volview.zip is loaded with the paint tool initially selected, clicking on the image did not paint. Fix is to pass correct idMap to paint tool and only `setActiveSegment` when a segmentGroup is selected.
If paint tool is activated when mouse is within a view, the stencil shape would not be correct. So call setSliceAxis at widget startup.
Small paint tool fixes: