Providers as dependent packages

[mbredif]

This issue is about discussing and deciding on a prescribed and supported way to write client code (with itowns as a npm dependency) that adds a new type of protocol/datasource.

Sample usecase

ROS is a robotics framework that is able (among other things) to stream raw or more processed sensor measurements (GPS tracks, lidar point clouds, color images...) through a websocket server.
ROSLIB and ROS3D are client js code that enable the visualization of these streams inside THREE.JS.
It sounds a lot like a datasource/protocol that itowns could visualize ! So I gave it a try and wrote itowns-ros, an npm project external to itowns that depends on both itowns and ros libraries and offers a simple ROSProvider class, with an extra register method to streamline the registration of the provider in the protocol registry maintained by the scheduler.

Questions / todos

1. Do we want to support providers/protocols as external dependencies ? (I do not think we want PRs for all possible protocols in the core!)
2. I got my usecase working, but I am sure my build configuration is fragile and should be improved a lot. I am presenting it to you as a starting point for discussion and feedback.
3. I do not understand all the parts I had to do to make it work (stuff about layerid, picking, change notification...)
4. my use case is particular in the sense that its feature (a gps track) changes due to external events (from the ws server) rather than from commands requested in the scheduler. The workaround I found was to disable frustumculling and notify changes in the update callback, but I am sure better can be done.
5. If we agree, we should write a tutorial describing this prescribed and supported way of doing it (that would help a lot future coders)
6. If we agree, should we port all current providers as separate npm packages ? (that may help enforce that providers developped inside itowns are not more priviledged than external provider packages)

[zarov]

1. Do we want to support providers/protocols as external dependencies ? (I do not think we want PRs for all possible protocols in the core!)

Yes, I think the Scheduler.addProtocolProvider is here for that, otherwise it shouldn't be public.

2. I got my usecase working, but I am sure my build configuration is fragile and should be improved a lot. I am presenting it to you as a starting point for discussion and feedback.

IMO I would simplify your example and reduce the class to a simpler one: get rid of the register method and do this in your example. But it is your provider, so do as you please ;)

5. If we agree, we should write a tutorial describing this prescribed and supported way of doing it (that would help a lot future coders)

Documentation for providers has been added recently http://www.itowns-project.org/itowns/API_Doc/Provider.html, but it can certainly be completed by a more thorough tutorial.

6. If we agree, should we port all current providers as separate npm packages ?

No, we need to keep basic ones, but some may go I think. iTowns shouldn't be bloated, but it shouldn't be naked at first sight. A new user needs to have some cases that works out of the box.

[autra]

+1 to everything @zarov said.

To elaborate a bit on 6: imho itowns should support widely-used openly specified protocols out-of-the-box.

So I gave it a try and wrote itowns-ros, an npm project external to itowns that depends on both itowns and ros libraries and offers a simple ROSProvider class,

From this formulation, I understand that you wanted to write a library, but your repo looks like an app (build results and bundles committed, no peer dependencies, a webpack config, a preinstall script etc). Is it intended?

notify changes in the update callback

I'm pretty sure you shouldn't do that, otherwise the update loop will never stop. notifyChange precisely tells iTowns to execute all the update functions. You should instead call it when you receive a message that contains new data to be displayed. In this case, you should call it when you receive a message of type sensor_msgs/NavSatFix.

