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

Ensure that the interpretation of bounding boxes is always half-open #4690

Merged
merged 2 commits into from
Jul 1, 2020

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Jul 1, 2020

When a bounding box is defined with [0, 0, 0, 10, 10, 10], the position [10, 10, 10] should be outside of the bounding box. This interpretation was not consistent within the code base which I cleaned up with this PR. I tried to go through all the relevant code and double-check / fix it where necessary. Predominantly, the following two cases needed fixing:

  • the dataset position view
  • the rendered cube mesh (since threeJS renders the mesh always inclusively, I subtracted an epsilon there)

URL of deployed dev instance (used for testing):

Steps to test:

  • I did the following:
    • open a task with a bounding box (e.g., 0, 0, 0, 10, 10, 10)
    • initialize the api with window.webknossos.apiReady(3).then(api => window.api = api)
    • test the rendering with api.tracing.centerPositionAnimated([8, 5, z]) where z can be something like 9, 9.9, 10, 10.01 etc.
    • before this PR, being at z=10 and z=10.1 behaved differently (one showed the box, the other did not). with this pr, both positions act correctly (i.e., behave like one is outside of the bounding box)

Issues:


@philippotto philippotto requested a review from daniel-wer July 1, 2020 09:30
@philippotto philippotto self-assigned this Jul 1, 2020
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.

Works great, thanks for having a detailed look at this 👍

@philippotto philippotto merged commit 590b4b0 into master Jul 1, 2020
@philippotto philippotto deleted the make-bbox-consistent branch June 14, 2022 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bounding Box off-by-one errors
2 participants