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 displayed segment #4480

Merged
merged 6 commits into from
Mar 24, 2020
Merged

Fix displayed segment #4480

merged 6 commits into from
Mar 24, 2020

Conversation

MichaelBuessemeyer
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer commented Mar 16, 2020

This PR adds looking for segmentation ids in higher resolutions when the current resolution has no data and render missing data black is disabled for the segmentation tab/mapping_info_view.

URL of deployed dev instance (used for testing):

  • _.webknossos.xyz

Steps to test:

  • It is best to test this on a local machine as manipulating datasets is easier.
  • Create a dataset
    • with at least one color layer with the resolutions 1-1-1, 2-2-2, 4-4-4 and so on
    • and with one segmentation layer that does not have the resolution 1-1-1.
  • Open the dataset in view mode, disable render missing data black and open the segmentation tab
  • Now zoom into mag 1
  • The shown segmentation should be the segmentation from mag 2.
  • The displayed segment ids in the segmentation tab should match the displayed data as well as there should be a new entry in the table of the tab displaying the resolution that is used to determine the segmentation ids.

Issues:


@MichaelBuessemeyer
Copy link
Contributor Author

@philippotto I would like to make the solution a bit more fancy:
I want to add an error message in case no valid segment ids could be found. I would position the error text right below the table.

My problem is that I cannot determine when there actually is no data when render missing data black is turned on. The reason is, that for the missing resolutions buckets filled with 0's are created which actually have data, but only fake data. Do you know why do determine whether a bucket is filled with fake data or with real data? Other than the "hasData" method from the bucket?

@philippotto
Copy link
Member

@philippotto I would like to make the solution a bit more fancy:
I want to add an error message in case no valid segment ids could be found. I would position the error text right below the table.

I'm not sure whether that's really necessary. What would be the usecase? For normal EM data, we don't show any information like that either. I'm afraid this makes the code and UI more convoluted without adding much value 🙈 But maybe I'm not understanding the usecase yet. So, feel free to argue back :D

@MichaelBuessemeyer
Copy link
Contributor Author

But maybe I'm not understanding the usecase yet.

My idea was to simply inform the user with a warning that the display segment id (0) is wrong and also tell why that might be the case. I especially had the dataset layer settings in mind. In the settings tab, a warning info icon is rendered when not all layers are visible. That is what I am thinking about here.

But on the other hand, the warning might not be necessary when we follow the rule "what you see is what you get". Because the warning would only be shown if no segmentation data can be found and rendered. Thus it might be obvious that the displayed 0 as segment id is wrong.

Leaving this point aside, I would be grateful if you would review the current code 😄

Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

See my one comment :)

@MichaelBuessemeyer
Copy link
Contributor Author

@philippotto Thanks for paying attention and finding my mistake 👍. I implemented your feedback. Could you please review the newest changes again :D?

Now the resolution/magnification has its own column and the zoom step for the mouse position is determined on its own. I tried to keep the code as understandable as possible. If you have any suggestions on how to improve it, I'll be glad to implement them.

Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

Awesome 🎉

@MichaelBuessemeyer MichaelBuessemeyer merged commit e224316 into master Mar 24, 2020
@philippotto philippotto deleted the fix-displayed-segment-id branch June 14, 2022 11:36
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.

"Segment ID at position" can be wrong in certain conditions
2 participants