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

Rework CityTiler and GeojsonTiler so they use lod_tree to create tilesets #12

Merged
merged 45 commits into from
Aug 31, 2021

Conversation

LorenzoMarnat
Copy link
Collaborator

@LorenzoMarnat LorenzoMarnat commented Jul 5, 2021

The main goal of this rework is to centralize the creation of the GLTF geometry. In the current state, all the tilers implement their own geometry creation, from raw data to triangles soup then to GLTF:

  GeojsonTiler          CitygmlTiler

  .geojson                 GML
     |                      |
     |                      |
Triangles soup        Triangles soup
     |                      |
     |                      |
   GLTF                    GLTF

The GLTF creation from triangles soup is the same for all tilers and doesn't depend on the type of data (gml, geojson...). That's why this rework centralizes the transformation of triangles soup to GLTF in a shared file (lod_tree). Now, the tilers only transform their data into triangles soup:

  GeojsonTiler       CitygmlTiler

  .geojson            GML
    \                /
     \              /
     Triangles soup
           |                      
           |                      
          GLTF 

The GLTF creation doesn't need to know the type of data, it takes ObjectsToTile (or derived classes) and creates tiles (.b3dm files).

Also, the new process allows to create tileset with level of details (LODs). The creation of LODs doesn't depend on the type of data, it takes triangles soup and creates the specified LODs. When no LOD is specified, lod_tree creates tilesets the old fashion way (with kd_tree). More details in this readme.

Contents of the PR:

  • Documentation about LODs creation
  • Tests for the GeojsonTiler and for the LODs creation
  • Rework of CityTiler and GeojsonTiler

Note: all the old features of the GeojsonTiler and the CityTiler still work. Those from GeojsonTiler are covered in test_geojsonTiler.py. Those from the CityTiler still need automated tests.

@jailln
Copy link
Contributor

jailln commented Jul 6, 2021

Will this PR allow to create a tileset from a geojson and a citygml seemlessly integrated in the tileset for instance? (i.e. one geojson + one citygml -> one tileset)?

Copy link
Contributor

@clementcolin clementcolin left a comment

Choose a reason for hiding this comment

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

Great job ! Still a few explanations to add (I think EBO will ask a lot of questions)

Thank you for the documentation 👍

I'm wondering if it is possible to split the creation of the b3dm (with the geom) from the lod_tree. Also, perhaps lod_tree is not the good name for this file since it does a lot more than a old tree

py3dtilers/CityTiler/CityTiler.py Outdated Show resolved Hide resolved
py3dtilers/CityTiler/CityTiler.py Outdated Show resolved Hide resolved
py3dtilers/CityTiler/CityTiler.py Outdated Show resolved Hide resolved
py3dtilers/CityTiler/CityTiler.py Outdated Show resolved Hide resolved
py3dtilers/CityTiler/citym_building.py Outdated Show resolved Hide resolved
py3dtilers/Common/lod_1.py Outdated Show resolved Hide resolved
py3dtilers/Common/lod_tree.py Outdated Show resolved Hide resolved
py3dtilers/Common/object_to_tile.py Outdated Show resolved Hide resolved
py3dtilers/GeojsonTiler/GeojsonTiler.py Outdated Show resolved Hide resolved
py3dtilers/GeojsonTiler/geojson.py Outdated Show resolved Hide resolved
@LorenzoMarnat
Copy link
Collaborator Author

Will this PR allow to create a tileset from a geojson and a citygml seemlessly integrated in the tileset for instance? (i.e. one geojson + one citygml -> one tileset)?

It won't be possible with this PR. However, I think it wouldn't require much work: we could easily mix geometries from both tilers, since the ObjectsToTile's geometry (triangles soup) doesn't depend on the type of source.
I thought about creating tilesets with a level of details from cityGML and a lower LOD from geojson, the only real problem with this is to link the geometries from both tilers together (i.e identify which feature correspond to a feature from the other tiler).

@LorenzoMarnat
Copy link
Collaborator Author

I'm wondering if it is possible to split the creation of the b3dm (with the geom) from the lod_tree. Also, perhaps lod_tree is not the good name for this file since it does a lot more than a old tree

Indeed, lod_tree does much more than just creating level of details. It also transforms triangles soup to GLTF. This transformation could (should ?) be moved in another file, so we keep only the part about LODs in lod_tree.

I've created an issue to list the possible improvements.

py3dtilers/__init__.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
tests/test_example.py Outdated Show resolved Hide resolved
py3dtilers/GeojsonTiler/geojson.py Outdated Show resolved Hide resolved
py3dtilers/GeojsonTiler/geojson.py Outdated Show resolved Hide resolved
py3dtilers/Common/tileset_creation.py Outdated Show resolved Hide resolved
Grouping methods are now in the group.py file
Polygon extrusion is now in the polygon_extrusion.py file
Loa and Lod1 inherit from LodNode to create nodes with specific geometries
- Add the data source in the test data
- Update CityTiler docstrings to match the code
- Update documentation to match the code
- Change some variable names for more explicit names
- Update, add or delete some comments in code
Copy link
Contributor

@EricBoix EricBoix left a comment

Choose a reason for hiding this comment

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

There will be another review....

- Replace some static methods by instance methods
- Delete useless methods (now done in class constructors)
- Apply flake8 on root directory and add folders in the exclude
- use tripy to triangulate the footprints, the old way was creating wrong triangles on non-convex polygons
- use a flag in the Geojson tiler to indicate if the features are roofprints or footprints (if they are roofprints, substract the height from the coordinates to reach the floor)
The atlas creation was targeting the 'junk_buildings' folder when adding texture. When creating relief, this folder was missing, causing a fatal error. Now we target a folder adapted to the type of data
Grouping geometries together is done (better) in the Common/group.py class
* Use shapely instead of tripy to triangulate the polygons. tripy was sometimes creating triangles outside of the polygon

Minor improvements:
* Change the default height (used when parsing a geojson file without height values)
* Dont create triangles under the shapes. Those triangles are hidden by the floor, so they are useless
* Handle MultiPolygon geometry when parsing geojson files
* use [earclip](https://github.com/lionfish0/earclip) to triangulate. Previous method was creating buildings with missing triangles
@EricBoix EricBoix mentioned this pull request Aug 31, 2021
4 tasks
@EricBoix EricBoix merged commit e1f5665 into master Aug 31, 2021
@EricBoix EricBoix deleted the CityTiler_with_LodTree branch August 31, 2021 12:56
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

Successfully merging this pull request may close these issues.

None yet

4 participants