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

Higher order transfer functions for volume rendering #2550

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

aganders3
Copy link
Contributor

As mentioned in #2539 (comment), I have been playing around with adding some features to volume rendering to allow the use of more advanced color features, specifically targeting 2D transfer functions (2D TF), mostly motivated by this paper and similar. I think there’s a way this could take the place of that work, but I’m not sure that it should. If we want both though, there should be a way to implement that feature in terms of this one (for example with ComplexMagnitudeTF, ComplexPhaseTF, etc. classes).

I believe the way I have implemented this is backwards compatible for the affected rendering methods (MIP, attenuated MIP, translucent, and additive). I can imaging including some generally useful transfer functions out of the box, but much like the already-scriptable colormaps this is pretty powerful.

I have a couple concerns/questions with my design/implementation so far (hence the Draft PR):

  1. I am not sure I am adding snippets to the shader in the most correct/optimal way, but I was having trouble defining functions that could still use other defined functions.
  2. I am not sure the implementation for MIP makes semantic sense given the parameters for the TF. I appreciate feedback on what arguments should be passed to the TF function.

I guess this is best illustrated by examples - these come from the included examples/scene/volume_2dtf.py.

This shows a volume rendering with an interactive 2D TF used for some segmentation. In the bottom view you see the 2D transfer function overlaid on the 2D (value, gradient magnitude) histogram. Just changing to the 2D TF is pretty subtle for this dataset - I will probably keep looking to try to show something more interesting here.
Screenshot 2023-11-17 at 5 04 27 PM copy

Here is a MIP colored by depth of the found max val (see this figure for a real-world example of this technique):
Screenshot 2023-11-17 at 5 02 37 PM copy

@aganders3 aganders3 marked this pull request as draft November 20, 2023 17:30
Copy link
Collaborator

@brisvag brisvag left a comment

Choose a reason for hiding this comment

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

First pass: I don't quite understand how the gradient one works (I have some reading to do), but I get the gist and this looks awesome :) I think having something built in the Volume that allows us to pass a snippet of glsl like the depthcolorTF is awesome; however, I'm a bit skeptical about adding the LUT logic in the base class... this seems like it should be handled entirely by the transfer function, like colormaps do.

Comment on lines +86 to +92
glsl_tf = """\
vec4 applyTransferFunction(vec4 color, vec3 loc, vec3 step) {
vec4 hue = applyColormap(length(loc));
vec4 final_color = vec4(color.r * hue.rgb, 1.0);
return final_color;
}
"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

@royerloic will love this, we couldn't figure it out last time but we just needed to use loc :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially I was just working on the 2D LUT stuff, but was inspired by a conversation with him to add this feature. It was very simple on this branch, which felt like a good sign! Note this is what I called out in the PR description though - in the MIP shader it's actually passing in max_loc_tex - start_loc for this parameter. I'm not sure if that's the best though. An alternative would be to pass in start_loc separately, but I also don't want to pass in a bunch of generally unused parameters.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see... well, I think it's fair to pass as many parameters as we think might be useful 🤷 It's easy to not use them.

Alternatively, glsl simply accessing stuff from the outer scope generally works (though it's uglier).

vec3 N; // normal
prev = $get_data(loc + vec3(-step.x, 0.0, 0.0));
next = $get_data(loc + vec3(step.x, 0.0, 0.0));
N[0] = colorToVal(prev) - colorToVal(next);
Copy link
Collaborator

Choose a reason for hiding this comment

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

just for consistency I'd use normal.x and so on instead of N[0], cause I thought it was an array at first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks - this is a good idea. To be honest I just copied this code from the iso shader (calculateColor) but I agree this would be more readable. It also could be just factored out as finding the gradient at a point may be otherwise useful.

@aganders3
Copy link
Contributor Author

Thanks for checking this out!

The gradient magnitude example was just the original inspiration because there are examples you can find online (example). It's apparently good for revealing interfaces or boundaries between different material types, but I don't find it particularly compelling on this volume. It is a nice example because it demonstrates doing some meaningful calculation on the fly, but similarly you could put a pre-calculated gradient magnitude into another channel.

I'm a bit skeptical about adding the LUT logic in the base class

I think this is fair. I feel like I had some idea that it was more composable as part of the base class, but I can't remember exactly how right now. I do think providing the LUT logic out-of-the-box is nice, so I will try instead to make this a subclass of BaseTransferFunction (for example TextureSamplingTF). It's easy to just drop it as well and put that in the example instead.

One other concern I have is that I think there is now perhaps a redundant texture lookup in some of the shaders. I want to see if it can be avoided or make sure that's not a big performance regression.

@brisvag
Copy link
Collaborator

brisvag commented Nov 21, 2023

I do think providing the LUT logic out-of-the-box is nice, so I will try instead to make this a subclass of BaseTransferFunction (for example TextureSamplingTF). It's easy to just drop it as well and put that in the example instead.

Yep, I like this!

One other concern I have is that I think there is now perhaps a redundant texture lookup in some of the shaders. I want to see if it can be avoided or make sure that's not a big performance regression.

We are extremely inefficient with texture lookups with isosurface. Things get exponentially worse if you use non-nearest, non-linear sampling (cause those are actually re-sampling the texture multiple times and doing calculations on the results).
This is quite bad in 3D raycasting, where the same texture position might be sampled dozens of time in a single render call... Unless the compiler can automatically solve this somehow, to me it seems super hard to solve without rewriting from scratch. I just don't know the hardware and driver internals enough to know ^^'

@aganders3
Copy link
Contributor Author

That sounds like a separate problem I'd be happy to investigate at some time :). What I meant was I want to make sure this feature doesn't make it significantly worse (right now this doesn't touch the iso method).

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.

None yet

2 participants