VCityTeam / py3dtilers

Tilers accepting various input formats (OBJ, 3DCity databases, GeoJson, IFC) and producing 3DTiles tilesets.
GNU Lesser General Public License v2.1
192 stars 52 forks source link

Rename ObjectsToTile to something with better semantics #59

Closed EricBoix closed 2 years ago

EricBoix commented 2 years ago

The name (of the type) "ObjectToTile" doesn't carry the right semantics (which is confusing when reading the code). Change that type name. Maybe a clearer name would be TilableObject ? In fact ObjectToTile is an interface (abstract class) to which some derived class must comply in order to be integrated into a Tile. Because "object" is a very broad term that doesn't add much semantics when used in TilableObject maybe we could rename "ObjectToTole" to "TilableInterface" ?

jailln commented 2 years ago

Good idea! I actually had a hard time with this name when going through the code yesterday :) +1 for "TilableInterface"

LorenzoMarnat commented 2 years ago

We really need the rename ObjectToTile and ObjectsToTile classes. The problem with TilableInterface is that ObjectToTile isn't an interface, since we actually instantiate ObjectToTile or ObjectsToTile in the common steps of the Tilers (for example when creating groups or LODs). In those steps, we could instanciate objects of the same class as the input, like we do in kd_tree, but if (in the future) we want to merge data from different input types (like IFC with CityGML), we will need a class that doesn't depend on the input type.

A solution would be to choose a name based on Cesium specification. It seems they use the word "feature" to describe an object which has its own position/properties. ("feature" also appears in 10. Tile formats of Cesium 3Dtiles reference card). However, the word "feature" don't seem to be part of the 3DTiles specification (I don't think there is an official word for what we call ObjectToTile in the specification).

clementcolin commented 2 years ago

I'm in for using a Feature class, as long as we explicitly describe that it comes from the Cesium Spec (Feature exist also in CityGML, ISO, GeoJSON, and so on, and can various meaning).

Another argument to use Feature is that it can be easier to understand the notion of feature and batch table, as they are described specifically related to Feature :

A Feature Table is a component of a tile's binary body and describes position and appearance properties required to render each feature in a tile.

A Batch Table is a component of a tile's binary body and contains per-feature application-specific properties in a tile

LorenzoMarnat commented 2 years ago

If we rename ObjectToTile to Feature, how sould we rename ObjectsToTile ? Features, FeatureCollection, something else ? Having just a 'S' to differentiate the classes can be confusing.