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

Volume traces #2753

Closed
wants to merge 23 commits into from
Closed

Volume traces #2753

wants to merge 23 commits into from

Conversation

kig
Copy link
Contributor

@kig kig commented Jun 25, 2018

Volume trace

volume_plotly2

@etpinard @alexcjohnson

Volume rendering trace for plotly.js. Renders 3D volumes as stacks of translucent images. Integration work in progress.

  • Volume data is passed in in the streamtube / isosurface fashion: x,y,z, and u arrays of coordinates and intensity, each array has the same length. The x, y, z arrays are used to construct a meshgrid that determines the shape of the u array.
  • scene.js transparent pass results in nothing drawn, the trace is now using a hack where it's rendered in the opaque pass.
  • There's an opacityscale parameter to control the mapping of intensity values to opacity multipliers, this is passed in as a 256-element array of numbers 0..1. Which is arbitrary and a bit of a pain to construct.
  • [x ] When rotating the volume, the rendering jumps when changing the image stack render order. The stack layers may not line up perfectly.
    • This is because the stacks should really be rendered as voxels to get proper z-sorting. Voxels would add up to around 6 triangles per voxel, or 12 million tris for a 128x128x128 volume. Rendering as a point cloud is another option.
  • The volume lib represents volumes with textures that have the layer images stacked vertically. This runs into trouble with higher-res volumes, exceeding max texture size.
    • Fixable by using 3D textures, which is easiest in WebGL2. Possible to do with a hack for WebGL1, but that'll require multiple textures for storing the volume.
  • Asymmetric volume dimensions seem buggy, try with 64x32x64 for example
  • Hover?
    • Hover doesn't really have a nice mapping here. We could ray march into the volume and look for an intensity value above a hover cutoff (so it'd work a bit like a voxel isosurface wrt picking.)

role: 'info',
editType: 'calc',
description: 'Sets the opacity scale of the volume, which opacity to use for which intensity. Array of 256 values in 0..1 range.'
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this mapping over the same range as colorscale? If so, can we specify it similarly, as a piecewise-linear list of pairs [[0, opacity0], [v1, opacity1], ..., [1, opacityN]]?

Is this used along with trace.opacity? Like the two are multiplied together perhaps? However that works it should be in the description.

role: 'info',
editType: 'calc',
description: 'Sets the opacity of the volume.'
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

opacity is a standard attribute for all traces (unless you disable it with 'noOpacity' in module.categories) so you shouldn't need this attribute, nor the coerce('opacity'), nor even trace.opacity === undefined ? 1 : trace.opacity - you should just be able to do volumeOpts.opacity = trace.opacity; since opacity defaults to 1.

coerce('opacityscale');

coerce('boundmin');
coerce('boundmax');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar to isosurface, could we change these to (x|y)(min|max)? Doesn't look like these are implemented yet, and it doesn't feel to me as though they're as important here as in isosurface, so I'd be OK with removing them for the first version of this trace type.

@@ -161,8 +182,6 @@ function convert(gl, scene, trace) {
trace.imax === undefined ? trace.cmax : trace.imax
];

volumeOpts.opacity = trace.opacity === undefined ? 1 : trace.opacity;
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can still set this, if it'll get used downstream - which it looks like it will based on your amended description for opacityscale, thanks for fleshing that out!

My point was just that you don't need to handle the undefined case here, since the global trace.opacity attribute has a default of 1 you know at this point that you can only have a number between 0 and 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh, right, thanks for catching this.

Copy link
Contributor

@etpinard etpinard left a comment

Choose a reason for hiding this comment

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

Unfortunately, I wasn't able to test @kig's volume on my laptop.

Punching in

    var width = 128
    var height = 128
    var depth = 128

    var xs = []
    var ys = []
    var zs = []

    var data = []
    for (var z=0; z<depth; z++)
    for (var y=0; y<height; y++)
    for (var x=0; x<width; x++) {
        xs.push(2*x/width);
        ys.push(3*y/height);
        zs.push(z/depth);
        var value = Math.pow(Math.abs((10000 + (0.5+Math.random())*500 * (
            Math.sin(2 * 2*Math.PI*(z/depth-0.5)) +
            Math.cos(3 * 2*Math.PI*(x*x/(width*width)-0.5)) +
            Math.sin(4 * 2*Math.PI*(y*z/(height*depth)-0.5))
        )) * Math.pow(z/depth,1/3) * (1-Math.sqrt(x*x / width*width + y*y / height*height)) % 500000)/1e6, 2);
        data[z*height*width + y*width + x] = value
    }

    var opacityscale = []
    for (var i=0; i<16; i++) {
        opacityscale[i] = [i/15, Math.pow(i/15, 1.2)]
    }

    Plotly.newPlot(gd, [{
      type: 'volume',

      x: xs,
      y: ys,
      z: zs,

      value: data,

      cmin: 0.05,
      cmax: 0.22,

      imin: 0.05,
      imax: 0.25,

      opacity: 0.05,

      colorscale: 'Portland',
      opacityscale: opacityscale
    }])

