Closed hugoledoux closed 2 years ago
Thanks for sharing your roadmap. It's not hard to guess that I am happy to see the plan to make CityJSON compliant with CityGML v3 :-) This is a great statement to the community. Let me know if and how I can support your work on this.
Some thoughts/questions:
"CityJSONFeature"
to support streaming"transform"
for "vertices"
. Doesn't this break backwards compatibility and would thus require a major release v2.0?"[none]"
in "boundaries"
means. Could you please point me to an example?Great that this makes you happy.
- I see the point for enforcing
"transform"
for"vertices"
. Doesn't this break backwards compatibility and would thus require a major release v2.0?
Hmmmm, doesn't pretty much any change break backwards compatibility? There are currently 2 ways, and only one will be accepted. If backwards means that all files from v1.0 are also v1.1, but then it means that any change require a major bump, and that one can only add things? I would argue that a patch bump should definitely be like this, but a minor bump not...
I wrote v1.1 because the same structure is kept, new CityObjects are added, and we're planning to fix small annoyances (please submit yours), and we'll add a update() to cjio.
- I am not sure what
"[none]"
in"boundaries"
means. Could you please point me to an example?
Oupsie, that was mistake. I meant to the "semantics"
arrays, fixed above, cheers.
My thought was that, for instance, a v1.1 parser should also be able to consume v1.0 datasets, which it couldn't when strictly implemented against the proposed change to enforce "transform
". Of course, there are valid reasons to pick v1.1. Just wanted to raise this question.
Oupsie, that was mistake. I meant to the
"semantics"
arrays, fixed above, cheers.
Ok, so it's about dropping the possibility to have null
in the "values"
member of "semantics"
? This would mean that either all surfaces of a geometry must be semantically classified or none, right? null
is also used for "material"
and "texture"
to indicate surfaces that shall not receive an appearance. I think it`s important to be able to express such cases.
for null
, no worries they would stay!
It's the trick that are written in the specs to save space that I would remove:
A null value is used to specify that a given surface has no semantics, but to avoid having arrays filled with null, it is also possible to specify null for a shell or a whole Solid in a MultiSolid, the null propagates to the nested arrays.
It's a pain to code and since you don't know then it proves my point: surely no one would implement this!
Some thoughts on the proposed 1.1 enhancements.
I dont think that CityJSON v1.1.0 can be considered as "totally compliant to CityGML v3.0" for the simple reason that the notion of Space (Occupied, Unoccupied, etc.) has not already been added. It will be an important and certainly a source of issues but still CityJSON v1.1 is very convincing.
Thanks for that thorough feedback!
Paragraphe 3.4 : I suggest to add it anyway as it is defined as a "TopLevel" object feature. To be totally consistent with the specs.
Hmmm, it's there, and we thought that being 100% aligned with the data model wasn't the aim since we're already diverting from it. If the same info can be stored then it's fine. As you said: we tried to simplified the complex data model too.
Paragraphe 4.3 : "LOD4 is omitted". It does not exist anymore, so is it really "omitted"?
Oupsie, will fix this, cheers.
OtherConstruction would enjoy a better definition. The difference with CityFurniture is not always trivial. For instance, is a pylon a construction or a furniture akin to a lamp post? I am not aware of a city environment typology stating on it. The example of the windmill is much more straightforward for me.
Very good point. The original work done by the WP had that codelist: https://github.com/opengeospatial/CityGML-3.0/blob/master/Archive/WP%2009%20Resources/Codelists/OtherConstruction_function.txt But since CityGML doesn't enforce codelist I left it empty... But I reckon adding a few examples in the text would help, right?
I dont think that CityJSON v1.1.0 can be considered as "totally compliant to CityGML v3.0" for the simple reason that the notion of Space (Occupied, Unoccupied, etc.) has not already been added.
I think it can be considered since the SPACES are not reflected anywhere in the implementation, they are only abstract classes.
I would like add a +1 for the '"metadata"/"triangulated": False'.
For the upcoming version (v1.1), the plan is to:
Concretely, those are the features that are considered:
BuildingStorey
,BuildingUnit
,BuildingRoom
as new CityObjectsTunnel
andBridge
accordinglyOtherConstruction
Transportation
classes"lod"
in geom to string“vertices”
are only integer, thus transform is always enforced. Better for everyone.CityObjectGroup
gets a role attribute for each part; this is in v2 but was forgotten[none]
in"boundaries"
that propagate, it’s just making things more difficult for devtoplevel
: check forparents
instead"geometry": []"
in Extension: for CO that have no geometry?CityJSONFeature
to support streaming? (more info) + metadatamyfile.city.json
to enforce?"toplevel"
in Extension"metadata"/"triangulated": False
GenericCityObject
b/c Extensions should be used insteadapplication/city+json
andapplication/city+json-seq
CityGMLv3 states that the modularisation is possible for encodings, so one can cherrypick the module they want. Thus, modules that will not be implemented:
Please comment below, also if I omitted or forgot something.