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

Cones are stretched out when axis spans don't match #4

Closed
etpinard opened this issue Apr 13, 2018 · 28 comments
Closed

Cones are stretched out when axis spans don't match #4

etpinard opened this issue Apr 13, 2018 · 28 comments

Comments

@etpinard
Copy link
Member

Here's what gl-cone3d looks like when wrapped in plotly.js:

Plotly.newPlot(gd, [{
  type: 'cone',
  cx: [1,2,3],
  cy: [1,2,3],
  cz: [1,2,3],
  u: [0.1,0,0],
  v: [0,0.1,0],
  w: [0,0,0.1],
  x: [1,2,3],
  y: [1,2,3],
  z: [1,2,3]
}])

gives:

image

In the above, the cones appear stretched out in the y & z direction and x axis covers a greater range.

Cones look fine when hard setting the axes ranges all to the same span:

image


We might be able to come up with a workaround in plotly.js that tweaks the positions and meshgrid values to un-stretched the cones. But perhaps a better solution would be to (1) make gl-cone3d aware of x/y/z axis ranges well building mesh positions (2) add a "pixel" mode to gl-mesh3d so that cone vertices can be specify in pixels offsets about data positions.

@kig @jackparmer

@jackparmer
Copy link
Member

But perhaps a better solution would be to (1) make gl-cone3d aware of x/y/z axis ranges while building mesh positions (2) add a "pixel" mode to gl-mesh3d so that cone vertices can be specified in pixel offsets about data positions.

Hey @kig @mikolalysenko What do you guys think? Are one of you free to grab this next week? We're so close to getting this into plotly.js.

@jackparmer
Copy link
Member

@etpinard I'm guessing streamtubes will have the same issue since they are also building on mesh3d?

@kig
Copy link
Collaborator

kig commented Apr 16, 2018

Hmm, interesting problem. How can I find out the scaling of the view? The cones are now batched into one big gl-mesh3d for rendering, so when the transform of the mesh changes, the cones get stretched.

I guess the way to go would be to apply the transform to the cone positions and the velocity vectors, then build the cones without the applied transform. Basically, convert the input data to the scaled view, then render the cone plot without the scaling.

@kig
Copy link
Collaborator

kig commented Apr 16, 2018

Alternatively, do the cones in a vertex shader that does the above.

Anyway, I'll try and get this working.

@etpinard
Copy link
Member Author

etpinard commented Apr 16, 2018

@etpinard I'm guessing streamtubes will have the same issue since they are also building on mesh3d?

Yes, streamtubes will exhibit the same problem as first in described in Robert Monfera's attempt: plotly/plotly.js#701 (comment)

How can I find out the scaling of the view?

@kig , I just pushed some uber wip plotly.js wrapper work to this branch https://github.com/plotly/plotly.js/compare/gl-vis-cone. To see it in action first, go through https://github.com/plotly/plotly.js/blob/master/CONTRIBUTING.md#development and then in the console enter:

Plotly.newPlot(gd, [{
  type: 'cone',
  cx: [1,2,3], cy: [1,2,3], cz: [1,2,3],
  u: [0.1,0,0], v: [0,0.1,0], w: [0,0,0.1],
  x: [1,2,3], y: [1,2,3], z: [1,2,3]
}])

to see the stretched view, and enter

Plotly.newPlot(gd, [{
  type: 'cone',
  cx: [1,2,3], cy: [1,2,3], cz: [1,2,3],
  u: [0.1,0,0], v: [0,0.1,0], w: [0,0,0.1],
  x: [1,2,3], y: [1,2,3], z: [1,2,3]
}], {
  scene: {
    xaxis: {range: [-5, 5]},
    yaxis: {range: [-5, 5]},
    zaxis: {range: [-5, 5]}
  }
})

to compare it with a non-stretched view.

Alternatively, do the cones in a vertex shader that does the above.

Awesome!

@kig
Copy link
Collaborator

kig commented Apr 22, 2018

I got the non-scaled cones on a scaled conemap working on a prototype. It's constructing the cones in the vertex shader and only transforming the cone root positions. The bonus feature of shader-built cones is that updating the cone model is very fast (it is rebuilding the model every frame after all), so you can animate the model just by changing the vectors buffer. The cone coloring needs to move to the shader for that though.

scaled_cones

Now to port these changes to the master branch and test it in plotly.js...

@kig
Copy link
Collaborator

kig commented Apr 23, 2018

Video of it in action - the cone direction and magnitude is scaled according to the coordinate system, but the shape stays the same. Need a tweak for normals and size, though.

@etpinard
Copy link
Member Author

Looking good @kig! Thanks for taking this on 🎉

It's constructing the cones in the vertex shader and only transforming the cone root positions.

Did you patch gl-mesh3d to make that happen, or will gl-cone3d have its own shaders? Personally, I prefer the former as I suspect your streamtubes implementation could reuse that logic. Either way, this isn't a deal-breaker by any means 👌

Now to port these changes to the master branch and test it in plotly.js...

Let me know if you need help with that!

@kig
Copy link
Collaborator

kig commented Apr 24, 2018

@etpinard It's doing both, patched gl-mesh3d to pass in the attribute buffers needed for cone construction, and a custom shader to build the cones based on the data. Streamtubes can use the same logic in mesh3d but needs another custom shader to construct tubes instead of cones.

@etpinard
Copy link
Member Author

It's doing both,

Sounds good! Thanks for the info.

@jackparmer
Copy link
Member

Hey @kig Just checking in, how is this going?

@kig
Copy link
Collaborator

kig commented May 1, 2018

Hi @jackparmer

Pushed an update to the master that merges in the shader-constructed cones. Now doing a small tweak to the cone lib to make it work with the plotly.js shim.

@kig
Copy link
Collaborator

kig commented May 1, 2018

@etpinard Can you give the latest gl-cone3d a go, I did the following in plotly.js to make the above examples run:

diff --git a/src/traces/cone/convert.js b/src/traces/cone/convert.js
index 587ee2565..da86e3092 100644
--- a/src/traces/cone/convert.js
+++ b/src/traces/cone/convert.js
@@ -10,7 +10,7 @@
 'use strict';

 var cone2mesh = require('gl-cone3d');
