ADE modularity - Githubissues

Issue by frogsapo Tuesday Jun 28, 2016 at 10:20 GMT Originally opened as https://github.com/MEPP-team/VCity/issues/140

Priority elements for roadmap establishment

Based on being sources or errors and bugs, and according to JSA, the top two priorities concerning the improvement of libCitGML are

make use of CityGML core language XSD
load the XSD files and realize on demand (lazy) validation
GUI and ADE modularity

When looking at the usage of GetWorkspace(...) method (which is "clearly" an ADE related notion) one encounters calls such as the one of the TreeView::AddTile(). Within TreeView::AddTile() the code needs to distinguish between nodes of Workspace type from nodes of other CityObject type (look at the generic loop on the CityObject). This need to separate (and distinguish) comes from the fact that domain semantics are such that the ADE::Workspace are not CityObject. This creates a strong dependency for the GUI TreeView (the display should be generic code) towards some ADE specifics.

Such dependency might be removed by having the implemented notions inherit from a single (GUI related) interface class. In other terms, in order for all VCity code to remain generic (without any specific knowledge of an ADE information) one not only needs to extend the libCityGML language with ADE but also to extend the GUI basic interface with the corresponding GUI extension.

BTW the ADE have their own GUI sub-directory and the Core GUI should not know about it.

Promote schema based validation

In order to validate a libCityGML xml file (any xml file) the canonical mechanism is to use XML schemas. For each considered ADE one should define an associated XSD and use it to validate xml files using such an ADE. Note that the basis of such validation is to:

assert the existence of an XML element,
assert the attributes (name and type) of such an XML element, (Note: the fact that libCityGML current implementation doesn't enforce this point can be inferred by the code of the MANAGE_OBJECT macro that reads in a series of strings attributes, without checking their name and type. It could be that the type of an attribute is plain wrong or that it uses external attributes (attributes of other nodes that shouldn't be encountered within the node at hand) without libCityGML detecting it).
assert the relationships between such XML elements (element Acan only be encountered within element B as opposed to C and which such cardinality. For example within CityGML a roof element can only be encountered within a Building element but not within vegetation element. (Note: the fact that libCityGML doesn't enforce this point can be inferred by the fact that parser.cpp is a simple flat collection of INSERTNODETYPEs without any structure relating them).

In its current implementation libCityGML's only asserts the first point and thus doesn't enforce the CityGML language (for example the parser won't complain if it encounters a roof element within vegetation and if the roof element has some unknown attribute to the CityGML informative language).

A bit more robust validation could be implemented in (at least) the following two ways:

at the (global) file level through xml schema validators: refer to libXML2 schema validation mechanisms
at the (local) level of "some" (to start with) elements by using the information of the given schema to assert its validity.

Because the second option, by being local, could be limited to "some" elements it would only offer partial validation. Besides, by bypassing XML library mechanisms it could be that a lot of tedious manual work is required. Although such method would be better than having no validation at all, it should probably not be recommended in the long run...

Recommendation: inquire for standard, yet ADE modular, validation mechanisms.

Remove dependency of libCityGML towards QT/OSG

The export/exporter.hpp includes QT's QDateTime.

Recommendation (note): drive the solving of issue #137 towards the removal of the only and single call to exporter::setDate(). Then simply remove that method from exporter, remove the QDateTime include will clear the QT dependency.

Note: move this item to libCityGML concerns (as opposed to this issue which is ADE related

libCityGML: separate ADEs from libCityGML-Standard

libCityGML-standard includes ADE defined CityObjects
- ADE/Temporal/Workspace* is included in e.g. citymodel.[hpp|cpp]
- More generaly look for the inclusion of any ADE/*/hpp file into libcityGML
- Many citymodel methods point to ADE code e.g. SetWorkspace setDocument() set/get-Version
citymodel.hpp includes many ADE/Documents* and Temporal* which it shouldn't
ADE-document: remove dependency to Qt

Use the same implementation mechanisms for libCityGML core and ADEs

Within the implementation, there doesn't seem to be any obvious (or documented) technical reason for not using the same formalism to define the CityGML core language and to define ADEs .

Yet actually:

the implementation of CityGML core language within libcitygml/parser.cpp is based on the [usage of the INSERTNODETYPE(...) macro (e.g. to define a Building element). like when defining a surfaceMember );`).
but ADE definitions are implemented as sets of methods whose coherence only coherence is drawn from the fact that they are initially written by using the same documented recommendation.

Recommendation: unite the implementations of CityGML core language and ADEs.

Remove the old temporal ADE version

This is just a reminder for issue #137 that is ADE related.

Respect XML namespace mechanism

A classic means to obtain modular XML files is to use XML namespace mechanism. In order to follow this standardized way of using XML each ADE should be "localized" within its own namespace. And the header of a given CityGML xml file (that uses one or many ADEs) should formally announce the respective namespaces of the ADEs it uses and then the xml elements should refer to those namespaces.

For the time being ADEs do not use the namespace mechanism at the xml file level. Yet under the hood the ADE implementation make use of namespaces whose names are hard coded in the parsers of the ADEs (with conventional keywords like temp, doc...).

With this discrepancy between the bypass of the namespace in the xml file headers and the underlying implementation that uses namespace, there is no hope for having an effective xml validation. LibCityGML is currently able to parse (possibly invalid) xml files because there is "hopefully" no form of xml validation (either global or even at level of some elements).

Recommendation: fix the parser and the used xml files to respect ADE based namespaces.

ADE extension automatic registration fails

There used to be a mechanism (refer to JE's commented out code ) that enabled an ADE to be automatically registered within the central parser. This mechanism seemed to have failed and some point and a kludge is now used: refer to This has to be manually done :refer to the void ADEHandlerFactory::getInstances(...) method. If we want to head for an ADE plugin mechanism (that wouldn't require to recompile libCityGML with every possible ADE support), we should at least avoid maintaining an explicit list of available ADEs...

Recommendation: fix the failing mechanism and consider (evaluate?) the technical possibility of having ADE as plugins.

libCityGML developers caveat emptor

Notes :

INSERTNODETYPE is a libCityGML defined macro and not a libXML2 macro as one could have expected.
When looking for how libCityGML deals with links/references grep recursively on "xlink:href". Parsing (importer) does the trick within the MANAGE_OBJECT macro.
The manner in which the load function is defined as a global funciton (within parserlibxml2.cpp) makes it hard to find...

VCityTeam / 3DUSE

ADE modularity #19