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

Also load up to three fallback layers for 'best quality first' #4470

Merged

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Mar 9, 2020

Right now, the rendering strategies define how many fallback magnifications will be loaded so that these can be rendered. For progressive loading, three fallback magnifications are loaded. For best quality, only one fallback mag was loaded until now. The reasoning behind this was that additional fallbacks shouldn't be necessary (since they are loaded with lower priority). However, this had the effect that missing data couldn't be covered by fallback data if that fallback was more than one mag apart.
I simply changed the value of how many fallback mags should be loaded to three. This can produce more traffic, but since the lower mags have lower priority when using "best quality first", this shouldn't affect the perceived loading speed.

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • Open a dataset for which data only exists in mag 4 or mag 8.
  • Select "best quality first" and disable "render missing data black"
  • Data should be visible when zooming to mag 1

Issues:


@philippotto philippotto self-assigned this Mar 9, 2020
@MichaelBuessemeyer
Copy link
Contributor

MichaelBuessemeyer commented Mar 9, 2020

Looks to me like this PR also resolves #4329 🧐

Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

The Also load up to three fallback layers for 'best quality first'-part works perfectly 👍

In the changes in the comment tab, I found only one thing in the code.

While testing the memoized version of the comment tab, I noticed that either the diffing between versions of the trees or that the relevant actions are not complete. At least these are my guesses.

Here is the "bug free" master:
comment_tab_not_buggy

And here is your branch on my local machine:
comment_tab_buggy
Could you please fix this before merging?

@MichaelBuessemeyer
Copy link
Contributor

MichaelBuessemeyer commented Mar 10, 2020

btw: Awesome idea to also memoize the tree-data 🚀

@philippotto philippotto force-pushed the deeper-fallback-rendering-for-best-quality-first branch from d8a3817 to 5e6f1dd Compare March 11, 2020 10:23
@philippotto
Copy link
Member Author

philippotto commented Mar 11, 2020

Looks to me like this PR also resolves #4329 🧐

Sorry, this was not my intention. I removed these changes again and created a new PR for them. Could you please re-review/accept the current PR? :)

@MichaelBuessemeyer MichaelBuessemeyer self-requested a review March 11, 2020 13:18
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

The Also load up to three fallback layers for 'best quality first'-part works perfectly 👍

I suppose that this still works as I cannot test this again at the moment. But this should not block this PR 😄

Feel free to merge

@bulldozer-boy bulldozer-boy bot merged commit 6f08833 into master Mar 17, 2020
@bulldozer-boy bulldozer-boy bot deleted the deeper-fallback-rendering-for-best-quality-first branch March 17, 2020 13:54
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.

"Best quality first" does not respect disabled "render missing data black" option
2 participants