[WIP] FEATURE: Convenience functions for volume rendering transfer functions#178
[WIP] FEATURE: Convenience functions for volume rendering transfer functions#178GenevieveBuckley wants to merge 9 commits intowidgetti:masterfrom
Conversation
|
Travis CI build is currently failing due to the issues with unpkg (unpkg/unpkg#134) |
maartenbreddels
left a comment
There was a problem hiding this comment.
Excellent work, this is also what I had in mind but never got to code up cleanly. Some comments on the code, maybe you always want to add some unittests?
ipyvolume/transferfunction.py
Outdated
| ) | ||
|
|
||
|
|
||
| def linear_transfer_function(rgb_values, |
There was a problem hiding this comment.
Instead of rgb_color, we can just name it color, and let matplotlib deal with it:
https://github.com/glue-viz/glue-jupyter/blob/7254451a2a671cdea07296861abd3f0cf50c6409/glue_jupyter/ipyvolume/volume.py#L43
It seems to accept named colors like 'red', hex values '#f0e6a4', and float tuples.
There was a problem hiding this comment.
That's much more elegant, thanks. Also means we can probably use this in conjunction with the color picker ipywidget, too! (Let me check that last part, but that would be great).
There was a problem hiding this comment.
It would be great if we do it on the frontend, and indeed use the color picker!
There was a problem hiding this comment.
What I've done in a previous project, was to put all the colormaps of matplotlib in 1 texture, of #texture x 1024 pixels, and then the index selected the row of the texture.
ipyvolume/transferfunction.py
Outdated
| return transfer_function | ||
|
|
||
|
|
||
| def load_transfer_functions(include_rgb_linear=True, |
There was a problem hiding this comment.
I would not call it load (get?), and I'd make it 'singleton' like, so it will always return the same widgets, what do you think? (it will basically cache the widgets).
My guess is this will be used as part of a gui to select a particular colormap?
There was a problem hiding this comment.
Let's call it 'predefined_transfer_functions'.
I'm not sure I understand correctly: do you want this to stay a function but always return the same dictionary, or have me make a singleton class for it?
I have no major plans for a colormap gui. But I do want to set future efforts at a GUI up for success as much as possible, so if you have advice on things I'm doing (or not doing here I'm happy to hear it.
What I want in the short term is a bunch of transfer function widgets so I can easily switch between them programmatically for the multivolume rendering I'm working on now.
There was a problem hiding this comment.
What I would like to see, is when a transfer function is created with the same set of parameters (same key for the dictionary), that it returns the same transfer function. That will limit the number of widgets created, what do you think?
There was a problem hiding this comment.
I'm not sure how best to implement the idea, but I think it's a good strategy/goal.
There was a problem hiding this comment.
sth like this:
tf_cache = {}
def get_transfer_functions(...)
tf = tf_cache.get(key)
if not tf:
tf = ....
tf_cache[key] = tf
...
|
I should also mention @vidartf 's work on ipyscales: |
|
Thanks for the link, I didn't know that about @vidartf. So far I've thought exactly zero about how to display colorbars next to figures, but that will be a pretty necessary next step. |
|
@maartenbreddels can you explain the difference between the four different transfer function classes in python? Those python classes are:
Found in And on the javascript side, there's Found in |
e861e87 to
6dead3f
Compare
I propose adding convenience functions for volume rendering transfer functions.
Example use
Transfer function from single RGB value with linear opacity ramp
Transfer functions based on matplotlib colormaps
Load all predefined transfer functions to a dictionary
And use them like so:
Discussion
Now that it's possible to have multiple volumes displayed in the same figure, we need something like this to increase the useability (and it looks like this has been suggested before):
I've also seen this issue play out too, since the default choice is very non-standard for my field:
Let me know what your thoughts are, happy to discuss etc.