So your best bet is to add your view.notifyChange in a callback to the same topic NavSatFix subscribes itself to. But maybe a better long-term solution would be to turn these sensors into event emitters? They'd have an update event, that will get fired each time they update their Object3D (so in NavSatFix.processMessage in this instance). This way sensors could choose not to notify itowns if the message they got didn't make any change in their 3d objects. That could be a contribution to Ros3Djs (you're already a contributor it seems)?

But the important conclusion here: as you don't actually need an update method, you don't need a provider here. Just add your 3D objects to the scene when you initiate the connection, subscribe to the right topic and call view.notifyChange(true) in it and you are done. The role of provider is actually already done by Ros3DJs: it already transforms the data it gets into threejs objects, that are perfectly compatible with iTowns.

As a rule of thumb: if you only ever have a fixed initial amount of 3d objects, you need neither layers nor Providers, providing you can react to data updates.

with an extra register method to streamline the registration of the provider in the protocol registry maintained by the scheduler.

addProtocolProvider is a bit buried indeed :-/

[mbredif]

From this formulation, I understand that you wanted to write a library, but your repo looks like an app (build results and bundles committed, no peer dependencies, a webpack config, a preinstall script etc). Is it intended?

The develop branch is where I work. the master branch is just push forced with the develop head with an extra "publication" commit to be able to host the demo as a ghpage using this script. index.html is indeed the demo app and src should be the library.

I must admit I stumbled onto 2 issues :

• the ros3djs package in npm is not yet es6 compatible. I patched an ongoing PR to make it work in this branch : mbredif/ros3djs#es6. I am not satisfied with the current preinstall script and the file dependendy.
• I was not able to configure peerdependencies and bundled everything while building itowns-ros.js.

If anyone could help me configure the package.json files of mbredif/ros3djs#es6 and mbredif/itowns-ros#develop correctly, that would help me a lot and provide a valuable sample/template to write itowns plugins !

[mbredif]

As for other aspects :

• About the update, It is a very good point in principle, I will update only as needed (but in practice here, the rate of object3d updates I am facing is like >20Hz so it does not change much).

• obj.frustumCulled = false; : I had to disable it to prevent off-nadir culling. How to make it work without this hack ?

• Picking : setting layer ids seemed like it helped picking to work more consistently, but I am not sure it is the right thing.

• my current approach needs a view, just to access to the Scheduler (which I believe is a singleton). IF we decide to keep that protocol registry functionality, should not this be exposed more directly such as itowns.addProtocolProvider ?

• modularization : I was not asking to explode itowns into multiple repositories, but I have read that a github repository can host multiple npm packages. for out-of-the-box functionalities, we could have a itowns metapackage depending on itowns-core and the other plugin packages of the github repository. I understand that may cause a maintenance burden, but that may be outweighted by having a more modular framework where eg each provider or parser may bring its own dependencies with no consequences on users that do not use them. Each internal plugin would also be a valid sample for any external plugin development... that is just a thought.

• Does itowns-ros need to be a provider ? I would say "currently, no" as per @autra arguments, but I think itowns should evolve such that even such use case of a plain THREE.JS object would benefit from being a layer, and automatize the setup of things such as picking, frustumculling, logdepth support, stylization. For instance, ros may publish lidar pointsets using a PointCloud2 topic, which style I would like to handle with the same code as potree and 3dtiles pointcloud sources, such as coloring with an ortho image layer.(I know this is not yet possible in itowns )

[autra]

Picking : setting layer ids seemed like it helped picking to work more consistently, but I am not sure it is the right thing.

What do you mean by "to work more consistently"? What happens if you don't? Btw, you can pass an object3D to picking.pickObjectsAt to restrict the hierarchy of pickable objects. Maybe passing your root object (of this layer) to it will help?

my current approach needs a view, just to access to the Scheduler (which I believe is a singleton). IF we decide to keep that protocol registry functionality, should not this be exposed more directly such as itowns.addProtocolProvider ?

yes, no problem about that. Or maybe view.addProtocolProvider, it depends if we want one Scheduler per webpage or per view.

modularization [...]

Not against per se, but 2 remarks:

• we are not there yet ;-)
• I already feel I spend too much time maintaining the infrastructure of iTowns (most of my PR are in this topic)
• the increase of complexity, even for the user, might not be worth it.

To be tried and seen in practice I'd say.

but I think itowns should evolve such that even such use case of a plain THREE.JS object would benefit from being a layer, and automatize the setup of things such as picking, frustumculling, logdepth support, stylization

