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

more than one texture per surface? #3

Closed
hugoledoux opened this issue Sep 14, 2017 · 14 comments
Closed

more than one texture per surface? #3

hugoledoux opened this issue Sep 14, 2017 · 14 comments

Comments

@hugoledoux
Copy link
Member

@jstoter gave the noise modelling example with different scenarios/params having diff results which become textures of the walls of buildings.

@hugoledoux
Copy link
Member Author

@kenohori @clausnagel
okay my proposal is as follows: a CityObject can have different "themes", either with materials or textures. The "material" is a JSON object with key-object pairs, the keys being the themes, eg:

"material": {
          "irradiation": { 
            "values": [[0, 0, null, null, 1, 1]] 
          },
          "irradiation-2": { 
            "values": [[2, 2, null, null, 1, 1]] 
          }
        }

and the values are just the references to the materials in the "appearance". "texture" has exactly the same behaviour with key-object pairs.

        "texture": {
          "winter-textures": {
            "values": [
              [ [[0, 10, 23, 22, 21]], [[null]], [[null]], [[0, 1, 2, 6, 5]], [[0, 2, 3, 7, 6]], [[0, 3, 0, 4, 7]] ],
              [ [[null]], [[null]], [[0, 0, 1, 5, 4]], [[0, 1, 2, 6, 5]] ]                      
            ]
          },
          "summer-textures": {
            "values": [
              [ [[1, 7, 3, 2, 1]], [[null]], [[null]], [[1, 1, 2, 6, 5]], [[1, 2, 3, 7, 6]], [[1, 3, 0, 4, 7]] ],
              [ [[null]], [[null]], [[1, 0, 1, 5, 4]], [[1, 1, 2, 6, 5]] ]                      
            ] 
          }
        }        

