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 zooming out for datasets with very large scale #6304

Merged
merged 5 commits into from
Jun 28, 2022

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Jun 23, 2022

Respect the dataset scale when calculating the maximum iteration limit for the max zoom value determination.

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • find a dataset which goes until mag 1024
  • set a dataset scale to 600, 600, 1
  • open the dataset and maximize the XY viewport
  • zoom out (without this PR, zooming out was not possible until mag 1024)

Issues:


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

@philippotto philippotto self-assigned this Jun 23, 2022
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 fix, but I'm not sure whether the maximum iteration count is computed correctly, see my comment :)

// From that, we calculate the theoretical maximum zoom value. The dataset scale is taken into account,
// because the entire scene is scaled with that.
const maxSupportedZoomValue = 2 ** MAX_SUPPORTED_MAGNIFICATION_COUNT * Math.max(...datasetScale);
const maximumIterationCount = Math.log(maxSupportedZoomValue) / Math.log(ZOOM_STEP_INTERVAL);
Copy link
Member

Choose a reason for hiding this comment

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

I think this would be the correct iteration count if the start zoom value were 1. However, it is 1 / ZOOM_STEP_INTERVAL ** 20 so 20 would need to be added.

Copy link
Member Author

Choose a reason for hiding this comment

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

Very good point 👍 I adapted the code.

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 👍

@philippotto philippotto enabled auto-merge (squash) June 28, 2022 17:15
@philippotto philippotto merged commit b917b84 into master Jun 28, 2022
@philippotto philippotto deleted the flexible-max-zoom branch June 28, 2022 17:39
hotzenklotz added a commit that referenced this pull request Jul 4, 2022
…type

* 'master' of github.com:scalableminds/webknossos:
  Ensure unique volume layer names when changing names in frontend (#6289)
  X-Auth Tokens everywhere (in datastore & tracingstore) (#6312)
  add mapping name param for rawcuboid (#6311)
  Fix zooming out for datasets with very large scale (#6304)
  Release 22.07.0 (#6307)
  added Sahil's publication to list
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.

Datasets with a large scale might not allow zooming to coarsest mags
2 participants