brown-ccv / react-volume-viewer

Volume Viewer package for react based websites
MIT License
0 stars 0 forks source link

ref: Move useColorMap outside of the individual model #143

Closed RobertGemmaJr closed 1 year ago

RobertGemmaJr commented 2 years ago

useColorMap is now a prop of the component and not the individual models. Every model becomes grayscale when !useColorMap. Added a checkbox in the controls and handle them in state

github-actions[bot] commented 2 years ago

Visit the preview URL for this PR (updated for commit 950cf08):

https://volume-viewer--pr143-usecolormap-c9uz4wwo.web.app

(expires Thu, 08 Sep 2022 14:32:17 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

RobertGemmaJr commented 2 years ago

Can you place the "Use Color Map" checkbox at the top of the options tab? It could be hard to know that the option exists.

The only issue is differentiating which controls are for an individual model (color map, transfer function) and which are for the whole volume (useColorMap and the sliders). I could flip them so the "global" controls are on top?

User case: When the "Use Color Map" is unchecked, then modify the clip planes, and then click on the reset button. The datasets is color mapped, but the "Use Color Map" is still unchecked.

Looks like this is a bug, I'll fix it in a second

RobertGemmaJr commented 2 years ago

Okay @kmilo9999 it should be working now, let me know what you think about the order of the controls

RobertGemmaJr commented 2 years ago

Rename the label to "Use gray scale color map" and is unchecked by default. When the user checks the box, then grayscale color map is applied.

We talked about what to do a fair bit at the GSDC meeting today. I'm not sure this logic really make sense, isn't grayscale literally the absence of color?

User case: I just added a control point to the transfer function and if I click on the reset button, the point remains.

I haven't been able to recreate this problem? Whatever is added to the transfer function from the controls disappears when the reset button is clicked

RobertGemmaJr commented 2 years ago

@kmilo9999 @mirestrepo @eldu I've made a different version (#144) where the global controls sit on top of the others. Let me know what you think. Just looking at the layout for right now, I can clean up the style of 144 if that's preferred

kmilo9999 commented 2 years ago

We talked about what to do a fair bit at the GSDC meeting today. I'm not sure this logic really make sense, isn't grayscale literally the absence of color?

Well, the absence of color is black. The data itself produce a grayscale image ( or volume ). We map the data value to a RGB color located in the texture. My opinion is that "color mapping" is a technical concept. The users that have seen the app ( at least Rachel and Jeff ) talk more about grayscale that color maps. You can make the argument that the title on the tabs says "Color Map", thus makes sense to call the checkbox "Use color map". What do you think?

User case: I just added a control point to the transfer function and if I click on the reset button, the point remains.

I haven't been able to recreate this problem? Whatever is added to the transfer function from the controls disappears when the reset button is clicked

You are right, I didnt notice you have a pre-defined transfer function burnt in the example app.

RobertGemmaJr commented 1 year ago

Closing as stale