And to ensure that a viewer picks the right theme when >1, one can define the following in the "appearance":

  "appearance": {
    "default-theme-texture": "summer-textures",
    "default-theme-material": "irradiation",
    "vertices-texture": [

If not define then the viewer just picks whatever it wants.

@hugoledoux
Copy link
Member Author

4e61127

@jstoter

@clausnagel
Copy link
Contributor

clausnagel commented Oct 6, 2017

Sorry for re-opening. The null values for non-textured surfaces are not really intuitive. And at least my Java Gson lib refuses to generate them automatically.

My suggestion is to simply use null instead of [[null]] in your example.

            "values": [
              [ [[0, 10, 23, 22, 21]], null, null, [[0, 1, 2, 6, 5]], [[0, 2, 3, 7, 6]], [[0, 3, 0, 4, 7]] ],
              [ null, null, [[0, 0, 1, 5, 4]], [[0, 1, 2, 6, 5]] ]                      
            ]

Just to illustrate a possible issue with the current [[null]] encoding: you could easily create a value like [[null, 5, 7, null]]. I think this should be avoided.

@phtj
Copy link

phtj commented Oct 7, 2017

I have been watching these discussion roll by, and I am no expert in the development of such formats. However, maybe I can just pitch in here. I would assume that models will often have many surfaces with no texture. For example, with irradiation, you may only want these textures on the glazed areas. In such cases, endless lists of null values might not be desirable.

An alternative may be to explicitly list the surface indices for each set of values rather than relying on the array position.

@kenohori
Copy link
Member

kenohori commented Oct 7, 2017

I don't think [null] or [[null]] make much sense. Since those arrays don't have the number of vertices as their corresponding ones in the geometry object, a developer cannot assume that they have the same structure. So, I'd second @clausnagel's suggestion. Going back to a discussion with @hugoledoux some time ago, maybe we can take it one step further and say that arrays consisting solely of null objects (e.g. [null, null, null] or [[null], [null]]) can always be flattened to null.

@hugoledoux hugoledoux reopened this Oct 7, 2017
@hugoledoux
Copy link
Member Author

hugoledoux commented Oct 8, 2017

Using null instead of [[null]] is indeed better. As Ken points out, that means that we have a "hierarchical representation" in a way: the null propagates to the other primitive "below".

This is one idea Ken and I discussed a few months ago, but we left it aside because the implementation is more complex. But perhaps time to bring it back now. It would solve most of the issues I reckon (@phtj: I dislike have new IDs to refer to the primitives... sorry) and have more compact files.

So the idea is that for "material" and "semantics" (and "texture" for null values) one can put a value at the level(s) higher in the hierarchy of arrays and that value propagates below. Imagine for instance that one Solid has all the same material "blue", which is the 7th in the array of appearance, then instead of enumerating for each surface the value 6, it can be put once:

"material": {
          "mycolours": { 
            "values": 6 
          }
  }

The same applies to null and for semantics with Semantic objects.

I've built in c++ a quick demo with a snippet showing how to extract the material/texture/semantics for a single geometry: https://github.com/tudelft3d/cityjson/blob/hierarchical-arrays/hierarchy-tests/main.cpp

Everything working. @kenohori tried for null with Ruby and it's also super simple.

What do you think?

@clausnagel
Copy link
Contributor

If I get your hierarchical idea right, then wouldn't it de-facto just make sense for "material"?

For example, isn't it rather unlikely that all surfaces of a "MultiSurface" or "[Composite|Multi]Solid" of a building share the same semantics? And if all values of a "texture" property are null, then wouldn't it be more straight forward to completely omit "texture" instead of assigning one null value?

For "material", I see the benefit (similar to CityGML, where you can assign an app:X3DMaterial to an aggregate surface such as gml:MultiSurface). But I agree with your concern that the implementation becomes more complex. I prefer using Json binding libs (like Gson) over having to manually parse the Json. In the binding world, you would have to deal with the fact that "values" can be both an integer and an array. Since this is not possible (well, at least in my Java world), you have to inject some manual parsing code. Doable, but more complex.

So if my understanding is correct, then perhaps adding one additional "value" property to "material" might be an alternative. If there is only one material for all surfaces, one can pick "value", and otherwise "values".

@clausnagel
Copy link
Contributor

Btw, I support the proposal of @kenohori to flatten arrays containing solely null values to a single null.

@hugoledoux
Copy link
Member Author

I'm also in favour of the null flattening, but it breaks the schema since the depth of the arrays is prescribed for the different Primitives... One solution is to modify the schema just so that it's only a JSON object, and write code to to further validate (cityjson-valschema does this to some extent already). Or perhaps there's another way to define this in a schema, but I am not aware and would have to investigate this further.

Also, a few points from the discussions above:

  • semantics could also be a lot of the same values sometimes I reckon, think of semantics for WaterBody for example, where the river is triangulated... but agreed it'll be rather rare.
  • gson doesn't have a function that gives you the type you have at runtime? like json["geometry"].is_array() or similar?
  • if the hierarchical arrays proposal is too complex (I'm ambivalent on this one I must say, it is more complex but if the 2 functions I wrote are translated in a few languages then it's done, no need to reinvent them), then I like the "value" for "material"" very much.

So yeah, let's do a poll I guess!

@clausnagel
Copy link
Contributor

No opinions so far? Well, here is my two cents: I was not aware that using null instead of [[null]] would break the schema. So personally I rate schema validation higher than a flattened array representation.

I wonder about the proposed change to the "values" property of "material". Is it possible to define "values" in the schema such that it can either take an integer or an array as value? If not, then I would also vote against this change :-)

Regarding Gson: The idea is that you define a Java class which represents a Json object and whose fields represent the properties of this object. The serialization and deserialization is then done automatically, so you do not have to fiddle around with Json elements. But this requries unique data types. As mentioned above, you can also write custom code in Gson like in your example in order to handle non-default cases.

Btw, I have completed a first CityGML-to-CityJSON support in citygml4j (against the current v0.3 schema). It should (hopefully) work for all sorts of building representations in CityGML. So I am happy to produce example CityJSON files which might be helpful in studying the layout of CityJSON datasets - just let me know. The changes are not yet on GitHub. I first have to implement a clean API and also want to work on CityJSON-to-CityGML.

@kenohori
Copy link
Member

kenohori commented Oct 10, 2017

We were busy thinking about and testing what flattening means in terms of implementation in different programming languages. Putting implementation first might be a philosophical difference with respect to CityGML 😉.

Languages like Ruby and Swift are a piece of cake. There are conditional evaluations that can be called even if something is null (?. and &. respectively). With flattened values, we do need a series of ifs at every level, but the code is relatively easy in all languages we tested. In theory, any language with type introspection (or a library that provides it) should work.

Hugo and I also looked into how to redefine the schema to allow for arrays of varying depth. I think it should work by defining a series of custom types at every level of the hierarchy. I'm in favour of this, but it does make the schema more complex...

Regarding the Gson serialisation, I reckon you could implement a custom (de)serialiser for the array types. Something like this:
https://stackoverflow.com/questions/19588020/gson-serialize-a-list-of-polymorphic-objects

Personally, if the implementation is not too messy, I'm in favour of flattening everything (nulls and values).

@clausnagel
Copy link
Contributor

@kenohori Thanks for your response. Don't get me wrong - I am not complaining about coding complexity. In fact, I have already implemented the flattened array approach in my CityJSON support in parallel to the v0.3 schema. Your link to the custom Json adapters is exactly what I meant with custom code in my previous post.

@hugoledoux
Copy link
Member Author

It seems that we decide here, and for me either way is good. It seems for you too Claus, and Ken too. If we're ambivalent, I'd then go for the flattened array approach because:

  1. better to introduce this early in the process. Otherwise if we wait and then at v2.0 (coming soon btw!) then it's too big of a change
  2. only deserialisation need to be supported in a way, writing CityJSON can be with an arrays of the same depth
  3. we all agree that for null values it is needed and good. Thus the schema need to be modified anyway. The current schema that defines the depth of each 3D Primitive doesn't work, but we can modify with a few tricks. The issue is that, just looking at the schema, perhaps it'll less expressive then the text of the specs. But the validator "cityjson-valschema" will have a proper validation. The question is: what is the CityJSON specs, the text or the schema? With JSON I reckon the text has to have more weight...

If you approve this (with a thumbs up) then I'll figure something out for the schema (or @kenohori will, I recall you promising :)).

And it's fantastic if citygml4j works, I do want some files yes, mine are all rather hacked.

@hugoledoux
Copy link
Member Author

hugoledoux commented Oct 13, 2017

with v0.5 the Semantic Surfaces are handled "better", and there is no need for hierarchical arrays. And "texture" can't have it either, so that leaves only "material"... I have decided, after an unanimous vote with myself, that the hierarchical arrays should be skipped. However, the idea of @clausnagel of having "value": 2 for the "material" if the the whole geometry has the same material has been implemented in v0.5.

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

No branches or pull requests

4 participants