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 occasional incorrect fallback rendering #6029

Merged
merged 12 commits into from
May 12, 2022

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Feb 8, 2022

This PR fixes how the look up buffer for the GPU rendering was written to. The new code only writes a bucket into a slot of the look up texture if there is not already another bucket there with a higher quality. The old code did not check this which is why sometimes data from a coarser mag was rendered even though a finer quality was available.

Todo:

  • fix flicker when zooming

URL of deployed dev instance (used for testing):

Steps to test:

Issues:


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

@philippotto philippotto self-assigned this Feb 8, 2022
@philippotto philippotto mentioned this pull request Feb 8, 2022
14 tasks
@daniel-wer
Copy link
Member

@philippotto I think I found the culprit for the flickering and fixed it :)

I was also able to roughly observe what was happening. See this log from my debugging session (The first number indicates the bucket's zoomstep and the second number posInBuffer. I've focused on a single bucket, the one with posInBuffer 66134):

Zoomed from zoomstep 0 to zoomstep 1 ...
###### Refresh lookup buffer for zoomstep 1
Base bucket fallback written 1 66134
Fallback written 0 66134
###### Refresh lookup buffer for zoomstep 1
Base bucket fallback written 1 66134
Fallback written 0 66134
###### Refresh lookup buffer for zoomstep 1
Base bucket fallback written 1 66134
###### Refresh lookup buffer for zoomstep 1
Base bucket fallback written 1 66134
...

Basically, during the first two refreshes of the lookup buffer, after zooming out from zoomstep 0 to zoomstep 1, the activeBucketToIndexMap still contained buckets in zoomstep 0. These buckets in the finer resolution were treated as fallback buckets (although they are anti-fallback buckets ^^), because isBaseBucket was false for them (zoomStepDifference === 0). As far as I can see, all of the remaining code expected zoomStepDifference to be positive, but it was negative in that case, leading to weird effects which I didn't fully debug.
This bug existed before, but apparently had no effect, because the buckets with zoomstep 0 were never written to the lookup buffer, because the ones with zoomstep 1 were written before (the bug you fixed). Your fix to write buckets to the lookup buffer if they have a better zoomstep brought this bug to light.

@philippotto
Copy link
Member Author

Ah, awesome debugging & find! Focusing on a single bucket is a great idea (which I wish to have had myself back then ^^). I'm very happy that this turned out to be such a plausible bug and not some weird GPU problem :)

https://media.giphy.com/media/3ohzdIuqJoo8QdKlnW/giphy.gif

I'll make the PR merge-ready next.

@philippotto philippotto changed the title [WIP] Fix occasional incorrect fallback rendering Fix occasional incorrect fallback rendering May 10, 2022
@philippotto philippotto requested a review from daniel-wer May 10, 2022 09:22
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.

Thanks for making this PR merge-ready! LGTM 👍

@philippotto philippotto enabled auto-merge (squash) May 11, 2022 12:28
@philippotto philippotto merged commit fcdd954 into master May 12, 2022
@philippotto philippotto deleted the ensure-correct-fallback-rendering branch May 12, 2022 08:58
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.

Annotating an anisotropic dataset with volume resolution restrictions behaves incorrectly
2 participants