Customising volume shader, depth occlusion...

[xinaesthete]

I'd quite like to be able to render something like a ScatterplotLayer along with a VolumeLayer. I can do that ok, e.g.

function getLayers() {
  return [
    new ScatterplotLayer({
      ...scatterProps
    }),
    ...volumeView.getLayers({props})
  ];
}

I'd like to make it such that the depth was handled nicely (ie, points occlude volume & vice-versa, notwithstanding vagueries of opacity)... now, that'd mean making a depth texture available to a modified xr-3d-volume shader, which gets into the territory of needing to make some fairly drastic changes to how the volume shader gets assembled and how the layers interact etc...

I'm not entirely sure if I'll push through with it, but I made a bit of a start on different versions some of the Viv classes, with the thought of hacking some of my own shader-mangling on top of what XR3DLayer does... I've been toying with doing so in a way that avoids forking viv itself, but may be ill-advised

class MyVolumeLayer extends VolumeLayer {
  renderLayers() {
    const xr3dlayer = super.renderLayers();
    const {getShaders} = xr3dlayer;

    function getShadersHack() {
      const shaders = getShaders.apply(xr3dlayer);
      /// attempt to jerry-rig some modifications
      // (I still need to get more used to how deck.gl / luma.gl work to figure
      // whether this is really a good way to try to hijack the pipeline)
      return shaders;
    }

    xr3dlayer.getShaders = getShadersHack;
    return xr3dlayer;
  }
}

Perhaps I'd be better off forking viv itself... I don't know how much trouble it would be to make it work with somewhat more arbitrary LayerExtensions. I noticed this issue which touches on related points of wanting to be able to add uniform declerations to modified versions of shaders...

[ilan-gold]

Hmmm, this is interesting - please don't fork Viv! Hopefully we can make this work - if you want this, others probably do as well! We can define hooks in the shader so placing one at a given point that you need should be doable (and again, if you need it, someone else probably does as well).

As for the depth texture, I remember getting this kind of working using gl_FragDepth (if this is what you mean by "depth texture") in this PR. If you wanted to fix and/or formalize this and add it to the core I am happy to review a PR. Maybe you can also open an issue if you think this is a feasible feature?

Thanks for your interest in the project!

[xinaesthete]

tl;dr:

If you're still not having luck, I can try to get working that PR with the current code base - in theory it shouldn't be too much.

That would be very much appreciated.

longer ramble

In terms of my comments on additive, I think it was just that I was thinking more in terms of additive blending from other contexts and initially slightly confused by that behaviour. I agree that I also want it not blending with surroundings, but occluding as appropriate.

I was a bit confused why taking an extensions array, you then basically ignore all but the last one.

If multiple extensions implement it, I only take the last one. I guess I could throw an error, which might have been more informative....sorry about that.

Not a problem, I just thought that since the logic of the english sentence in the error message doesn’t seem to match the logic of the code at that point it might be worth flagging (and maybe changing while we think about it just in case it causes confusion in future).

I’m not sure I have a strong stance on what the best architecture for extensions would be - as I say, I’m glad that this extensibility has been considered at all in the design, and am appreciating a bit better how this renderer may have limits in that regard. I might try to think about that a bit more, for now my priority is figuring out why I still don’t seem to be managing to write out gl_FragDepth in any meaningful way.

I’ve made a change so that I can switch on and off visualisation of the value being written to depth (including via a LayerExtension used on my ScatterplotLayer, as well as a change to _AFTER_RENDER in the volume shader). This lets me see that something approximating reasonable values of depth are being computed (you can see that in the vercel thing I linked elsewhere, in the bottom visualisation, you need to select a different resolution for the volume renderer to update). There seems to be some discrepancy between the scaling of values rendered by the scatterplot vs the volume - I’ve tried to apply equivalent linear_to_srgb() etc, there is something that isn’t right there (could possibly be related to renderDepthCoord issues you flagged) but really these details are academic while I’m failing to make the volume shader write to depth.

Right now it’d represent significant progress if I could write gl_FragDepth = 0.; in my _AFTER_RENDER and see the scatterplot mostly occluded.

I imagine that at some point I’ll stumble on whatever trivial flag needs to be set, or whatever other change needs to be made…

I’m putting the Scatterplot after the Volume in the layer array, so scatterplot should be getting any depth written by volume and occluding appropriately (I’ve tried it the other way around just in case).

I’ve just been trying hauling more of VolumeLayer.renderLayers() into my override version, so that rather than const xr3dlayer = super.renderLayers(), I have my own version of your code and can specify the parameters the XR3DLayer is created with. Not sure that’s getting me anywhere, though.

I haven’t seemed to manage to parse the part of your PR that makes depth work (at all) for you. Perhaps I could try checking out that branch.

I’m still wondering what disabledCommandIds in the spector dump is.

Ho hum. I'm sure I'll kick myself when I finally know what the problem is.

[ilan-gold]

Alright @xinaesthete it seems I have hit on what might be the issue at play here. I think you should play around with where you're applying WebGL parameters - i.e on the per-layer level, or globally. For example, here is a branch that does look a bit dumb for the letters, but the additive blending on the volume + axis actually works as intended so hopefully this is a good starting point. Have a look at the diff: https://github.com/hms-dbmi/viv/compare/ig/new_3d_axes?expand=1 I think we need to figure out a correct alpha value to get MIP working as well (or at least that is my suspicion). See https://deck.gl/docs/api-reference/core/deck#parameters and https://deck.gl/docs/developer-guide/tips-and-tricks#per-layer-control-of-webgl-parameters

