Fix segment registering in coarser mags for non-mag-aligned bounding boxes #8364
Conversation
📝 WalkthroughWalkthroughThe pull request introduces changes to WEBKNOSSOS, focusing on new features for dataset rotation configuration, streamlined annotation versioning, and enhanced bounding box handling. Users can initiate a split-merger evaluation during neuron inference, and the "Restore older Version" feature now consolidates annotation history. Bug fixes address issues with tree merging and segment registration in non-mag-aligned bounding boxes. The changelog also notes the removal of volume annotation downsampling. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| // getDataForBoundingBox grows the bounding box to be mag-aligned which can change the dimensions | ||
| const boundingBoxInMag1MagAligned = boundingBoxInMag1.alignWithMag(currentMag, "grow"); |
There was a problem hiding this comment.
Just for my understanding, where can I find the code in which getDataForBoundingBox might be growing the bbox?
There was a problem hiding this comment.
It's quite hidden because the code is complex, which is unfortunate. getDataForBoundingBox requests all buckets that intersect with the mag 1 bounding box (getBucketAddressesInCuboid) so the gathered data usually is larger than the mag 1 bounding box, because the bounding box is not aligned to 32³ buckets. Then, cutOutCuboid is called which converts the mag 1 bounding box to the request mag by using the scaleGlobalPositionWithMagnification method. This method has a ceil argument which is not set for top left (so that is floored) and set for bottom right. This possibly grows the bounding box (when thinking about it in mag 1 coordinates, which is what is done to determine the segment positions).
Does that make sense?
There was a problem hiding this comment.
Okay, that makes sense! I understand that to align the bounding box with the given mag, both in cutOutCuboid->scaleGlobalPositionWithMagnification the topLeft (min) is floored and the bottomRight (max) is ceiled, which is the same as what is happening in alignWithMag when it's called with the argument "grow".
When registering segments for a bounding box in a mag other than the finest one and if the bounding box is not mag-aligned it was not taken into account that the
getDataForBoundingBoxmight grow the bounding box to be mag-aligned. In that case more data is returned and the indexing may change, resulting in incorrect segment positions.URL of deployed dev instance (used for testing):
Steps to test:
(Please delete unneeded items, merge only when none are left open)