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

Color thumbnails with view config #7255

Merged
merged 13 commits into from
Sep 4, 2023
Merged

Color thumbnails with view config #7255

merged 13 commits into from
Sep 4, 2023

Conversation

frcroth
Copy link
Member

@frcroth frcroth commented Aug 8, 2023

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • Set view config of some dataset to include color
  • View thumbnail
    image

Notes

  • Only tested with one dataset and uint16 data Tested with uint8 and unit16
  • I changed the intensity value calculation to use ints instead of bytes. Since bytes are signed in jvm (not a fan), clamping from 0 to 255 and then setting toByte lead to negative values, which messed up some calculations. Now it uses positive ints, which should work just as well.
  • I used the Color class I found here and in Automatically assign color and channel name for omero ngff #7251, don't know if that class should be used.
  • Optional TODO: Respect inverted setting Done

Issues:


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

  • Updated changelog
  • Needs datastore update after deployment
  • Remove dev-only changes (Use cache again)

@frcroth frcroth self-assigned this Aug 8, 2023
@frcroth frcroth requested a review from fm3 August 8, 2023 14:01
Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

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

Sweet, looking forward to this!

  • Reusing the existing Color class is fine by me
  • Changing the data types of the intermediate results is also fine, as long as it does not create data loss
  • Passing the invert setting would be a good follow up issue :) (I guess if you consider it to be easily done, feel free to include it here as well)
  • I think there are still some overflows happening, I’m getting something like this
    image
    for a uint8 dataset (ROI2017_wkw) with color setting [200,5,5]
    The frontend rendering shows this (not the exact same location, sorry, but you get the idea)
    image

@frcroth
Copy link
Member Author

frcroth commented Aug 23, 2023

@fm3 I can't reproduce the faulty behavior. Maybe you can take another look at this?
I tested it with ROI2017 and got this (h=400, w=400)
image

@fm3
Copy link
Member

fm3 commented Aug 23, 2023

I just tried again and I can actually still reproduce it.

This his what my dataset configuration looks like (from the dataset settings view)

image

And visiting http://localhost:9000/api/datasets/sample_organization/ROI2017_wkw/layers/color/thumbnail?w=100&h=100 (with ctrl+f5 for cache-clear refresh) yields this image
image

@fm3
Copy link
Member

fm3 commented Aug 23, 2023

Looks like it does work when intensityRange is also set. 🤔 Even when it covers the full range (0,255)

@frcroth
Copy link
Member Author

frcroth commented Aug 24, 2023

Ok, I think it was once again a weirdness of the byte type, which resulted in no intesity range being different to given full intesity range. Should be fixed now.

@frcroth frcroth requested a review from fm3 August 24, 2023 10:37
Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

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

Works nicely now, thanks!

Added two more small comments about naming.

Also don’t forget to enable the caching again, then this should be good to go :)

@frcroth frcroth requested a review from fm3 September 4, 2023 07:51
Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

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

🎉

@frcroth frcroth merged commit 6c05922 into master Sep 4, 2023
2 checks passed
@frcroth frcroth deleted the thumbnail-color branch September 4, 2023 12:51
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.

Thumbnails should use layer color from view configuration
2 participants