cityjson / specs

Specifications for CityJSON, a JSON-based encoding for 3D city models
https://cityjson.org
Creative Commons Zero v1.0 Universal
109 stars 25 forks source link

Make changes to CityObjectGroup #74

Closed liberostelios closed 3 years ago

liberostelios commented 3 years ago

Additions to the CityObjectGroup.

The point is to make its use for specific use cases, instead of using it as a "dummy" object to group things. We prescribed a few cases for which they can be used (e.g. as a way to group roads together or model neighbourhoods).

Also, we remove the geometry property as this seems unnecessary. Instead, we add a spatialBoundaries property which can only be a (Multi/Composite) Solid that defines the boundaries of the area that the group represents.

hugoledoux commented 3 years ago

Instead, we add a spatialBoundaries property which can only be a (Multi/Composite) Solid that defines the boundaries of the area that the group represents.

Should we allow spatialBoundaries to be MultiSurface, eg for a neighbourhood? Using only Solid sounds cumbersome, no?

Athelena commented 3 years ago

Instead, we add a spatialBoundaries property which can only be a (Multi/Composite) Solid that defines the boundaries of the area that the group represents.

Should we allow spatialBoundaries to be MultiSurface, eg for a neighbourhood? Using only Solid sounds cumbersome, no?

We discussed this as well. Decided to leave it out for simplicity but I'm not against it.

kenohori commented 3 years ago

is there any reason for calling it spatialBoundaries? why not stick to geometry?

Athelena commented 3 years ago

is there any reason for calling it spatialBoundaries? why not stick to geometry?

We discussed this a lot as well and it was to differentiate it and make it more explicit/clear. We went through many name options 😅

liberostelios commented 3 years ago

is there any reason for calling it spatialBoundaries? why not stick to geometry?

Conceptually, because this isn't supposed to be real geometry but more of an administrative boundaries representation. Practically, because this isn't supposed to be geometry because: a) it doesn't support all types (only Solids allowed) and b) in the future it shouldn't have an "lod" property.

The purpose is to underpin the importance of CityObjectGroups altogether and not treat them as yet another type of dummy object.

hugoledoux commented 3 years ago

I went for a walk this pm and thought about all this.

First, I don't know why you mention that only Solids should be supported for groups, why not MultiSolid or MultiSurface? A district could be disconnected, or there's surely a case where a Solid is not sufficient, no?

Second, spatial boundaries and geometries mean roughly the same, and as mentioned, changing the name makes it perhaps simpler for the user to see the difference, but from a code point-of-view it's not simpler. CityObjectGroup becomes different from the other COs, and this is one slippery slope... A CO should have a geometry, or not. The fact that there's a "lod" enforced is certainly not ideal, but in CityGML v3 if my UML skills are good enough the lod needs to be given, so since v1.1 not a major update but just v3 compliance, I think this makes most sense to keep it.

Third, I think to harmonise with Extensions the members should become children, then all CO are consistent.

Of course the docs should be expanded, we can add the example of the roads as an example?

liberostelios commented 3 years ago

I wrote Solid in the post for convenience, but in the proposed schema MultiSolid and CompositeSolid is also allowed. I am not sure about MultiSurface because the purpose is to have 3D boundaries and only Solids are supposed to provide this (although I know there is an argument for 2D representation via MultiSurface but I am not a big fan).

From an OO perspective, there can be multiple hierarchies of objects. For instance, we can assume all CityObjects have type and attributes but only physical objects have geometry, while logical objects (such as CityObjectGroups) have spatialBoundaries. I think ObjectGroups are special enough to have their own properties but not too much to not be considered CityObjects.

I don't mind the members-children argument that much. After all, functionally they are the same, while for geometry vs spatialBoundaries there is a reason to flag to devs that this is not a typical geometry and needs special treatment.

At the end of the day, I see that CityGML 3 posses restrictions on what we can do for 1.1 so I'll let you decide. For me, it's more of the vision of what purpose CityObjectGroups could serve in the long term.