I think generally, you probably want the right "global" blending parameter to control how the layers interact, and then some sort of per-layer control as well dependent on your use-case. I'll probably get back to look at this soon since it is very exciting, but now things seem to work as intended. Previously, on that very old branch, I would say things were not really working, so my apologies. Screenshots can lie. This one, though, does not!

If you do get something working, please let me know and we can probably incorporate it into the core since you'll probably beat me in figuring this out.

[ilan-gold]

Just so it's clear - you can see where the blue channel is present and where it isn't. The space is where the scale axis appears as desired.

[ilan-gold]

It's not perfect, but I think this is a start:

[xinaesthete]

Great, I'll try to see what I can do with your PR shortly. I had started to deduce that parameters seemed like the relevant place for whatever flags need to be set, hadn't quite managed the right incantation just yet but hopefully this'll get me on the right track.

Incidentally, I believe writing gl_FragDepth can have a significant impact on performance, so it may be worth trying to do some benchmarking of that, and not applying it in situations where it's not necessary.

Ultimately there could still be value in rendering other layers first and passing a depth-texture into volume shader. That applies particularly to when you want the volume looking translucent, but it may be that it turns out to perform better in other situations as well. It does seem like it'd require more work in managing how the different passes interact and pass information to each other, in a way that if it wasn't well designed could lead to future maintenance etc.

[xinaesthete]

Well, I tried a bit more playing with parameters, with your PR diff as a reference. Didn't get results I wanted straight away in my project, so I'm trying to run your branch on my machine.

Not quite sure what's going on - looking at Avivator on localhost, when I first loaded a volume I could see axes along with the 'loading' text, but then when the volume finished loading they were nowhere to be seen and since then I haven't seen them even when going to a completely separate browser. I think before my sense of reality disintegrates any further I'm going to carry on with other things for now, but hopefully I'll make some kind of sense of it next week.

Regards,
Peter

[xinaesthete]

As I noted previously, I think the best approach may be to prepare a depth-texture first, and use that to avoid marching in to any parts of the volume that are occluded.

[ilan-gold]

@xinaesthete That would be a good nice. Could you screenshot what is not working with MIP? I'm curious. My issue with MIP was generally that each fragment pass involves "rendering" different depths at a single point whereas in Additive, we combine the different colors on each step. Concretely, the difference I'm getting at is that the color is aggregated within the _RENDER for loop in Additive but not in MIP, where it happens in _AFTER_RENDER. So there's no way to know what the "true" depth of the volume is. But I haven't looked at this in a second - at one point this might have been clearer to me, but now I'm just in the recesses of memory...

[xinaesthete]

there's no way to know what the "true" depth of the volume is

Right - which is generally the case and why rendering translucent things makes depth-buffer a less than ideal approach... but mostly the simplest way to approach it is to render opaque things first (with depth test & write), then translucent things (with depth test, but not write). Because there is no 'true depth' value for those things.

[xinaesthete]

So, the issue I had with MIP was that I was overriding the _AFTER_RENDER with whatever happened to be entered into my ShaderEdit gui, so although my changes to the source in the extensions themselves were ok, I wasn't getting any useful results out because I skipped the color computation. I only just bothered to look into that.

That said, I don't see any sensible output from 'Minimum Intensity Projection' even on the main production website, which I initially attributed to my own ignorance, but should probably be reviewed.

My current status, then, once I clean up a little:

• Additive rendering mode works well.
• Maximum Intensity Projection is no longer broken, but the depth isn't very useful, because there is no correct depth value to write to gl_FragDepth. The correct approach IMO will be to pass in a depth buffer, influencing canShow in the ray-march loop (or similar). I may implement that soon as I feel like this is a "show don't tell" situation... not sure when I'll get 'round to it though.
• I haven't tried to think much about Minimum Intensity Projection - seems like there's an underlying bug that may have been around a while and is not impacted by my changes (or changes in new_3d_axes).
• I've not got my changes to work outside AVIVATOR context and am still not entirely sure why, likely to do with my Deck configuration.

So I'll maybe prepare an initial PR soon, not necessarily for merging straight into master but a useful contribution to new_3d_axes at least.

I don't necessarily think depth should be enabled by default, as in many cases it is likely not being used and may have some performance impact. Ideally it should probably respect what is passed in parameters: { depthTest, depthWrite }, with caveats around depthWrite with MIP (possibly issuing a warning to the user). Maybe there should be a separate hook for depthWrite (and later depthTest when it's implemented).

Maybe further down the line, particularly if trying to render multiple overlapping MIP volumes in a scene, depth-peeling may be something to consider, but for now that would be an unneeded level of complexity.

The change to homogenous coordinates was only relevant at the final depth computation, so I've reverted changes to the earlier stages.

[ilan-gold]

Yes, Minimum Intensity Projection is broken - I should probably remove it. The other comments sound good to me - I agree about the depth defaults and the new hook (especially since it is not a breaking changing). Thanks for your (hopefully) future contribution!