-
Notifications
You must be signed in to change notification settings - Fork 22
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
Allow to convert agglomerate skeletons to normal ones #7537
Conversation
… name already exists
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.
Great :) I left a comment regarding the bug fix. maybe it's worth to incorporate this.
frontend/javascripts/oxalis/view/right-border-tabs/tree_hierarchy_view.tsx
Outdated
Show resolved
Hide resolved
frontend/javascripts/oxalis/view/left-border-tabs/mapping_settings_view.tsx
Outdated
Show resolved
Hide resolved
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.
Testing went very smooth 👍 I noticed only two things:
When trying to modify an agglomerate skeleton with the skeleton tool, this message appears (rightfully):
Agglomerate skeletons can only be modified when using the proofreading tool to add or delete edges.
There, you could add "Consider switching to the proofreading tool or converting the skeleton to a normal tree via right-click in the Skeleton tab." or something like this.
I wondered how undo/redo worked for this conversion. It turns out to work well 👍 I know that undo/redo is not supported in the proofreading tool, but one can switch to the skeleton tool and then undo stuff that was done in the proofreading tool. I assume that this is not new to this PR and also not too easy to fix. However, I wanted to mention it here since it caught my attention. The impact is hopefully not too bad? The proper goal would probably be to allow undo/redo in the proofreading tool...
Thanks for testing! I incorporated your first suggestion 👍 Regarding the second one, you are correct, that's already been the case before this PR. I'd second your opinion to rather support undo/redo for the proofreading tool in the near future :) |
I also fixed the initialization of the mapping list for agglomerate skeletons iff json mappings are present. This was broken due to a regression before. The
mappings
property of a layer might already be set when initially obtaining the layer information, so it's not sufficient to check for that when checking whether mappings (and agglomerates) were already loaded.URL of deployed dev instance (used for testing):
Steps to test:
TODOs:
Issues:
(Please delete unneeded items, merge only when none are left open)