Add Volume Tasks #3712
Add Volume Tasks #3712
Conversation
|
status: creating + requesting + tracing + finishing volume tasks works
|
|
I added the necessary UI to the task creation form (but not bulk). For bulk creation, we need to think about how we want to extend the format. At the moment, every value is mandatory (except for script id). Adding a second optional parameter can get ugly. Making it mandatory is not backwards compatible. Not sure how we should handle this. |
… tracing type in dashboard
|
On a second thought, I'm not sure anymore whether it makes sense to model the skeleton / volume distinction within the actual task instances. Wouldn't it make sense to encode this within the |
There was a problem hiding this comment.
Backend code LGTM, also tested it 👍 . I noticed two things that are probably related to the frontend, probably they are not supposed to work yet: I was not able to brush or trace anything in a volume task (might be because I enabled default settings). And the task types view shows the Settings that are only related to skeleton tracings also for volume tracings.
Actually, this should work. Did you select the "brush tool" in the top bar?
Right now, these settings are very generic (e.g., move value is independent of tracing type). They include a lot of skeleton settings, but could include volume settings in the future, as well. I'd leave it as is to reduce complexity. In the future, we could still separate this, if we want to.
I just pushed the necessary changes for this. |
Sure. You can try this yourself with on the dev-instance:
In thins case I'm meant the |
There was a problem hiding this comment.
Code looks good, great that this didn't cause lots of code changes! 🎉
I'd say if the testing goes well we can deploy this as is, but in the next days I would definitely want to take an hour to go through the code and look for "tasks-are-skeleton-only" assumptions - I know that we made those a few times in the past :)
Will report back after testing!
| topLeft: boundary.lowerBoundary, | ||
| }; | ||
| } | ||
|
|
||
| export function convertPointToVecInBoundingBox(boundingBox: ServerBoundingBox): BoundingBoxObject { |
There was a problem hiding this comment.
Was this function unused now?
But we can also check that when we refactor the bounding box/upper-lower boundary mess, maybe add a comment.
There was a problem hiding this comment.
Yes, it's unused. I'd leave it as is, though, since the cost of recreating the function (in case it's needed again) is higher than having the few additional unused lines in the code base. Getting rid of some bounding box types will be more rewarding :)
|
Testing went well, great work @fm3 and @philippotto 🚢 |
URL of deployed dev instance (used for testing):
Steps to test:
Issues:
[ ] Updated documentation if applicable