feat: add channel prop support to the volume layer#587
Conversation
in this only the first channel would be supported but lays the framework for using channel props for multi-channel rendering
| } | ||
| } | ||
|
|
||
| public setChannelProps(newProps: ChannelProps[]) { |
There was a problem hiding this comment.
Nit: If copied over, might as well use same naming (i.e. channelProps instead of newProps).
There was a problem hiding this comment.
Thank you for spotting this, changing it. This came from the fact that we actually wrote this setChannelProps quite a while ago before realising there was an established structure and then essentially copying that code over from the chunked image layer instead. Forgot to update this one in the process, thanks for noticing. The same applies to #587 (comment), so fixing that also
| this.channelProps_ = newProps; | ||
| this.currentChunks_.forEach((chunk) => { | ||
| chunk.setChannelProps(newProps); | ||
| }); |
There was a problem hiding this comment.
The chunked image layer invokes callbacks when channel props are set:
this.channelChangeCallbacks_.forEach((callback) => {
callback();
});Why did we skip it here?
|
|
||
| public setChannelProps(newProps: ChannelProps[]) { | ||
| this.channelProps_ = newProps; | ||
| this.currentChunks_.forEach((chunk) => { |
There was a problem hiding this comment.
When we get a pooled image in the chunked image layer, we set its channel props:
if (pooled) {
const texture = pooled.textures[0] as Texture2DArray;
texture.updateWithChunk(chunk, this.getDataForImage(chunk));
this.updateImageChunk(pooled, chunk);
if (this.channelProps_) {
pooled.setChannelProps(this.channelProps_);
}
return pooled;
}Why did we skip it here?
There was a problem hiding this comment.
No reason for skipping, glad you noticed it and apologies for missing in the first place, fixing that problem
| this.programName = dataTypeToVolumeShader(texture.dataType); | ||
| this.cullFaceMode = "front"; | ||
| this.depthTest = false; | ||
| // TODO handle visibility property of channels |
There was a problem hiding this comment.
Is it a TODO because you still want to limit the scope of this PR to one channel? So this PR only covers the plumbing of channel props?
There was a problem hiding this comment.
Yes exactly, we could plumb it so that visibility is available as a uniform. But I think that might be better not to do right now as OIT and non-OIT would want to handle that visibility differently.
OIT would want to avoid rendering the chunk for that channel altogether if the channel is not visible, so this would be in the renderer. While non-OIT would have access to all the textures in the shader and conditionally sample based on the visibility of the channel, so having a uniform would be useful (it could also compile and use a different shader altogether to avoid the conditional in the shader but that's a bit of overhead to introduce right now)
There was a problem hiding this comment.
In general that is also why this PR isn't further updating getUniforms right now, as for OIT and without OIT the getUniforms would look fairly different
to be more consistent with the chunked image layer in the volume layer
seankmartin
deleted the
seankmartin-aranega/volume-rendering-channel-props
branch
|
🎉 This PR is included in version 0.11.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
Summary
One thing is common to both #564 and #583 - which is that there needs to be some way to define the properties of the channels for volume rendering. We've extracted that common piece here, as it is more self contained and would be helpful to get feedback on so then #564 and #583 can be adding on top of that.
The
VolumeLayerhas copied some of thechannelPropsfunctionality from the other image layers with the callbacks on channel change, how to set channels etc. And how theVolumeRenderablesets those as a uniform is similar to theImageRenderable. This also helps in moving towards #555 and removing uniforms from the volume layer. In this only the first channel would be supported for passing the uniforms, but it lays the framework for using channel props for multi-channel rendering.Tests & Checks
I manually tested the volume rendering examples and checked other examples to see if still working. Visually this shouldn't really change much, outside of the transfer function bounds being a little different for the invlerp mapping.
New:
Old:
