Optional vtkjs

[zachmullen]

This merges latest master with the changes on @sankhesh 's vtk branch, as well merging the dual-builds branch.

@sankhesh @manthey during the merge there was one conflict I wasn't sure about -- are the changes in src/camera.js to the far and near camera bounds still necessary?

[manthey]

@zachmullen I'm reworking how the camera class allows setting the clip bounds (including near and far). I hope to have a PR for that soon. This PR ultimately won't change camera.js but will have the renderer do something like this.map().camera().clipbounds = {near: <x>, far: <y>} (but I'm not certain of that interface yet).

[zachmullen]

I've updated to the the earliest version of vtk.js that this works with, but the rendering is not working:

It just shows the background of the vtk render window. Panning the map causes that to disappear and be replaced with the normal map tiles, but no points appear in either case. Any thoughts?

[manthey]

To fix the pan issue, I'm guessing that the geoOn(geo_event.pan, ... function should call m_this._render. I'll have to look further than that.

[zachmullen]

@aashish24 @sankhesh @manthey I've pushed changes that now make the vtk.js example that @sankhesh wrote load into the list and have access to the data, but as I mentioned before, the vtkjs renderer is only showing the background, no map or points.

Is this expected? Did this ever work, or was it just not completed? Something is clearly wrong in the renderer implementation itself.

[sankhesh]

@zachmullen It did work at one point. However, I must admit that it was all hacked together back then. My understanding was that between the changes to vtk-js and this branch, it should all work. I can take a look if you like.

[manthey]

@zachmullen I just pushed some changes that make it so that when I use the simple_point tutorial I see points in the appropriate spots. You can zoom and pan via the console: tutorial.map.zoom(5), tutorial.map.center({x: -90, y: 30}). The mouse interactor seems to be disabled because the vtkjs window is on top of the geojs div.

There are a bunch of points in the code I've marked as TODO. At the very least, we need to use the non-fullscreen vtk.js renderer so we don't break interaction. I don't know why vtk.js renders a background color once but then not again -- we'd rather it never do so.

[manthey]

@aashish24 I think, at the very least, we have to resolve the initial color overlay and the mouse interaction issue before we could merge this. There are many other smaller issues which we could defer or wait until they are resolved before merging. I'd also like to see us using the latest vtk.js version (the latest version doesn't import the same way, so we have to figure that out).

[aashish24]

There are a bunch of points in the code I've marked as TODO. At the very least, we need to use the non-fullscreen vtk.js renderer so we don't break interaction

+1, yes that is the right thing to do.

t the very least, we have to resolve the initial color overlay

What is the initial color overlay issue is? just wanted to make sure I understand.

mouse interaction issue

Are we still having some interaction issues? I believe @sankhesh fixed some but wondering if there are more.

[manthey]

What is the initial color overlay issue is? just wanted to make sure I understand.

For whatever reason, the vtkjs renderer fills its background with a color. As @zachmullen just commented, we can ask for transparent to avoid this.

Are we still having some interaction issues? I believe @sankhesh fixed some but wondering if there are more.

Yes -- the vtkjs render window is the full screen renderer, which makes a div on top of everything. This is wrong for two reasons: (1) the map may not be the full windows, and (2) the div eats the mouse events. I crudely switched to the vtkGenericRenderWindow, and set its container to the layer div. This fixed the interaction issue, but the vtk renderer had the wrong resolution (and may have had other issues), so I assume we just need to not use the full screen renderer and configure a partial screen renderer properly.

However, if I manually pan, zoom, etc., the drawn points are all in the correct spots.

[sankhesh]

Perhaps, we could use the vtkGenericRenderWindow and set the size on it each frame. That should fix the issue of wrong viewport size.

[aashish24]

Perhaps, we could use the vtkGenericRenderWindow and set the size on it each frame. That should fix the issue of wrong viewport size.

I believe that is what David did. David, with these two fixes, do you still think we need to address these issues in depth before we approve this PR? Perhaps, we can list all of these issues and check if they can be addressed in another PR?

[manthey]

@zachmullen, @aashish24 There are still a pile of "TODO" issues, but this is superficially working. It even has coverage of much of the new code. I recommend that we merge this and fix issues in future PRs.

The biggest defect, in my opinion, is that it uses a single value for radius, color, and opacity for all points (and doesn't distinguish between fill and stroke values at all). I don't (yet) know enough about vtkjs to know how to color and size each point distinctly in an efficient manner.

[aashish24]

@zachmullen, @aashish24 There are still a pile of "TODO" issues, but this is superficially working. It even has coverage of much of the new code. I recommend that we merge this and fix issues in future PRs.

The biggest defect, in my opinion, is that it uses a single value for radius, color, and opacity for all points (and doesn't distinguish between fill and stroke values at all). I don't (yet) know enough about vtkjs to know how to color and size each point distinctly in an efficient manner.

I agree, lets merge this if it is okay with @zachmullen and @sankhesh as well.