-
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
Make ad-hoc meshes compatible with ND segmentations #7394
Conversation
…e.getState() occurences
frontend/javascripts/oxalis/controller/segment_mesh_controller.ts
Outdated
Show resolved
Hide resolved
… of known cycles is always correct
…leminds/webknossos into adhoc-mesh-additional-coordinates
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 refactoring 🎉 The code looks way more concise now :) I left a couple of comments, but mostly only small stuff and some questions for clarifications. The finish line is visible 🏁 😃
frontend/javascripts/oxalis/controller/segment_mesh_controller.ts
Outdated
Show resolved
Hide resolved
frontend/javascripts/oxalis/model/reducers/annotation_reducer.ts
Outdated
Show resolved
Hide resolved
frontend/javascripts/oxalis/model/reducers/annotation_reducer.ts
Outdated
Show resolved
Hide resolved
one last thing... the meshes are always requested for the same position, even if the additional coordinates change. I think sometimes when creating annotation, this can lead to a bug. If I create an annotation and create segments in multiple dimensions, and I load a mesh for segment A in dimension 1, I will have to click on the segment in dimension 2 before being able to load the mesh in that dimension. However, this occurs only in the beginning of creating the dataset and I wasn't able to reproduce it afterwards. It seems to be a separate bug that has to do with the segment's position, so I feel like it's a separate issue. |
Excellent work! The code looks very good now and testing went well, too :) 🕺 Let's ship it! Kudos for pulling this through the finish line :)
Let's iron it out later then :) |
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.
Good catch, thanks! This didn't occur in my test dataset ( |
4f4755c
to
7d92806
Compare
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.
Thank you for fixing this! 😃 I pushed a tiny commit, because I noticed that two methods had an implicit any
return type. Feel free to merge now 💪
URL of deployed dev instance (used for testing):
@dieknolle3333 I think backend should be completed, though I have not tested it yet.
Steps to test:
TODOs:
additionalCoordinates
getAdditionalCoordinatesString
to method, e.g. in flycam_accessorIssues:
(Please delete unneeded items, merge only when none are left open)