I'd say exactly the contrary for picking: it should work on simple Object3D, not only on a layer. That would cover your use case too I guess. But I think it's already the case.... I'm not sure why you have trouble with that (maybe a bug?).

Stylization

how you stylize an object3D already constructed? But apart from that, again I think the contrary: you should be able to stylize and display stuffs without layers imho. Ex, you have a gpx as a string objects (because you constructed it, or have it by other means), you could feed it directly to a parser then a stylizer then a sink and voilà, you have a Object3D you can add to view.scene directly, skipping providers (or sources) and processing (because really you don't need them). Layers should be seen as a dynamic collection of Object3D, that can update itself.

but yeah at the end we want to be able to do this:

For instance, ros may publish lidar pointsets using a PointCloud2 topic, which style I would like to handle with the same code as potree and 3dtiles pointcloud sources, such as coloring with an ortho image layer.(I know this is not yet possible in itowns )

Does itowns-ros need to be a provider ?

Imho the problem you have here, is that Ros3Djs already gives you Object3D, which makes providers and layers of itowns useless. Maybe a alternative solution would be to use roslibjs directly in itowns-ros, if you want tighter integration? For me itowns-ros and Rods3DJs look more like alternate solutions to solve the same problem, a bit differently depending on the degree of integration you want.

[mbredif]

What do you mean by "to work more consistently"?

I opened #747 for this.

modularization [...]

I agree we are not there yet, I am just proposing the idea of a multi-package repo to get opinions.
Maybe a first step could be to provide a quickstart project for libraries/plugins that extend itowns. See iTowns/itowns_app_examples#1

obj.frustumCulled = false; : I had to disable it to prevent off-nadir culling. How to make it work without this hack ?

any thought ?

how you stylize an object3D already constructed?

You can change the material properties, or even the whole material, you can process its attributes to create another object3d... That is what is being done in the wfs example with onMeshCreated callbacks for instance.

Layers should be seen as a dynamic collection of Object3D, that can update itself.

I find it difficult to draw this line : #747 may help to have a common agreement on that.

For me itowns-ros and Rods3DJs look more like alternate solutions to solve the same problem, a bit differently depending on the degree of integration you want.

Rods3DJs does a great job at decoding messages and converting it into object3d, it would be a pity if your advice was to recode that myself ! I admit that the view and materials should be replaced by the ones of itowns but I will definitely reuse their decoded attributes

Ex, you have a gpx as a string objects (because you constructed it, or have it by other means), you could feed it directly to a parser then a stylizer then a sink and voilà, you have a Object3D you can add to view.scene directly, skipping providers (or sources) and processing (because really you don't need them). Layers should be seen as a dynamic collection of Object3D, that can update itself.

If I rephrase that, I am proposing to be able to provide Object3D sources as itowns libraries that can be plugged in seamlessly (after we split the providers) and you are proposing that in the case of sources that produce an Object3D, the user would have to manually call whatever any styling, transformation, material patching, picking fixing code he requires.
I think both make sense (even if my phrasing is surely biased :) ), we will see how it goes in practice !

[autra]

Yes your phrasing is biaised, I didn't say the user would have to manually call , we can still do it for them. I did say Object3D should not be coerced into layers, that'd be contrived and artificial.

[autra]

Rods3DJs does a great job at decoding messages and converting it into object3d, it would be a pity if your advice was to recode that myself ! I admit that the view and materials should be replaced by the ones of itowns but I will definitely reuse their decoded attributes

I may have missed something (I'm not familiar with Ros3Djs): what does it give you apart from the 20 lines of processMessage here (actually you wrote this method, so you already coded it :-) ). For me NavSatFix is already nearly the provider you're looking for, isn't it?

[mbredif]

For me NavSatFix is already nearly the provider you're looking for, isn't it?

That is right for NavSatFix, but there are plenty of other messages and the handling of moving frames by SceneNode. I would rather write the ros->three code I need in an existing package maintained by a small but active community than rewrite yet another one.

[zarov]

This issue has been covered now, see the documentation and tutorials.