-
Notifications
You must be signed in to change notification settings - Fork 24
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
Rescale layers for different voxel sizes on remote import #7213
Conversation
@frcroth I just pushed the necessary frontend changes. let me know if you need anything else 📫 |
@philippotto |
If I see it correctly, the backend defines the coordinate transformations once for the entire datasource. However, the transformations should be defined per layer. For example, if there are two layers (with differing voxel sizes), the first shouldn't have a transformation, but the second should get one so that it matches the 1st. Does this make sense? |
b793ccc
to
e1b0a5c
Compare
@philippotto Yes, thank you, your comment helped me and now it works 🎉 |
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.
Backend looking good, just left two cosmetic suggestions.
As I mentioned before, this variant is not going to be able to register situations where e.g. the segmentation exists only starting at mag2 (but theoretically has the same voxel size). With this, it will register both starting at their individual mag1 with a transformation. In that particular case, my guess is that we would prefer calling it mag2, and have a uniform voxel size. However, I don’t know how to properly distinguish which case we want, with the limited information we are getting.
@philippotto Do you have an idea how this distinction could be made? I guess we could look for power-of-two divisibility, but that may yield false positives.
Otherwise, I’d say it is fine as is. One could also argue it is more correct to always call the “finest” existing mag of a layer mag1.
I've thought a bit about this and think that this is a pretty good heuristic. In the rare event of a false positive, the rendering should still be fine. Enumerating the mags differently could still be seen as a plausible way of bringing two layers together, in my opinion. |
Ok, in this case, I’d like to ask you @frcroth to add this heuristic. I assume an epsilon check is needed for checking the divisibility. For a test dataset, you could use the zarr streaming links from a webknossos instance (directly link to a mag, so no ngff metadata is used) |
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.
If I read the code correctly, the rounding of minVoxelSize/preferredVoxelSize the could be a problem. If one layer has 2.3 times the other’s voxel size, this might be wrongly detected as mag2, right? I think the isPowerOfTwo check has to be done on the doubles there, possibly with epsilon comparison.
Apart fromt hat, it looks good to me now :)
@fm3 Was this what you had in mind? |
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, perfect!
URL of deployed dev instance (used for testing):
Steps to test:
To-dos:
Issues:
(Please delete unneeded items, merge only when none are left open)