-
Notifications
You must be signed in to change notification settings - Fork 7
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
First easy start tutorial #15
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks to start this! It's a nice work already. Before doing an exhaustive review, some general remarks:
-
Did you know you can create jekyll page with markdown files? They would have the site general style, and you could delete the generated html file from this PR.
-
I think overall, there is too much text and not enough code. On overall, after reading it, I don't feel like I'll be autonomous with itowns. This should be the main goal of a tutorial. this is linked with my next point:
-
This tutorial can be useful for beginners in iTowns and Javascript but also for more experienced users.
Let's choose an audience, it'll be more efficient because it could be shorter and more "to the point". In my opinion, there are 2 important things:- teach people to use itowns. In this case, just detailing how to create a planar view, a globe view and add layers to them (especially OSM one), then how to create a pointcloud is enough. Maybe a better starting point would be to create tutorials directly from our examples. It's possible to generate webpages from code and comments for instance (@peppsac did it with docco, and the result was very good). This is for beginners.
- teach people to use itowns for their own need (extend it, actually): make them code a simple custom layer (with an update function they'll code themselves). This is a more advanced use case (but important nonetheless).
-
I'm not sure we should abstract the layer configuration part in external json file. This is actually the hardest part of itowns for the "beginner" use case. It should not be hidden, but quite the contrary: a significant part of this tutorial should be about learning how to configure a layer.
-
Apart from that, I like the "important classes" part, but it's missing MainLoop.js, which is the actual most important part of itowns. Notably, MainLoop allows you to understand than at its core, a layer is just an update function. It allows you to understand that you can have children layers, and that the layer concept of itowns is more general than just SIG layer. You can see them as computation or processing units, or as a sort of pipeline. Of course, this belongs more to the advanced use case.
-
You have completely left out the pointcloud possibilities of itowns. It's too bad, because it may be the most mature part of itowns nowadays! Same thing for 3dtiles, you don't speak about it at all. More generally, this tutorial is actually a tutorial on how to use the globe with IGN data in itowns.... I'm not against it, but I think it should be clearer from the start.
-
Here follows my main disagreement about this PR: the diagrams.
The UML diagrams are not clear and not adapted to javascript. As I've already said elsewhere, We DO NOT WANT class diagram in js. What you want is maybe a simpler version of component diagram.
Beside that, UML generally does a poor job explaining to beginner, because they are not beginner-friendly to start with. I'm not against diagrams, but it'd be a lot better to have a more free-form diagram explaining which component interact with others. To have a broad overview of how itowns works for a beginner, we need to provide answers to the question"What is using what to do which tasks?".
Moreover, in their current form, they are incomplete. As a start, the artificial split between core, process and renderer suppressed some very important links and dependencies (TileGeometry using OBB for instance).
A lot of info they carry are at best irrelevant, at worst incorrect. Some examples:
- c3DEngine does not have a 1 to 1 link with LayeredMaterial of any sort. It's not because c3DEngine import LayeredMaterial that it should appear that way in the diagram (especially considering what c3DEngine imports from LayeredMaterial: a simple function !)
- in UML, simple links must have a label explaining what this links is about. There is none here. This is simply useless to draw links without saying what it is
- indicating that modules depends on THREE is irrevelant. Itowns depends on three everywhere
- do we really care that there is a dependency between GlobeTileProcessing and Coordinates??
And there are other problems too. In short: again let's stop trying to use auto-generated class diagrams in a dynamic language like js. This simply does not work. They are not made for that. This will never get relevant or even correct. Sorry to be so blunt about this, but it's the reality. They may be very good in java, but not in js. Let's remove them from this PR, please.
Following issue #14 I propose an easy tutorial for a quick start with itowns with a link on the front page with examples and all.