Skip to content
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

Fix ad-hoc meshing with agglomerates and cumsum.json #7449

Merged
merged 4 commits into from
Nov 22, 2023

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Nov 21, 2023

Ad-Hoc meshing can request data from bboxes that are not aligned with buckets, and thus also not aligned with the segmentation chunks described in the cumsum.json.

This lead to an edge case where the cumsum-json-based agglomerate mapping had to gather the correct segmentation chunks that the requested bbox intersects. This is supported, but there was a bug in that logic, which is now fixed in this PR. The currDimensions used to advance to the next chunk bbox in each dimension has to be reset before each inner loop starts, so that the correct topleft for the next box can be found. Thanks to @daniel-wer and @philippotto for the debugging help :)

Other small cleanups:

  • converting the requested box to mag1 performed unnecessary arithmetic, with potential rounding errors
  • some renamings.
  • removed .toLong on things that are already Long

Steps to test:

  • Request ad-hoc mesh with agglomerate mapping (with a cumsum.json) for a segment that goes across multiple segmentation chunks in the oversegmentation. Should not fail

  • Updated changelog
  • Needs datastore update after deployment

@fm3 fm3 self-assigned this Nov 21, 2023
@fm3 fm3 marked this pull request as ready for review November 22, 2023 09:58
@fm3 fm3 requested a review from daniel-wer November 22, 2023 09:58
Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, worked well during my tests 👍

@fm3 fm3 merged commit aa6b0c0 into master Nov 22, 2023
2 checks passed
@fm3 fm3 deleted the fix-cumsum-across-boxes branch November 22, 2023 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants