hms-dbmi / viv

Library for multiscale visualization of high-resolution multiplexed bioimaging data on the web. Directly renders Zarr and OME-TIFF.
http://avivator.gehlenborglab.org
MIT License
284 stars 46 forks source link

Max channels allowed #687

Open vadzimvashchanka opened 1 year ago

vadzimvashchanka commented 1 year ago

Hey! I'm using IF slides with 8 IF channels. Is there a way do display them all at once? Are there any cons against increasing the limit here? https://github.com/hms-dbmi/viv/blob/master/packages/constants/src/index.ts

vadzimvashchanka commented 1 year ago

@ilan-gold @manzt anyone?

manzt commented 1 year ago

Hi there. IIRC the limit is primarily set due to our shader implementation (which requires a fixed number of channels) and to account for the challenges in human color perception. While technically feasible to display more channels simultaneously, the benefit is increasingly limited due to the difficulty in distinguishing a high number of colors at once.

That being said, I'm open to understanding your use case and happy to be convinced otherwise!

ilan-gold commented 1 year ago

I would agree with Trevor here. I would add that if someone (ideally not us) were willing to write the necessary code to make the number arbitrary, I would be interested in that. It would be mostly just string manipulation/shader injection. @manzt would be curious to hear your thoughts.

manzt commented 1 year ago

Right. We have an open discussion on this topic as well. Supporting such a feature would indeed require manipulating shaders at runtime, but iirc, recompiling shaders often depending on the number of channels might not be desireable perf wise.

ilan-gold commented 1 year ago

I think this could be mitigated with a maxAllowableChannels (or similar) prop. Default could be recompilation but people could set the prop if they don't want it to be dynamic

vadzimvashchanka commented 1 year ago

Hi guys! Thank you for your response. So the context is the following. I'm a dev who takes part in integration of your package into the portal to visualize IF slides. Usually we have from 3 to 6 channels. But recently the users complained that they can't see some slides. There was an error in console about max number of channels allowed (6) for slides which have 8 channels. So as I see there is no way to avoid this error except forcing the app to display max 6 channels at once. Anyways the user is able to switch the channels and display a preferable list so it's not a big issue. But it would be great to have this value configurable ideally though props as @ilan-gold suggested so the package users could decide if they want to display more slides channels (prolly with some sacrifice for performance) or less. image

ilan-gold commented 1 year ago

Let's see what Trevor has to say. Others (@Nanguage and @YammyHuang) have the same request. I would be open to one of you implementing a solution, although want to see what Trevor's take is.

vadzimvashchanka commented 1 year ago

@manzt what do you think?

manzt commented 1 year ago

Yes, I am for supporting an implementation! I am not sure I'll have the time to implement myself but would be happy to provide a review.

I think this could be mitigated with a maxAllowableChannels (or similar) prop. Default could be recompilation but people could set the prop if they don't want it to be dynamic.

I think ideally we should avoid exposing more props. Leaky abstraction. We just want to avoid recompiling shaders on every render (e.g., when we adjust contrast limits or change colors), but I think recompiling shaders if channels are added or removed should probably be OK. Not a super frequent interaction, and the UI for triggering adding/removing channels is typically some kind of dropdown with clicks.

ilan-gold commented 1 year ago

Ok @manzt I could see there being low-cost to that and/or a way to avoid it. So if anyone wants to implement this, we would take it. I may also have time to do this next month but am also not sure I will have the time.

tymiao1220 commented 1 year ago

Thank you for this nice application. We have the same request for multiple channels visualization. Currently, it can handle 6 channels as expected when we try to add more, the entire image disappears. So we now only add to max 6 channels and then switch between each channel to avoid that.

tymiao1220 commented 1 year ago

@ilan-gold @manzt Do you have any plan of this?

manzt commented 1 year ago

As @ilan-gold mentioned, we are in support of and would be willing to review a PR implementing this feature, but do not have a plan to implement ourselves.

xinaesthete commented 11 months ago

I might consider making a PR for this if I have time - maybe relatively soon.

I wouldn't be concerned about the performance cost of recompiling shaders at runtime as the number of channels changes; the shaders are relatively simple.