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

Prevent 'negative' buckets from being created #7124

Merged
merged 9 commits into from
Jun 7, 2023

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Jun 5, 2023

URL of deployed dev instance (used for testing):

Steps to test:

  • brush at negative positions (nothing should happen)
  • check that no buckets are sent to the server
  • note that updateSegments will still be created with negative anchor positions, but I think it's alright (the case is very unlikely anyway and shouldn't do any harm)

Issues:


(Please delete unneeded items, merge only when none are left open)

@philippotto philippotto self-assigned this Jun 5, 2023
Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

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

Backend LGTM

@daniel-wer
Copy link
Member

@philippotto Could you explain the fix? As far as I can see, the tracing's bounding box being negative caused "negative" buckets to be created and sent to the server. How did the tracing's bounding box become negative and shouldn't this be prevented, too?

@philippotto
Copy link
Member Author

@philippotto Could you explain the fix? As far as I can see, the tracing's bounding box being negative caused "negative" buckets to be created and sent to the server. How did the tracing's bounding box become negative and shouldn't this be prevented, too?

The DataCube instantiates the BoundingBox class and stores that in this.boundingBox. That box is used to prevent buckets from being created outside of the DataCube's volume.
The BoundingBox class itself uses -Infinity and +Infinity as defaults for min and max. These defaults were partially overriden. Namely:

  • when a volume task exists, min and max were overriden (--> no problems, in that case)
  • otherwise, only max was overriden (with this.upperBoundary)

Thus, if no volume task existed, min was left at its default of -Infinity. To fix this, I changed the code to always intersect with min=0,0,0 and max=this.upperBoundary.

I hope, this makes sense. Otherwise, let's call about this 🤙 =)

@daniel-wer
Copy link
Member

Thank you, that makes sense 👍 Does it make sense to use the lower boundary of the dataset instead, then? Otherwise, the behavior is inconsistent with the upper boundary being strict and the lower boundary always being at [0, 0, 0].

@philippotto
Copy link
Member Author

Thank you, that makes sense +1 Does it make sense to use the lower boundary of the dataset instead, then? Otherwise, the behavior is inconsistent with the upper boundary being strict and the lower boundary always being at [0, 0, 0].

Good point! I also refactored the code a bit so that we have one bbox type less. please have another look :)

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.

Nice cleanup, LGTM! 👍

@philippotto philippotto enabled auto-merge (squash) June 7, 2023 14:59
@philippotto philippotto merged commit 889a87a into master Jun 7, 2023
@philippotto philippotto deleted the no-negative-brushing branch June 7, 2023 15:12
hotzenklotz added a commit that referenced this pull request Jun 8, 2023
…esign-right-sidebar

* 'master' of github.com:scalableminds/webknossos:
  Create bounding box by dragging with box tool (#7118)
  Prevent 'negative' buckets from being created (#7124)
  Lazy load onnx and canvas2html module (#7121)
  Disable editing of super voxel skeletons in skeleton mode (#7086)
  add missing evolution to migration guide (#7126)
  Change sttp backend to HttpURLConnectionBackend (#7125)
  Implement Zarr v3 and sharding codec (#7079)
  Fix decompression of garbage data after valid gzip data causing decompression to fail (#7119)
  When scanning volume buckets, skip those with unparseable key (#7115)
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.

Volume annotations should not save buckets with negative bucket position
3 participants