-var createMesh = require('gl-mesh3d');
+var createMesh = cone2mesh.createConeMesh;

 function Mesh3DTrace(scene, mesh, uid) {
     this.scene = scene;

The result for the -5..5 example:
cone_plotly

The other example creates tiny invisible cones for some reason, and tweaking the scaling factors in the shaders has no visible effect. Been banging my head against that, dunno what could be causing it. The model matrix has a (-1,-1,-1) translate & scaling factor of 1, which seems to be the only difference between it and the example that works (that one has 0 translate and 0.2 scaling).

@etpinard
Copy link
Member Author

etpinard commented May 4, 2018

@kig I'm sure it's nothing major, but all the examples under example/ are broken with the current master and gl-mesh3d@2.0.0. They spit out:

image

@etpinard
Copy link
Member Author

etpinard commented May 4, 2018

P.S. I updated https://github.com/plotly/plotly.js/compare/gl-vis-cone to work with the latest gl-cone3d master. Feel free to push commits unto that branch!

@kig
Copy link
Collaborator

kig commented May 11, 2018

Ok, figured out what's going on with the cones not showing up. The fragment shader was clipping the rendering based on mesh3d clipBounds. I've disabled it now, so the cones show up on both of the plotly.js examples.

@etpinard
Copy link
Member Author

The example/ folder is still broken for me off your latest commit a041539

image

@kig
Copy link
Collaborator

kig commented May 12, 2018

@etpinard Huh, haven't seen that error. I'll test with a fresh clone of the repo. It shouldn't be using gl-mesh3d any longer, but a local version that adds: a couple of uniforms to control cone scaling, a vectors attribute, and a fourth element to position attribute to use as vertex index (... that should be moved to its own attribute come think of it).

In other news, streamtube3d is now running in a vertex shader as well, so shouldn't exhibit non-uniform scaling in non-uniform coordinate systems. There are live demos of the cones and the streamtubes at https://gl-vis.github.io/gl-streamtube3d/ and https://gl-vis.github.io/gl-cone3d/

@kig
Copy link
Collaborator

kig commented May 13, 2018

I started doing a streamtube convert.js based on the cone3d one, this is early, just baked-in data, next step is to put in some dataset.
streamtube_plotly_wip

@kig
Copy link
Collaborator

kig commented May 14, 2018

I don't have commit access on the plotly.js repo, so I'm working on https://github.com/kig/plotly.js/tree/gl-vis-cone

Here's the output for

var x = [-5, -4, -3, -2, -1, 0, 1, 2, 3, 4, 5];
var y = x, z = x;
var len = x.length * y.length * z.length;
var u=[],v=[],w=[]; for (var i=0; i<len; i++) { u.push(1+Math.sin(i)); v.push(Math.cos(i)); w.push(Math.sin(i*0.3)*0.3); }
var cx=[],cy=[],cz=[]; for (var i=0; i<7; i++) for(var j=0; j<7; j++) { cx.push(-5); cy.push(i-3); cz.push(j-3); }
Plotly.newPlot(gd, [{
  type: 'streamtube',
  cx, cy, cz,
  u, v, w,
  x, y, z, 
  widthScale: 100, 
  colormap:'portland'
}])

streamtube_plotly_1

@etpinard
Copy link
Member Author

There are live demos of the cones and the streamtubes at https://gl-vis.github.io/gl-streamtube3d/ and https://gl-vis.github.io/gl-cone3d/

both https://gl-vis.github.io/gl-streamtube3d/ and https://gl-vis.github.io/gl-cone3d/ don't work for me

image

@etpinard
Copy link
Member Author

@kig on what OS / browser are you deving on? Looks like https://gl-vis.github.io/gl-cone3d/ is working on Chrome 66 on High Sierra (tested on https://www.browserstack.com).

Maybe my issues only appear in Ubuntu 😕

@etpinard
Copy link
Member Author

Maybe @dy could help out.

Can you see any potential issues in https://github.com/gl-vis/gl-cone3d/tree/master/lib to could cause cross-browser / cross-OS problems?

etpinard added a commit that referenced this issue May 14, 2018
- this fixes issues reported in
  #4 (comment)
  that seem to only appear on etpinard's thinkpad running
  Ubuntu 16.04
@etpinard
Copy link
Member Author

Great news. Using #5, I'm getting cones to render in plotly.js 🎉

Plotly.newPlot(gd, [{
  type: 'cone',
  cx: [1,2,3], cy: [1,2,3], cz: [1,2,3],
  u: [0.1,0,0], v: [0,0.1,0], w: [0,0,0.1],
  x: [1,2,3], y: [1,2,3], z: [1,2,3]
}])

gives:

image

and

Plotly.newPlot(gd, [{
  type: 'cone',
  cx: [1,2,3], cy: [1,2,3], cz: [1,2,3],
  u: [0.1,0,0], v: [0,0.1,0], w: [0,0,0.1],
  x: [1,2,3], y: [1,2,3], z: [1,2,3]
}], {
  scene: {
    xaxis: {range: [-5, 5]},
    yaxis: {range: [-5, 5]},
    zaxis: {range: [-5, 5]}
  }
})

gives

image

Note that the autorange logic for cones will need to be patched, e.g. rotation the first figure gives:

image

@jackparmer
Copy link
Member

I don't have commit access on the plotly.js repo

@kig added you as a collaborator:

https://github.com/plotly/plotly.js/invitations

@kig
Copy link
Collaborator

kig commented May 17, 2018

@etpinard I'm on Windows & Chrome (half the stuff I do is Unity, which is why I'm stuck there..). Yeah, that fix sounds like it could be the native OpenGL driver bleeding out some implementation details through ANGLE.

@jackparmer Thanks, I'll create branches for the streamtube and streamline there.

@etpinard
Copy link
Member Author

Thanks for sorting this out @kig

Looking forward to your streamtube and streamline branches.

Going to close this issue. Go to plotly/plotly.js#2641 for updates on cones.

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

No branches or pull requests

3 participants