gives me a blank scene and:

image

Applying a patch similar to gl-vis/gl-cone3d@a9e5bb3 does not seem to resolve the issue. @kig do these console errors look familiar to you?

Moreover, @kig , would you be interested in adding 2 or 3 "data"/"layout" mocks in test/image/mocks/ and a few jasmine tests similar to:

Volume trace don't have hover, so that's one less thing to worry about, but we should make sure autorange and Plotly.restyle behave correctly.

return simpleMap(arr, function(v) { return ax.d2l(v) * scale; });
}

var xs = getSequence(trace.x);
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this different / better than

function distinctVals(col) {
return Lib.distinctVals(col).vals;
}

used in streamtubes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooh, good to see. I'll change it to use Lib.distinctVals to have a single reference for this kind of function.

FWIW, getSequence operates on an array of sequences sorted in ascending order and returns the unique values of the first sequence. E.g. [1,1,2,2,3,3,1,1,2,2,3,3] -> [1,2,3]. So it is a bit more performant for this kind of data (no need to sort, can bail out after the first sequence), but I don't think it's worth the maintenance burden.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the info @kig !

I don't think it's worth the maintenance burden.

I think so too. 👍

].join(' ')
},

value: {
Copy link
Contributor

Choose a reason for hiding this comment

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

I pushed this in a couple months ago, but thinking about this again, I think values (n.b. plural) would be better. It would match values attributes in pie, splom and parcoords traces.

var proto = Volume.prototype;

proto.handlePick = function(selection) {
// TODO
Copy link
Contributor

Choose a reason for hiding this comment

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

We should just return here if we're not going to implement hover for volume traces.

].join(' ')
},

imin: {
Copy link
Contributor

Choose a reason for hiding this comment

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

I forget. Did we settle on imin / imax in the past? If not, I think I would prefer vmin and vmax to match the v in values.

We should also explain how this pair of attributes is related to cmin / cmax

    volumeOpts.isoBounds = [trace.cmin, trace.cmax];
    volumeOpts.intensityBounds = [
        trace.imin === undefined ? trace.cmin : trace.imin,
        trace.imax === undefined ? trace.cmax : trace.imax
    ];

in the attribute description.

attrs.hoverinfo = extendFlat({}, baseAttrs.hoverinfo, {
editType: 'calc',
flags: ['x', 'y', 'z', 'intensity', 'text', 'name'],
dflt: 'x+y+z+intensity+text+name'
Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove hoverinfo completely, as we're not implementing a hover handler in this first iteration.

@kig
Copy link
Contributor Author

kig commented Oct 3, 2018

@etpinard The gl-texture2d failure is likely because a 128x128x128 volume tries to create a 16kpx high texture. 64x64x64 should work. I'm working on a proper fix to this issue by laying out the slices on a grid (and eventually using that as a raymarchable volume).

@etpinard
Copy link
Contributor

etpinard commented Oct 3, 2018

The gl-texture2d failure is likely because a 128x128x128 volume tries to create a 16kpx high texture. 64x64x64 should work. I'm working on a proper fix to this issue by laying out the slices on a grid (and eventually using that as a raymarchable volume).

Awesome 👌

@etpinard
Copy link
Contributor

@archmoj could you try pulling down this branch, npm i, npm start and opening up mock gl3d_volume-simple. I'm getting

image

and I'd like to confirm I'm not the only one.

@archmoj
Copy link
Contributor

archmoj commented Oct 30, 2018

@etpinard Yes I am getting the same errors on Ubuntu & Windows too.

@archmoj archmoj mentioned this pull request Jan 24, 2019
@archmoj archmoj closed this Jan 24, 2019
@etpinard etpinard deleted the volume-traces branch February 12, 2019 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants